[get] Specify expiration of credentials #2
Merged
Conversation
This file contains 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
|
@tjgq PTAL |
ulfjack
reviewed
Jan 18, 2024
| "type": "object" | ||
| }, | ||
| "expires": { | ||
| "description": "The time when the credentials expire and the tool should re-validate them, formatted as ISO 8601.\n\nIf set, tools **SHOULD** not invoke the `get` command of the Credential Helper for the URI before this time has passed or before the remote service indicates the credentials are expired, revoked, or invalid for any other reason. Otherwise, tools **MAY** assume that the credentials are valid indefinitely or for a tool-specified duration before re-validating them.", |
There was a problem hiding this comment.
What about the corner case where the credentials (are expected to) expire in the middle of an operation?
There was a problem hiding this comment.
+1, recommend adding an "unless ..." clause in here.
There was a problem hiding this comment.
Added something to indicate that revalidating if the server returns a permission error is always ok.
There was a problem hiding this comment.
Also added something to allow tools to refresh early if they expect the credentials to expire mid-operation. It's up to the server to decide what to do if they do expire mid-operation
jayconrod
reviewed
Jan 18, 2024
| "type": "object" | ||
| }, | ||
| "expires": { | ||
| "description": "The time when the credentials expire and the tool should re-validate them, formatted as ISO 8601.\n\nIf set, tools **SHOULD** not invoke the `get` command of the Credential Helper for the URI before this time has passed or before the remote service indicates the credentials are expired, revoked, or invalid for any other reason. Otherwise, tools **MAY** assume that the credentials are valid indefinitely or for a tool-specified duration before re-validating them.", |
There was a problem hiding this comment.
+1, recommend adding an "unless ..." clause in here.
tjgq
reviewed
Jan 18, 2024
| "type": "object" | ||
| }, | ||
| "expires": { | ||
| "description": "The time when the credentials expire and the tool should re-validate them, formatted as ISO 8601.\n\nIf set, tools **SHOULD** not invoke the `get` command of the Credential Helper for the URI before this time has passed or before the remote service indicates the credentials are expired, revoked, or invalid for any other reason. Otherwise, tools **MAY** assume that the credentials are valid indefinitely or for a tool-specified duration before re-validating them.", |
There was a problem hiding this comment.
The ISO 8601 spec allows for a (in my opinion) ridiculous number of variants, including ambiguous timestamps in the "local" timezone and esoteric formats such as week and ordinal dates. Should we require it to be in the much smaller subset defined by RFC 3339?
|
PTAL |
jayconrod
approved these changes
Jan 29, 2024
Yannic
added a commit
to EngFlow/bazel
that referenced
this pull request
Feb 8, 2024
This was recently specified in EngFlow/credential-helper-spec#2. RELNOTES[NEW]: Bazel now repects `expires` from Credential Helpers.
copybara-service bot
pushed a commit
to bazelbuild/bazel
that referenced
this pull request
Feb 19, 2024
This was recently specified in EngFlow/credential-helper-spec#2. RELNOTES[NEW]: Bazel now respects `expires` from Credential Helpers. Closes #21249. PiperOrigin-RevId: 608208538 Change-Id: Id168f654093c7491a40364e3988af66ad1767443
bazel-io
pushed a commit
to bazel-io/bazel
that referenced
this pull request
Feb 19, 2024
This was recently specified in EngFlow/credential-helper-spec#2. RELNOTES[NEW]: Bazel now respects `expires` from Credential Helpers. Closes bazelbuild#21249. PiperOrigin-RevId: 608208538 Change-Id: Id168f654093c7491a40364e3988af66ad1767443
tjgq
pushed a commit
to tjgq/bazel
that referenced
this pull request
Feb 20, 2024
This was recently specified in EngFlow/credential-helper-spec#2. RELNOTES[NEW]: Bazel now respects `expires` from Credential Helpers. Closes bazelbuild#21249. PiperOrigin-RevId: 608208538 Change-Id: Id168f654093c7491a40364e3988af66ad1767443
github-merge-queue bot
pushed a commit
to bazelbuild/bazel
that referenced
this pull request
Feb 20, 2024
This was recently specified in EngFlow/credential-helper-spec#2. RELNOTES[NEW]: Bazel now respects `expires` from Credential Helpers. Closes #21249. PiperOrigin-RevId: 608208538 Change-Id: Id168f654093c7491a40364e3988af66ad1767443 Co-authored-by: Yannic Bonenberger <contact@yannic-bonenberger.com>
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.
No description provided.