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.

Sign up for GitHub

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

Jump to bottom

Update controller RBAC for ConfigMap and PersistentVolumeClaim #2187

Merged
merged 1 commit into from
Sep 24, 2024
Merged

Update controller RBAC for ConfigMap and PersistentVolumeClaim #2187

merged 1 commit into from
Sep 24, 2024

Conversation

ChenYi015
Copy link
Contributor

@ChenYi015 ChenYi015 commented Sep 23, 2024 •
edited
Loading

Purpose of this PR

Close #2185

Proposed changes:

  • Add list and watch permissions for configmaps
  • Add get, list, watch, create, update, patch and delete permissions for persistentvolumeclaims

Change Category

  • Bugfix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that could affect existing functionality)
  • Documentation update

Checklist

Before submitting your PR, please review the following:

  • I have conducted a self-review of my own code.
  • I have updated documentation accordingly.
  • I have added tests that prove my changes are effective or that my feature works.
  • Existing unit tests pass locally with my changes.

Additional Notes

@ChenYi015
Update controller RBAC for ConfigMap and PersistentVolumeClaim
10bb135 
Signed-off-by: Yi Chen <github@chenyicn.net>
@ChenYi015
Copy link
Contributor Author

/assign @yuchaoran2011 @vara-bonthu @jacobsalway

jacobsalway
jacobsalway approved these changes Sep 24, 2024
View reviewed changes
Copy link
Member

@jacobsalway jacobsalway left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

This is a subset of the permissions available to the controller service account pre 2.0.0, so I don't see any issue with adding these and creating a 2.0.1 release to fix.

As you said in #2185 (comment), I would imagine these permissions are required for the informers in the caches inside the controller-runtime framework. Not familiar with all the internals but my rough understanding is that a list is required on informer startup while a watch stream is used to stay up to date.

@google-oss-prow google-oss-prow bot added the lgtm label Sep 24, 2024
@ChenYi015
Copy link
Contributor Author

/approve

@google-oss-prow Google OSS Prow
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ChenYi015, jacobsalway

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:

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

@google-oss-prow google-oss-prow bot merged commit 73caefd into kubeflow:master Sep 24, 2024
7 checks passed
ChenYi015 added a commit that referenced this pull request Sep 26, 2024
@ChenYi015
Update controller RBAC for ConfigMap and PersistentVolumeClaim (#2187)
f4f5bea 
Signed-off-by: Yi Chen <github@chenyicn.net>
(cherry picked from commit 73caefd)
ChenYi015 added a commit that referenced this pull request Sep 26, 2024
@ChenYi015
Update controller RBAC for ConfigMap and PersistentVolumeClaim (#2187)
78c85f8 
Signed-off-by: Yi Chen <github@chenyicn.net>
(cherry picked from commit 73caefd)
jbhalodia-slack pushed a commit to jbhalodia-slack/spark-operator that referenced this pull request Oct 4, 2024
@ChenYi015
Update controller RBAC for ConfigMap and PersistentVolumeClaim (kubef…
3f96958 
…low#2187)

Signed-off-by: Yi Chen <github@chenyicn.net>
(cherry picked from commit 73caefd)
@ChenYi015 ChenYi015 deleted the fix/controller-rbac branch October 11, 2024 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobsalway jacobsalway jacobsalway approved these changes

@mwielgus mwielgus Awaiting requested review from mwielgus

@vara-bonthu vara-bonthu Awaiting requested review from vara-bonthu

Assignees

@yuchaoran2011 yuchaoran2011

@jacobsalway jacobsalway

@vara-bonthu vara-bonthu

Labels
approved lgtm size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] RBAC issue with V2.0.0
4 participants
@ChenYi015 @jacobsalway @yuchaoran2011 @vara-bonthu