openshift / open-service-broker-sdk

A starting point for creating service brokers implementing the Open Service Broker API
Apache License 2.0
31 stars 33 forks source link

Attempt to resolve issues with roles differences between k8 and oscp #24

Closed maleck13 closed 7 years ago

maleck13 commented 7 years ago

@bparees PTAL when you have a min. Should resolve https://github.com/openshift/open-service-broker-sdk/issues/20 I believe. Most of the changes are moving the files to the new dir structure. I tested the changes against my own kube cluster, but would be good to have someone else try it if possible as this is the first time I have done anything with the rbac stuff in kubernetes.

bparees commented 7 years ago

Can you remove the test-scripts/resources dir/files?

enj commented 7 years ago

This is the wrong approach. It is nearly impossible to manually convert to RBAC and guarantee that everything is in sync. Write a small helper program to handle the conversion as that will save you time and work in the future.

maleck13 commented 7 years ago

@enj Thanks for response. Do you have an example in a code base that I could use for inspiration or is this something that would need to be done from scratch. @bparees Does this PR provide enough value and we can create a separate issue for the tool, or would you prefer we keep this one open?

maleck13 commented 7 years ago

@enj sorry missed your link to the code in the related issue. I will take a look at what you linked to

maleck13 commented 7 years ago

@bparees @enj Hey guys starting to put a tool together. Little unsure about decoding the objects. I have used the UniversalDecoder and UnstructuredJSONScheme is there a way to decode objects like the ClusterRole into their correct types currently only getting *runtime.Unstructured. gist of my very rough code to show how I am decoding https://gist.github.com/maleck13/fac92043989a8a425c247e5e6f4f0432#file-convert-go-L75

enj commented 7 years ago

You are missing the imports that register objects. That file has logic for decoding objects as well.

maleck13 commented 7 years ago

@enj excellent thanks

enj commented 7 years ago

As a side note, I feel like the correct approach would be to define (in Go, not YAML) the RBAC roles you need. Then convert them to Openshift roles (in Go code). Then dump both of list as YAML. So the input to the tool is Go code, not YAML.

pmorie commented 7 years ago

In general I think we should install the broker with a technology conventional for the environment it's being installed into. For k8s, many people are going to want a helm chart to install their broker. On OpenShift, people are going to expect templates. I think there's probably enough divergence in the roles to maintain both an OpenShift template and a k8s template.

I am not saying no to this PR, but stating my own opinion about what this dimension of the experience should be like for users.

maleck13 commented 7 years ago

@pmorie @enj @bparees How about we take this PR as an incremental improvement? Which it is as the roles and installation didn't work on k8s at all and now it does. We could then add new issues for helm installation and the roles generation tool?

bparees commented 7 years ago

@pmorie @enj @bparees How about we take this PR as an incremental improvement? Which it is as the roles and installation didn't work on k8s at all and now it does. We could then add new issues for helm installation and the roles generation tool?

i'm perfectly happy with that approach.

enj commented 7 years ago

SGTM

maleck13 commented 7 years ago

@bparees made that change

bparees commented 7 years ago

awesome, thanks @maleck13!