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

allow for top level terraform flag assignment #558

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

Conversation

ThisGuyCodes
Copy link

@ThisGuyCodes ThisGuyCodes commented Jun 16, 2020

fixes #517

also adds tests for FormatTerraformBackendConfigAsArgs


This change is Reviewable

@ThisGuyCodes ThisGuyCodes force-pushed the feature/backend-config-file branch from 43962b9 to 9921f57 Compare June 16, 2020 15:52
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

I'm a bit confused by this PR. Is it missing files? The changes seem to affect how Terraform sets -var flags rather than -backend-config flags?

modules/terraform/format.go Show resolved Hide resolved
modules/helm/upgrade.go Outdated Show resolved Hide resolved
@ThisGuyCodes ThisGuyCodes force-pushed the feature/backend-config-file branch from 9921f57 to 5847837 Compare June 17, 2020 16:08
@ThisGuyCodes
Copy link
Author

ThisGuyCodes commented Jun 17, 2020

@brikis98 FormatTerraformBackendConfigAsArgs uses the generic formatTerraformArgs, which is what I modified:

// FormatTerraformBackendConfigAsArgs formats the given variables as backend config args for Terraform (e.g. of the
// format -backend-config=key=value).
func FormatTerraformBackendConfigAsArgs(vars map[string]interface{}) []string {
return formatTerraformArgs(vars, "-backend-config", false)
}

@ThisGuyCodes
Copy link
Author

oh I feel silly. That's the wrong test. I'll get back to you.

@ThisGuyCodes ThisGuyCodes marked this pull request as draft June 17, 2020 16:14
@ThisGuyCodes ThisGuyCodes force-pushed the feature/backend-config-file branch from 5847837 to f11c061 Compare June 17, 2020 16:23
@ThisGuyCodes ThisGuyCodes marked this pull request as ready for review June 17, 2020 16:25
Copy link
Member

@brikis98 brikis98 left a comment

Choose a reason for hiding this comment

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

Ah, OK, this makes more sense now!

Could you add comments to terraform.Options to indicate that terraform.KeyOnly can be used as a value in the BackendConfig map to indicate only the key should be passed to Terraform?

modules/terraform/format_test.go Outdated Show resolved Hide resolved
modules/terraform/format.go Outdated Show resolved Hide resolved
modules/terraform/format.go Outdated Show resolved Hide resolved
Co-authored-by: Yevgeniy Brikman <brikis98@users.noreply.github.com>
@brikis98
Copy link
Member

brikis98 commented Jul 7, 2020

Could you add comments to terraform.Options to indicate that terraform.KeyOnly can be used as a value in the BackendConfig map to indicate only the key should be passed to Terraform?

Bumping this one up too :)

@jai
Copy link

jai commented Jul 22, 2020

@ThisGuyCodes lmk if you could use a hand on this, could deff use this feature.

@ThisGuyCodes ThisGuyCodes requested a review from denis256 as a code owner July 9, 2024 17:48
@ThisGuyCodes
Copy link
Author

bump? (4 years later)

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

Successfully merging this pull request may close these issues.

Terraform options does not allow to specify backend file
3 participants