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

Allocate mux in master.New() #2044

Merged
merged 1 commit into from
Oct 31, 2014
Merged

Allocate mux in master.New() #2044

merged 1 commit into from
Oct 31, 2014

Conversation

erictune
Copy link
Member

Master installs auth in front of other handlers,
but code that uses the master was not giving
this handler to the http servers.
Fix test, now that /_whoami sits behind auth.

@erictune
Copy link
Member Author

@smarterclayton this may affect you.

Actually, I'd like to take this one step further and have Master be responsible for allocating the mux.
There would then be two choices for someone who wants to serve extra endpoints beyond those created by m := master.New()

  • if you want to add endpoints with their own auth, then write your own handler or mux and send anything you can't handle to m.Handler
  • if you want to add endpoint which use the same auth, then do m.Handle(pattern string, handler Handler) and it will add that to the list of things handled behind the auth checks.

Will that work for you?

@smarterclayton
Copy link
Contributor

On Oct 28, 2014, at 6:30 PM, Eric Tune notifications@github.com wrote:

@smarterclayton this may affect you.

Actually, I'd like to take this one step further and have Master be responsible for allocating the mux.
There would then be two choices for someone who wants to serve extra endpoints beyond those created by m := master.New()

if you want to add endpoints with their own auth, then write your own handler or mux and send anything you can't handle to m.Handler
if you want to add endpoint which use the same auth, then do m.Handle(pattern string, handler Handler) and it will add that to the list of things handled behind the auth checks.
Trying to think here - we expose everything that kube exposes, plus our api (covered by auth, under osapi/*) plus the oauth server which shouldn't be covered by auth. We also have a mode where we don't start the kube master, we just connect to it (ComponentPlugin mode). Accepting for the moment the latter is still something we have to solve (how do clients discover plugins plus how does authz cover them), I think what you proposed works for the former. We'd register osapi in the mux and oauth outside it.
Will that work for you?

—
Reply to this email directly or view it on GitHub.

@smarterclayton smarterclayton self-assigned this Oct 28, 2014
@smarterclayton
Copy link
Contributor

I'll try it once you have working code just to be sure in case I missed something.

@erictune
Allocate mux in master.New()
9713b58 
Callsites no longer allocate a mux.
Master now exposes method to install handlers
which use the master's auth code.  Not used
but forks (openshift) are expected to use these
methods.  These methods will later be a point
for additional plug-in functionality.
Integration tests now use the master-provided
handler which has auth, rather than using the mux,
which didn't.  Fix TestWhoAmI now that /_whoami
sits behind auth.
@erictune
Copy link
Member Author

Okay. New stuff to look at. I think the story about plugins and auth is getting slightly clearer.

@erictune erictune changed the title Actually use the handler that has auth installed Allocate mux in master.New() Oct 29, 2014
@erictune
Copy link
Member Author

@smarterclayton PTAL

@erictune erictune mentioned this pull request Oct 30, 2014
Merged
@smarterclayton
Copy link
Contributor

LGTM

@smarterclayton
Copy link
Contributor

Will merge shortly

smarterclayton added a commit that referenced this pull request Oct 31, 2014
@smarterclayton
Merge pull request #2044 from erictune/fix_mux
41f0929 
Allocate mux in master.New()
@smarterclayton smarterclayton merged commit 41f0929 into kubernetes:master Oct 31, 2014
@erictune erictune deleted the fix_mux branch October 31, 2014 04:34
haircommander pushed a commit to haircommander/kubernetes that referenced this pull request Aug 12, 2024
@openshift-merge-bot
Merge pull request kubernetes#2044 from dinhxuanvu/routine-alpha
b2fd74b 
OCPBUGS-35297: UPSTREAM: 126470: Move APIServingWithRoutine to alpha and disabled by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@smarterclayton smarterclayton

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@erictune @smarterclayton