-
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
let builders create new imagestreams for pushes #9066
let builders create new imagestreams for pushes #9066
Conversation
230d419
to
a451237
Compare
@@ -397,6 +397,8 @@ func GetBootstrapClusterRoles() []authorizationapi.ClusterRole { | |||
Rules: []authorizationapi.PolicyRule{ | |||
// push and pull images | |||
authorizationapi.NewRule("get", "update").Groups(imageGroup).Resources("imagestreams/layers").RuleOrDie(), | |||
// allow auto-provisioning when pushing an image that doesn't have an imagestream yet |
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.
do we always want builders to have this power?
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.
do we always want builders to have this power?
I can't think of a reasonable reason not to. If a cluster-admin comes up with one, he can change the default role.
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.
does this allow custom builders to create imagestream for arbitrary images?
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.
does this allow custom builders to create imagestream for arbitrary images?
Yes, but given push rights, they can destroy any existing one already. The ability to create new ones doesn't do much if nothing is wired to use them.
@deads2k i would be nice to have something like |
@juanvallejo Want to give that a try? I'd want it to support @fabianofranz not sure what else you've got on his plate. |
yay! |
a451237
to
3973054
Compare
test complete |
[test] |
[merge] |
Evaluated for origin test up to 3973054 |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4364/) (Image: devenv-rhel7_4304) |
Evaluated for origin merge up to 3973054 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4364/) |
Reminder to myself to add a test and make sure it works:
@pruan-rht I think its you.