stork icon indicating copy to clipboard operation
stork copied to clipboard

CRD permissions refinement and restriction

Open cehoffman opened this issue 7 years ago • 3 comments

Is this a BUG REPORT or FEATURE REQUEST?:

FEATURE

What happened:

The permissions granted to stork is little larger that I would really like in the CRD group. The default install gives it full ability to create and destroy new CRDS. Since CRDS are a sort of global namespace, we have taken the approach to creating the CRDs an application needs outside of the application itself, e.g. a barebones definition, and then granting the application full control of that instance so it can update and manage those resources.

Currently stork will bomb out when attempting this since it tries to create the CRD always and only checks the error to see if the CRD create failed because it already exists. It does exist but kubernetes rejects the create because stork does not have that permission.

What you expected to happen:

A flag on stork to control CRD creations, i.e. --create-crd=false to disable. Documentation in release notes about new CRDs or migrations that need to happen.

How to reproduce it (as minimally and precisely as possible):

Deploy stork and don't give it permission to create CRDs.

Anything else we need to know?:

Also it appears the example RBAC for stork daemonset has some nonsensical rules in it.

https://github.com/libopenstorage/stork/blob/abf1e99a5f902c630bbafdea4059ca1cfe318da2/specs/stork-daemonset.yaml#L89-L94

Deployments and Statefulsets exist in the extensions or apps api group. AFAIK there is not deployments/extensions or statefulsets/extensions resource.

Environment:

  • Kubernetes version (use kubectl version):
  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):
  • Kernel (e.g. uname -a):
  • Install tools:
  • Others:

cehoffman avatar Oct 22 '18 19:10 cehoffman

We only really need permission to create CRDs. Will it work for you if we remove the other permissions for CRDs?

Also the deployment and statefulset extensions permissions are required for the initializer.

disrani-px avatar Oct 22 '18 19:10 disrani-px

Really want to only give access to the CRD that the application manages. Create unfortunately can't be scoped to an instance since the name is unknown at the time of authorization. We're fine creating the instance outside of the application lifecycle and giving full permission to that instance.

Scoping to just create is a good first step though.

As for the extensions. Isn't the api group still wrong for that though.

cehoffman avatar Oct 22 '18 20:10 cehoffman

@cehoffman Updated CRD permissions to create and get (we need get to wait for it to be ready). Also fixed the api group in the daemonset.

Will add the flag to skip creating CRDs in a future change

disrani-px avatar Oct 24 '18 04:10 disrani-px