Merged
Conversation
18fdc24 to
2f81570
Compare
robmonte
reviewed
Apr 29, 2025
Member
robmonte
left a comment
There was a problem hiding this comment.
For my initial review this looks good. Depending on any final changes made to the PR based on the feedback in Slack I think the design for the support of this feature is done well.
I would consider this a soft approval, as someone else will need to give it a bit more thorough of a review and test, since I will be out on PTO.
6 tasks
robmonte
reviewed
May 19, 2025
robmonte
approved these changes
May 19, 2025
Contributor
Author
|
I was also asked to show a test that pre-existing legacy tokens are not effect: i created a token using the legacy api: curl \
--header "Authorization: Bearer $TFE_TOKEN" \
--header "Content-Type: application/vnd.api+json" \
--request POST \
"https://app.terraform.io/api/v2/teams/$TFE_TEAM_ID/authentication-token" \
--data '{
"data": {
"type": "authentication-token",
"attributes": {
"expired-at": "2026-04-06T12:00:00.000Z"
}
}
}
'then i created one with the new vault team tokens from the secret engine $ vault write terraform/role/tfc-mgmt team_id=$TFE_TEAM_ID description="test team description" credential_type="team" ttl=200 max_ttl=300
$ vault read terraform/creds/tfc-mgmt
Key Value
--- -----
lease_id terraform/creds/tfc-mgmt/lB7WOlEDcXFhPJeHc2cwbcvf
lease_duration 3m20s
lease_renewable true
description test team description(7135)
expired_at 2025-06-23T18:51:34Z
token jRRTLapDx9NK7A.atlasv1.<>
token_id at-fVMZ1TyGDtLgm2Gx |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Partially closes: #6
add new role parameter
credential_typeto set to new team token behavior. if you setteam_idandcredential_type=teamthen youll get new behavior. leaving outcredential_typedefaults toteam_legacy(if also settingteam_id) which maintains previous behaviors. if empty,credential_typeis set automatically for users based on the id type they passed and is used for internal logic decisions.Vault Docs PR: hashicorp/vault#30477
Notes:
max_ttlis set we also set the ExpiredAt field. Tokens in API remain even after they expire so vault can still delete and maintain lease as expected.(#####)). This will help with identifying the token in the GUI.TeamLegacymax_ttlandttlon the legacy token but they didnt do anything. This was deceptive to users so I have added an error when trying to set a role like this.with a max_ttl
**properly removed after 10s
when max_ttl set larger than system max_ttl (respected in HCP tf api)
**notice respected max ttl of 768h (default)
with no max_ttl set
uses system max_ttl