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

Remove "expiration manager is nil on tokenstore" error log for dr secondary #22137

Merged
merged 4 commits into from Jul 31, 2023

Conversation

akshya96
Copy link
Contributor

@akshya96 akshya96 commented Jul 31, 2023

Jira: https://hashicorp.atlassian.net/browse/VAULT-18066
Prudential log findings: https://docs.google.com/document/d/17FBuLY6Ce584XOjLH2Kfen9Si9_xXFGQwha1bvb0qhY/edit#bookmark=id.ehl8pojz06uk

On DR secondary when we have unauth requests with some token being set in req.ClientToken (not root/ batch token), we log the error "expiration manager is nil on tokenstore". We have this check in lookupInternal to check if we are still restoring the expiration manager as we want to ensure that the token is not expired but DR secondaries do not have expiration manager https://github.com/hashicorp/vault/blob/8253e59752751ce44284ef45130776c2b2812231/vault/token_store.go#L1694C1-L1697C1.
This error is just logged but the command never fails because in CheckToken function, we ignore these errors as non-errors for unauth requests as we do not expect a token for unauth requests https://github.com/hashicorp/vault/blob/8253e59752751ce44284ef45130776c2b2812231/vault/request_handling.go#L281C1-L287C1.
Most of the paths for auth requests are disabled for DR secondary mode so this will not fail in case of auth requests.

This PR removes this error log for DR secondaries.

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jul 31, 2023
@akshya96 akshya96 modified the milestones: 1.14.2, 1.15 Jul 31, 2023
@akshya96 akshya96 requested review from mpalmi and hghaf099 July 31, 2023 19:27
@akshya96 akshya96 marked this pull request as ready for review July 31, 2023 19:28
@github-actions
Copy link

Build Results:
All builds succeeded! ✅

@github-actions
Copy link

CI Results:
All Go tests succeeded! ✅

Copy link
Contributor

@hghaf099 hghaf099 left a comment

Choose a reason for hiding this comment

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

LGTM!

@akshya96 akshya96 added backport/1.12.x backport/1.13.x Backport changes to `release/1.13.x` backport/1.14.x Backport changes to `release/1.14.x` labels Jul 31, 2023
akshya96 added a commit that referenced this pull request Jul 31, 2023
…ondary (#22137)

* add check for dr secondary case

* add changelog
akshya96 added a commit that referenced this pull request Jul 31, 2023
…ondary (#22137) (#22139)

* add check for dr secondary case

* add changelog

Co-authored-by: akshya96 <87045294+akshya96@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.13.x Backport changes to `release/1.13.x` backport/1.14.x Backport changes to `release/1.14.x` hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants