aws-deployment-framework icon indicating copy to clipboard operation
aws-deployment-framework copied to clipboard

Improve variable precedence in terraform global.auto.tfvars

Open asosso opened this issue 9 months ago • 5 comments

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 in tfvars/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:

project_name = "default"
project_env = "test"

The file tfvars/TARGET_ACCOUNT_ID/environment.auto.tfvars:

project_env = "prod"

The file tfvars/TARGET_ACCOUNT_ID/project.auto.tfvars:

project_name = "mybrand"

Issue

When running the terraform plan on this code:

variable "project_name" {
  description = "Project name"
  type        = string
}

variable "project_env" {
  description = "Project environment"
  type        = string
}

output "project_name" {
  value = var.project_name
}

output "project_env" {
  value = var.project_env
}

I get:

Changes to Outputs:
  + project_env  = "test"
  + project_name = "mybrand"

Instead of what I would expect:

Changes to Outputs:
  + project_env  = "prod"
  + project_name = "mybrand"

This happens because the *.auto.tfvars files are processed in the lexical order of their filenames. See Variable Definition Precedence

What?

Description of changes:

To address this issue, I suggest modifying the file naming convention. Specifically, adding a numeric prefix (e.g., 0-) to the global.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 in global.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.

asosso avatar Oct 13 '23 12:10 asosso

Hi @igordust,

The file name global.auto.tfvars would be renamed only in the temporary /tmp/ folder created for executing Terraform commands. The source file will still be named global.auto.tfvars, ensuring backward compatibility and requiring no-changes for those already using ADF.

Your suggestion can still work, allowing users the flexibility to add any prefix to the *global.auto.tfvars file in the Git repository.

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}"

asosso avatar Oct 13 '23 14:10 asosso

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.

igordust avatar Oct 13 '23 15:10 igordust

@igordust Thank you for the suggestion! PR updated

asosso avatar Oct 13 '23 15:10 asosso

I tested the patch, it's not working, because this test: if [ -f "${CURRENT}/tfvars/global.auto.tfvars" ]; then is matching only if the file is named global.auto.tfvars, you should a more elaborate test to match any file matching the pattern *global.auto.tfvars.

igordust avatar Jan 12 '24 17:01 igordust

Thank you for testing this @igordust!

@asosso could you address the two open comments above?

sbkok avatar Jan 12 '24 18:01 sbkok