Stack-action vs. action variables (TERRAFORM_APPLY | DESTROY)

[LeoDiazL]

As it is right now, you can define a stack-action and yet pass TERRAFORM_APPLY / TERRAFORM_DESTROY as an environment variable. Hence, you could create the environment and destroy it in the same run.

The idea would be to:

• Sanitize options. Make sure only one of them is chosen. (What should we do if stack-action collides with TERRAFORM_* ?)
• Run through a case statement, avoiding unnecessary validations and keeping the code cleaner.
• Print warning for the environment variables we want to remove in the future ( if we want to. 😄 )

[PhillypHenning]

Keep variables
If we keep the variables as override then we should update them to use a _OVERRIDE postfix

Remove variables
If we remove the variables, it just locks the actions to only be set using the stack action which I think is a positive for keeping things standardized.

Additional considerations
Cloudformation
Cloudformation uses the stack-action however doesn't have an override.

if [[ "${CFN_STACK_ACTION}" == "deploy" ]] || [[ "${CFN_STACK_ACTION}" == "Deploy" ]]; then
  echo "Running Cloudformation Deploy Stack"
  bash $CLOUDFORMATION_ROOT_SCRIPTS/scripts/cloudformation_deploy.sh "$CFN_TEMPLATE_FILENAME" "$CFN_PARAMS_FLAG" "$CFN_TEMPLATE_PARAMS_FILENAME" "$CFN_STACK_NAME" "$CFN_CAPABILITY" "$CFN_TEMPLATE_S3_BUCKET" "$CFN_S3_PREFIX"
fi

if [[ "${CFN_STACK_ACTION}" == "delete" ]] || [[ "${CFN_STACK_ACTION}" == "Delete" ]]; then
  echo "Running Cloudformation Delete Stack"
  bash $CLOUDFORMATION_ROOT_SCRIPTS/scripts/cloudformation_delete.sh "$CFN_STACK_NAME"
fi

[PhillypHenning]

My suggestion would be to use only stack-action, remove the terraform TERRAFORM_(APPLY|DESTORY) variables

[arm4b]

Good find!

+1 to catching the corner cases + proper error reporting.
Eg. you can pass ENV_A, ENV_B, but not both and allow one action based on merged config + ENV results, but not both.

Also I think it's OK that the ENV variable overrides the setting coming from config.
For instance, in Ansible there's a level of precedence when evaluating CLI params, vs config vs ENV where the ENV value come as a highest priority and overrides everything else.

[arm4b]

I also understand that having an ENV var counterpart to the config setting is helpful when using BitOps in CI/CD and pipeline runners. This way ENV value could be changed on fly depending on context without relying on the whole config file.

[mickmcgrath13]

My suggestion would be to use only stack-action, remove the terraform TERRAFORM_(APPLY|DESTORY) variables
this might be a breaking change (would at least need to be highlighted in migration docs for repos that are using those vars.

I agree with @armab that it's nice to be able to specify env vars in the pipeline config to drive behavior