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

Larger diff than expected when updating helm_release 'set' 'value' #916

Merged
merged 2 commits into from
Oct 18, 2022

Conversation

adcharre
Copy link
Contributor

Description

Fix issue #915 where when re-deploying a helm_release after changing a 'set' 'value' the plan generated indicates that values not changing will be deleted and re-applied even though they have not been changed.

The issue is caused by the resource diff process removing the 'set' 'type' value from the state as it was not set in the original terraform code. When initially applying the plan the default value "" was used for the 'set' 'type' and stored in the state, when redeploying the original value of 'set' 'type' is requested but as no default has been set in the schema it is marked as removed, which then results in null being stored in the state for the value of 'set' 'type'.
This causes an internal warning to be generated in the logs:
"Provider "registry.terraform.io/hashicorp/helm" produced an unexpected new value for helm_release.test during refresh."

Acceptance tests

  • [Y] Have you added an acceptance test for the functionality being added?
    • TestAccResourceRelease_updateSetValue - confirms that 'type' is not lost when updating value.

Release Note

Release note for CHANGELOG:

Fix large diffs being generated when updating a 'set' 'value'

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Fix issue hashicorp#915 where when re-deploying a helm_release after changing a 'set' 'value' the plan generated indicates that values not changing will be deleted and re-applied even though they have not been changed.

The issue is caused by the resource diff process removing the 'set' 'type' value from the state as it was not set in the original terraform code. When initially applying the plan the default value "" was used for the 'set' 'type' and stored in the state, when redeploying the original value of 'set' 'type' is requested  but as no default has been set in the schema it is marked as removed, which then results in null being stored in the state for the value of 'set' 'type'.
This cause an internal warning to be generated in the logs:
"Provider "registry.terraform.io/hashicorp/helm" produced an unexpected new value for helm_release.test during refresh."
@hashicorp-cla
Copy link

hashicorp-cla commented Jul 11, 2022

CLA assistant check
All committers have signed the CLA.

@adcharre
Copy link
Contributor Author

@jrhouston - as this is a first PR for terraform helm provider, is there anything more you might need/I've missed in this PR?

@fcrespofastly
Copy link

Hey @adcharre! did this fix #711 for you as well?

@adcharre
Copy link
Contributor Author

Hey @adcharre! did this fix #711 for you as well?

I've certainly not seen any issues since using this fix and the unit tests also suggests that it fixes problems with set blocks.
Any help with a thumbs up on this PR to get it merged would be appreciated!

@fcrespofastly
Copy link

I've certainly not seen any issues since using this fix and the unit tests also suggests that it fixes problems with set blocks. Any help with a thumbs up on this PR to get it merged would be appreciated!

Thanks! do you have kind of your own private registry where you uploaded your custom build of the provider?

@fcrespofastly
Copy link

Who should we be requesting a review from to get this merged? seems like a very straightorward PR ...

@xThomo
Copy link

xThomo commented Aug 24, 2022

Who should we be requesting a review from to get this merged? seems like a very straightorward PR ...

@fcrespofastly looking at the contributors and some previous PRs it looks like a key contributor is @jrhouston

@fcrespofastly
Copy link

Hey @jrhouston! 👋🏻 👋🏻 this seems like a pretty reasonable and straightforward PR, any chance we can get this merged? We currently can't render manifests which impacts our ability to validate the plan prior to apply.

@fcrespofastly
Copy link

Hey @jrhouston any chance you can take a look at this one?

@dkulchinsky
Copy link

Dear @jrhouston, @BBBmau

I noticed you recently helped merge and release few fixes, can you please spare a minute to get this fix in? this is a straightforward change and addresses both #915 & #711 🙏🏼

@fcrespofastly
Copy link

Hey @jrhouston 👋🏻 👋🏻 !

I noticed you reacted to @dkulchinsky's comment. Did you have a chance to review this PR? 🙏🏻

Thanks in advance!!

Copy link
Contributor

@jrhouston jrhouston left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this @adcharre! 🚀

@jrhouston jrhouston merged commit 2131abb into hashicorp:main Oct 18, 2022
@dkulchinsky
Copy link

@jrhouston many thanks for reviewing and getting this merged 🙏🏼

could you also help to get this fix released in a new patch version?

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants