feat(account): support API token operations #363
Conversation
kangasta
commented
Jan 30, 2025
•
kangasta
commented
- Depends on feat(tokens): API tokens support upcloud-go-api#353
- Use released version of Go SDK
- Update CHANGELOG
| @@ -122,6 +123,13 @@ func BuildCommands(rootCmd *cobra.Command, conf *config.Config) { | |||
| permissionsCommand := commands.BuildCommand(permissions.BasePermissionsCommand(), accountCommand.Cobra(), conf) | |||
| commands.BuildCommand(permissions.ListCommand(), permissionsCommand.Cobra(), conf) | |||
|
|
|||
| // Token | |||
There was a problem hiding this comment.
These could be under account.
There was a problem hiding this comment.
Yeah, I was on the verge with this, so up to you. If we want to have the tokens hierarchically under accounts like they are in the API (makes sense), we should have tokens under account as a subcommand, i.e. upctl account token create ....
On the other hand, it just feels better as a top level command/resource, i.e. upctl token create ....
In either case, we should follow the same hierarchy in code (style, packaging...).
| type createParams struct { | ||
| request.CreateTokenRequest | ||
| name string | ||
| expiresIn time.Duration |
There was a problem hiding this comment.
This could use support for d unit, e.g. 90d or 365d could be common values defined by user 🤔
There was a problem hiding this comment.
Definitely should use as hours is not convenient. I suppose one could do it with some custom parser, but the current approach uses flags.DurationVarP which uses time.ParseDuration and only supports up to hours.
| fs.StringVar(&dst.expiresAt, "expires-at", def.expiresAt, "Exact time when the token expires in RFC3339 format. e.g. 2025-01-01T00:00:00Z") | ||
| fs.DurationVar(&dst.expiresIn, "expires-in", def.expiresIn, "Duration until the token expires. e.g. 24h") | ||
| fs.BoolVar(&dst.canCreateTokens, "can-create-tokens", def.canCreateTokens, "Allow token to be used to create further tokens.") | ||
| fs.StringArrayVar(&dst.allowedIPRanges, "allowed-ip-ranges", def.allowedIPRanges, "Allowed IP ranges for the token.") |
There was a problem hiding this comment.
This needs more documentation as token with empty list is invalid(?). Maybe this could be made required as well 🤔
There was a problem hiding this comment.
It is technically not invalid, it just can't be used from anywhere to access API. We deliberately allow creating such tokens if one would like to use them as canary tokens. We could give a warning that the token will not be able to access any API though.
kangasta
force-pushed
the
feat/api_tokens
branch
from
2e336c1 to
f31f8f7
Compare
expires_in is useful for short lived tokens, but not so much for > 24h tokens as it only supports units up to an hour, e.g. 168h for one week.
For consistency with other commands.
For consistency with other options.
