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

Scope secret informers per namespace #737

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JoelSpeed
Copy link

What this PR does / why we need it:

When trying to set up the cloud provider with stricter permissions (not requiring get/list/watch for secrets across all namespaces), I realised that the way that the vSphere provider currently retrieves secrets for credentials requires these wide permissions.

To make the RBAC surface smaller, this PR updates the way we watch secrets to only watch the required namespaces for the secrets from the configuration.

This should be backwards compatible as existing users require get/list/watch on secrets across all namespaces currently.
Future configurations will now be able to specify secret get/list/watch for only the namespaces they need, eg the one containing their credentials for vCenter.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

I have manually tested this but have not updated any unit tests as I wasn't sure where to go for that, any pointers to appropriate tests to update would be appreciated.

Release note:

Provider no longer requires global secret reader permissions and may be scoped to reading secrets only in required namespaces.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 17, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JoelSpeed
Once this PR has been reviewed and has the lgtm label, please assign nicolehanjing for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 17, 2023
@JoelSpeed
Copy link
Author

@lubronzhan Could I ask you to take a look at this PR to give an initial go/no-go on whether you think this is something we want to fix?

@JoelSpeed
Copy link
Author

Any maintainers able to take a look at this PR?

@JoelSpeed
Copy link
Author

@lubronzhan can you help with this optimisation PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants