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.

Sign up for GitHub

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

Jump to bottom

Check for valid serviceaccount JWT token before inspecting claims #28542

Merged
merged 2 commits into from
Jul 8, 2016
Merged

Check for valid serviceaccount JWT token before inspecting claims #28542

merged 2 commits into from
Jul 8, 2016

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Jul 6, 2016 •
edited
Loading

Moved claims check after the error check that ensures we have a valid JWT token

@liggitt liggitt mentioned this pull request Jul 6, 2016
Merged
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Jul 6, 2016
@liggitt liggitt force-pushed the sa-invalid-token branch from 7e9418a to c95045d Compare July 6, 2016 17:54
@liggitt
Copy link
Member Author

liggitt commented Jul 6, 2016

@krousey fixes non-jwt bearer token auth

@liggitt liggitt added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Jul 6, 2016
@liggitt liggitt mentioned this pull request Jul 6, 2016
Merged
@liggitt
Copy link
Member Author

liggitt commented Jul 6, 2016

@cjcullen PTAL

@liggitt liggitt assigned cjcullen and unassigned dchen1107 Jul 6, 2016
@liggitt
Copy link
Member Author

liggitt commented Jul 6, 2016

cc @colemickens

@cjcullen
Copy link
Member

cjcullen commented Jul 6, 2016

LGTM. Thanks for fixing this up.

@cjcullen cjcullen added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 6, 2016
@liggitt liggitt force-pushed the sa-invalid-token branch from c95045d to 83ebcfd Compare July 6, 2016 18:24
@googlebot googlebot added cla: no and removed cla: yes labels Jul 6, 2016
@liggitt liggitt force-pushed the sa-invalid-token branch from 83ebcfd to cce6772 Compare July 6, 2016 18:25
@googlebot googlebot added cla: yes and removed cla: no labels Jul 6, 2016
@liggitt
Copy link
Member Author

liggitt commented Jul 6, 2016 •
edited
Loading

picked in a revert of @krousey's revert, retagging

@liggitt liggitt added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jul 6, 2016
@k8s-bot
Copy link

k8s-bot commented Jul 6, 2016

GCE e2e build/test passed for commit c95045d7df319f23c0e19d1d8c8da014baf84b0f.

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 6, 2016
@k8s-bot
Copy link

k8s-bot commented Jul 6, 2016

GCE e2e build/test passed for commit 83ebcfdefd3cdacadffaa567d6398f37d8fcc390.

@k8s-bot
Copy link

k8s-bot commented Jul 6, 2016

GCE e2e build/test passed for commit cce6772.

@jessfraz
Copy link
Contributor

jessfraz commented Jul 7, 2016

ah you hit the jwt pkg api update, I did this in a different project ;)

@jessfraz
Copy link
Contributor

jessfraz commented Jul 7, 2016

changes look good to me

@colemickens
Copy link
Contributor

Thanks for catching and fixing this @liggitt, sorry I didn't catch it in my initial update PR.

Do the GKE e2e tests not run for PRs, maybe just on some interval?

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jul 8, 2016

GCE e2e build/test passed for commit cce6772.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 9a414d2 into kubernetes:master Jul 8, 2016
@cjcullen
Copy link
Member

cjcullen commented Jul 8, 2016

@colemickens We're still working on hooking in some small amount of GKE smoke tests into the PR builder.

@liggitt liggitt deleted the sa-invalid-token branch July 11, 2016 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cjcullen cjcullen

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@liggitt @cjcullen @k8s-bot @jessfraz @colemickens @k8s-github-robot @googlebot @dchen1107