Skip to content

Retry reactive Vault login after failures#1006

Open
goutamadwant wants to merge 1 commit into
spring-projects:mainfrom
goutamadwant:GH-940-reactive-vault-token-cache
Open

Retry reactive Vault login after failures#1006
goutamadwant wants to merge 1 commit into
spring-projects:mainfrom
goutamadwant:GH-940-reactive-vault-token-cache

Conversation

@goutamadwant

Copy link
Copy Markdown

Summary

  • Reset the cached reactive login attempt when authentication fails, so later token requests can retry instead of replaying the same cached error.
  • Keep successful login caching in place so concurrent callers still share a single token lookup.
  • Add a regression test that fails the first login attempt and verifies the next call retries and succeeds.

Issue

Addresses spring-cloud/spring-cloud-vault#940.

Testing

  • ./mvnw -pl spring-vault-core -Dtest=ReactiveLifecycleAwareSessionManagerUnitTests#shouldRetryLoginAfterFailure test
  • ./mvnw -pl spring-vault-core -Dtest=ReactiveLifecycleAwareSessionManagerUnitTests test
  • src/test/bash/create_certificates.sh
  • env -u SPRING_PROFILES_ACTIVE ./mvnw -pl spring-vault-core test

Signed-off-by: Goutam Adwant <workwithgoutam@gmail.com>
@raghusujju

Copy link
Copy Markdown

@goutamadwant Just to re-confirm, when the token is refreshed for a failed login, will there be an immediate retry after the token set to empty ? I am asking this as in our case, the scheduler runs a few seconds late , by then the token gets expired and this failure scenario is cached and retried.
As I understand from the code changes, your fix will rectify it by setting the cached token to EMPTY. However, will it also try to refresh the token once to see if it can fetch a new token immediately?.

@goutamadwant

Copy link
Copy Markdown
Author

@goutamadwant Just to re-confirm, when the token is refreshed for a failed login, will there be an immediate retry after the token set to empty ? I am asking this as in our case, the scheduler runs a few seconds late , by then the token gets expired and this failure scenario is cached and retried. As I understand from the code changes, your fix will rectify it by setting the cached token to EMPTY. However, will it also try to refresh the token once to see if it can fetch a new token immediately?.

@raghusujju This PR does not start a separate background login immediately after the failed login signal. The failed call still returns the login error.

The change is that the cached failed login Mono is cleared back to EMPTY, so the next call to getSessionToken() / getVaultToken() creates a new login attempt instead of replaying the cached failure. So if the scheduler or caller asks for a token again a few seconds later, that call should go back to Vault and can obtain a fresh token.

We have a regression test which covers this with two consecutive token requests: the first login fails, and the second request invokes the token supplier again and succeeds.

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