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

fix(pulumi): prevent existing pulumi configs from being overwritten #6526

Conversation

BraedonLeonard
Copy link
Contributor

What this PR does / why we need it:

This PR addresses an issue where existing pulumi config files fail to be read in, and are then overwritten by Garden, essentially deleting all existing config from the file.

Special notes for your reviewer:

In applyConfig(), there's this line here that overwrites the pulumi config that gets read in:

stackConfig.config = vars

The docs for the pulumi provider and actions aren't clear about how existing pulumi config files are handled, so I'm not sure if this is intentional or not, but I'm pretty sure this is meant to merge these values into the existing pulumi config, rather than overwriting it. Since I wasn't certain about whether or not this was intentional, I left it alone and just called it out here.

@BraedonLeonard BraedonLeonard marked this pull request as ready for review October 8, 2024 19:30
Copy link
Collaborator

@vvagaytsev vvagaytsev left a comment

Choose a reason for hiding this comment

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

Great catch! Thanks for fixing this @BraedonLeonard! 💯 👍

@vvagaytsev vvagaytsev added this pull request to the merge queue Oct 10, 2024
@vvagaytsev vvagaytsev removed this pull request from the merge queue due to a manual request Oct 10, 2024
@vvagaytsev vvagaytsev changed the title fix: issue with existing pulumi config being overwritten fix(pulumi): prevent existing pulumi configs from being overwritten Oct 10, 2024
@vvagaytsev vvagaytsev added this pull request to the merge queue Oct 10, 2024
Merged via the queue into garden-io:main with commit dcfef40 Oct 10, 2024
17 checks passed
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.

2 participants