authorize personalSAR based on selfsubjectaccessreviews.authorization.k8s.io#13256
Conversation
|
[test] |
1e245e5 to
891ef8b
Compare
| func (a *personalSARRequestInfoResolver) NewRequestInfo(req *http.Request) (*request.RequestInfo, error) { | ||
| requestInfo, err := a.infoFactory.NewRequestInfo(req) | ||
| if err != nil { | ||
| return requestInfo, err |
There was a problem hiding this comment.
Actually no, The upstream function docs like this: "If error is not nil, RequestInfo holds the information as best it is known before the failure", so that at least some error comes back that might make sense.
|
|
||
| default: | ||
| return false, fmt.Errorf("unexpected request attributes for checking personal access review: %v", extendedAttributes) | ||
| // only match SAR and LSAR requests for personal review |
|
Having recently read the pile of tests we have for the |
|
@enj is that like "minor comments, then LGTM"? |
|
Yup. |
891ef8b to
e3f7aaa
Compare
|
Evaluated for origin test up to e3f7aaa |
|
Removing merge tag -- seeing issues with node dep starting, fix is probably in #13317 so let's let that go first. |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/56/) (Base Commit: c9ed4e5) |
|
[merge] |
|
Evaluated for origin merge up to e3f7aaa |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/23/) (Base Commit: 39ceb75) (Image: devenv-rhel7_6062) |
This eliminates our usage of the AttributeRestrictions for policy rules and the matching authorization attributes. It helps us collapse onto the upstream authorizer interface.
Instead of authorizing using the AttributeRestrictions, the requestInfoResolver uses the information to lie about the resource being accessed. When its a personalSAR or personalLSAR, the requestInfo is written to selfsubjectaccessreviews.authorization.k8s.io
@liggitt This is what we spoke about
@enj ptal
@mfojtik @soltysh fyi