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

[FEATURE]: Incorrect merge result of multiple pulumi var files #6726

Closed
ishantagarwal-x213235 opened this issue Dec 17, 2024 · 8 comments · Fixed by #6729 or #6735
Closed

[FEATURE]: Incorrect merge result of multiple pulumi var files #6726

ishantagarwal-x213235 opened this issue Dec 17, 2024 · 8 comments · Fixed by #6729 or #6735
Assignees

Comments

@ishantagarwal-x213235
Copy link
Contributor

ishantagarwal-x213235 commented Dec 17, 2024

There is a specification of pulumivarfile where we need to add configs in one or more files and garden merges them all-together during the deployment.
https://docs.garden.io/reference/action-types/deploy/pulumi#spec.pulumivarfiles

There is a also a note added in documentation
Note: There is no need to nest the variables under a config field as is done in a pulumi config. Simply specify all the config variables at the top level.

It needs an improvement actually, In pulumi - we've a secret provider containing the secretsprovider and encryptedkey keys which is not the part of application configuration but these represent the meta-level settings and looks like in the following way:-
Expected result after merge the files

secretsprovider: gcpkms://projects/xyz/locations/global/keyRings/pulumi/cryptoKeys/pulumi-secrets
encryptedkey: AQICAHgYk+123...

config:
  someOtherConfig: value

Currently the file after merging looks like which is incorrect merged result.
Actual result after merge the files

config:
  secretsprovider: gcpkms://projects/xyz/locations/global/keyRings/pulumi/cryptoKeys/pulumi-secrets
  encryptedkey: AQICAHgYk+123...  # Wrong! Pulumi won't recognize this.
  someOtherConfig: value

Please looks into this and provide the optimum solution for this

garden version 0.13.38
pulumi version v3.108.1
Command: garden plugins pulumi preview --env={env-name}

@twelvemo
Copy link
Collaborator

Hello @ishantagarwal-x213235 , thank you for this feature request. It does make sense to allow specifying other top level keys in the varfiles. As you have observed correctly, right now we assume they are all values that go under config. Since this is a breaking change in the varfile schema, i will put the new behavior behind a feature flag and we can switch over to that new behavior on the next major release.

@ishantagarwal-x213235
Copy link
Contributor Author

Thanks @twelvemo for considering it as a breaking changes. Can you please take this on priority as I'm blocked on using the secret provider pulumi feature with garden.

@twelvemo
Copy link
Collaborator

Hi @ishantagarwal-x213235, i started working on the implementation see linked PR.

@ishantagarwal-x213235
Copy link
Contributor Author

Thanks @twelvemo to take on priority.
Please keep in mind that we’re currently using Garden version 0.13.x, which relies on the Modules configuration. While we’re aware that Modules will be removed in version 0.14, this transition is planned for later. For now, any fix should support the Modules setup in 0.13.x.

@stefreak stefreak reopened this Dec 19, 2024
@stefreak
Copy link
Member

@ishantagarwal-x213235 I'll have a look at the module support.

@stefreak
Copy link
Member

@ishantagarwal-x213235 you can now update using garden self-update edge-bonsai. Please let us know if it works for you.

@ishantagarwal-x213235
Copy link
Contributor Author

@stefreak It's working for me. Thanks for your support.

@stefreak
Copy link
Member

great, thanks for letting us know. we will release this early next year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment