terraform icon indicating copy to clipboard operation
terraform copied to clipboard

Read state lock info and lock holder info from environment variables

Open hsiam261 opened this issue 2 years ago • 5 comments

The defult LockInfo object created by terraform duirng state locking does not use the Info field present in LockInfo object which is useful for storing metadata. Neither does the it effectively set the Who field. Currently the Who field default to user@host. Which isn't useful when the lock is being created from inside a docker image or in a CI/CD pipeline. In those cases, the user might find it useful if the Who field was instead configurable.

This PR adds the feature of reading Info and Who fields for the LockInfo object from the TF_LOCK_METADATA and TF_LOCK_OWNER_ID environment variables if present from the statemgr.NewLockInfo method which is used as the default LockInfo factory in all backends in this repo. If the environment variables are not present, the code defaults to the old behavior and thus wouldn't change anything.

As a rationale for the usages of this feature, consider the case where terraform plans are ran from a CI/CD pipeline like in Jenkins. One might want to know which build of which job caused the plan to lock. Then setting TF_LOCK_OWNER_ID=${JOB_NAME}-${JOB_NUMBER} will resolve the issue.

Fixes #

  • https://github.com/hashicorp/terraform/issues/23910
  • https://github.com/hashicorp/terraform/issues/26928

Target Release

1.5.x

Draft CHANGELOG entry

ENHANCEMENTS

  • User can add custom information about state lock if they want by adding the information in TF_LOCK_METADATA environment variable.
  • User can add customize the who field of LockInfo by exporting the TF_LOCK_OWNER_ID environment variable.
  • These features will only be present in backends that do not write those two fields themselves i.e, uses the default LockInfo factory as is i.e the PR only enhances the behavior of the default LockInfo generation process.
  • If those environment variables are not present the behavior will remain unchanged i.e the Info field won't be present, and the Who field will default to user@host i.e this PR only adds on top of existing features are keeps it backward compatible.

hsiam261 avatar Nov 17 '23 15:11 hsiam261

CLA assistant check
All committers have signed the CLA.

hashicorp-cla avatar Nov 17 '23 15:11 hashicorp-cla

Thanks for this submission! I'll raise it in triage.

crw avatar Nov 17 '23 23:11 crw

Hi @hsiam261, the summary of the feedback from triage is that there is an internal conversation around changing a big feature adjacent to this pull request, and reviewing/accepting this now would make that change more difficult. The bad news is that discussion will likely take months / quarters to resolve, and so you are unlikely to see much movement on this PR in that time frame.

Assuming we did review this in the future, there is one piece of feedback to note. We've been moving away from directly accessing environment variables from inside Terraform's "guts", because that makes it much harder to keep all of the env vars in mind under future maintenance and requires us to do undesirable things in tests to fake the environment variables. However, I do not recommend making any major changes at this time (to plumb the values through multiple layers of code to avoid using environment variables) since it is uncertain at this time whether we will even move forward with this PR.

Thanks for this submission, and I am sorry we cannot give you a definitive answer today on this PR. If you want to check in in a few months, we might have more to share; if you chose to close it we understand. Thanks again!

crw avatar Dec 01 '23 19:12 crw

Hello @crw

Are there any news on this?

I was going to prepare a very similar PR - just using "TF_LOCK_INGO"

If you use Terraform in a CI - pipeline it would be really helpful to have a short extra info like a runId in the lock.

bvoss avatar Oct 28 '24 12:10 bvoss

Hi @bvoss no update, but depending on future developments it seems unlikely this particular solution would be adopted due to previously-mentioned concerns. That said, the future isn't written and so I will keep this PR open in case we reconsider (it does happen!)

crw avatar Nov 21 '24 20:11 crw