[helm/v3] Pass provider options to helm invokes #1919
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
|
CC @guineveresaenger who is planning on taking this over. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
|
@viveklak @guineveresaenger can I help get this merged? |
Sorry for the delay in responding @rawkode. I think we are waiting on an implementation for dotnet at the moment. I do think we can merge this as is and follow up with the SDK version too. @guineveresaenger thoughts? |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
|
Hi! This fell off my radar, and I apologize. I can take a stab at this tomorrow, but if you think we can merge as-is and that would unblock you @RawCode, that's fine by me. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
2 similar comments
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
I think we can merge as-is for now. We have more work to be done here than just helm chart for Dotnet anyway. We still need to reflect the option to ConfigFile and ConfigGroup as well. I will open a separate ticket to track those once this is merged. |
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
for Pulumi's contribution guidelines.
Help us merge your changes more quickly by adding more details such
as labels, milestones, and reviewers.-->
### 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](https://www.pulumi.com/docs/concepts/options/) 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](pulumi/pulumi#8969)). |
| `import` | no | |
| `parent` | **yes** | The component becomes the parent. |
| `protect` | no | Inherited (see [p/p
bug](pulumi/pulumi#12431)). |
| `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.<br/>Should propagate when child resource is from the
same provider type. |
| `pluginDownloadURL` | **yes** | Influences default provider selection
logic during invokes.<br/>Should propagate when child resource is from
the same provider type. |
### Testing
A new test case is provided ([test
case](https://github.com/pulumi/pulumi-kubernetes/blob/073b9dc64e32e4f14fa6691e1b11049007ca2db7/tests/sdk/go/go_test.go#L808),
[test
program](https://github.com/pulumi/pulumi-kubernetes/blob/073b9dc64e32e4f14fa6691e1b11049007ca2db7/tests/sdk/go/options/main.go))
that exercises option propagation across the component resources:
-
[kubernetes.helm.sh.v3.Chart](https://www.pulumi.com/registry/packages/kubernetes/api-docs/helm/v3/chart/)
-
[kubernetes.kustomize.Directory](https://www.pulumi.com/registry/packages/kubernetes/api-docs/kustomize/directory/)
-
[kubernetes.yaml.ConfigGroup](https://www.pulumi.com/registry/packages/kubernetes/api-docs/yaml/configgroup/)
-
[kubernetes.yaml.ConfigFile](https://www.pulumi.com/registry/packages/kubernetes/api-docs/yaml/configfile/)
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:
- pulumi/pulumi#8796
- #1601
- #1919
- #2005
We still need to add support for dotnet and make similar changes for ConfigGroup and ConfigFile as well. I will open a separate issue to track those.
Proposed changes
Related issues (optional)
Fixes #1912