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/workspace_ids: Require exact match when no wildcard #752

Conversation

brandonc
Copy link
Collaborator

@brandonc brandonc commented Jan 4, 2023

Description

There was a bug in the new d/tfe_workspace_ids wildcard logic that performed a substring match even when no wildcards were present

External links

Closes #747

Output from acceptance tests

$ TESTARGS="-run TestAccTFEWorkspaceIDsDataSource_noMatch" make testacc
TF_ACC=1 TF_LOG_SDK_PROTO=OFF go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEWorkspaceIDsDataSource_noMatch -timeout 15m
?   	github.com/hashicorp/terraform-provider-tfe	[no test files]
=== RUN   TestAccTFEWorkspaceIDsDataSource_noMatch
--- PASS: TestAccTFEWorkspaceIDsDataSource_noMatch (25.57s)
PASS
ok  	github.com/hashicorp/terraform-provider-tfe/tfe	26.388s

@brandonc brandonc requested a review from a team as a code owner January 4, 2023 19:39
@brandonc brandonc force-pushed the TF-3556-terraform-provider-tfe-gh-issue-748-tfe-project-invalid-value-for-name-for-simple-project-name-with-in-it branch from 6dcf862 to bb4f587 Compare January 4, 2023 21:27
@@ -56,7 +56,7 @@ func includedByName(names map[string]bool, workspaceName string) bool {
case len(name) == 0:
continue
case !strings.HasPrefix(name, "*") && !strings.HasSuffix(name, "*"):
if strings.Contains(workspaceName, name) {
if name == workspaceName {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Nice catch. Wondering how the behavior of strings.Contains(workspaceName, name) creates a bug, since they both perform a string comparison?

Copy link
Member

Choose a reason for hiding this comment

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

It's because Contains doesn't match the whole string -- strings.Contains("partial", "art") will return true, but "partial" == "art" is false.

Copy link
Member

@nfagerlund nfagerlund 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 good. ✅ However! You've got the wrong jira issue number in your branch name, so something unrelated ended up in the review column on our board. 😆

@laurenolivia
Copy link
Contributor

👍 thanks @nfagerlund.

Copy link
Contributor

@laurenolivia laurenolivia left a comment

Choose a reason for hiding this comment

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

@brandonc brandonc merged commit dfcb59e into main Jan 4, 2023
@brandonc brandonc deleted the TF-3556-terraform-provider-tfe-gh-issue-748-tfe-project-invalid-value-for-name-for-simple-project-name-with-in-it branch January 4, 2023 23:21
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 data source returns unexpected workspaces
3 participants