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

fix: Replace wildcards in RBAC objects with explicit resources and verbs #682

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

legal90
Copy link
Contributor

@legal90 legal90 commented Sep 2, 2024

According to Kubernetes documentation and various k8s security guidelines, wildcards in resource and verb entries should be avoided:

Warning

Using wildcards in resource and verb entries could result in overly permissive access being granted to sensitive resources. For instance, if a new resource type is added, or a new subresource is added, or a new custom verb is checked, the wildcard entry automatically grants access, which may be undesirable. The principle of least privilege should be employed, using specific resources and verbs to ensure only the permissions required for the workload to function correctly are applied.

Refs:

This PR could be seen as a continuation of a previous work for hardening the RBAC: #625
It replaces * with explicit verbs and resources, according to KEDA needs.

Checklist

  • I have verified that my change is according to the deprecations & breaking changes policy
  • Commits are signed with Developer Certificate of Origin (DCO - learn more)
  • README is updated with new configuration values (if applicable) learn more
  • A PR is opened to update KEDA core (repo) (if applicable, ie. when deployment manifests are modified)

Similar PR to the main keda repo: kedacore/keda#6129

@legal90 legal90 requested a review from a team as a code owner September 2, 2024 15:32
@JorTurFer
Copy link
Member

JorTurFer commented Sep 2, 2024

This PR look nice, but we can revert if by mistake during the release if we copy the RBAC from KEDA repo. Could you open a PR there too, changing the RBAC? 🙏

@legal90
Copy link
Contributor Author

legal90 commented Sep 3, 2024

Thank you, @JorTurFer ! I submitted the similar PR to the main KEDA repo: kedacore/keda#6129

@legal90 legal90 force-pushed the rbac-no-wildcards branch 2 times, most recently from 3bfed4e to 55e2919 Compare September 10, 2024 18:51
Remove unnecessary "create" and "delete" verbs for CRs which are supposed to be
created and deleted only by a user, and not by an operator.

Signed-off-by: Mikhail Zholobov <[email protected]>
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

@legal90 can we please fix the build?

@JorTurFer
Copy link
Member

It was a CI problem, the fix is merged and I've rebased your PR

@JorTurFer JorTurFer merged commit d626197 into kedacore:main Nov 7, 2024
36 checks passed
@qlikcoe
Copy link

qlikcoe commented Nov 22, 2024

@JorTurFer @zroubalik Will this break a scenario when horizontal-pod-autoscaler ServiceAccount's token was used for connecting to the Datadog Cluster Agent?

As per KEDA's documentation:

Bearer authentication:

token - The ServiceAccount token to connect to the Datadog Cluster Agent. 
The service account needs to have permissions to get, watch, and 
list all external.metrics.k8s.io resources.

UPD
Nevermind. There is another ClusterRole called datadog-cluster-agent-external-metrics-reader that's also attached to the horizontal-pod-autoscaler SA and that role has all the neccessary RBAC rules.

wozniakjan pushed a commit to wozniakjan/keda-charts that referenced this pull request Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants