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

vault: ensure that token revocation is idempotent #7959

Merged
merged 2 commits into from
May 14, 2020

Conversation

notnoop
Copy link
Contributor

@notnoop notnoop commented May 14, 2020

This ensures that token revocation is idempotent and can handle when
tokens are revoked out of band. Also, on retries, ensure that nomad doesn't extend the token TTL indefinitely.

Idempotency is important to handle some transient failures and retries.
Consider when a single token of a batch fails to be revoked, nomad would
retry revoking the entire batch; tokens already revoked should be
gracefully handled, otherwise, nomad may retry revoking the same
tokens forever.

I've added two tests that fail initially in https://circleci.com/gh/hashicorp/nomad/66794 but pass in the follow up commit.

Fixes #7953

Mahmood Ali added 2 commits May 14, 2020 11:30
This ensures that token revocation is idempotent and can handle when
tokens are revoked out of band.

Idempotency is important to handle some transient failures and retries.
Consider when a single token of a batch fails to be revoked, nomad would
retry revoking the entire batch; tokens already revoked should be
gracefully handled, otherwise, nomad may retry revoking the same
tokens forever.
@notnoop notnoop requested a review from drewbailey May 14, 2020 16:31
@notnoop notnoop self-assigned this May 14, 2020
Copy link
Contributor

@drewbailey drewbailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@notnoop notnoop merged commit b771142 into master May 14, 2020
@notnoop notnoop added this to Done in Nomad - Community Issues Triage via automation May 14, 2020
@notnoop notnoop deleted the b-deleted-vault-accessors branch May 14, 2020 16:39
cgbaker added a commit that referenced this pull request May 14, 2020
@schmichael schmichael added this to the 0.11.2 milestone May 21, 2020
@github-actions
Copy link

github-actions bot commented Jan 5, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vault accessors doesn't get deleted
3 participants