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.

Sign up for GitHub

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

Jump to bottom

Keep resource refs when invoking pulumi:pulumi:getResource #11382

Merged
merged 1 commit into from Nov 16, 2022
Merged

Keep resource refs when invoking pulumi:pulumi:getResource #11382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...ng/20221116--engine--always-keep-resources-when-pulumi-pulumi-getresource-is-invoked.yaml
View file
Open in desktop
@@ -0,0 +1,4 @@
changes:
- type: fix
scope: engine
description: Always keep resources when pulumi:pulumi:getResource is invoked
4 changes: 4 additions & 0 deletions ...6--sdk-go-nodejs-python--set-acceptresources-when-invoking-pulumi-pulumi-getresource.yaml
View file
Open in desktop
@@ -0,0 +1,4 @@
changes:
- type: fix
scope: sdk/go,nodejs,python
description: Set acceptResources when invoking pulumi:pulumi:getResource
11 changes: 10 additions & 1 deletion pkg/resource/deploy/source_eval.go
View file
Open in desktop
Expand Up @@ -710,11 +710,20 @@ func (rm *resmon) Invoke(ctx context.Context, req *pulumirpc.ResourceInvokeReque
if err != nil {
return nil, fmt.Errorf("invocation of %v returned an error: %w", tok, err)
}

// Respect `AcceptResources` unless `tok` is for the built-in `pulumi:pulumi:getResource` function,
// in which case always keep resources to maintain the original behavior for older SDKs that are not
// setting the `AccceptResources` flag.
keepResources := req.GetAcceptResources()
if tok == "pulumi:pulumi:getResource" {
keepResources = true
}

mret, err := plugin.MarshalProperties(ret, plugin.MarshalOptions{
Label: label,
KeepUnknowns: true,
KeepSecrets: true,
KeepResources: req.GetAcceptResources(),
KeepResources: keepResources,
})
if err != nil {
return nil, fmt.Errorf("failed to marshal %v return: %w", tok, err)
Expand Down
5 changes: 3 additions & 2 deletions sdk/go/pulumi/context.go
View file
Open in desktop
Expand Up @@ -723,8 +723,9 @@ func (ctx *Context) getResource(urn string) (*pulumirpc.RegisterResourceResponse
tok := "pulumi:pulumi:getResource"
logging.V(9).Infof("Invoke(%s, #args=%d): RPC call being made synchronously", tok, len(resolvedArgsMap))
resp, err := ctx.monitor.Invoke(ctx.ctx, &pulumirpc.ResourceInvokeRequest{
Tok: "pulumi:pulumi:getResource",
Args: rpcArgs,
Tok: "pulumi:pulumi:getResource",
Args: rpcArgs,
AcceptResources: !disableResourceReferences,
})
if err != nil {
return nil, fmt.Errorf("invoke(%s, ...): error: %v", tok, err)
Expand Down
1 change: 1 addition & 0 deletions sdk/nodejs/runtime/resource.ts
View file
Open in desktop
Expand Up @@ -114,6 +114,7 @@ export function getResource(res: Resource, parent: Resource | undefined, props:
req.setArgs(gstruct.Struct.fromJavaScript(inputs));
req.setProvider("");
req.setVersion("");
req.setAcceptresources(!utils.disableResourceReferences);

// Now run the operation, serializing the invocation if necessary.
const opLabel = `monitor.getResource(${label})`;
Expand Down
11 changes: 10 additions & 1 deletion sdk/python/lib/pulumi/runtime/resource.py
View file
Open in desktop
Expand Up @@ -243,8 +243,17 @@ async def do_get():

monitor = settings.get_monitor()
inputs = await rpc.serialize_properties({"urn": urn}, {})

accept_resources = not (
os.getenv("PULUMI_DISABLE_RESOURCE_REFERENCES", "").upper()
in {"TRUE", "1"}
)
Comment on lines +247 to +250
Copy link
Member Author

@justinvp justinvp Nov 16, 2022 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's debatable whether we even need to continue respecting PULUMI_DISABLE_RESOURCE_REFERENCES (especially in these new cases), which was used when we were originally rolling out resource refs as a way to be able to turn off the feature if something went wrong during the rollout. We still check the envvar elsewhere, so I'm continuing to use it in these new places. Separately, we should consider removing it throughout as it's not really needed anymore (resource refs have been working fine E2E -- there hasn't been a need to disable them).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets go with this as is for now. Sounds like a good thing to do a cleanup pass on though.

req = resource_pb2.ResourceInvokeRequest(
tok="pulumi:pulumi:getResource", args=inputs, provider="", version=""
tok="pulumi:pulumi:getResource",
args=inputs,
provider="",
version="",
acceptResources=accept_resources,
)

def do_invoke():
Expand Down