openshift / origin

Conformance test suite for OpenShift
http://www.openshift.org
Apache License 2.0
8.49k stars 4.7k forks source link

Project viewers should be able to see service instances #16585

Closed spadgett closed 7 years ago

spadgett commented 7 years ago

Right now users with the view role can't see service catalog serviceinstances (at least not using oc cluster up --version=latest --service-catalog). I believe this was originally done since instance parameters might contain sensitive data. Now that instance and binding parameters can reference values from a secret, should we let viewers list instances and bindings?

@bparees @pmorie @ewolinetz

bparees commented 7 years ago

are instance and binding parameters always provided via a secret now?

spadgett commented 7 years ago

The web console always provides them in a secret

spadgett commented 7 years ago

https://bugzilla.redhat.com/show_bug.cgi?id=1496753

gabemontero commented 7 years ago

For expediency, here is the key nugget from the bugzilla: Error from server (Forbidden): User "user2" cannot list serviceinstances.servicecatalog.k8s.io in the namespace "test": User "user2" cannot list serviceinstances.servicecatalog.k8s.io in project "test" (get serviceinstances.servicecatalog.k8s.io)

gabemontero commented 7 years ago

FYI - this is ultimately blocking the 3.7 card https://trello.com/c/EXHlyops/1230-5-use-groups-extra-information-for-authorization-in-template-service-broker-templatebroker from being accepted.

simo5 commented 7 years ago

circling in @enj too. Is the ask here to change the bootsrap roles to allow this ?

gabemontero commented 7 years ago

On Thu, Oct 5, 2017 at 11:09 AM, Simo Sorce notifications@github.com wrote:

circling in @enj https://github.com/enj too. Is the ask here to change the bootsrap roles to allow this ?

I believe so.

At least for the oc cluster up --service-catalog path, we are talking https://github.com/openshift/origin/blob/master/pkg/oc/bootstrap/docker/openshift/servicecatalog.go#L46-L74 , and updating the view role to get/list for sure, and maybe watch.

You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openshift/origin/issues/16585#issuecomment-334495326, or mute the thread https://github.com/notifications/unsubscribe-auth/ADbadCCpL6gOjZAStu-h0isxE2nXnnl7ks5spPFAgaJpZM4PmKJk .

spadgett commented 7 years ago

The web console needs instance and binding watch

enj commented 7 years ago

@bparees please update pkg/oc/bootstrap/docker/openshift/servicecatalog.go if you agree with this change. I suggest changing your rules to be RuleOrDie since you should never have an error.

bparees commented 7 years ago

@pmorie it's my view that service catalog owns getting these roles straightened out both for ansible and oc cluster up.

enj commented 7 years ago

oc auth reconcile is probably useful here. @deads2k may has some opinions as well.

jpeeler commented 7 years ago

PR in progress: https://github.com/openshift/origin/pull/16872

jpeeler commented 7 years ago

PR merged.