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

Don't cache clients that are created (without auth) when app loads. #3338

Merged
merged 1 commit into from
Sep 1, 2021

Conversation

absoludity
Copy link
Contributor

Description of the change

One of the most difficult (yet satisfying to find the solution) bugs I've debugged in a long time :P

Our CI for the release was failing only for tests on GKE, but it turned out to be just because the GKE tests are the only ones which use token authentication.

Reproducing the issue locally was tricky at first, because it seemed to go away if you refresh the page (which explained some of the screenshots taken on CI which had errors at first, but passed on refresh).

The issue turned out to be that we were initializing the grpc clients when the app loads. Since there is no auth token present in local storage when the app loads and you're unauthenticated, the clients do not have any authentication metadata. If you re-load after authenticating, no issue. If you're configured with OIDC, this issue is not present since the metadata is added by auth-proxy for requests in-flight.

Benefits

Tests pass when using token auth. Bug fixed when using token auth. Note that it would also mean that if you authed with one token with high privs, refreshed to have things working, logged out and logged in with a low-priv token, you'd still have a client initialized with the high prived token.

Possible drawbacks

microseconds slower? Not sure.

Applicable issues

Additional information

Signed-off-by: Michael Nelson <minelson@vmware.com>
Copy link
Contributor

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

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

Awesome. Good catch. THANKS!!!

@absoludity absoludity merged commit b063daa into vmware-tanzu:master Sep 1, 2021
@absoludity absoludity deleted the dont-cache-grpc-clients branch September 1, 2021 07:49
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.

2 participants