Skip to content

Fixed incorrect secret creation on drift detection of secret value #2499

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jorgecarleitao
Copy link

@jorgecarleitao jorgecarleitao commented Dec 5, 2024

Resolves #1383
Resolves #2288


Before the change?

Given a .tf with a resource with any secret
When .tf is applied
And the secret's value is externally modified
And .tf is applied
Then the new plan results in the creation of a new secret

After the change?

Given a .tf with a resource with any secret
When .tf is applied
And the secret's value is externally modified
And .tf is applied
Then the plan results in the update of the existing secret

In particular, this behavior enables the use of the lifecycle meta argument to ignore changes to externally updated secrets.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

NOTE: I could not find any test that I could re-use to introduce an external change to an existing resource. Need support.

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

  • Yes
  • No

@jorgecarleitao jorgecarleitao changed the title Fixed incorrect secrets creation on externally updated secret Fixed incorrect secrets creation on drift detection of secret value Dec 5, 2024
@jorgecarleitao jorgecarleitao changed the title Fixed incorrect secrets creation on drift detection of secret value Fixed incorrect secret creation on drift detection of secret value Dec 5, 2024
Copy link

@nbali nbali left a comment

Choose a reason for hiding this comment

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

yeah, pretty much what I had in mind, ty for your time

@mothilraj
Copy link

Please let us know when this fix will be merged.

@dcfsc
Copy link
Contributor

dcfsc commented Mar 12, 2025

This issue is making me reconsider the use of Terraform to manage repository secrets, or split secrets into a new configuration.

We create a placeholder secret and ignore the changes, so the team can update the secrets and we will not detect a drift.
Every plan now shows dirty if the value is changed, and if I apply without a target, it replaces the new desired value with the placeholder value. So I have to warn teams NOT to apply when secrets are changed. In most cases the secret comes from somewhere else, but that is additional work AND the plan is now dirty AGAIN.
I will probably grab the code for the latest provider and try to make a patched version for myself. Would love if we could merge this.

@dcfsc
Copy link
Contributor

dcfsc commented Mar 14, 2025

I built the latest version of the provider and cherry picked your 1 commit on the "fix" branch. It works like a dream.

@jacobkretz-bf
Copy link

@nbali This is a really big issue. Any ETA.

@nbali
Copy link

nbali commented Apr 24, 2025

@jacobkretz-bf I'm not sure why are you mentioning me :) I just suggested a solution to the issue, that @jorgecarleitao essentially implemented.

@grobbinsBF
Copy link

@kfcampbell What does it take to get this PR approved and released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants