Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading sdk form 3.43.1 to 3.46.1 throws eror #11316

Closed
rshade opened this issue Nov 9, 2022 · 3 comments · Fixed by #11509
Closed

Upgrading sdk form 3.43.1 to 3.46.1 throws eror #11316

rshade opened this issue Nov 9, 2022 · 3 comments · Fixed by #11509
Assignees
Labels
area/sdks Pulumi language SDKs impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec language/javascript resolution/fixed This issue was fixed
Milestone

Comments

@rshade
Copy link
Contributor

rshade commented Nov 9, 2022

What happened?

Automated testing against upgrading from 3.43.1 -> 3.46.1 thru the following error:

[stdout]          stderr: error: Running program '/tmp/binding-directory2497179436' failed with an unhandled exception:
[stdout]          Error: failed to register new resource example--s3-adv-log-bucket [example:aws/s3:S3Bucket]: 2 UNKNOWN: plugins that can construct components must support secrets
[stdout]              at Object.registerResource (/tmp/binding-directory2497179436/node_modules/@pulumi/runtime/resource.ts:293:27)
[stdout]              at new Resource (/tmp/binding-directory2497179436/node_modules/@pulumi/resource.ts:402:13)
[stdout]              at new ComponentResource (/tmp/binding-directory2497179436/node_modules/@pulumi/resource.ts:895:9)
[stdout]              at new S3Bucket (/tmp/binding-directory2497179436/node_modules/@example/aws/s3/S3Bucket.ts:99:9)
[stdout]              at /tmp/binding-directory2497179436/src/index.ts:70:31
[stdout]              at Generator.next (<anonymous>)
[stdout]              at fulfilled (/tmp/binding-directory2497179436/src/index.ts:5:58)
[stdout]              at processTicksAndRejections (node:internal/process/task_queues:96:5)
[stdout]          error: Running program '/tmp/binding-directory2497179436' failed with an unhandled exception:
[stdout]          Error: failed to register new resource example--s3-simple-bucket [example:aws/s3:S3Bucket]: 2 UNKNOWN: plugins that can construct components must support secrets
[stdout]              at Object.registerResource (/tmp/binding-directory2497179436/node_modules/@pulumi/runtime/resource.ts:293:27)
[stdout]              at new Resource (/tmp/binding-directory2497179436/node_modules/@pulumi/resource.ts:402:13)
[stdout]              at new ComponentResource (/tmp/binding-directory2497179436/node_modules/@pulumi/resource.ts:895:9)
[stdout]              at new S3Bucket (/tmp/binding-directory2497179436/node_modules/@example/aws/s3/S3Bucket.ts:99:9)
[stdout]              at /tmp/binding-directory2497179436/src/index.ts:49:26
[stdout]              at Generator.next (<anonymous>)
[stdout]              at fulfilled (/tmp/binding-directory2497179436/src/index.ts:5:58)
[stdout]              at processTicksAndRejections (node:internal/process/task_queues:96:5)

Steps to reproduce

  1. Using the existing test infrastructure upgrade from 3.43.1 to 3.46.1
  2. Only raises an issue in typescript, not python or go.

Expected Behavior

  1. error not to be thrown

Actual Behavior

  1. Error thrown.

Output of pulumi about

No response

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@rshade rshade added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Nov 9, 2022
@kpitzen kpitzen added area/sdks Pulumi language SDKs language/javascript and removed needs-triage Needs attention from the triage team labels Nov 10, 2022
@RobbieMcKinstry
Copy link
Contributor

@rshade Could you link us to the automated test that failed? Is the code sample that caused the failure available?

@justinvp
Copy link
Member

@RobbieMcKinstry, there's a repro in pulumi/pulumi-awsx#948, if that helps. I still haven't figured out what's going on. It appears to happen when passing an explicit provider when creating an MLC. Interestingly, if the provider is passed using providers: rather than provider:, the program works without the error.

@justinvp justinvp assigned justinvp and unassigned RobbieMcKinstry Dec 1, 2022
@justinvp
Copy link
Member

justinvp commented Dec 1, 2022

This is a regression in the Node.js SDK introduced by #11093.

let providerRef: string | undefined;
let importID: ID | undefined;
if (custom || remote) {
const customOpts = <CustomResourceOptions>opts;
importID = customOpts.import;
providerRef = await ProviderResource.register(opts.provider);
}

#11093 added the || remote to the if, which, in this case, causes the engine to call Construct (to create the Vpc MLC component) on the specified aws provider, rather than the intended awsx provider.

@justinvp justinvp added the impact/regression Something that used to work, but is now broken label Dec 1, 2022
bors bot added a commit that referenced this issue Dec 2, 2022
11509: [sdk/nodejs] Fix regression when passing a provider to a MLC r=justinvp a=justinvp

#11093 changed the Node.js SDK to pass a provider specified in a MLC's `ResourceOptions.provider` to the engine.

Unfortunately, this regresses behavior that existing programs rely on. For example:

```ts
import * as aws from "`@pulumi/aws";`
import * as awsx from "`@pulumi/awsx";`

const myRegion = new aws.Provider("us-east-1", {
  region: "us-east-1",
});

const vpc = new awsx.ec2.Vpc("awsx-nodejs-default-args", {}, { provider: myRegion });
```

In the above program, an explicit _aws_ provider is being passed to the _aws**x**_ `VPC` component, with the intention that the _aws_ provider will be used as the provider for all of `Vpc`'s children.

With the change in #11093, the engine would try to call `Construct` for the `Vpc` using the specified `aws` provider, which does not work (it fails with `plugins that can construct components must support secrets`).

This change reverts the problematic change from #11093 and adds a regression test to lock-in the previous behavior.

Note: We do want to be able to support specifying a MLC's provider (to allow explicit providers for MLCs), but we'll address that in a separate change. (I'll open an issue).

Fixes #11316

Co-authored-by: Justin Van Patten <jvp@justinvp.com>
bors bot added a commit that referenced this issue Dec 2, 2022
11509: [sdk/nodejs] Fix regression when passing a provider to a MLC r=justinvp a=justinvp

#11093 changed the Node.js SDK to pass a provider specified in a MLC's `ResourceOptions.provider` to the engine.

Unfortunately, this regresses behavior that existing programs rely on. For example:

```ts
import * as aws from "`@pulumi/aws";`
import * as awsx from "`@pulumi/awsx";`

const myRegion = new aws.Provider("us-east-1", {
  region: "us-east-1",
});

const vpc = new awsx.ec2.Vpc("awsx-nodejs-default-args", {}, { provider: myRegion });
```

In the above program, an explicit _aws_ provider is being passed to the _aws**x**_ `VPC` component, with the intention that the _aws_ provider will be used as the provider for all of `Vpc`'s children.

With the change in #11093, the engine would try to call `Construct` for the `Vpc` using the specified `aws` provider, which does not work (it fails with `plugins that can construct components must support secrets`).

This change reverts the problematic change from #11093 and adds a regression test to lock-in the previous behavior.

Note: We do want to be able to support specifying a MLC's provider (to allow explicit providers for MLCs), but we'll address that in a separate change. (I'll open an issue).

Fixes #11316

Co-authored-by: Justin Van Patten <jvp@justinvp.com>
@bors bors bot closed this as completed in 712145c Dec 2, 2022
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sdks Pulumi language SDKs impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec language/javascript resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants