-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Use TypeDurationSecond for TTL values in PKI. #3270
Conversation
This has the side effect of simplifying logic quite a bit. This also fixes/changes parseutil.ParseDurationSecond to return a duration of 0 if the input is a string but empty.
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.
Minor comments. Otherwise LGTM!
t.Fatal(err) | ||
} | ||
if resp != nil && resp.IsError() { | ||
t.Fatalf(resp.Error().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.
Is there a need for double Error()
here?
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.
maybe if I switch it to t.Fatal -- t.Fatalf expects a string, and resp.Error() returns an error
builtin/logical/pki/cert_util.go
Outdated
ttl, err = parseutil.ParseDurationSecond(role.TTL) | ||
if err != nil { | ||
return nil, errutil.UserError{Err: fmt.Sprintf( | ||
"invalid requested ttl: %s", err)} |
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.
This may not be a requested ttl
. Its coming from role. Can we change the error message?
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 yeah
builtin/logical/pki/cert_util.go
Outdated
if len(role.MaxTTL) == 0 { | ||
maxTTL = b.System().MaxLeaseTTL() | ||
} else { | ||
if role.MaxTTL != "" { | ||
maxTTL, err = parseutil.ParseDurationSecond(role.MaxTTL) | ||
if err != nil { | ||
return nil, errutil.UserError{Err: fmt.Sprintf( | ||
"invalid ttl: %s", err)} |
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.
max_ttl
* oss/master: Plugin Version Update (#3275) Lazy-load plugin mounts (#3255) changelog++ changelog++ Add pki/root/sign-self-issued. (#3274) Travis, be happier please changelog++ Change auth helper interface to api.Secret. (#3263) changelog++ Try reconnecting Mongo on EOF (#3269) Don't append a trailing slash to the request path if it doesn't actually help find something (#3271) changelog++ Use TypeDurationSecond for TTL values in PKI. (#3270) changelog++ changelog++ Use net.SplitHostPort on Consul address (#3268) Normalize plugin_name option for mount and enable-auth (#3202) Updating Okta lib for credential backend (#3245) Explicitly mention that aws/aws-ec2 were unified under aws.
This has the side effect of simplifying logic quite a bit.
This also fixes/changes parseutil.ParseDurationSecond to return a
duration of 0 if the input is a string but empty.