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

d/tfe_project: Output workspace names #1429

Merged
merged 7 commits into from
Aug 5, 2024
Merged

Conversation

1natedawg
Copy link
Contributor

@1natedawg 1natedawg commented Jul 29, 2024

Description

This PR is to emit the names of all the workspaces within a project. My use case is is follows:

  • I have a known project name and want to act on certain workspaces within it (based on a workspace name suffix)
  • I can use tfe_project to look up the IDs of all the workspaces in that project
  • I don't have a clean way to look up those workspaces by ID to get their names

So, this allows me to capture the workspace names within a project and act on them. If there is a better alternative, let me know!

Remember to:

Testing plan

  1. Create a project called foo
  2. Create a tfe_project data source with name foo, e.g.:
data "tfe_project" "foo" {
  organization = "my-org"
  name = "foo"
}

output "project" {
  value = data.tfe_project.foo
}
  1. Run terraform plan/terraform apply
  2. Output should contain a list called workspace_names in addition to the attributes that were already there.

Output from acceptance tests

Please run applicable acceptance tests locally and include the output here. See testing.md to learn how to run acceptance tests.

If you are an external contributor, your contribution(s) will first be reviewed before running them against the project's CI pipeline.

TESTARGS="-run TestAccTFEProjectDataSource_caseInsensitive" make testacc                                                   [13:00:03]
TF_ACC=1 TF_LOG_SDK_PROTO=OFF go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEProjectDataSource_caseInsensitive -timeout 15m
?       github.com/hashicorp/terraform-provider-tfe     [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/client     0.838s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/logging    0.749s [no tests to run]
?       github.com/hashicorp/terraform-provider-tfe/internal/provider/validators        [no test files]
?       github.com/hashicorp/terraform-provider-tfe/version     [no test files]
=== RUN   TestAccTFEProjectDataSource_caseInsensitive
--- PASS: TestAccTFEProjectDataSource_caseInsensitive (5.01s)
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/provider   5.817s

...

Copy link

hashicorp-cla-app bot commented Jul 29, 2024

CLA assistant check
All committers have signed the CLA.

@1natedawg 1natedawg marked this pull request as ready for review July 29, 2024 17:26
@1natedawg 1natedawg requested a review from a team as a code owner July 29, 2024 17:26
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.

This looks like a reasonable change but I want to point out that there is a subtle drawback to having two "Set" type attributes side-by-side like this: if you convert it to a list, the indices may or may not correspond to each other. In other words, tolist(data.tfe_project.this.workspace_ids)[0] may or may not correspond to tolist(data.tfe_project.this.workspace_names)[0] and so it seems to me like the symmetry suggested by the attributes could introduce some unintended bugs downstream.

@brandonc
Copy link
Collaborator

brandonc commented Jul 30, 2024

It may be acceptable to highlight in the docs that the attributes are sets and cannot be meaningfully mapped to each other... Unless you can think of a better design. I thought about several solutions but nothing really jumped out at me as being obviously better.

@brandonc
Copy link
Collaborator

Also, can you add a test assertion to TestAccTFEProjectDataSource_basic ? Thanks for the contribution!

@1natedawg
Copy link
Contributor Author

Thanks for the feedback @brandonc ! Yeah, I'm with you - I'd love to be able to tie them together, but I couldn't come up with a good way to do so. I've added a test assertion and a note in the docs, let me know if you need any additional changes!

Acceptance tests w/ additional assertion:

TESTARGS="-run TestAccTFEProjectDataSource*" make testacc                
TF_ACC=1 TF_LOG_SDK_PROTO=OFF go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEProjectDataSource* -timeout 15m
?       github.com/hashicorp/terraform-provider-tfe     [no test files]
?       github.com/hashicorp/terraform-provider-tfe/internal/provider/validators        [no test files]
?       github.com/hashicorp/terraform-provider-tfe/version     [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/client     0.494s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/logging    0.287s [no tests to run]
=== RUN   TestAccTFEProjectDataSource_basic
--- PASS: TestAccTFEProjectDataSource_basic (10.95s)
=== RUN   TestAccTFEProjectDataSource_caseInsensitive
--- PASS: TestAccTFEProjectDataSource_caseInsensitive (7.93s)
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/provider   19.320s

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.

CHANGELOG Just needs a rebase and I'll be glad to merge it

@@ -9,6 +9,8 @@ Get information on a Project.

Use this data source to get information about a project.

~> **NOTE:** The `workspace_ids` and `workspace_names` attributes are not guaranteed to return values in the same order, so they cannot be reliably mapped to one another. To map workspace names to IDs reliably, it is recommended to pass those names into the `tfe_workspace_ids` data source.
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@brandonc brandonc merged commit 68c4a0e into hashicorp:main Aug 5, 2024
3 of 9 checks passed
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.

2 participants