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.

Sign up for GitHub

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

feat: TF_CLI_ARGS_* Handling for -var-file #898

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: TF_CLI_ARGS_* Handling for -var-file #898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9b0d2d
added warning message when TF_CLI var-file specified
milldr Dec 27, 2024
42c3026
Merge branch 'main' into feat/DEV-2793_dont-squash-tfcli
milldr Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions internal/exec/shell_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,13 +185,19 @@ func execTerraformShellCommand(
}
}()

// Set the Terraform environment variables to reference the var file
componentEnvList = append(componentEnvList, fmt.Sprintf("TF_CLI_ARGS_plan=-var-file=%s", varFile))
componentEnvList = append(componentEnvList, fmt.Sprintf("TF_CLI_ARGS_apply=-var-file=%s", varFile))
componentEnvList = append(componentEnvList, fmt.Sprintf("TF_CLI_ARGS_refresh=-var-file=%s", varFile))
componentEnvList = append(componentEnvList, fmt.Sprintf("TF_CLI_ARGS_import=-var-file=%s", varFile))
componentEnvList = append(componentEnvList, fmt.Sprintf("TF_CLI_ARGS_destroy=-var-file=%s", varFile))
componentEnvList = append(componentEnvList, fmt.Sprintf("TF_CLI_ARGS_console=-var-file=%s", varFile))
// Define the Terraform commands that may use var-file configuration
tfCommands := []string{"plan", "apply", "refresh", "import", "destroy", "console"}

// Check for existing var-file arguments in TF_CLI environment variables
for _, cmd := range tfCommands {
envVar := fmt.Sprintf("TF_CLI_ARGS_%s", cmd)
existing := os.Getenv(envVar)
if existing != "" && strings.Contains(existing, "-var-file=") {
u.LogWarning(atmosConfig, "Found var-file in environment! This may be overwritten by Atmos")
}
// Set the Terraform environment variable to reference the var file
componentEnvList = append(componentEnvList, fmt.Sprintf("%s=-var-file=%s", envVar, varFile))
Copy link
Member

@osterman osterman Dec 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We still haven't solved the underlying intent here which is that we should also allow the user to specify their own TF_CLI_ARGS, which would be appending to the current value of the corresponding ENV. Now we squash whatever value was there. If the value is set to anything, not just a var file, we should warn that this could interfere with atmos behavior.

Copy link
Member Author

@milldr milldr Dec 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

TF_CLI_ARGS_name does already work with the current version of atmos:

TF_CLI_ARGS_plan="-var='foo=bar'" atmos terraform plan station -s dev
...

│ Error: Value for undeclared variable
│
│ A variable named "foo" was assigned on the command line, but the root module does not declare a variable of that name. To use this value, add a "variable" block to
│ the configuration.
╵
exit status 1

Or with another example (not var):

TF_CLI_ARGS_plan="-input=false" atmos terraform plan station -s dev

Initializing the backend...

Initializing provider plugins...
- Reusing previous version of hashicorp/local from the dependency lock file
- Reusing previous version of hashicorp/http from the dependency lock file
- Using previously-installed hashicorp/local v2.5.2
- Using previously-installed hashicorp/http v3.4.5

Terraform has been successfully initialized!
Switched to workspace "dev-station".
╷
│ Error: No value for required variable
│
│   on variables.tf line 30:
│   30: variable "units" {
│
│ The root module input variable "units" is not set, and has no default value. Use a -var or -var-file command line argument to provide a value for this variable.
╵
exit status 1

Copy link
Member Author

Choose a reason for hiding this comment

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

However, TF_CLI_ARGS (without _name) does not work in practice. The TF_CLI_ARGS is passed to all TF CLI commands -- init, workspace, plan. So if it's an invalid arg for 1 of the 3, then that command fails:

TF_CLI_ARGS="-var 'foo=bar'" atmos terraform plan station -s dev

Initializing the backend...

Initializing provider plugins...
- Reusing previous version of hashicorp/local from the dependency lock file
- Reusing previous version of hashicorp/http from the dependency lock file
- Using previously-installed hashicorp/local v2.5.2
- Using previously-installed hashicorp/http v3.4.5

Terraform has been successfully initialized!
Usage: terraform [global options] workspace select NAME

  Select a different Terraform workspace.

Options:

    -or-create=false    Create the Terraform workspace if it doesn't exist.
Error parsing command-line flags: flag provided but not defined: -var

Usage: terraform [global options] workspace new [OPTIONS] NAME

  Create a new Terraform workspace.

Options:

    -lock=false         Don't hold a state lock during the operation. This is
                        dangerous if others might concurrently run commands
                        against the same workspace.

    -lock-timeout=0s    Duration to retry a state lock.

    -state=path         Copy an existing state file into the new workspace.
Error parsing command-line flags: flag provided but not defined: -var

exit status 1

Copy link
Member Author

Choose a reason for hiding this comment

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

Plus, any other ENV var does not work. For example, this doesnt do anything

TF_VAR_foo='bar' atmos terraform plan station -s dev

Copy link
Member Author

Choose a reason for hiding this comment

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

@osterman what is the goal with this task?

  1. Do we simply want to warn if TF_CLI_ARGS_name is set (no new behavior)
  2. Do we want to support TF_CLI_ARGS somehow?
  3. Do we want to support other env vars, such as TF_VAR_varname?

Copy link
Member

Choose a reason for hiding this comment

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

I am on my phone and TF_CLI_ARGS is just my short hand. I don't mean that literally.

I guess I have to take a closer look at the code. I don't understand where we preserve the current value while appending new values to it .

}

// Set environment variables to indicate the details of the Atmos shell configuration
componentEnvList = append(componentEnvList, fmt.Sprintf("ATMOS_STACK=%s", stack))
Expand Down
13 changes: 13 additions & 0 deletions internal/exec/terraform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,23 @@ func ExecuteTerraform(info schema.ConfigAndStacksInfo) error {
info.ComponentEnvList = append(info.ComponentEnvList, fmt.Sprintf("TF_APPEND_USER_AGENT=%s", appendUserAgent))
}

// Check for existing var-file arguments in TF_CLI environment variables
tfCommands := []string{"plan", "apply", "refresh", "import", "destroy", "console"}
for _, cmd := range tfCommands {
envVar := fmt.Sprintf("TF_CLI_ARGS_%s", cmd)
existing := os.Getenv(envVar)
if existing != "" && strings.Contains(existing, "-var-file=") {
u.LogWarning(atmosConfig, "Found var-file in environment! This may be overwritten by Atmos")
}
}

// Print ENV vars if they are found in the component's stack config
if len(info.ComponentEnvList) > 0 {
u.LogDebug(atmosConfig, "\nUsing ENV vars:")
for _, v := range info.ComponentEnvList {
if strings.Contains(v, "-var-file=") {
u.LogWarning(atmosConfig, "Found var-file in component environment! This may be overwritten by Atmos")
}
u.LogDebug(atmosConfig, v)
}
}
Expand Down
Loading