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

Add exclude_tags to data_source_workspace_ids #523

Merged
merged 2 commits into from Jul 8, 2022

Conversation

Uk1288
Copy link
Contributor

@Uk1288 Uk1288 commented Jun 20, 2022

Description

Currently the list of Workspaces within an Organization can be filtered with tag_names to include. This PR adds the ability to exclude workspaces by tags using theexclude_tags parameter.

See usecase here: #393

Remember to:

Testing plan

  1. Create an organization
  2. Create workspaces with tags within the organization
  3. Filtering workspaces using the exclude_tags options, should list all matching workspaces excluding those with the excluded tags.
  4. See example block below:
resource "tfe_organization" "foobar" {
  name  = "tst-terraform-1"
  email = "admin@company.com"
}
resource "tfe_workspace" "foo" {
  name         = "workspace-foo-1"
  organization = tfe_organization.foobar.id
  tag_names    = ["good", "happy"]
}
resource "tfe_workspace" "bar" {
  name         = "workspace-bar-1"
  organization = tfe_organization.foobar.id
  tag_names    = ["good"]
}
data "tfe_workspace_ids" "good" {
  tag_names    = ["good"]
  exclude_tags    = ["happy"]
  organization = tfe_workspace.foo.organization
  depends_on = [
    tfe_workspace.foo,
    tfe_workspace.bar
  ]
}

External links

Include any links here that might be helpful for people reviewing your PR. If there are none, feel free to delete this section.

Output from acceptance tests

Please run applicable acceptance tests locally and include the output here. See TESTS.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 TestAccTFEWorkspace" make testacc

=== RUN   TestAccTFEWorkspaceIDsDataSource_basic
--- PASS: TestAccTFEWorkspaceIDsDataSource_basic (9.51s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_wildcard
--- PASS: TestAccTFEWorkspaceIDsDataSource_wildcard (11.21s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_tags
--- PASS: TestAccTFEWorkspaceIDsDataSource_tags (9.77s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_searchByTagAndName
--- PASS: TestAccTFEWorkspaceIDsDataSource_searchByTagAndName (8.96s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_empty
--- PASS: TestAccTFEWorkspaceIDsDataSource_empty (1.42s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_namesEmpty
--- PASS: TestAccTFEWorkspaceIDsDataSource_namesEmpty (8.94s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_excludeTags
--- PASS: TestAccTFEWorkspaceIDsDataSource_excludeTags (14.33s)
=== RUN   TestAccTFEWorkspaceIDsDataSource_sameTagInTagNamesAndExcludeTags
--- PASS: TestAccTFEWorkspaceIDsDataSource_sameTagInTagNamesAndExcludeTags (10.58s)
PASS

Screen Shot 2022-06-21 at 2 12 10 PM

@Uk1288 Uk1288 force-pushed the uk1288-add-exclude-tags-to-ds-workspace-ids branch from b1ae554 to 71302cd Compare June 21, 2022 18:23
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.

OK, unfortunately, some versions of TFE that we support will not have the "exclude_tags" field for awhile, so we have to provide a fallback as well. That means excluding workspaces if they contain excluded tags when building up the list of ids.

@brandonc brandonc dismissed their stale review July 6, 2022 22:41

My feedback was addressed but I didn't have a chance to smoke test this PR

@annawinkler
Copy link
Contributor

I reached out to @sudomateo about testing this PR with the last TFE. Maybe we could do that tomorrow? cc @sebasslash

@Uk1288 Uk1288 force-pushed the uk1288-add-exclude-tags-to-ds-workspace-ids branch from 7075dce to f0730fa Compare July 7, 2022 20:10
Copy link
Contributor

@sebasslash sebasslash left a comment

Choose a reason for hiding this comment

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

Smoke tested ✅ Overall code changes look 🔥 with some really minor things below ⬇️

@@ -26,6 +26,12 @@ func dataSourceTFEWorkspaceIDs() *schema.Resource {
Optional: true,
},

"exclude_tags": {
Type: schema.TypeList,
Copy link
Contributor

Choose a reason for hiding this comment

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

exclude_tags should be Type: schema.TypeSet since the order of the tags to exclude does not matter.

// Create a search string with all the tag names we are looking for.
var tagSearchParts []string
for _, tagName := range d.Get("tag_names").([]interface{}) {
name := tagName.(string)
if len(strings.TrimSpace(name)) != 0 {
if name, ok := tagName.(string); ok && len(strings.TrimSpace(name)) != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch 👍


data "tfe_workspace_ids" "good" {
tag_names = ["good"]
exclude_tags = ["happy"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there's some extra spaces here

Copy link

@faithlum faithlum Jul 8, 2022

Choose a reason for hiding this comment

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

Tested with a TFE instance that does not support exclude_tags to check if any other elements would break due to the change.

  1. With exclude_tags included: argument not supported as expected (CLI & VCS)

Screen Shot 2022-07-08 at 1 45 37 PM

Screen Shot 2022-07-08 at 2 04 37 PM

  1. Without exclude_tags included: tfe_workspace resources were still successfully created without error

Screen Shot 2022-07-08 at 1 46 15 PM

@Uk1288 Uk1288 force-pushed the uk1288-add-exclude-tags-to-ds-workspace-ids branch from f0730fa to 631f35f Compare July 8, 2022 16:27
@Uk1288 Uk1288 force-pushed the uk1288-add-exclude-tags-to-ds-workspace-ids branch from bf5e957 to 5e0f62a Compare July 8, 2022 16:43
Copy link
Contributor

@sebasslash sebasslash left a comment

Choose a reason for hiding this comment

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

🕺

@annawinkler annawinkler merged commit 1bb6ade into main Jul 8, 2022
@annawinkler annawinkler deleted the uk1288-add-exclude-tags-to-ds-workspace-ids branch July 8, 2022 20:31
@Uk1288 Uk1288 linked an issue Jul 11, 2022 that may be closed by this pull request
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.

tfe_workspace_ids support to filter excluding specific tags
5 participants