Image change trigger must be able to create all build types #14792
Conversation
|
[severity:bug] (not required for stage exit, but required for release) |
|
lgtm |
smarterclayton
force-pushed
the
trigger_policy
branch
3 times, most recently
from
ff179d2 to
708a770
Compare
|
Two flakes deployment scale to zero and an ovs rpm failure, [test] |
|
👍 sorry for not having catching this |
|
It was extremely non obvious in how it interplayed with the system.
…On Wed, Jun 21, 2017 at 11:47 AM, Michal Fojtik ***@***.***> wrote:
👍 sorry for not having catching this
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#14792 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p0LGTl0jB_TcrVdQ6YNkSGDrke3Nks5sGTsUgaJpZM4OAYjo>
.
|
| @@ -7,6 +7,8 @@ import ( | |||
|
|
|||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | |||
| rbac "k8s.io/kubernetes/pkg/apis/rbac" | |||
|
|
|||
| authorizationapi "github.com/openshift/origin/pkg/authorization/api" | |||
There was a problem hiding this comment.
I'd prefer to avoid this import since the bootstrap policy doesn't logically require an API type dependency.
There was a problem hiding this comment.
We already import this dependency into this package for the other use
There was a problem hiding this comment.
We already import this dependency into this package for the other use
Yes, the mistake will be more obvious moving forward. I won't block on it, but it is a problem.
| @@ -165,6 +167,15 @@ func init() { | |||
| rbac.NewRule("get", "update").Groups(batchGroup).Resources("cronjobs").RuleOrDie(), | |||
| rbac.NewRule("get", "update").Groups(deployGroup, legacyDeployGroup).Resources("deploymentconfigs").RuleOrDie(), | |||
| rbac.NewRule("create").Groups(buildGroup, legacyBuildGroup).Resources("buildconfigs/instantiate").RuleOrDie(), | |||
| // trigger controller must be able to modify these build types | |||
| // TODO: move to a new custom binding that can be removed separately from end user access? | |||
There was a problem hiding this comment.
I think we're ok to add these powers here. The image trigger controller logically requires these to do its job.
smarterclayton
force-pushed
the
trigger_policy
branch
2 times, most recently
from
ed42d30 to
8160d27
Compare
|
[merge][severity:bug] |
|
continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 14 |
|
Evaluated for origin merge up to 8160d27 |
|
re[test] |
Build admission prevents users who don't have access to the synthetic kinds from mutating builds, which includes the image trigger controller now that it is not using the privileged loopback client.
smarterclayton
force-pushed
the
trigger_policy
branch
from
8160d27 to
74ba8ad
Compare
|
[test]
…On Mon, Jun 26, 2017 at 8:23 PM, OpenShift Bot ***@***.***> wrote:
continuous-integration/openshift-jenkins/test FAILURE (
https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2650/)
(Base Commit: 3c328f7
<3c328f7>)
(PR Branch Commit: 74ba8ad
<74ba8ad>
)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#14792 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_pymZlF7DkCT9T8Ur4M-oC2Sd2BPrks5sIEuOgaJpZM4OAYjo>
.
|
|
Evaluated for origin test up to 74ba8ad |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2666/) (Base Commit: 955efb1) (PR Branch Commit: 74ba8ad) |
Build admission prevents users who don't have access to the synthetic
kinds from mutating builds, which includes the image trigger controller
now that it is not using the privileged loopback client.
Fixes #14725
[test] @bparees
@deads2k we don't want admins to remove this binding, so putting it in the existing bindings for this didn't seem appropriate. If you want this somewhere else let me know.