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

Component option propagation (Go SDK) #2709

Merged
merged 3 commits into from
Jan 3, 2024
Merged

Conversation

EronWright
Copy link
Contributor

@EronWright EronWright commented Dec 12, 2023

Proposed changes

Epic: #2254
Fixes: #2710

This PR standardizes the option propagation logic for the component resources in the pulumi-kubernetes Go SDK. The general approach is:

  1. In the component resource constructor, compute the child options to be propagated to any children. The child options consist of the component as parent, and with version and pluginDownloadURL if specified.
  2. Compute the invoke options by copying the child options.

Specification

The component resource is responsible for computing sub-options for invokes and for child resource declarations. This table outlines the expected behavior for each resource option when presented to a component resource.

Propagated Remarks
additionalSecretOutputs no "does not apply to component resources"
aliases no Inherited via parent-child relationship.
customTimeouts no "does not apply to component resources"
deleteBeforeReplace no
deletedWith no
dependsOn no The children implicitly wait for the dependency.
ignoreChanges no Nonsensical to apply directly to children (see discussion).
import no
parent yes The component becomes the parent.
protect no Inherited (see p/p bug).
provider no Combined into providers map, then inherited via parent-child relationship.
providers no Inherited.
replaceOnChanges no "does not apply to component resources"
retainOnDelete no "does not apply to component resources"
transformations no Inherited.
version yes Influences default provider selection logic during invokes.
Should propagate when child resource is from the same provider type.
pluginDownloadURL yes Influences default provider selection logic during invokes.
Should propagate when child resource is from the same provider type.

Testing

A new test case is provided (test case, test program) that exercises option propagation across the component resources:

Upgrade testing must be done manually, with an emphasis on avoiding replacement due to reparenting.

Related issues (optional)

The Go SDK doesn't allow for options to be mutated via the "kubernetes-style" transformation function.
#2666

During testing it was observed that the parent field is occasionally missing from RegisterResource RPC call.
pulumi/pulumi#14826

Here's some key related PRs for additional context:

Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@EronWright EronWright changed the title Option propagation (Go SDK) Standardized option propagation (Go SDK) Dec 12, 2023
@EronWright EronWright changed the title Standardized option propagation (Go SDK) Component option propagation (Go SDK) Dec 13, 2023
tests/gomega/matchers.go Show resolved Hide resolved
tests/pulumirpc/log.go Outdated Show resolved Hide resolved
tests/sdk/go/go_test.go Show resolved Hide resolved
Copy link
Contributor

@rquitales rquitales left a comment

Choose a reason for hiding this comment

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

The code looks pretty good to me, but I'll defer the final approval to Levi since he is more familiar with overlays. One suggestion I'd make is to break up the PR into either multiple PRs or commits at the very least. It looks like this PR does a few things, add more test capabilities and utility functions as well as actual logic updates. Having all these squashed in 1 commit makes it harder to review, especially since some of these files are autogenerated.

I'd even go as far as to suggest separating the test changes to its own PR. This way, if we ever did need to revert this PR, the test improvements won't be reverted together.

@EronWright EronWright merged commit c9fa74e into master Jan 3, 2024
18 checks passed
@EronWright EronWright deleted the eronwright/issue-2254-go branch January 3, 2024 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[sdk/go] ConfigFile doesn't respect SkipAwait argument
3 participants