move the oauth server out of the main API server for structure #15744
Conversation
openshift-merge-robot
added
the
size/L
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-merge-robot
added
the
approved
|
I don't see how this can conflict without conflicting. Marking |
|
Stop abusing retest :) |
|
verify failed on an accidental match. #15733 will fix it. |
deads2k
force-pushed
the
oauth-01-split
branch
from
04daab7
to
f918985
Compare
| @@ -194,7 +194,7 @@ func TestAccessOriginWebConsoleMultipleIdentityProviders(t *testing.T) { | |||
| linkRegexps := make([]string, 0) | |||
|
|
|||
| // Verify that the plain /login URI is unavailable when multiple IDPs are in use. | |||
| urlMap["/login"] = urlResults{http.StatusForbidden, ""} | |||
| urlMap["/login"] = urlResults{http.StatusNotFound, ""} | |||
There was a problem hiding this comment.
this status code changed because the oauth server no longer falls through to our "normal" chain. If you request a path handled by the oauth server, it is the end of the chain and will 404 on you if you fall through.
| @@ -33,6 +33,12 @@ import ( | |||
| "github.com/openshift/origin/pkg/user/cache" | |||
| ) | |||
|
|
|||
| const ( | |||
There was a problem hiding this comment.
in later cleanup, we may move these to the oauth/apiserver package. We just don't really want people depending on the package to get a constant through.
|
/retest |
1 similar comment
|
/retest |
deads2k
force-pushed
the
oauth-01-split
branch
from
f918985
to
d6b4f20
Compare
|
/retest |
openshift-merge-robot
added
the
needs-rebase
deads2k
force-pushed
the
oauth-01-split
branch
from
d6b4f20
to
1cee229
Compare
openshift-merge-robot
added
needs-rebase
deads2k
force-pushed
the
oauth-01-split
branch
from
1cee229
to
3bebaf4
Compare
openshift-merge-robot
removed
the
needs-rebase
Not stated in the description, but it also seems to be a step towards pluggable oauth servers in general. Based on my limited exposure to this code and the overall context, everything looks good provided we have good regression tests. |
deads2k
added
lgtm
|
Automatic merge from submit-queue |
@openshift/sig-security fyi
This moves the oauth API server to its own package to make it a little more obvious where pieces come from. It does not restructure the oauth server.
Doing this enables us to organize our handler chain into before and after the standard upstream chain.