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

genericapiserver: turn APIContainer.SecretRoutes into a real ServeMux #38826

Merged
merged 2 commits into from
Dec 16, 2016
Merged

genericapiserver: turn APIContainer.SecretRoutes into a real ServeMux #38826

merged 2 commits into from
Dec 16, 2016

Conversation

sttts
Copy link
Contributor

@sttts sttts commented Dec 15, 2016

The secret routes Mux is actually a http.ServeMux and we are type-casting to it. For downstream we want to wrap it into a restful container which also needs a real http.ServeMux.

@sttts sttts added this to the v1.6 milestone Dec 15, 2016
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 15, 2016
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Dec 15, 2016
deads2k
deads2k reviewed Dec 15, 2016
View reviewed changes
pkg/genericapiserver/mux/container.go
SecretRoutes Mux

// SecretRoutes are not recorded, are not visible at / and do not show up in Swagger.
SecretRoutes *http.ServeMux
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we record this so that we can later call HandledPaths?

Also, the name is bad. AuthzProtectedRouted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are all protected in the HandlerContainer.

The SecretRoutes are not recorded because they are secret ;-) They don't show up anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

Find a better name while you're in here with it. I didn't/don't know what this is other than its behind authz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UnlistedRoutes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NotListedRoutes? InvisibleRoutes?

Copy link
Contributor

Choose a reason for hiding this comment

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

UnlistedRoutes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to UnlistedRoutes. That's a lgtm from you?

@sttts sttts added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Dec 15, 2016
@sttts sttts force-pushed the sttts-secret-routes-real-mux branch from 66769dd to ec79791 Compare December 16, 2016 08:19
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 16, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins Bazel Build failed for commit ec79791c68d4e768797489615537442e958c55e6. Full PR test history.

The magic incantation to run this job again is @k8s-bot bazel test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit ec79791c68d4e768797489615537442e958c55e6. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit ec79791c68d4e768797489615537442e958c55e6. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@sttts sttts force-pushed the sttts-secret-routes-real-mux branch from ec79791 to e49fb2f Compare December 16, 2016 10:04
@deads2k
Copy link
Contributor

deads2k commented Dec 16, 2016

lgtm

@sttts sttts added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 16, 2016
@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit de3b73b into kubernetes:master Dec 16, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deads2k deads2k deads2k left review comments

Assignees

@deads2k deads2k

Labels
area/apiserver cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.6
Development

Successfully merging this pull request may close these issues.
5 participants
@sttts @k8s-reviewable @k8s-ci-robot @deads2k @k8s-github-robot