NodeJS transforms #15532

Frassle · 2024-02-28T12:39:25Z

Description

This adds a new experimental feature to the NodeJS SDK to register remote transform functions. These are currently all prefixed 'X' to show they're experimental.

These transform functions will run even for resources created inside MLCs.

Checklist

I have run make tidy to update any new dependencies
I have run make lint to verify my code passes the lint check
- I have formatted my code using gofumpt

I have added tests that prove my fix is effective or that my feature works

I have run make changelog and committed the changelog/pending/<file> documenting my change

Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

pulumi-bot · 2024-02-28T12:40:31Z

Changelog

[uncommitted] (2024-03-06)

Features

[sdk/nodejs] Add experimental support to the NodeJS SDK for the new transforms system.
#15532

Frassle · 2024-02-28T12:40:49Z

sdk/nodejs/cmd/run/run.ts

@@ -487,7 +487,8 @@ ${defaultMessage}`,
    };

    // Construct a `Stack` resource to represent the outputs of the program.
-    const stackOutputs = stack.runInPulumiStack(runProgram);
+    const stackOutputs = await stack.runInPulumiStack(runProgram);
+    await settings.disconnect()


We need to disconnect here so that the callback server can be shutdown, otherwise it leaves the nodejs event loop forever running and we never hit process.on("exit", settings.disconnectSync);

Frassle · 2024-02-28T12:41:44Z

sdk/nodejs/resource.ts

+ * of the original call to the `Resource` constructor.  If the transform returns undefined,
+ * this indicates that the resource will not be transformed.
+ */
+export type ResourceTransform = (args: ResourceTransformArgs) => Promise<ResourceTransformResult | undefined>;


Made this a promise because it's running async anyway so if people want to do async work inside transforms they can easily do so now.

But this is now one more thing we're going to have to note in the migration guide -- that you basically have to use an async function now or return a promise.

Should we make it not return Promise or make it optionally return promise?

...9--sdk-nodejs--add-experimental-support-to-the-nodejs-sdk-for-the-new-transforms-system.yaml

sdk/nodejs/cmd/run/run.ts

sdk/nodejs/runtime/callbacks.ts

tgummerer · 2024-03-04T12:39:06Z

sdk/nodejs/runtime/callbacks.ts

+                response.setProperties(gstruct.Struct.fromJavaScript(sprops));
+
+                // Copy the options over.
+                if (result.opts !== undefined) {


This seems a bit worrying. I don't know a lot of typescript, but having to copy every option here manually seems quite brittle for the future, in case we ever add any resource options.

Indeed but it's changing from one domain to another, we have to do this manually :(

I do worry about this also. When adding a new resource option there's a bunch of places that need to be kept up-to-date, this now being one of them. Wondering if there's a way to make that more obvious. Perhaps a comment on the ResourceOptions at a minimum?

sdk/nodejs/resource.ts

sdk/nodejs/runtime/mocks.ts

justinvp · 2024-03-06T14:26:22Z

sdk/nodejs/resource.ts

+ * of the original call to the `Resource` constructor.  If the transform returns undefined,
+ * this indicates that the resource will not be transformed.
+ */
+export type ResourceTransform = (args: ResourceTransformArgs) => Promise<ResourceTransformResult | undefined>;


But this is now one more thing we're going to have to note in the migration guide -- that you basically have to use an async function now or return a promise.

Should we make it not return Promise or make it optionally return promise?

sdk/nodejs/runtime/callbacks.ts

justinvp · 2024-03-06T14:35:51Z

sdk/nodejs/runtime/callbacks.ts

+                response.setProperties(gstruct.Struct.fromJavaScript(sprops));
+
+                // Copy the options over.
+                if (result.opts !== undefined) {


I do worry about this also. When adding a new resource option there's a bunch of places that need to be kept up-to-date, this now being one of them. Wondering if there's a way to make that more obvious. Perhaps a comment on the ResourceOptions at a minimum?

sdk/nodejs/runtime/callbacks.ts

tests/integration/transforms/nodejs/simple/Pulumi.yaml

tests/integration/transforms/nodejs/simple/package.json

tests/integration/transforms/transforms_nodejs_test.go

sdk/nodejs/runtime/callbacks.ts

pulumi.sln

sdk/nodejs/runtime/rpc.ts

sdk/nodejs/runtime/callbacks.ts

Draft changelog: ### Features - [auto/{go,nodejs,python}] Add support for suppress progress and suppress outputs parameters in the Automation API [#15596](#15596) - [pkg] Make schema.NewPluginLoader respect PULUMI_DEBUG_PROVIDERS, which enables Pulumi YAML programs to work correctly with this feature [#15526](#15526) - [sdk/dotnet] Update dotnet language host to 3.60.0 [#15609](#15609) - [sdk/nodejs] Add experimental support to the NodeJS SDK for the new transforms system. [#15532](#15532) - [sdk/python] Add support for asynchronous invokes via a new `invoke_async` function [#15602](#15602) - [sdkgen/dotnet] Support for non-overlay components in codegen for pulumi-kubernetes provider [#15490](#15490) ### Bug Fixes - [cli] Fix a panic when the secrets provider is missing from the deployment snapshot [#15599](#15599) - [backend/service] Make decrypt/encrypt network calls retryable to help work around network hiccups [#15600](#15600) - [cli/new] Strip credentials and query strings from template URLs saved to project [#15586](#15586) - [engine] Fix an issue where snapshots could become invalid when doing a targeted up [#15476](#15476) - [pkg/testing] Make ProgramTest use a temporary PULUMI_HOME for each test [#15568](#15568) - [sdkgen/dotnet] Codegen fix for resources without constant input properties [#15488](#15488) - [sdk/nodejs] Properly capture node:crypto and global.crypto in node 19+ - [sdk/python] Fix determining plugins for old packages in the Python language host [#15576](#15576)

Frassle force-pushed the fraser/nodejsTransform branch from de9102d to d526b6f Compare February 28, 2024 12:42

Frassle commented Feb 28, 2024

View reviewed changes

Frassle force-pushed the fraser/nodejsTransform branch 7 times, most recently from 55edde6 to 0fa1875 Compare March 4, 2024 08:57

Frassle marked this pull request as ready for review March 4, 2024 08:59

Frassle requested a review from a team as a code owner March 4, 2024 08:59

tgummerer reviewed Mar 4, 2024

View reviewed changes

Frassle force-pushed the fraser/nodejsTransform branch from 1455c54 to dbb8950 Compare March 4, 2024 17:34

tgummerer approved these changes Mar 5, 2024

View reviewed changes

Frassle added this pull request to the merge queue Mar 5, 2024

justinvp reviewed Mar 5, 2024

View reviewed changes

sdk/nodejs/resource.ts Show resolved Hide resolved

sdk/nodejs/runtime/mocks.ts Outdated Show resolved Hide resolved

Frassle removed this pull request from the merge queue due to a manual request Mar 5, 2024

Frassle force-pushed the fraser/nodejsTransform branch 2 times, most recently from 7405297 to e364ce2 Compare March 6, 2024 08:30

Frassle enabled auto-merge March 6, 2024 08:46

Frassle added this pull request to the merge queue Mar 6, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 6, 2024

Frassle added this pull request to the merge queue Mar 6, 2024

Frassle removed this pull request from the merge queue due to a manual request Mar 6, 2024

justinvp reviewed Mar 6, 2024

View reviewed changes

sdk/nodejs/runtime/rpc.ts Outdated Show resolved Hide resolved

justinvp reviewed Mar 6, 2024

View reviewed changes

sdk/nodejs/runtime/callbacks.ts Outdated Show resolved Hide resolved

sdk/nodejs/runtime/callbacks.ts Outdated Show resolved Hide resolved

Frassle force-pushed the fraser/nodejsTransform branch 2 times, most recently from 901ba55 to 24601e6 Compare March 6, 2024 16:43

Frassle force-pushed the fraser/nodejsTransform branch 2 times, most recently from 47ca169 to 7dceb54 Compare March 6, 2024 19:16

NodeJS transforms

65c11d3

Frassle force-pushed the fraser/nodejsTransform branch from 7dceb54 to 65c11d3 Compare March 6, 2024 19:17

Frassle added this pull request to the merge queue Mar 7, 2024

Merged via the queue into master with commit a183992 Mar 7, 2024
48 checks passed

Frassle deleted the fraser/nodejsTransform branch March 7, 2024 10:23

julienp mentioned this pull request Mar 7, 2024

Remove stray console.log #15616

Merged

6 tasks

justinvp mentioned this pull request Mar 7, 2024

Freeze v3.109.0 #15620

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NodeJS transforms #15532

NodeJS transforms #15532

Frassle commented Feb 28, 2024 •

edited

pulumi-bot commented Feb 28, 2024 •

edited

Frassle Feb 28, 2024

Frassle Feb 28, 2024

justinvp Mar 6, 2024

tgummerer Mar 4, 2024

Frassle Mar 4, 2024

justinvp Mar 6, 2024

justinvp Mar 6, 2024

justinvp Mar 6, 2024

NodeJS transforms #15532

NodeJS transforms #15532

Conversation

Frassle commented Feb 28, 2024 • edited

Description

Checklist

pulumi-bot commented Feb 28, 2024 • edited

Changelog

[uncommitted] (2024-03-06)

Features

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Frassle commented Feb 28, 2024 •

edited

pulumi-bot commented Feb 28, 2024 •

edited