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.

Sign up for GitHub

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

Jump to bottom

Filtering by and updating tag-bindings #987

Merged
merged 3 commits into from
Oct 17, 2024
Merged

Filtering by and updating tag-bindings #987

merged 3 commits into from
Oct 17, 2024

Conversation

brandonc
Copy link
Collaborator

@brandonc brandonc commented Oct 16, 2024 •
edited
Loading

  • Filter workspaces and projects by kv tags
  • Fetch tags on workspaces and projects
  • Create workspaces and projects with kv tags
  • Replace kv tags on workspace and projects
$ ENABLE_BETA=1 go test ./... -v -run "Test(Projects|Workspaces)Update/with_valid_options"
?       github.com/hashicorp/go-tfe/examples/backing_data       [no test files]
?       github.com/hashicorp/go-tfe/examples/organizations      [no test files]
?       github.com/hashicorp/go-tfe/examples/configuration_versions     [no test files]
?       github.com/hashicorp/go-tfe/examples/run_errors [no test files]
?       github.com/hashicorp/go-tfe/examples/state_versions     [no test files]
?       github.com/hashicorp/go-tfe/examples/users      [no test files]
?       github.com/hashicorp/go-tfe/examples/registry_modules   [no test files]
?       github.com/hashicorp/go-tfe/mocks       [no test files]
?       github.com/hashicorp/go-tfe/examples/workspaces [no test files]
=== RUN   TestProjectsUpdate
=== RUN   TestProjectsUpdate/with_valid_options
--- PASS: TestProjectsUpdate (2.02s)
    --- PASS: TestProjectsUpdate/with_valid_options (0.96s)
=== RUN   TestWorkspacesUpdate
=== RUN   TestWorkspacesUpdate/with_valid_options
--- PASS: TestWorkspacesUpdate (2.43s)
    --- PASS: TestWorkspacesUpdate/with_valid_options (0.80s)
=== RUN   TestWorkspacesUpdateTableDriven
--- PASS: TestWorkspacesUpdateTableDriven (1.64s)
=== RUN   TestWorkspacesUpdateTableDrivenWithGithubApp
    workspace_integration_test.go:1583: Export a valid GITHUB_APP_INSTALLATION_ID before running this test!
--- SKIP: TestWorkspacesUpdateTableDrivenWithGithubApp (0.00s)
=== RUN   TestWorkspacesUpdateByID
=== RUN   TestWorkspacesUpdateByID/with_valid_options
--- PASS: TestWorkspacesUpdateByID (2.52s)
    --- PASS: TestWorkspacesUpdateByID/with_valid_options (0.57s)
=== RUN   TestWorkspacesUpdateWithDefaultExecutionMode
--- PASS: TestWorkspacesUpdateWithDefaultExecutionMode (2.17s)
PASS
ok      github.com/hashicorp/go-tfe

?       github.com/hashicorp/go-tfe/examples/backing_data       [no test files]
?       github.com/hashicorp/go-tfe/examples/configuration_versions     [no test files]
?       github.com/hashicorp/go-tfe/examples/organizations      [no test files]
?       github.com/hashicorp/go-tfe/examples/registry_modules   [no test files]
?       github.com/hashicorp/go-tfe/examples/run_errors [no test files]
?       github.com/hashicorp/go-tfe/examples/state_versions     [no test files]
?       github.com/hashicorp/go-tfe/examples/users      [no test files]
?       github.com/hashicorp/go-tfe/examples/workspaces [no test files]
?       github.com/hashicorp/go-tfe/mocks       [no test files]
=== RUN   TestProjectsList
=== RUN   TestProjectsList/when_using_a_tags_filter
--- PASS: TestProjectsList (2.88s)
    --- PASS: TestProjectsList/when_using_a_tags_filter (1.34s)
=== RUN   TestWorkspacesList
=== RUN   TestWorkspacesList/when_using_a_tags_filter
--- PASS: TestWorkspacesList (3.89s)
    --- PASS: TestWorkspacesList/when_using_a_tags_filter (1.69s)
PASS
ok      github.com/hashicorp/go-tfe     7.038s

@brandonc brandonc force-pushed the TF-20596-go-tfe-go-tfe-support-for-tags branch 2 times, most recently from 993c619 to 28cbb9a Compare October 16, 2024 22:52
@brandonc brandonc force-pushed the TF-20596-go-tfe-go-tfe-support-for-tags branch from 2b3f087 to 89f0127 Compare October 17, 2024 17:22
ctrombley
ctrombley reviewed Oct 17, 2024
View reviewed changes
Copy link
Contributor

@ctrombley ctrombley 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 great, just a few minor comments but no blockers.

Do we want to test around including both kinds of filters in the same request? Or enforce mutually exclusivity?

helper_test.go
Comment on lines +2522 to +2527
return createProjectWithOptions(t, client, org, ProjectCreateOptions{
Name: randomStringWithoutSpecialChar(t),
})
}

func createProjectWithOptions(t *testing.T, client *Client, org *Organization, options ProjectCreateOptions) (*Project, func()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

project.go
}

var list struct {
*Pagination
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we chose not to paginate the list tag bindings results because of the maximum tags constraint.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I should have highlighted this in the notes. Proper decoding inside go-tfe depends on the collection being this type, but there is no pagination. That's why I kind of hid it inside this anonymous type and didn't return a List type with pagination.

workspace.go
@@ -610,6 +621,10 @@ type WorkspaceUpdateOptions struct {
// Associated Project with the workspace. If not provided, default project
// of the organization will be assigned to the workspace
Project *Project `jsonapi:"relation,project,omitempty"`

// Associated TagBindings of the project. Note that this will replace
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: consider "Note that populating this field will..."?

workspace.go
}

var list struct {
*Pagination
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, I don't think we are paginating the api response.

workspace.go
@@ -1436,7 +1481,7 @@ func (o WorkspaceCreateOptions) valid() error {
if o.AgentPoolID == nil && (o.ExecutionMode != nil && *o.ExecutionMode == "agent") {
return ErrRequiredAgentPoolID
}
if o.TriggerPrefixes != nil && len(o.TriggerPrefixes) > 0 &&
if len(o.TriggerPrefixes) > 0 &&
Copy link
Contributor

Choose a reason for hiding this comment

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

This change had me do a double take. I didn't realize nil slices allowed calls to len!
You can't do that with other nils. TIL.

@brandonc brandonc merged commit 9359d0a into main Oct 17, 2024
7 checks passed
@brandonc brandonc deleted the TF-20596-go-tfe-go-tfe-support-for-tags branch October 17, 2024 20:30
@github-actions GitHub Actions
Copy link

Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ctrombley ctrombley ctrombley approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@brandonc @ctrombley @sebasslash