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

Fix identity token panic during invalidation #8015

Merged
merged 5 commits into from
Dec 17, 2019

Conversation

kalafut
Copy link
Contributor

@kalafut kalafut commented Dec 12, 2019

No description provided.

@kalafut kalafut added this to the 1.3.1 milestone Dec 12, 2019
@kalafut kalafut requested review from calvn and a user December 12, 2019 20:17
briankassouf
briankassouf previously approved these changes Dec 12, 2019
Copy link
Member

@jefferai jefferai left a comment

Choose a reason for hiding this comment

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

Don't know if it's the right fix but seems reasonable.

I'd also like to see some nil checks added to oidcCache though.

@kalafut kalafut requested a review from jefferai December 16, 2019 06:16
briankassouf
briankassouf previously approved these changes Dec 16, 2019
@@ -1531,7 +1570,7 @@ func (i *IdentityStore) oidcPeriodicFunc(ctx context.Context) {
i.Logger().Warn("error expiring OIDC public keys", "err", err)
}

i.oidcCache.Flush(nilNamespace)
i.oidcCache.Flush(noNamespace)
Copy link
Member

Choose a reason for hiding this comment

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

Here (and elsewhere in this PR where you call flush), and in SetDefault below, you've added error returns to the functions but aren't actually checking them here.

@briankassouf briankassouf merged commit a8de99b into master Dec 17, 2019
@briankassouf briankassouf deleted the fix-identity-cache-flush branch December 17, 2019 18:45
kalafut pushed a commit that referenced this pull request Dec 17, 2019
* Fix identity token crash during invalidation

* Check for nil namespace

* Fix test

* Add nil check test

* Check OIDC cache errors
briankassouf pushed a commit that referenced this pull request Dec 17, 2019
* Fix identity token crash during invalidation

* Check for nil namespace

* Fix test

* Add nil check test

* Check OIDC cache errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants