update RBAC file

[kubernetes-csi]

update RBAC file

[pohly]

Michelle Au <[email protected]> writes:

There are other big changes coming to cluster driver registrar soon, namely that the crd is going to be replaced by an intree beta object. Do you think it's worth it to have a 1.1, or move straight to a 2.0? The crds were technically alpha so should we consider breaking them to warrant a major version bump?

Right now there is no "good" release that works on Kubernetes 1.13. V1.0.1 works but has the broken RBAC file. I guess we could fix this with an updated release note where we quote the correct RBAC rules. A v2.0 won't work on Kubernetes 1.13. I would prefer to tag v1.1.0 on master before replacing the CRDs. If you set a v1.1.0-rc1, then I can do some testing with it.

[msau42]

Looking through the code I think it only needs CSIDriver create and delete permissions

cc @gnufied who is working on testing this in our e2es

[pohly]

Michelle Au <[email protected]> writes:

cc @gnufied who is working on testing this in our e2es

@gnufied: please make sure that the RBAC file in Kubernetes E2E stays in sync with the one here. I have a PR pending (kubernetes/kubernetes#71703) which pulls yaml files via URLs, but that can only work when the right file content is really available elsewhere.

[gnufied]

/assign

[k8s-ci-robot]

@gnufied: GitHub didn't allow me to assign the following users: gnufied.

Note that only kubernetes-csi members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ferdinandhuebner]

The current RBAC definitions don't include permissions to create the CSIDriver CRD which are created on startup:

cluster-driver-registrar/cmd/cluster-driver-registrar/k8s_register.go

Line 54 in e705135

_, err = crdv1beta1client.Create(k8scsicrd.CSIDriverCRD())

At the moment, the cluster-driver-registrar exits with os.Exit(1) if it doesn't have the permission to create the CRD. I'm not quite sure if it would make sense to ignore that error and only exit if it is unable to register the CSIDriver object.

From the current CSI documentation, it is unclear to me who the responsible party for creating the CRDs is. The HostPath example mentions that the CRDs need to be created manually by a kubernetes administrator.

[msau42]

@ferdinandhuebner good catch, our intention is that the CRDs have to be created by the deployment tool or an administrator beforehand.

@saad-ali wdyt of the sidecar trying to install the CRD?

[msau42]

Discussed a bit with @saad-ali wrt sidecar installing the CRD. We need to think through potential version skew issues, if we add new fields to the CRD and want to update the CRD definition. Also we should be consistent with whatever we do for CSINodeInfo too. Right now, we're leaning towards not having the sidecar install the CRD.

[pohly]

The v1.0.1 release was tagged without fixing the RBAC rules contained in that release. We should prepare a release v1.1.0 which no longer installs the CRD and has correct RBAC rules.

@msau42 The master branch has that fixed and looks like it could be turned into v1.1.0.

[msau42]

There are other big changes coming to cluster driver registrar soon, namely that the crd is going to be replaced by an intree beta object. Do you think it's worth it to have a 1.1, or move straight to a 2.0? The crds were technically alpha so should we consider breaking them to warrant a major version bump?

[msau42]

How about cherry picking #19 to release 1.0 and cut a new 1.0.x?

[pohly]

Michelle Au <[email protected]> writes:

How about cherry picking #19 to release 1.0 and cut a new 1.0.x?

As there's no code change in that case, I think it's enough to add a note to the 1.0.1 release note with the errata - basically just a link to the PR and the resulting revision of the RBAC file. Remember that the release process in 1.0.x is still manual, so it would be more work to do a new release there compared to master.

[pohly]

"Patrick Ohly" <[email protected]> writes:

Michelle Au ***@***.***> writes: > How about cherry picking #19 to release 1.0 and cut a new 1.0.x? As there's no code change in that case, I think it's enough to add a note to the 1.0.1 release note with the errata - basically just a link to the PR and the resulting revision of the RBAC file.

Actually, the RBAC rules from PR #19 do not work for v1.0.1, because that release still needs permission to create CRDs. I ended up backporting two more commits from master. See #36

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[fejta-bot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

[k8s-ci-robot]

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.