-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Use webhook authentication from node #9808
Conversation
[test] |
@@ -0,0 +1,49 @@ | |||
package remotemaster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
package name mismatch, filename mismatch
@@ -33,7 +33,9 @@ func (a *TokenAuthenticator) AuthenticateToken(value string) (kuser.Info, bool, | |||
ctx := api.NewContext() | |||
|
|||
token, err := a.tokens.GetAccessToken(ctx, value) | |||
fmt.Printf("#### 1b %v\n", token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove debugging?
c85dac3
to
d65d72d
Compare
re[test] |
comments look minor. Barring anything else, I'll fix up the comments, re-pick the upstream, and merge tomorrow. |
shape looks fine... I don't want to carry an API though, do the upstreams have LGTM? |
I'd be shocked if they were rejected. You could do the user one. |
Don't merge until you have LGTM on API review upstream. On Thu, Jul 14, 2016 at 4:58 PM, David Eads [email protected]
|
Already asked Brian for it. Of we don't get it, we'll be cloning our own and hacking it in. |
5728c44
to
7697e2b
Compare
[merge] |
#9775 re[test] |
#9775 re[test] |
re[test] re[merge] |
continuous-integration/openshift-jenkins/test Running (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6767/) |
secret mount again, re[test] |
Evaluated for origin test up to 4c61f4e |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6770/) (Image: devenv-rhel7_4651) |
Evaluated for origin merge up to 4c61f4e |
Fixes #8988
This introduces a REST endpoint to back the webhook authentication type from upstream. We use it to plumb through token scopes. Also fixes the upstream authorization attributes so we can roundtrip through it.
I had to remove the ugorji codecs in the patch because they won't compile unless we pick down some codec changes. We were already using the reflection path in the old code, so its not any worse. I also can't get protobuf to generate compiling code, tagged @smarterclayton into the upstream pull.
@liggitt review?