fnproject / fn-helm

Helm Chart for Fn
Apache License 2.0
56 stars 24 forks source link

Separated RBAC definitions #19

Closed ahma closed 6 years ago

ahma commented 6 years ago

This pull request creates a new RBAC definition with serviceaccount, clusterrole, and clusterrolebinding support with release name tag. This is a great improvement for running more than one deployment per namespace.

fn-bot commented 6 years ago

CLA Bot

Thank you for your submission! It appears that the following authors have not signed our Contributor License Agreement:

Please do so now by visiting http://www.oracle.com/technetwork/community/oca-486395.html

Once complete, let us know in our community Slack and we’ll send you an Fn T-shirt.

We are working on modernizing the CLA process into a digital signature but it isn’t quite ready yet.

Thank you for being a part of the Fn Community!

derekschultz commented 6 years ago

Thanks for this. I ran into an issue when testing this on minikube with RBAC enabled. The PVCs for MySQL and Redis are not bound and are stuck in a pending state, thereby rendering the services useless. I tried adding persistentvolumes and persistentvolumeclaims to the RBAC resources list, but that didn't resolve the issue. I also tried allowing access to (all) "*" resources to no avail. Not an RBAC expert here, but something seems to be missing.

Lastly, this appears to be a very broad list of resources, some of which aren't necessary to run the Fn service. Is this intentional or can we narrow this down to only what is required?

ahma commented 6 years ago

Thank you for checking this pull request @derekschultz . We have tried this with Pipeline on 3 different cloud providers/managed K8s and it works for us. Minikube is good for quick and dirty testing but fails many times when we deal with complex scenarios, so usually we always test in a real Kubernetes cluster. I investigated the problem and I figured out that when you use minikube with the RBAC extra parameter (--extra-config=apiserver.Authorization.Mode=RBAC) you also need to use the kubeadm bootstrap method (--bootstrapper kubeadm). So you need to use something like this -> minikube start --bootstrapper kubeadm --extra-config=apiserver.Authorization.Mode=RBAC

The second topic: The reason why I use this board list of resources... I think that because the kubernetes offers a lot of useful information (Ex. resourcequotas, limitranges etc..) you probably will use these in your LB app in the near future. With that broad list you mentioned I tried to prevent the permission problems in the future. If you want I can change the list depending on what the code (https://github.com/fnproject/lb/blob/master/lib/k8s_store.go#L142) requires now.
If I can help anything else, please let me know.

derekschultz commented 6 years ago

@ahma Sorry for the delay on this, I've been battling a nasty sickness for what seems like the last 3 weeks. Anyway, I figured out the issue on my end (side note: the kubeadm bootstrapper enables rbac authorization by default). Overall, I'm okay with this PR, but would prefer to limit the scope of the rbac rules as previously mentioned, as currently we don't require more than just the ability to watch pods. We can expand the list as needed/necessary.

So, I think we just need:

rules:
- apiGroups:
  - ""
  resources:
  - pods
  verbs:
  - watch

By the way, have you signed the CLA? If not, please see the CLA bot comment above. If you have already signed it, let me know and I'll see what's going on.

ahma commented 6 years ago

@derekschultz I'm glad to hear that you are better now. I made the resource limitation what you mentioned before. I sent the OCA on 12th of March but I didn't get any answer until now. I will send it again.

fn-bot commented 6 years ago

CLA Bot

All committers have signed the CLA.

derekschultz commented 6 years ago

Got CLA taken care of, thanks!