Skip to content

Commit

Permalink
[sdk/nodejs] Fix provider for resource methods
Browse files Browse the repository at this point in the history
The `Resource` class in the Node.js SDK has the following internal
property:

```typescript
/** @internal */
readonly __prov?: ProviderResource;
```

When a resource is created, the provider specified for the resource is
stored in this property. If it is set, it is passed along in the `Call`
request when a method is called on the resource.

Prior to #13282, the property was only set for custom resources in
`Resource`'s constructor:

```typescript
this.__prov = custom ? opts.provider : undefined;
```

With #13282, it was changed to also store the value for remote
components:

```diff
- this.__prov = custom ? opts.provider : undefined;
+ this.__prov = custom || remote ? opts.provider : undefined;
```

This regressed the behavior when calling a method on a remote component
that had an explicit provider that wasn't the component provider,
but some other provider (e.g. AWS provider) specified as:

```typescript
const component = new MyRemoteComponent("comp", {
}, { provider: awsProvider });
```

The `awsProvider` was being stored in `Resource.__prov`, and when making
the method call on the resource, it would try to invoke `Call` on the
AWS provider, rather than calling the remote component provider's
`Call`, which resulted in an error.

Note that specifying the AWS provider using the more verbose
`providers: [awsProvider]` works around the issue.

The fix is to only set `__prov` if the provider's package is the same as
the resource's package. Otherwise, don't set it, because the user is
specifying a provider with the `provider: awsProvider` syntax as
shorthand for `providers: [awsProvider]`.
  • Loading branch information
justinvp committed Aug 30, 2023
1 parent ed2ea3e commit 26769af
Show file tree
Hide file tree
Showing 33 changed files with 1,146 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changes:
- type: fix
scope: sdk/nodejs
description: Fix provider used for resource methods
23 changes: 20 additions & 3 deletions sdk/nodejs/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,9 @@ export abstract class Resource {
private readonly __providers: Record<string, ProviderResource>;

/**
* The specified provider or provider determined from the parent for custom resources.
* The specified provider or provider determined from the parent for custom or remote resources.
* It is passed along in the `Call` gRPC request for resource method calls (when set) so that the
* call goes to the same provider as the resource.
* @internal
*/
// Note: This is deliberately not named `__provider` as that conflicts with the property
Expand Down Expand Up @@ -439,12 +441,13 @@ export abstract class Resource {
...convertToProvidersMap(opts.provider ? [opts.provider] : {}),
};

const pkg = pkgFromType(t);

// provider is the first option that does not return none
// 1. opts.provider
// 2. a matching provider in opts.providers
// 3. a matching provider inherited from opts.parent
if ((custom || remote) && opts.provider === undefined) {
const pkg = pkgFromType(t);
const parentProvider = parent?.getProvider(t);

if (pkg && pkg in this.__providers) {
Expand All @@ -454,8 +457,22 @@ export abstract class Resource {
}
}

// Custom and remote resources have a backing provider. If this is a custom or
// remote resource and a provider has been specified that has the same package
// as the resource's package, save it in `__prov`.
// If the provider's package isn't the same as the resource's package, don't
// save it in `__prov` because the user specified `provider: someProvider` as
// shorthand for `providers: [someProvider]`, which is a provider intended for
// the component's children and not for this resource itself.
// `__prov` is passed along in `Call` gRPC requests for resource method calls
// (when set) so that the call goes to the same provider as the resource.
if ((custom || remote) && opts.provider) {
if (pkg && pkg === opts.provider.getPackage()) {
this.__prov = opts.provider;
}
}

this.__protect = !!opts.protect;
this.__prov = custom || remote ? opts.provider : undefined;
this.__version = opts.version;
this.__pluginDownloadURL = opts.pluginDownloadURL;

Expand Down
62 changes: 62 additions & 0 deletions sdk/nodejs/tests/resource.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,31 @@ describe("DependencyProviderResource", () => {
});
});

describe("CustomResource", () => {
runtime.setMocks({
call: (_) => {
throw new Error("unexpected call");
},
newResource: (args) => {
return { id: `${args.name}_id`, state: {} };
},
});

// https://github.com/pulumi/pulumi/issues/13777
it("saves provider with same package as the resource in __prov", async () => {
const provider = new MyProvider("prov");
const custom = new MyCustomResource("custom", { provider: provider });
assert.strictEqual(custom.__prov, provider);
});

// https://github.com/pulumi/pulumi/issues/13777
it("does not save provider with different package as the resource in __prov", async () => {
const provider = new MyOtherProvider("prov");
const custom = new MyCustomResource("custom", { provider: provider });
assert.strictEqual(custom.__prov, undefined);
});
});

describe("ComponentResource", () => {
runtime.setMocks({
call: (_) => {
Expand Down Expand Up @@ -198,18 +223,55 @@ describe("ComponentResource", () => {
});
});

describe("RemoteComponentResource", () => {
runtime.setMocks({
call: (_) => {
throw new Error("unexpected call");
},
newResource: (args) => {
return { id: `${args.name}_id`, state: {} };
},
});

// https://github.com/pulumi/pulumi/issues/13777
it("saves provider with same package as the resource in __prov", async () => {
const provider = new MyProvider("prov");
const comp = new MyRemoteComponentResource("comp", { provider: provider });
assert.strictEqual(comp.__prov, provider);
});

// https://github.com/pulumi/pulumi/issues/13777
it("does not save provider with different package as the resource in __prov", async () => {
const provider = new MyOtherProvider("prov");
const comp = new MyRemoteComponentResource("comp", { provider: provider });
assert.strictEqual(comp.__prov, undefined);
});
});

class MyProvider extends ProviderResource {
constructor(name: string) {
super("test", name);
}
}

class MyOtherProvider extends ProviderResource {
constructor(name: string) {
super("other", name);
}
}

class MyCustomResource extends CustomResource {
constructor(name: string, opts?: CustomResourceOptions) {
super("test:index:MyCustomResource", name, {}, opts);
}
}

class MyRemoteComponentResource extends ComponentResource {
constructor(name: string, opts?: ComponentResourceOptions) {
super("test:index:MyRemoteComponentResource", name, {}, opts, true /*remote*/);
}
}

// Regression test for https://github.com/pulumi/pulumi/issues/12032
describe("parent and dependsOn are the same 12032", () => {
runtime.setMocks({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name: construct_component_methods_provider_go
description: A program that constructs remote component resources with methods and an explicit provider set for another package.
runtime: go
66 changes: 66 additions & 0 deletions tests/integration/construct_component_methods_provider/go/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
module github.com/pulumi/pulumi/tests/construct_component_methods_provider

go 1.18

require github.com/pulumi/pulumi/sdk/v3 v3.59.0

require (
github.com/Microsoft/go-winio v0.5.2 // indirect
github.com/ProtonMail/go-crypto v0.0.0-20221026131551-cf6655e29de4 // indirect
github.com/acomagu/bufpipe v1.0.3 // indirect
github.com/aead/chacha20 v0.0.0-20180709150244-8b13a72661da // indirect
github.com/blang/semver v3.5.1+incompatible // indirect
github.com/cheggaaa/pb v1.0.29 // indirect
github.com/cloudflare/circl v1.3.3 // indirect
github.com/djherbis/times v1.5.0 // indirect
github.com/emirpasic/gods v1.18.1 // indirect
github.com/go-git/gcfg v1.5.0 // indirect
github.com/go-git/go-billy/v5 v5.4.0 // indirect
github.com/go-git/go-git/v5 v5.6.0 // indirect
github.com/gofrs/uuid v4.2.0+incompatible // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/glog v1.1.0 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/grpc-ecosystem/grpc-opentracing v0.0.0-20180507213350-8e809c8a8645 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
github.com/imdario/mergo v0.3.13 // indirect
github.com/inconshreveable/mousetrap v1.0.1 // indirect
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
github.com/kevinburke/ssh_config v1.2.0 // indirect
github.com/mattn/go-isatty v0.0.14 // indirect
github.com/mattn/go-runewidth v0.0.13 // indirect
github.com/mitchellh/go-ps v1.0.0 // indirect
github.com/opentracing/basictracer-go v1.1.0 // indirect
github.com/opentracing/opentracing-go v1.2.0 // indirect
github.com/pjbgf/sha1cd v0.3.0 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pkg/term v1.1.0 // indirect
github.com/rivo/uniseg v0.2.0 // indirect
github.com/rogpeppe/go-internal v1.9.0 // indirect
github.com/sabhiram/go-gitignore v0.0.0-20210923224102-525f6e181f06 // indirect
github.com/santhosh-tekuri/jsonschema/v5 v5.0.0 // indirect
github.com/sergi/go-diff v1.2.0 // indirect
github.com/skeema/knownhosts v1.1.0 // indirect
github.com/spf13/cobra v1.6.1 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/stretchr/objx v0.5.0 // indirect
github.com/texttheater/golang-levenshtein v1.0.1 // indirect
github.com/tweekmonster/luser v0.0.0-20161003172636-3fa38070dbd7 // indirect
github.com/uber/jaeger-client-go v2.30.0+incompatible // indirect
github.com/uber/jaeger-lib v2.4.1+incompatible // indirect
github.com/xanzy/ssh-agent v0.3.3 // indirect
go.uber.org/atomic v1.9.0 // indirect
golang.org/x/crypto v0.3.1-0.20221117191849-2c476679df9a // indirect
golang.org/x/net v0.10.0 // indirect
golang.org/x/sys v0.8.0 // indirect
golang.org/x/term v0.8.0 // indirect
golang.org/x/text v0.9.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20230706204954-ccb25ca9f130 // indirect
google.golang.org/grpc v1.57.0 // indirect
google.golang.org/protobuf v1.31.0 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
lukechampine.com/frand v1.4.2 // indirect
sourcegraph.com/sourcegraph/appdash v0.0.0-20211028080628-e2786a622600 // indirect
)

0 comments on commit 26769af

Please sign in to comment.