-
Notifications
You must be signed in to change notification settings - Fork 936
feat(cli): extend duration to longer units #15040
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
Changes from all commits
93e2e47
617fda1
6ce2600
950c056
e294630
02cfb2b
c51cf20
256c085
12e2c2d
7723ae9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,7 @@ func (r *RootCmd) tokens() *serpent.Command { | |
|
||
func (r *RootCmd) createToken() *serpent.Command { | ||
var ( | ||
tokenLifetime time.Duration | ||
tokenLifetime string | ||
name string | ||
user string | ||
) | ||
|
@@ -63,8 +63,30 @@ func (r *RootCmd) createToken() *serpent.Command { | |
if user != "" { | ||
userID = user | ||
} | ||
|
||
var parsedLifetime time.Duration | ||
var err error | ||
|
||
tokenConfig, err := client.GetTokenConfig(inv.Context(), userID) | ||
if err != nil { | ||
return xerrors.Errorf("get token config: %w", err) | ||
} | ||
|
||
if tokenLifetime == "" { | ||
parsedLifetime = tokenConfig.MaxTokenLifetime | ||
} else { | ||
parsedLifetime, err = extendedParseDuration(tokenLifetime) | ||
if err != nil { | ||
return xerrors.Errorf("parse lifetime: %w", err) | ||
} | ||
Comment on lines
+78
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: given that we have the token config available, we can check if the requested lifetime is greater than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah you're completely right - makes sense I just changed it a bit to handle the config fetched ! 👌 |
||
|
||
if parsedLifetime > tokenConfig.MaxTokenLifetime { | ||
return xerrors.Errorf("lifetime (%s) is greater than the maximum allowed lifetime (%s)", parsedLifetime, tokenConfig.MaxTokenLifetime) | ||
} | ||
} | ||
|
||
res, err := client.CreateToken(inv.Context(), userID, codersdk.CreateTokenRequest{ | ||
Lifetime: tokenLifetime, | ||
Lifetime: parsedLifetime, | ||
TokenName: name, | ||
}) | ||
if err != nil { | ||
|
@@ -82,8 +104,7 @@ func (r *RootCmd) createToken() *serpent.Command { | |
Flag: "lifetime", | ||
Env: "CODER_TOKEN_LIFETIME", | ||
Description: "Specify a duration for the lifetime of the token.", | ||
Default: (time.Hour * 24 * 30).String(), | ||
Value: serpent.DurationOf(&tokenLifetime), | ||
Value: serpent.StringOf(&tokenLifetime), | ||
}, | ||
{ | ||
Flag: "name", | ||
|
defelmnq marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,50 @@ func TestDurationDisplay(t *testing.T) { | |
} | ||
} | ||
|
||
func TestExtendedParseDuration(t *testing.T) { | ||
t.Parallel() | ||
for _, testCase := range []struct { | ||
Duration string | ||
Expected time.Duration | ||
ExpectedOk bool | ||
}{ | ||
{"1d", 24 * time.Hour, true}, | ||
{"1y", 365 * 24 * time.Hour, true}, | ||
defelmnq marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{"10s", 10 * time.Second, true}, | ||
{"1m", 1 * time.Minute, true}, | ||
{"20h", 20 * time.Hour, true}, | ||
mtojek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{"10y10d10s", 10*365*24*time.Hour + 10*24*time.Hour + 10*time.Second, true}, | ||
defelmnq marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{"10ms", 10 * time.Millisecond, true}, | ||
defelmnq marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{"5y10d10s5y2ms8ms", 10*365*24*time.Hour + 10*24*time.Hour + 10*time.Second + 10*time.Millisecond, true}, | ||
{"10yz10d10s", 0, false}, | ||
Comment on lines
+51
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: What happens if we try to parse a duration greater than
defelmnq marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{"1µs2h1d", 1*time.Microsecond + 2*time.Hour + 1*24*time.Hour, true}, | ||
{"1y365d", 2 * 365 * 24 * time.Hour, true}, | ||
{"1µs10us", 1*time.Microsecond + 10*time.Microsecond, true}, | ||
// negative related tests | ||
{"-", 0, false}, | ||
{"-2h10m", -2*time.Hour - 10*time.Minute, true}, | ||
{"--10s", 0, false}, | ||
{"10s-10m", 0, false}, | ||
// overflow related tests | ||
{"-20000000000000h", 0, false}, | ||
{"92233754775807y", 0, false}, | ||
{"200y200y200y200y200y", 0, false}, | ||
{"9223372036854775807s", 0, false}, | ||
} { | ||
testCase := testCase | ||
t.Run(testCase.Duration, func(t *testing.T) { | ||
t.Parallel() | ||
actual, err := extendedParseDuration(testCase.Duration) | ||
if testCase.ExpectedOk { | ||
require.NoError(t, err) | ||
assert.Equal(t, testCase.Expected, actual) | ||
} else { | ||
assert.Error(t, err) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func TestRelative(t *testing.T) { | ||
t.Parallel() | ||
assert.Equal(t, relative(time.Minute), "in 1m") | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
@defelmnq @johnstcn WDYT about defining a new type in coder/serpent,
time.LongDuration
?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.
Could be done as a follow-up!
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 created a follow-up issue here - would definitely be a cool one to do indeed.