-
Notifications
You must be signed in to change notification settings - Fork 229
Improve variable precedence in terraform global.auto.tfvars #660
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the file name of global.auto.tfvars would cause all existing pipelines to fail, I suggest to use "*global.auto.tfvars" as pattern to copy to the workspace
Hi @igordust, The file name Your suggestion can still work, allowing users the flexibility to add any prefix to the Let me know if you prefer this file change, and I'll update the PR: cp -R "${CURRENT}/tfvars/*global.auto.tfvars" "${CURRENT}/tmp/${TF_VAR_TARGET_ACCOUNT_ID}-${AWS_REGION}" |
IMHO, it's better, in this way you give the user the freedom to call the file 0-global.auto.tfvars, 00-global.auto.tfvars or whatever it fits his/her needs. Please note that for files in account folders under tfvars there isn't such a limitation, it just copies everything in the folder into the root of the temp folder. |
…global.auto.tfvars
@igordust Thank you for the suggestion! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @asosso for raising this PR.
Considering the above conversation, and the issue you highlighted, I think it would be great to highlight this potential issue and how to fix it in the documentation.
So people not familiar with this lookup order will not need waste time debugging it.
For example, add a section to recommend a specific global.auto.tfvars
file name to use.
Could you add this to this PR please?
I tested the patch, it's not working, because this test: |
Why?
For the sake of convenience in managing variables within a multi-project and multi-environment setting, there's a need to utilize multiple
*.tvars
intfvars/TARGET_ACCOUNT_ID/
.For example:
tfvars/TARGET_ACCOUNT_ID/environment.auto.tfvars
tfvars/TARGET_ACCOUNT_ID/local.auto.tfvars
tfvars/TARGET_ACCOUNT_ID/project.auto.tfvars
The file
tfvars/global.auto.tfvars
is structured as follows:The file
tfvars/TARGET_ACCOUNT_ID/environment.auto.tfvars
:The file
tfvars/TARGET_ACCOUNT_ID/project.auto.tfvars
:Issue
When running the
terraform plan
on this code:I get:
Instead of what I would expect:
This happens because the
*.auto.tfvars
files are processed in the lexical order of their filenames. See Variable Definition PrecedenceWhat?
Description of changes:
To address this issue, I suggest modifying the file naming convention. Specifically, adding a numeric prefix (e.g.,
0-
) to theglobal.auto.tfvars
file when it's copied from${CURRENT}/tfvars/global.auto.tfvars
to${CURRENT}/tmp/${TF_VAR_TARGET_ACCOUNT_ID}-${AWS_REGION}/0-global.auto.tfvars
.By doing this, we can ensure (at least for file that not start with a number) that all variables defined in
tfvars/TARGET_ACCOUNT_ID/
take precedence over those inglobal.auto.tfvars
.By submitting this pull request, I confirm that you can use, modify, copy, and
redistribute this contribution, under the terms of your choice.