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

Read state lock info and lock holder info from environment variables #34265

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

Conversation

hsiam261
Copy link

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 #

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.

The defult LockInfo object created by terraform duirng state lock does
not use the Info field present in StateLock object that 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 userful if the Who
field was instead configurable.

This commit 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.
@hashicorp-cla
Copy link

hashicorp-cla commented Nov 17, 2023

CLA assistant check
All committers have signed the CLA.

@crw
Copy link
Collaborator

crw commented Nov 17, 2023

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

@crw
Copy link
Collaborator

crw commented Dec 1, 2023

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!

@bvoss
Copy link

bvoss commented Oct 28, 2024

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.

@crw
Copy link
Collaborator

crw commented Nov 21, 2024

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!)

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

Successfully merging this pull request may close these issues.

4 participants