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

update okta-sdk-golang to v2.9.1 #13439

Merged
merged 3 commits into from Jan 6, 2022
Merged

update okta-sdk-golang to v2.9.1 #13439

merged 3 commits into from Jan 6, 2022

Conversation

ccapurso
Copy link
Contributor

@ccapurso ccapurso commented Dec 15, 2021

It is possible for Vault to get into a deadlock with the current version of the Okta SDK based on how it handles backoff for 429 responses. The SDK trusts the backoff time from the Okta API without any means of short-circuiting with a maximum backoff limit.

We have witnessed long lived Okta login requests resulting in the k8s service registry incorrectly labeling two nodes as vault-active=true. In this scenario, Okta login request goroutines were sleeping in a function called backoffPause. The following is the call chain from Vault’s Okta backend:

In order to enter backoffPause, Vault needed to receive a 429 Too Many Requests response from the Okta API. The implementation of the backoffPause function in version 2.0.0 of the SDK trusts the X-Rate-Limit-Reset header value from the Okta API which is used to calculate the sleep time. My theory is that the SDK calculated an extremely large backoff time given that the default max retries is set to 2. The backoff logic has since changed in newer versions of the SDK by introducing a config parameter Okta.Client.RateLimit.MaxBackoff which defaults to 30 seconds when using NewClient (see builtin/credential/okta/path_config.go:326). The important change is that if the SDK calculates a backoff time using the response headers that exceeds the one configured on the Okta client, the SDK will default to using the one configured on the client, see okta-sdk-golang/requestExecutor.go at v2.9.1.

This PR updates okta-sdk-golang to v2.9.1 in order to consume refactored request handling logic which introduces this maximum backoff limit. In addition to the existing unit tests, I have also manually tested the Okta auth method with MFA to ensure that there have not been any regressions.

@vercel vercel bot temporarily deployed to Preview – vault-storybook December 15, 2021 15:15 Inactive
@vercel vercel bot temporarily deployed to Preview – vault December 15, 2021 15:15 Inactive
@vercel vercel bot temporarily deployed to Preview – vault December 15, 2021 17:48 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook December 15, 2021 17:48 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook January 5, 2022 15:30 Inactive
@vercel vercel bot temporarily deployed to Preview – vault January 5, 2022 15:30 Inactive
@ccapurso ccapurso added this to the 1.10 milestone Jan 5, 2022
Copy link
Member

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

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

LGTM!

@ccapurso ccapurso merged commit 98ef77d into main Jan 6, 2022
@ccapurso ccapurso deleted the okta-sdk-update-2.9.1 branch January 6, 2022 14:42
heppu pushed a commit to heppu/vault that referenced this pull request Jan 13, 2022
* update okta-sdk-golang to v2.9.1

* go mod tidy

* add changelog entry
joatmon08 pushed a commit that referenced this pull request Jan 25, 2022
* update okta-sdk-golang to v2.9.1

* go mod tidy

* add changelog entry
qk4l pushed a commit to qk4l/vault that referenced this pull request Feb 4, 2022
* update okta-sdk-golang to v2.9.1

* go mod tidy

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

Successfully merging this pull request may close these issues.

None yet

2 participants