-
Notifications
You must be signed in to change notification settings - Fork 104
Support creation of multiple team tokens #1083
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
Conversation
Datadog ReportBranch report: ✅ 0 Failed, 1437 Passed, 167 Skipped, 20m 46.89s Total Time |
43190a6
to
6a08f10
Compare
team_token.go
Outdated
@@ -13,23 +13,36 @@ import ( | |||
// Compile-time proof of interface implementation. | |||
var _ TeamTokens = (*teamTokens)(nil) | |||
|
|||
const ( | |||
AuthenticationTokensPath = "authentication-tokens/%s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linter complained that this needed to be made a constant. I didn't see a good common file to place this in and didn't really want to create a new file with just one constant, so I put it here. Open to feedback, though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference, here's the lint error:
golangci-lint run .
Error: agent_token.go:119:19: string `authentication-tokens/%s` has 6 occurrences, make it a constant (goconst)
u := fmt.Sprintf("authentication-tokens/%s", url.PathEscape(agentTokenID))
^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes! I see that our threshold for that lint rule is 5 occurrences of the same string. Can you please create a const.go file and add the constant there?
Also, can you update the delete
method in agent_token.go to also use this constant? There is 1 occurrence there too.
Tokens created with the new token API that supports multiple team tokens must be read and deleted by ID rather than by team ID. Legacy tokens can also be deleted via this endpoint.
To appease the linter.
402d08e
to
53fa3ea
Compare
When fetching by ID, we do not necessarily know the team ID, so include the team information as part of the fetch response.
5433106
to
494878b
Compare
team_token.go
Outdated
@@ -13,23 +13,36 @@ import ( | |||
// Compile-time proof of interface implementation. | |||
var _ TeamTokens = (*teamTokens)(nil) | |||
|
|||
const ( | |||
AuthenticationTokensPath = "authentication-tokens/%s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes! I see that our threshold for that lint rule is 5 occurrences of the same string. Can you please create a const.go file and add the constant there?
Also, can you update the delete
method in agent_token.go to also use this constant? There is 1 occurrence there too.
CHANGELOG.md
Outdated
@@ -3,6 +3,8 @@ | |||
## Enhancements | |||
|
|||
* Remove `DefaultProject` from `OrganizationUpdateOptions` to prevent updating an organization's default project, by @netramali [#1078](https://github.com/hashicorp/go-tfe/pull/1078) | |||
* Adds support for creating multiple team tokens by adding `Description` to `TeamTokenCreateOptions`. This provides BETA support, which is EXPERIMENTAL, SUBJECT TO CHANGE, and may not be available to all users, by @mkam [#1056](https://github.com/hashicorp/go-tfe/pull/1083) | |||
* Adds support for reading and deleting team tokens by ID, by @mkam [#1056](https://github.com/hashicorp/go-tfe/pull/1083) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to link PR 1083 instead of 1056?
c75fb4c
to
054efca
Compare
Delete(ctx context.Context, teamID string) error | ||
|
||
// Delete a team token by its token ID. | ||
DeleteByID(ctx context.Context, tokenID string) error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be more clear to say DeleteByTokenID? because Delete and DeleteByID is a bit ambigious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally prefer DeleteByID since the struct and interface is TeamToken
, so this implies that the ID is the team token ID. Here's a similar example with workspaces:
Lines 31 to 42 in 303a2a8
// Read a workspace by its name and organization name. | |
Read(ctx context.Context, organization string, workspace string) (*Workspace, error) | |
// ReadWithOptions reads a workspace by name and organization name with given options. | |
ReadWithOptions(ctx context.Context, organization string, workspace string, options *WorkspaceReadOptions) (*Workspace, error) | |
// Readme gets the readme of a workspace by its ID. | |
Readme(ctx context.Context, workspaceID string) (io.Reader, error) | |
// ReadByID reads a workspace by its ID. | |
ReadByID(ctx context.Context, workspaceID string) (*Workspace, error) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a small nit comment.
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. |
Description
This PR adds support for creating multiple team tokens via the new team token APIs (
teams/:team_id/authentication-tokens
). The changes made are backwards compatible with the existing team token API (teams/:team_id/authentication-token
) that assumes there is only a single token for the team.The new team token API requires the tokens to have a unique description per team. I’ve used the presence of the description in the create options to determine whether
CreateWithOptions
should make the request against the old endpoint or the new endpoint.For deleting and reading tokens, I added new methods for getting the team token by its token ID, as opposed to the existing methods that use the team ID. These methods work still with the legacy style tokens, so they are not part of the beta feature.
Testing plan
Output from tests
Test output of all team token tests