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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug (Go): yaml.Transformation does not return options slice #2666

Open
ghostsquad opened this issue Nov 14, 2023 · 12 comments
Open

Bug (Go): yaml.Transformation does not return options slice #2666

ghostsquad opened this issue Nov 14, 2023 · 12 comments
Labels
area/yaml kind/enhancement Improvements or new features

Comments

@ghostsquad
Copy link

ghostsquad commented Nov 14, 2023

Hello!

  • Vote on this issue by adding a 馃憤 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

I'm unsure how to implement ignoreChanges resource option with the yaml.ConfigGroup resource. It would be nice to have an example of this.

EDIT: I was pointed to this: https://www.pulumi.com/registry/packages/kubernetes/api-docs/yaml/configgroup/#yaml-with-transformations

Of which would seemingly allow me to transform a resource's resourceOptions, including the ignoreChanges resourceOption. However, upon further investigation, I noticed that this won't work (in Golang). Slices in Golang are not reference values. Appending to a slice doesn't change the original slice, it makes a new one. The transformation here is not returning the new slice.

// Set a resource alias for a previous name.
func(state map[string]interface{}, opts ...pulumi.ResourceOption) {
	if state["kind"] == "Deployment" {
		aliases := pulumi.Aliases([]pulumi.Alias{
			{
				Name: pulumi.String("oldName"),
			},
		})
		opts = append(opts, aliases)
	}
},

Affected area/feature

yaml.ConfigGroup / Go

@ghostsquad ghostsquad added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Nov 14, 2023
@ghostsquad ghostsquad changed the title Provide examples for how to ignoreChanges for a ConfigGroup Bug (Go): cannot ignoreChanges for a ConfigGroup Nov 16, 2023
@mikhailshilkov

This comment was marked as resolved.

@mikhailshilkov

This comment was marked as resolved.

@mikhailshilkov mikhailshilkov added awaiting-upstream The issue cannot be resolved without action in another repository (may be owned by Pulumi). and removed needs-triage Needs attention from the triage team labels Nov 17, 2023
@ghostsquad

This comment was marked as resolved.

@ghostsquad

This comment was marked as resolved.

@ghostsquad ghostsquad changed the title Bug (Go): cannot ignoreChanges for a ConfigGroup Bug (Go): yaml.Transformation does not return options slice Nov 18, 2023
@ghostsquad
Copy link
Author

@mikhailshilkov alternatively, if there's a way to pass a pulumi.ResourceTransformation to the ConfigGroup that modifies some set of resources that configGroup manages, then that actually means that the yaml.Transformation is redundant. If this scenario is possible, I'd like to know how that can be accomplished with an example.

@EronWright
Copy link
Contributor

@ghostsquad thanks for the report and sorry about the confusion about which type of transformation you're referring to. It's true there's two variations - the general-purpose transformation as described here, and a transformation provided by the pulumi-kubernetes provider as described here. The key difference is that the former presents you with Pulumi resource properties, whereas the latter presents you with the Kubernetes object.

It is also true that the Go SDK for pulumi-kubernetes has a design flaw in that you cannot mutate the resource options within the kubernetes-centric transformation function, despite what the "YAML with Transformations" example shows.

As a workaround, I do think that you could mutate the resource options using the general-purpose transformation.

@ghostsquad
Copy link
Author

@EronWright what I don't totally understand is how that would work. A ConfigGroup generates resources for each k8s object it finds. Could you provide an example of how to use the general-purpose transformation that targets a specific resource within the ConfigGroup?

@EronWright
Copy link
Contributor

EronWright commented Nov 30, 2023

@ghostsquad the general-purpose transformation function that's attached to a given resource is invoked for each child resource (in addition to the resource itself). Please take a look at the example here that shows how to identify the sub-resource you seek to mutate. Also, feel free to reach out to me on the Pulumi Community slack workspace for more assistance.

@EronWright
Copy link
Contributor

Note to implementors, be sure to make a copy of the opts before passing them to transform function, as the other SDKs do. For example:

// Create a copy of opts to pass into potentially mutating transforms that will be applied to this resource.
opts = Object.assign({}, opts);
// Allow users to change API objects before any validation.
for (const t of transformations || []) {
t(obj, opts);
}

Regarding the API change we'd need to make, consider making use of the ResourceOptions struct that was introduced in pulumi/pulumi#11698.

@ghostsquad
Copy link
Author

Ok, I think this makes sense. I see now, by looking at the state, what the subresources look like:

image

@EronWright if I get this working, I'd like to submit an example of how to do this for everyone else. I often find the pulumi documentation to be lacking in thorough examples for Go 馃槄 . I think Go is one of the harder languages to use effectively with Pulumi. How would I go about submitting an example that would show up somewhere on the site?

@EronWright
Copy link
Contributor

@ghostsquad I believe you'd contribute to pulumi-hugo; cc @pulumi/docs .

@EronWright EronWright removed the awaiting-upstream The issue cannot be resolved without action in another repository (may be owned by Pulumi). label Nov 30, 2023
@toriancrane
Copy link

toriancrane commented Dec 1, 2023

Hi @ghostsquad,

The best place to submit an example would be in the pulumi/examples repo. That will then get funneled to the How-To guides section on the site.

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/yaml kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

4 participants