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.

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

allow self SARs using old policy: #4767

Merged
merged 1 commit into from Sep 23, 2015

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Sep 23, 2015

Backwards compatibility without policy updates is broken without this fix. Without this fix, old registry images against new openshift servers running with old policy (before oadm policy reconcile-cluster-roles), will suddenly start failing

There's an extra check in an SAR to ensure that the user can run an SAR against the namespace he's requesting. This is done via an authorization check against the localSAR resource for that namespace. That check should only happen for requests that weren't already authorized via the authorizer before being assigned to a go-restful route.

The integration test was broken because the server starts up with new policy instead of old policy. A new test was added to cover this case.

@sdodson @brenton please evaluate for patch to OSE.

@deads2k
Copy link
Contributor Author

deads2k commented Sep 23, 2015

@smarterclayton @liggitt ptal

@deads2k
Copy link
Contributor Author

deads2k commented Sep 23, 2015

Oh, and since @liggitt is going to ask. There's no default policy granting rights to localRARs across multiple namespaces, so a user would have had to create a custom role to create the situation with an RAR. That makes an integration tests for the RAR side more difficult and a lot less valuable. I made the registry code congruent, but only test one side explictly.

@sdodson
Copy link
Member

sdodson commented Sep 23, 2015

@deads2k Was this broken in the v1.0.6 tag or after that?

Is this is mitigated by the fact that we tell people to run oadm policy reconcile-cluster-roles during upgrades to check for policy changes?

@liggitt
Copy link
Contributor

liggitt commented Sep 23, 2015

Is this is mitigated by the fact that we tell people to run oadm policy reconcile-cluster-roles during upgrades to check for policy changes?

Yes, reconcile roles resolves this

@liggitt
Copy link
Contributor

liggitt commented Sep 23, 2015

lgtm

@deads2k
Copy link
Contributor Author

deads2k commented Sep 23, 2015

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/3376/) (Image: devenv-fedora_2386)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 6abaa95

openshift-bot pushed a commit that referenced this pull request Sep 23, 2015
@openshift-bot openshift-bot merged commit 1b11e86 into openshift:master Sep 23, 2015
@deads2k deads2k deleted the broken-backwards-compat branch November 5, 2015 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants