minimize rbac permission by setting secrets as optional #130

hoyho · 2019-06-19T16:38:12Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug

/kind cleanup

/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
Set secret rbac as optional.
Some drivers may require a secret while some don't.
By default, we should give less permission as possible because ClusterRole is quite a broad.
Other sidecars like external-provisioner and external-attacher have comment out that. It should be ok to do it in external-snapshotter.
ref https://kubernetes-csi.github.io/docs/secrets-and-credentials.html#secret-rbac-rules
related issue kubernetes-csi/external-provisioner#164

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

None

k8s-ci-robot · 2019-06-19T16:38:13Z

Welcome @hoyho!

It looks like this is your first PR to kubernetes-csi/external-snapshotter 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-csi/external-snapshotter has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2019-06-19T16:38:27Z

Hi @hoyho. Thanks for your PR.

I'm waiting for a kubernetes-csi or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

xing-yang · 2019-06-19T18:15:11Z

deploy/kubernetes/rbac.yaml

@@ -51,6 +51,14 @@ rules:
    resources: ["customresourcedefinitions"]
    verbs: ["create", "list", "watch", "delete", "get", "update"]

+#Secret permission is optional.


Add a space after #

thanks for the advice.

xing-yang · 2019-06-19T18:15:51Z

deploy/kubernetes/rbac.yaml

@@ -51,6 +51,14 @@ rules:
    resources: ["customresourcedefinitions"]
    verbs: ["create", "list", "watch", "delete", "get", "update"]

+#Secret permission is optional.
+#Enable it if you need value from secret.


s/if you need value from secret/if your driver needs secret

xing-yang · 2019-06-19T18:18:03Z

deploy/kubernetes/rbac.yaml

@@ -51,6 +51,14 @@ rules:
    resources: ["customresourcedefinitions"]
    verbs: ["create", "list", "watch", "delete", "get", "update"]

+#Secret permission is optional.
+#Enable it if you need value from secret.
+#For example, you have key `csi.storage.k8s.io/controller-publish-secret-name` in StorageClass.parameters.


For example, csi.storage.k8s.io/snapshotter-secret-name is set in VolumeSnapshotClass.

xing-yang · 2019-06-19T18:21:31Z

/ok-to-test

xing-yang · 2019-07-03T15:37:41Z

deploy/kubernetes/rbac.yaml

+# Secret permission is optional.
+# Enable it if your driver needs secret.
+# For example, `csi.storage.k8s.io/snapshotter-secret-name` is set in VolumeSnapshotClass.
+# See https://kubernetes-csi.github.io/docs/secrets-and-credentials.html for more details.


Comments should be added above lines #35-37 and comment out lines #35-37.

xing-yang · 2019-07-03T17:00:49Z

deploy/kubernetes/rbac.yaml

-  - apiGroups: [""]
-    resources: ["secrets"]
-    verbs: ["get", "list"]
+# Secret permission is optional.


I think you need to add space and make sure this aligns with - apiGroups: [""] on line 32. yaml file is space sensitive.

Actually, no need to worry about that. Comment lines in yaml file will be ignored. I have test it and it can apply/create. In case of some CO may have different behavior, I also update the yaml file as you suggested

xing-yang · 2019-07-08T13:31:22Z

/lgtm
/approve

k8s-ci-robot · 2019-07-08T13:31:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hoyho, xing-yang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [xing-yang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…ncy-openshift-4.16-ose-csi-snapshot-controller OCPBUGS-25124: Updating ose-csi-snapshot-controller-container image to be consistent with ART

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 19, 2019

k8s-ci-robot requested review from jingxu97 and saad-ali June 19, 2019 16:38

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jun 19, 2019

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 19, 2019

xing-yang reviewed Jun 19, 2019

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 19, 2019

hoyho force-pushed the rbac_secret branch from ec7496a to 551764b Compare June 19, 2019 18:24

hoyho mentioned this pull request Jun 20, 2019

REQUEST: New membership for hoyho kubernetes/org#935

Closed

6 tasks

xing-yang reviewed Jul 3, 2019

View reviewed changes

hoyho force-pushed the rbac_secret branch 2 times, most recently from e1cddee to dc72235 Compare July 3, 2019 15:51

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 3, 2019

hoyho force-pushed the rbac_secret branch from dc72235 to 4711fd2 Compare July 3, 2019 15:52

xing-yang reviewed Jul 3, 2019

View reviewed changes

minimize rbac permission by setting secret as optional

8963b9d

hoyho force-pushed the rbac_secret branch from 4711fd2 to 8963b9d Compare July 6, 2019 12:15

k8s-ci-robot assigned xing-yang Jul 8, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 8, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2019

k8s-ci-robot merged commit c933d6d into kubernetes-csi:master Jul 8, 2019

hoyho deleted the rbac_secret branch August 15, 2019 12:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

minimize rbac permission by setting secrets as optional #130

minimize rbac permission by setting secrets as optional #130

hoyho commented Jun 19, 2019

k8s-ci-robot commented Jun 19, 2019

k8s-ci-robot commented Jun 19, 2019

xing-yang Jun 19, 2019

hoyho Jun 19, 2019

xing-yang Jun 19, 2019

hoyho Jun 19, 2019

xing-yang Jun 19, 2019

hoyho Jun 19, 2019

xing-yang commented Jun 19, 2019

xing-yang Jul 3, 2019

hoyho Jul 3, 2019

xing-yang Jul 3, 2019

hoyho Jul 6, 2019 •

edited

Loading

xing-yang commented Jul 8, 2019

k8s-ci-robot commented Jul 8, 2019

minimize rbac permission by setting secrets as optional #130

minimize rbac permission by setting secrets as optional #130

Conversation

hoyho commented Jun 19, 2019

k8s-ci-robot commented Jun 19, 2019

k8s-ci-robot commented Jun 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xing-yang commented Jun 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoyho Jul 6, 2019 • edited Loading

Choose a reason for hiding this comment

xing-yang commented Jul 8, 2019

k8s-ci-robot commented Jul 8, 2019

hoyho Jul 6, 2019 •

edited

Loading