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: restrict WriteConfigField to atomic writes #1109

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

charliecruzan-stripe
Copy link
Contributor

Reviewers

cc @stripe/developer-products

Summary

It's possible to get into a state where the values stored in the config file do not equal the values that viper has in memory. When this happens, if we call .WriteConfigField, viper will write other values besides the one we are explicitly passing in, causing unforeseen behavior and very difficult to track down bugs. This change makes it so that viper reads from the current config file, then sets the key we are explicitly trying to override, then writes to the file.

@charliecruzan-stripe charliecruzan-stripe requested a review from a team as a code owner July 25, 2023 00:29
Copy link
Collaborator

@vcheung-stripe vcheung-stripe left a comment

Choose a reason for hiding this comment

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

The change makes sense, one additional thing we could verify is that this works when there's more than one profile in the config

@charliecruzan-stripe
Copy link
Contributor Author

good call! Tested and works as expected when using the --project-name flag: switching accounts in two different profiles only affects the appropriate profile

@charliecruzan-stripe charliecruzan-stripe merged commit a2a9670 into master Jul 25, 2023
7 checks passed
@charliecruzan-stripe charliecruzan-stripe deleted the charliecruzanRUN_DX-1931 branch July 25, 2023 20:09
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.

None yet

2 participants