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

yaml.ConfigFile doesn't respect providers in opts #2254

Closed
antdking opened this issue Dec 6, 2022 · 6 comments
Closed

yaml.ConfigFile doesn't respect providers in opts #2254

antdking opened this issue Dec 6, 2022 · 6 comments
Assignees
Labels
impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec mro2 Monica's list of 2st tier overlay related issues
Milestone

Comments

@antdking
Copy link

antdking commented Dec 6, 2022

What happened?

Attempted to migrate to using providers=[...] for our resource options, however this isn't propagated to the invoke for yaml.ConfigFile.
This results in a failure, as we have default providers disabled.

Steps to reproduce

with config: pulumi:disable-default-providers: [kubernetes]

import pulumi as p
import pulumi_kubernetes as k8s

k8s_provider = k8s.Provider("gke")
default_opts = p.ResourceOptions(
  providers=[
    k8s_provider,
    # ... other providers
  ],
)

# use cert-manager as an example
k8s.yaml.ConfigFile(
  "cert-manager",
  file="https://github.com/cert-manager/cert-manager/releases/download/v1.9.1/cert-manager.yaml",
  opts=default_opts,
)

Expected Behavior

For yaml.ConfigFile to be invoked with the "kubernetes" provider defined in the providers list

Actual Behavior

Attempts to invoke with the default "kubernetes" provider

Output of pulumi about

$ pulumi about
CLI          
Version      3.48.0
Go Version   go1.19.2
Go Compiler  gc

Plugins
NAME           VERSION
command        0.6.0
gcp            6.44.0
google-native  0.27.0
kubernetes     3.22.1
python         unknown

Host     
OS       arch
Version  22.0.0
Arch     x86_64

This project is written in python: executable='/home/anthony/.pyenv/shims/python3' version='3.10.6
'

Current Stack: datapane/test

TYPE                                                                       URN
<redacted>
pulumi:providers:kubernetes                                                urn:pulumi:test::dp-infra::gcp:container/cluster:Cluster$pulumi:providers:kubernetes::gke
kubernetes:yaml:ConfigFile                                                 urn:pulumi:test::dp-infra::kubernetes:yaml:ConfigFile::cert-manager

Found no pending operations associated with datapane/test

Backend        
Name           pulumi.com
URL            https://app.pulumi.com/antdking
User           antdking
Organizations  antdking, datapane, antdking-testing

Dependencies:
NAME                     VERSION
bump2version             1.0.1
cloudflare               2.10.1
datapane                 0.15.5
docker                   6.0.1
environs                 9.5.0
google-cloud-monitoring  2.11.1
grpcio-status            1.47.0
invoke                   1.7.1
joblib                   1.1.0
kubernetes               24.2.0
mypy                     0.971.0
pip                      22.3.0
pre-commit               2.20.0
pulumi-command           0.6.0
pulumi-gcp               6.44.0
pulumi-google-native     0.27.0
pulumi-kubernetes        3.22.1
semgrep                  0.122.0
sh                       1.14.3
wheel                    0.37.1

Pulumi locates its logs in /tmp by default

not sure why the python sdk version didn't show:

$ pip show
Name: pulumi
Version: 3.48.0

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).

@antdking antdking added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Dec 6, 2022
@roothorp
Copy link

roothorp commented Dec 7, 2022

Hi @antdking, thanks for the issue! I've raised this with the team to be prioritized and added to our work stack.

@roothorp roothorp added impact/usability Something that impacts users' ability to use the product easily and intuitively and removed needs-triage Needs attention from the triage team labels Dec 7, 2022
@antdking
Copy link
Author

antdking commented Dec 7, 2022

Thanks, @roothorp

I did a little digging earlier, and tested a fix locally.
It looks as if provider is passed explicitly to the InvokeOptions here

The fixed I used was to pass parent=self in instead, which will rely on the Resource.get_provider functionality.

ie:

invoke_opts = pulumi.InvokeOptions(version=_utilities.get_version(), parent=self)

In the nodejs + python sdks, this looks to give the intended behaviour.

I was going to provide a PR to patch all languages; however I'm unable to get tests running locally, so won't be contributing more today

@roothorp
Copy link

roothorp commented Dec 7, 2022

That's brilliant, thank you! I'll pass that along to the team.

@mnlumi mnlumi added the mro2 Monica's list of 2st tier overlay related issues label Mar 28, 2023
bacek added a commit to bacek/pulumi-kubernetes that referenced this issue May 11, 2023
Proposed changes to fix pulumi#2254
@bacek bacek mentioned this issue May 11, 2023
@EronWright EronWright added this to the 0.95 milestone Oct 2, 2023
@EronWright

This comment was marked as resolved.

@EronWright

This comment was marked as outdated.

@lukehoban lukehoban modified the milestones: 0.97, 0.98 Dec 20, 2023
EronWright added a commit that referenced this issue Jan 3, 2024
<!--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
EronWright added a commit that referenced this issue Jan 3, 2024
<!--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: #1938
Fixes: #2049
Fixes: #812

This PR standardizes the option propagation logic for the component
resources in the pulumi-kubernetes NodeJS 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. |
| `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/e1ad541a9c86e8eb207bd1d8a276822862425109/tests/sdk/nodejs/nodejs_test.go#L2107),
[test
program](https://github.com/pulumi/pulumi-kubernetes/blob/e1ad541a9c86e8eb207bd1d8a276822862425109/tests/sdk/nodejs/options/index.ts))
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 p/p NodeJS SDK doesn't propagate the `pluginDownloadURL` option to
the Invoke RPC.
pulumi/pulumi#14839
EronWright added a commit that referenced this issue Jan 4, 2024
<!--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: #1214
Fixes: #2345

This PR standardizes the option propagation logic for the component
resources in the pulumi-kubernetes .NET 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. |
| `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/db3cd71f3e5780bfe0bf4584d667b10e446761f7/tests/sdk/dotnet/dotnet_test.go#L316),
[test
program](https://github.com/pulumi/pulumi-kubernetes/blob/db3cd71f3e5780bfe0bf4584d667b10e446761f7/tests/sdk/dotnet/options/Program.cs))
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.

### Upgrade Considerations
In previous versions, the `ConfigFile` and `ConfigGroup` component
resources inadvertently assigned the wrong parent to the child
resource(s). This would happen when the component resource itself had a
parent; the child would be assigned that same parent. This also had the
effect of disregarding the component resource's provider in favor of the
parent's provider.

For example, here's a before/after look at the component hierarchy:
Before:
```
├─ pkg:index:MyComponent            parent                                               
│  ├─ kubernetes:core/v1:ConfigMap  cg-options-cg-options-cm-1                           
│  ├─ kubernetes:yaml:ConfigFile    cg-options-testdata/options/configgroup/manifest.yaml
│  ├─ kubernetes:core/v1:ConfigMap  cg-options-configgroup-cm-1                          
│  ├─ kubernetes:yaml:ConfigFile    cg-options-testdata/options/configgroup/empty.yaml   
│  └─ kubernetes:yaml:ConfigGroup   cg-options                                                                                     
```
After:
```
└─ pkg:index:MyComponent                  parent                                               
   └─ kubernetes:yaml:ConfigGroup         cg-options                                           
      ├─ kubernetes:yaml:ConfigFile       cg-options-testdata/options/configgroup/manifest.yaml
      │  └─ kubernetes:core/v1:ConfigMap  cg-options-configgroup-cm-1                          
      └─ kubernetes:core/v1:ConfigMap     cg-options-cg-options-cm-1      
```

This PR addresses this issue and attempts to heal existing stacks using
aliases. This is effective at avoiding a replacement except in the case
where the child was created with the wrong provider. In this case,
**Pulumi will suggest a replacement of the child resource(s)**, such
that they use the correct provider.

### Related issues (optional)
@EronWright
Copy link
Contributor

Posted an announcement to the community slack:
https://pulumi-community.slack.com/archives/CB36DSVSA/p1704325722193319

EronWright added a commit that referenced this issue Jan 4, 2024
<!--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: #2356

This PR standardizes the option propagation logic for the component
resources in the pulumi-kubernetes Python 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. |
| `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/183db21e939ac960f3474859138c0b13985fc627/tests/sdk/python/python_test.go#L629),
[test
program](https://github.com/pulumi/pulumi-kubernetes/blob/183db21e939ac960f3474859138c0b13985fc627/tests/sdk/python/options/__main__.py))
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/usability Something that impacts users' ability to use the product easily and intuitively kind/bug Some behavior is incorrect or out of spec mro2 Monica's list of 2st tier overlay related issues
Projects
None yet
7 participants