Skip to content

Distinguish between empty and nil team token description#1088

Merged
mkam merged 2 commits intomainfrom
mkam/team-token-optional-description
Apr 18, 2025
Merged

Distinguish between empty and nil team token description#1088
mkam merged 2 commits intomainfrom
mkam/team-token-optional-description

Conversation

@mkam
Copy link
Copy Markdown
Contributor

@mkam mkam commented Apr 17, 2025

Description

An empty string for a description should be unique from an omitted, nil description, and only nil descriptions should use the legacy token API. This PR updates Description to be a string pointer to distinguish between these two values.

Testing plan

  1. Create a team token with a nil description
  2. Create a second team token with an empty string description. This should not fail because it's considered unique from a nil description.

Output from tests

Test output of all team token tests
-> % ENABLE_BETA=1 go test ./... -v -run "TestTeamTokens"
?   	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/projects	[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   TestTeamTokensCreate
=== RUN   TestTeamTokensCreate/with_valid_options
=== RUN   TestTeamTokensCreate/when_a_token_already_exists
=== RUN   TestTeamTokensCreate/without_valid_team_ID
--- PASS: TestTeamTokensCreate (2.42s)
    --- PASS: TestTeamTokensCreate/with_valid_options (0.23s)
    --- PASS: TestTeamTokensCreate/when_a_token_already_exists (0.48s)
    --- PASS: TestTeamTokensCreate/without_valid_team_ID (0.00s)
=== RUN   TestTeamTokens_CreateWithOptions
=== RUN   TestTeamTokens_CreateWithOptions/with_valid_options
=== RUN   TestTeamTokens_CreateWithOptions/when_a_token_already_exists
=== RUN   TestTeamTokens_CreateWithOptions/without_valid_team_ID
=== RUN   TestTeamTokens_CreateWithOptions/without_an_expiration_date
=== RUN   TestTeamTokens_CreateWithOptions/with_an_expiration_date
--- PASS: TestTeamTokens_CreateWithOptions (3.79s)
    --- PASS: TestTeamTokens_CreateWithOptions/with_valid_options (0.22s)
    --- PASS: TestTeamTokens_CreateWithOptions/when_a_token_already_exists (0.61s)
    --- PASS: TestTeamTokens_CreateWithOptions/without_valid_team_ID (0.00s)
    --- PASS: TestTeamTokens_CreateWithOptions/without_an_expiration_date (0.58s)
    --- PASS: TestTeamTokens_CreateWithOptions/with_an_expiration_date (0.72s)
=== RUN   TestTeamTokens_CreateWithOptions_MultipleTokens
=== RUN   TestTeamTokens_CreateWithOptions_MultipleTokens/with_multiple_tokens
=== RUN   TestTeamTokens_CreateWithOptions_MultipleTokens/with_an_expiration_date
=== RUN   TestTeamTokens_CreateWithOptions_MultipleTokens/without_an_expiration_date
=== RUN   TestTeamTokens_CreateWithOptions_MultipleTokens/when_a_token_already_exists_with_the_same_description
=== RUN   TestTeamTokens_CreateWithOptions_MultipleTokens/without_valid_team_ID
--- PASS: TestTeamTokens_CreateWithOptions_MultipleTokens (5.55s)
    --- PASS: TestTeamTokens_CreateWithOptions_MultipleTokens/with_multiple_tokens (2.14s)
    --- PASS: TestTeamTokens_CreateWithOptions_MultipleTokens/with_an_expiration_date (0.66s)
    --- PASS: TestTeamTokens_CreateWithOptions_MultipleTokens/without_an_expiration_date (0.58s)
    --- PASS: TestTeamTokens_CreateWithOptions_MultipleTokens/when_a_token_already_exists_with_the_same_description (0.89s)
    --- PASS: TestTeamTokens_CreateWithOptions_MultipleTokens/without_valid_team_ID (0.00s)
=== RUN   TestTeamTokensRead
=== RUN   TestTeamTokensRead/with_valid_options
=== RUN   TestTeamTokensRead/with_an_expiration_date_passed_as_a_valid_option
=== RUN   TestTeamTokensRead/when_a_token_doesn't_exists
=== RUN   TestTeamTokensRead/without_valid_organization
--- PASS: TestTeamTokensRead (3.57s)
    --- PASS: TestTeamTokensRead/with_valid_options (1.00s)
    --- PASS: TestTeamTokensRead/with_an_expiration_date_passed_as_a_valid_option (1.08s)
    --- PASS: TestTeamTokensRead/when_a_token_doesn't_exists (0.21s)
    --- PASS: TestTeamTokensRead/without_valid_organization (0.00s)
=== RUN   TestTeamTokensReadByID
=== RUN   TestTeamTokensReadByID/with_legacy,_descriptionless_tokens
=== RUN   TestTeamTokensReadByID/with_multiple_team_tokens
=== RUN   TestTeamTokensReadByID/when_a_token_doesn't_exists
--- PASS: TestTeamTokensReadByID (4.68s)
    --- PASS: TestTeamTokensReadByID/with_legacy,_descriptionless_tokens (0.96s)
    --- PASS: TestTeamTokensReadByID/with_multiple_team_tokens (2.19s)
    --- PASS: TestTeamTokensReadByID/when_a_token_doesn't_exists (0.20s)
=== RUN   TestTeamTokensDelete
=== RUN   TestTeamTokensDelete/with_valid_options
=== RUN   TestTeamTokensDelete/when_a_token_does_not_exist
=== RUN   TestTeamTokensDelete/without_valid_team_ID
--- PASS: TestTeamTokensDelete (2.27s)
    --- PASS: TestTeamTokensDelete/with_valid_options (0.51s)
    --- PASS: TestTeamTokensDelete/when_a_token_does_not_exist (0.21s)
    --- PASS: TestTeamTokensDelete/without_valid_team_ID (0.00s)
=== RUN   TestTeamTokensDeleteByID
=== RUN   TestTeamTokensDeleteByID/with_legacy,_descriptionless_tokens
=== RUN   TestTeamTokensDeleteByID/with_multiple_team_tokens
=== RUN   TestTeamTokensDeleteByID/when_a_token_does_not_exist
=== RUN   TestTeamTokensDeleteByID/with_invalid_token_ID
--- PASS: TestTeamTokensDeleteByID (3.97s)
    --- PASS: TestTeamTokensDeleteByID/with_legacy,_descriptionless_tokens (0.77s)
    --- PASS: TestTeamTokensDeleteByID/with_multiple_team_tokens (1.68s)
    --- PASS: TestTeamTokensDeleteByID/when_a_token_does_not_exist (0.22s)
    --- PASS: TestTeamTokensDeleteByID/with_invalid_token_ID (0.00s)
PASS
ok  	github.com/hashicorp/go-tfe	26.798s

...

An empty string for a description should be unique from an omitted, nil
description. Only nil descriptions should use the legacy token API.
@datadog-terraform-cloud-hashicorp
Copy link
Copy Markdown

datadog-terraform-cloud-hashicorp bot commented Apr 17, 2025

Datadog Report

Branch report: mkam/team-token-optional-description
Commit report: f06daee
Test service: hashicorp/go-tfe

✅ 0 Failed, 1439 Passed, 168 Skipped, 17m 17.29s Total Time
➡️ Test Sessions change in coverage: 1 no change

@mkam mkam force-pushed the mkam/team-token-optional-description branch from eda4110 to 8250e0c Compare April 17, 2025 20:09
@mkam mkam marked this pull request as ready for review April 17, 2025 21:00
@mkam mkam requested a review from a team as a code owner April 17, 2025 21:00
@mkam mkam merged commit da71abb into main Apr 18, 2025
10 checks passed
@mkam mkam deleted the mkam/team-token-optional-description branch April 18, 2025 17:00
@github-actions
Copy link
Copy Markdown

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.

@glennsarti
Copy link
Copy Markdown
Contributor

Hi @mkam ... The release notes say this is breaking change and yet it says "BETA, which is EXPERIMENTAL, SUBJECT TO CHANGE,"

These things seem to be at odds... A breaking experimental change? Would we instead wait for it to not be experimental before merging this?

@mkam
Copy link
Copy Markdown
Contributor Author

mkam commented Apr 23, 2025

@glennsarti Multiple team tokens is the new feature that hasn't been GA'd yet, and there shouldn't be any users of this feature yet. We shouldn't be actually breaking anyone in this change from string to string pointer.

However, the initial go-tfe related code for multiple team tokens where I added Description was merged as part of v1.77.0, so it felt to me that technically this was a breaking change. I also saw in that the breaking change in v1.77.0 mentioned as this feature approaches general availablity.

If this is confusing, I can change the heading for the changelog entry or add more clarification!

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.

3 participants