Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move CSI RBAC Role definitions from kubernetes/kubernetes to external repos #69379

Closed
saad-ali opened this issue Oct 3, 2018 · 9 comments
Closed
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Milestone

Comments

@saad-ali
Copy link
Member

saad-ali commented Oct 3, 2018

Is this a BUG REPORT or FEATURE REQUEST?:

Uncomment only one, leave it on its own line:

/kind bug

/kind feature

What happened:
RBAC Roles for external CSI components (external-provisioner, external-attacher, etc.) are defined in https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go#L462 and pre-installed on k8s

What you expected to happen:
#68819 (comment):

  1. We should use manifest yamls instead of bootstrapped roles or roles in code (1) (3) (Update RBAC rules for CSIDriver #68821 (comment))
  2. We should deprecate the bootstrapped cluster roles ASAP (3) (1) (CSI E2E tests: fail with upcoming CSI release #68819 (comment))
  3. Cluster Role names should be unique, potentially even tagging UUID's onto the end of the names when creating from manifests (2) (1)
  4. Ideally some sort of sync/import of the manifests between the external repo (source of truth) and the test manifest (4)

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

Anything else we need to know?:

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:
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 3, 2018
@k8s-ci-robot
Copy link
Contributor

@saad-ali: There are no sig labels on this issue. Please add a sig label by either:

  1. mentioning a sig: @kubernetes/sig-<group-name>-<group-suffix>
    e.g., @kubernetes/sig-contributor-experience-<group-suffix> to notify the contributor experience sig, OR

  2. specifying the label manually: /sig <group-name>
    e.g., /sig scalability to apply the sig/scalability label

Note: Method 1 will trigger an email to the group. See the group list.
The <group-suffix> in method 1 has to be replaced with one of these: bugs, feature-requests, pr-reviews, test-failures, proposals.

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.

@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Oct 3, 2018
@saad-ali saad-ali added sig/storage Categorizes an issue or PR as relevant to SIG Storage. sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Oct 3, 2018
@saad-ali saad-ali added this to the v1.13 milestone Oct 3, 2018
@liggitt liggitt added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed kind/bug Categorizes issue or PR as related to a bug. labels Oct 16, 2018
@nikopen
Copy link
Contributor

nikopen commented Oct 31, 2018

Going through v1.13 labeled issues and pinging/putting priority labels

I assume this is low priority - can be removed from the milestone if it doesn't make it

/priority backlog

@k8s-ci-robot k8s-ci-robot added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Oct 31, 2018
@AishSundar
Copy link
Contributor

@saad-ali is this still in scope for 1.13? I dont see any associated PR(s) here. is priority/backlog right for this issue? If so since we are nearing code slush for 1.13 next Friday 11/9 and Code freeze in 2 weeks, can you please move this out of 1.13 if it isnt criitical? thanks

@msau42
Copy link
Member

msau42 commented Nov 5, 2018

@pohly can you update the latest status on this, and also reference any prs already merged or in-flight?

@pohly
Copy link
Contributor

pohly commented Nov 5, 2018

The builtin rules are still there, but are going to be declared as "deprecated" in the 1.13 release notes and instead external RBAC definitions are getting used (see #69868).

Updating all those external RBAC definitions is in progress. Some upstream repos have them already, some don't (external-snapshotter). I also found (a bit late unfortunately) that the existing rules don't pass kubectl validation. I now have the following PRs pending:

This blocks updating the kubernetes-csi/docs.

@AishSundar
Copy link
Contributor

@marpaia for the appropriate Release notes updates for this change.

@pohly we are nearing Code slush (11/9) and Code freeze (11/16) pretty rapidly. Do you think all the 3 pending PRs and Docs can be wrangled by then? Also if you want this to be in 1.13, plz update the priority to important-soon for it to be mergeable once in code slush.

@msau42
Copy link
Member

msau42 commented Nov 5, 2018

@AishSundar the remaining tasks are being done outside kubernetes core. All kubernetes core work has been completed.

@nikopen
Copy link
Contributor

nikopen commented Nov 12, 2018

@AishSundar The three PRs mentioned above have been merged outside k/k, so this has been done for tracking purposes. Closing this.

/close

@pohly I suppose kubernetes-csi/docs can be updated now ^^

@marpaia re:release notes, see above post and #69868 just to be sure

@k8s-ci-robot
Copy link
Contributor

@nikopen: Closing this issue.

In response to this:

@AishSundar The three PRs mentioned above have been merged outside k/k, so this has been done for tracking purposes. Closing this.

/close

@pohly I suppose kubernetes-csi/docs can be updated now ^^

@marpaia re:release notes, see above post and #69868 just to be sure

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Projects
None yet
Development

No branches or pull requests

7 participants