Skip to content
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

Allow setting NotBefore property via init and sign commands #64

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jmecom
Copy link

@jmecom jmecom commented Oct 29, 2018

Adds notbefore as an argument to init and sign to allow setting NotBefore property. If notbefore is not supplied, the NotBefore will be set as 10 minutes earlier than now. I believe this was intended to be the default behavior before, e.g.

// NotBefore is set to be 10min earlier to fix gap on time difference in cluster
NotBefore: time.Now().Add(-600).UTC(),

However, Add takes a Duration as an argument, which is an int64 representing nanoseconds - so I don't think this behaved as described by the comment. I've changed the argument to -time.Minute * 10. Let me know if I should keep it as-is.

@CLAassistant
Copy link

CLAassistant commented Oct 29, 2018

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


jm seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mcpherrinm
Copy link
Contributor

It's kinda weird to call it "NotBefore", but it actually takes an offset from Now(). I'm not sure what exactly to do here. I'd expect NotBefore to take a date, I think. I wonder if we should use a different name for this, but I'm not sure what.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants