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

Explain how to disable builds by strategy using policy #780

Merged
merged 1 commit into from
Jul 23, 2015

Conversation

csrwng
Copy link

@csrwng csrwng commented Jul 20, 2015

Documents how to disable a build strategy globally and how to grant access only to specified users or to users within a project.

@csrwng
Copy link
Author

csrwng commented Jul 20, 2015

@bparees @deads2k ptal

@@ -912,3 +912,131 @@ case `*scmsecret*`:
<1> The URL of private repository is usually in the form
`[email protected]:<username>/<repository>`.
====


== Limiting the type of build strategies for builds
Copy link
Contributor

Choose a reason for hiding this comment

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

since this requires cluster-admin, maybe it belongs under the admin guide, not the dev guide?

Copy link
Author

Choose a reason for hiding this comment

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

makes sense, I'll move it there

@bparees
Copy link
Contributor

bparees commented Jul 20, 2015

lgtm.

@bparees
Copy link
Contributor

bparees commented Jul 20, 2015

(aside from relocating it and one nit)

$ oadm edit clusterrole edit
----

For each role, remove the line that corresponds to the resource of the strategy to disable:
Copy link
Contributor

Choose a reason for hiding this comment

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

How hard is this to do with a json patch?

Copy link
Author

Choose a reason for hiding this comment

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

I didn't realize we had 'oc patch' until you asked the question :) I will try to come up with a patch... I think that's better than interactive editing

@csrwng csrwng force-pushed the build_type_security branch from 9f37a18 to ba77210 Compare July 20, 2015 21:22
@csrwng
Copy link
Author

csrwng commented Jul 20, 2015

Updated based on review comments. Thx

@bparees
Copy link
Contributor

bparees commented Jul 20, 2015

lgtm

$ oc get clusterrole admin -o yaml | grep -v "builds\/docker" | \
oc replace clusterrole admin -f -

$ oc get clusterrole edit -o yaml | grep -v "builds\/docker" | \
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't object, but why is patch hard to use?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather make patch easy as opposed to working around it.

Copy link
Author

Choose a reason for hiding this comment

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

kubectl patch in its current form only adds to your object. I didn't find a way to delete keys. It doesn't take a real json patch as input.

@deads2k
Copy link
Contributor

deads2k commented Jul 21, 2015

This doc is very clear on how to do this, but I don't think it's obvious why I would want to. Is that described somewhere else?

@csrwng
Copy link
Author

csrwng commented Jul 21, 2015

This doc is very clear on how to do this, but I don't think it's obvious why I would want to. Is that described somewhere else?

It's not. I can add an explanation to the overview

permission to use all strategies (Docker, Source-to-Image, and Custom).


=== Disabling a build strategy globally
Copy link
Contributor

Choose a reason for hiding this comment

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

All the === level headings can be == like Overview.

@adellape
Copy link
Contributor

@csrwng Overall LGTM (one comment inline). Will leave open for the moment in case more was incoming per:

I can add an explanation to the overview

@csrwng csrwng force-pushed the build_type_security branch from ba77210 to bd91645 Compare July 22, 2015 21:26
@csrwng
Copy link
Author

csrwng commented Jul 22, 2015

updated overview and fixed level headings

...

----
<1> Delete this line to disable Docker builds globally
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this line to disable Docker builds globally for users with the admin role.

@bparees
Copy link
Contributor

bparees commented Jul 22, 2015

one comment/question and then lgtm.


----
$ oadm edit clusterrole admin
$ oadm edit clusterrole edit
Copy link

Choose a reason for hiding this comment

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

this should be "oc edit", oadm don't have command edit.

@csrwng csrwng force-pushed the build_type_security branch from bd91645 to 4fd773f Compare July 23, 2015 14:15
@csrwng
Copy link
Author

csrwng commented Jul 23, 2015

addressed comments

@bparees
Copy link
Contributor

bparees commented Jul 23, 2015

lgtm.

@adellape
Copy link
Contributor

👍

adellape added a commit that referenced this pull request Jul 23, 2015
Explain how to disable builds by strategy using policy
@adellape adellape merged commit 0145aa1 into openshift:master Jul 23, 2015
@csrwng csrwng deleted the build_type_security branch December 13, 2016 17:08
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.

5 participants