Conversation
|
[test] |
pkg/cmd/server/origin/handlers.go
Outdated
| } | ||
| // if groups are not specified, then we need to look them up differently depending on the type of user | ||
| // if they are specified, then they are the authority | ||
| groupsSpecified := len(subjects) > 1 |
There was a problem hiding this comment.
subjects might not be groups... shouldn't this be checking whether the header was specified?
There was a problem hiding this comment.
subjects might not be groups... shouldn't this be checking whether the header was specified?
In order to get more than one subject, you must have specified a group. The requestedUser is guaranteed to produce one subject.
I can rewrite if you find the other comparison more obvious, but it is effectively identical.
|
LGTM (except @liggitt comment, I think we should check it header was specified). Also tests looks superb. |
8eabff4 to
e8180e5
Compare
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4233/) (Image: devenv-rhel7_4298) |
e8180e5 to
a13fe3d
Compare
|
Evaluated for origin test up to a13fe3d |
|
Evaluated for origin merge up to a13fe3d |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4233/) |
Adds the ability to specify groups on an impersonation request. This allows clean impersonation on requests through a loopback to our API. This is needed to enable the jenkins template to be created using user credentials in an admission plugin.
Impersonate-Group@openshift/api-review