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

add system:image-auditor #8455

Merged
merged 1 commit into from
Apr 12, 2016
Merged

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Apr 11, 2016

Adds a system:image-auditor role for components that want to monitor new images in the docker registry and annotate the image as "good" or "bad" based on scan results.

@smarterclayton Approval? I think this is low risk and helps teams trying to integrate with us.

@simon3z @moolitayer Comments on whether this satisfies your use-case?

@deads2k
Copy link
Contributor Author

deads2k commented Apr 11, 2016

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 7f6db1f

@smarterclayton
Copy link
Contributor

@pweil- concerns for 1.2?

@simon3z
Copy link

simon3z commented Apr 11, 2016

@pweil- concerns for 1.2?

@pweil- @smarterclayton as you're mentioning, +1 for having this in 1.2 or something equivalent in openshift-ansible for management-admin.

@pweil-
Copy link

pweil- commented Apr 11, 2016

No concerns on a new role, low risk.

Rules: []authorizationapi.PolicyRule{
{
APIGroups: []string{imageapi.GroupName},
Verbs: sets.NewString("get", "list", "watch", "patch", "update"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not expect an auditor role to have any write permissions. Is there a different name we could use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not expect an auditor role to have any write permissions. Is there a different name we could use?

This is a thing that vets images to indicate whether they're good or bad. Alternate name suggestions welcome. I thought of this one and image-inspector, but thought that auditor was seemed like a better fit.

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/2880/)

@moolitayer
Copy link
Contributor

Relates to ManageIQ/manageiq#7536

@deads2k Thanks!
Note we don't need read permissions on images - we don't currently read images. Having said that giving all those together might make sense for other/future auditors so it's up to you.

@smarterclayton
Copy link
Contributor

Approved per pweil's assessment

On Mon, Apr 11, 2016 at 10:27 AM, Mooli Tayer [email protected]
wrote:

Relates to ManageIQ/manageiq#7536
ManageIQ/manageiq#7536

@deads2k https://github.com/deads2k Thanks!
Note we don't need read permissions on images - we don't currently read
images
https://github.com/ManageIQ/manageiq/blob/master/app/models/manageiq/providers/openshift/container_manager/refresher.rb#L9.
Having said that giving all those together might make sense for
other/future auditors so it's up to you.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8455 (comment)

@deads2k
Copy link
Contributor Author

deads2k commented Apr 11, 2016

@liggitt barring any better names, the rest looks clean?

@deads2k
Copy link
Contributor Author

deads2k commented Apr 12, 2016

[merge]

@openshift-bot
Copy link
Contributor

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

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 7f6db1f

@liggitt
Copy link
Contributor

liggitt commented Apr 12, 2016

Would "approver" be more indicative of giving write access?

@smarterclayton
Copy link
Contributor

The role definitely is intended to be vague - to cover viewing AND
modifying if necessary. Pruner might technically have this role.

On Tue, Apr 12, 2016 at 9:46 AM, Jordan Liggitt [email protected]
wrote:

Would "approver" be more indicative of giving write access?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8455 (comment)

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.

7 participants