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

Feature/variables data source #369

Merged
merged 21 commits into from
Sep 30, 2021
Merged

Feature/variables data source #369

merged 21 commits into from
Sep 30, 2021

Conversation

kilwa0
Copy link
Contributor

@kilwa0 kilwa0 commented Sep 20, 2021

Description

Hi all, this change add the variables data source. It outputs into a list some useful values from the configured variables on a given worskpace

It outputs three values:

  1. variables - the whole set
  2. terraform - only the terraform categorized variables
  3. environment - the environment variables into the worskpace

As you can tell English is not my primary language so please double check documentation for misspelled words :)
Please let me know if you need anything from me

Regards

Testing plan

Acceptance test creates:

resource "tfe_organization" "foobar" {
  name  = "org-%d"
  email = "admin@company.com"
}

resource "tfe_workspace" "foobar" {
  name         = "workspace-foo-%d"
  organization = tfe_organization.foobar.id
}

resource "tfe_variable" "terrbar" {
	key          = "foo"
	value        = "bar"
	category     = "terraform"
	workspace_id = tfe_workspace.foobar.id
}

resource "tfe_variable" "envbar" {
	key          = "foo"
	value        = "bar"
	category     = "env"
	workspace_id = tfe_workspace.foobar.id
}

data "tfe_variables" "foobar" {
	workspace_id = tfe_workspace.foobar.id
	depends_on = [
    tfe_variable.terrbar,
		tfe_variable.envbar
  ]
}

output "variables" {
	value = data.tfe_variables.foobar.variables[0]["name"]
}

output "environment" {
	value = data.tfe_variables.foobar.environment[0]["name"]
}

output "terraform" {
	value = data.tfe_variables.foobar.terraform[0]["name"]
}

End checks the id and the outputs of the data source

Output from acceptance tests

TESTARGS="-run TestAccTFEVariablesDataSource_basic" make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEVariablesDataSource_basic -timeout 15m
?   	github.com/hashicorp/terraform-provider-tfe	[no test files]
=== RUN   TestAccTFEVariablesDataSource_basic
--- PASS: TestAccTFEVariablesDataSource_basic (19.16s)
PASS
ok  	github.com/hashicorp/terraform-provider-tfe/tfe	19.168s
?   	github.com/hashicorp/terraform-provider-tfe/version	[no test files]

@kilwa0 kilwa0 requested a review from a team as a code owner September 20, 2021 21:11
Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

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

Thanks for implementing this! It looks very promising. Here's my initial code review. Tomorrow I'll take some time to smoke test and ensure everything works as I imagine it did.

tfe/data_source_variables.go Outdated Show resolved Hide resolved
tfe/data_source_variables.go Outdated Show resolved Hide resolved
Comment on lines 130 to 156
if variable.Category == "terraform" {
result := make(map[string]interface{})
result["id"] = variable.ID
result["name"] = variable.Key
result["category"] = variable.Category
result["hcl"] = variable.HCL
if variable.Sensitive != true {
result["value"] = variable.Value
} else {
result["value"] = "***"
}

terraformVars = append(terraformVars, result)
} else if variable.Category == "env" {
result := make(map[string]interface{})
result["id"] = variable.ID
result["name"] = variable.Key
result["category"] = variable.Category
result["hcl"] = variable.HCL
if variable.Sensitive != true {
result["value"] = variable.Value
} else {
result["value"] = "***"
}

envVars = append(envVars, result)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since these two conditions are identical, you can use the condition to figure out which slice to append result to:

I would also add "sensitive" to the schema and set that:

Suggested change
if variable.Category == "terraform" {
result := make(map[string]interface{})
result["id"] = variable.ID
result["name"] = variable.Key
result["category"] = variable.Category
result["hcl"] = variable.HCL
if variable.Sensitive != true {
result["value"] = variable.Value
} else {
result["value"] = "***"
}
terraformVars = append(terraformVars, result)
} else if variable.Category == "env" {
result := make(map[string]interface{})
result["id"] = variable.ID
result["name"] = variable.Key
result["category"] = variable.Category
result["hcl"] = variable.HCL
if variable.Sensitive != true {
result["value"] = variable.Value
} else {
result["value"] = "***"
}
envVars = append(envVars, result)
}
result := make(map[string]interface{})
result["id"] = variable.ID
result["name"] = variable.Key
result["category"] = variable.Category
result["hcl"] = variable.HCL
result["sensitive"] = variable.Sensitive
result["value"] = variable.Value
if variable.Category == "terraform" {
terraformVars = append(terraformVars, result)
} else if variable.Category == "env" {
envVars = append(envVars, result)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

big commit incoming solving also this

tfe/data_source_variables.go Outdated Show resolved Hide resolved
Comment on lines 158 to 159
totalVariables = append(totalVariables, terraformVars...)
totalVariables = append(totalVariables, envVars...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to not interleave pages of terraform and env variables in this collection. At the end, you can just append totalEnvVariables to totalTerraformVariables (see below) so that all the terraform variables come first and all the env variables come after. It also simplifies the code and reduces copying.

Suggested change
totalVariables = append(totalVariables, terraformVars...)
totalVariables = append(totalVariables, envVars...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

website/docs/d/variables.html.markdown Outdated Show resolved Hide resolved
website/docs/d/variables.html.markdown Outdated Show resolved Hide resolved
website/docs/d/variables.html.markdown Outdated Show resolved Hide resolved
website/docs/d/variables.html.markdown Show resolved Hide resolved
website/docs/d/variables.html.markdown Outdated Show resolved Hide resolved
kilwa0 and others added 9 commits September 21, 2021 11:02
Co-authored-by: Brandon Croft <brandon.croft@gmail.com>
Co-authored-by: Brandon Croft <brandon.croft@gmail.com>
Co-authored-by: Brandon Croft <brandon.croft@gmail.com>
Co-authored-by: Brandon Croft <brandon.croft@gmail.com>
Co-authored-by: Brandon Croft <brandon.croft@gmail.com>
Co-authored-by: Brandon Croft <brandon.croft@gmail.com>
Co-authored-by: Brandon Croft <brandon.croft@gmail.com>
Co-authored-by: Brandon Croft <brandon.croft@gmail.com>
@brandonc
Copy link
Collaborator

@kilwa0 Thanks for making my recommended changes! I didn't have time to finalize my testing yet but this is still on my radar.

Copy link
Collaborator

@brandonc brandonc left a comment

Choose a reason for hiding this comment

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

One final tweak! Thanks again

tfe/data_source_variables.go Outdated Show resolved Hide resolved
tfe/data_source_variables.go Show resolved Hide resolved
@hashicorp hashicorp deleted a comment from kilwa0 Sep 28, 2021
@hashicorp hashicorp deleted a comment from kilwa0 Sep 28, 2021
@brandonc brandonc merged commit 642ea61 into hashicorp:main Sep 30, 2021
@brandonc
Copy link
Collaborator

@kilwa0 Congrats and thanks for the excellent contribution. You're always welcome to submit more changes that simplify your workflow.

@kilwa0
Copy link
Contributor Author

kilwa0 commented Sep 30, 2021

@brandonc My pleasure.
Thank you for your patience and guidance. See you soon 👍

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.

None yet

2 participants