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

Create rolebinding for .Release.Namespace implicitly #643

Merged
merged 5 commits into from
Sep 2, 2024

Conversation

jkremser
Copy link
Contributor

When helm chart is installed with a non-empty watchNamespace field, it will create the RoleBinding resource for the cluster role that contains most of the operator rights for each specified namespace in this CSV field. However, not for the namespace into which we actually install KEDA (mostly called keda).

So this PR adds the .Release.Namespace to the list

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) N/A

Fixes #641

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>
@jkremser jkremser requested a review from a team as a code owner May 24, 2024 15:41
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.

LGTM, thanks for the fix

@kamialie
Copy link
Contributor

kamialie commented Jun 5, 2024

I guess you could also increment patch part of the Helm chart version.

@calvinbui
Copy link

i dont think this will solve the problem as the clusterrole is still missing the list and watch actions if .Values.permissions.operator.restrict.secret: true?

https://github.com/kedacore/charts/blob/v2.14.2/keda/templates/manager/clusterrole.yaml#L33-L40

…certs)

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>
@jkremser
Copy link
Contributor Author

@calvinbui i think you are right, I've added this in another commit (only for the .Release.Namespace)
@kamialie I'd let the versioning and releasing to the maintainer

@JorTurFer JorTurFer merged commit 3ab87fb into kedacore:main Sep 2, 2024
36 checks passed
@joebowbeer
Copy link
Contributor

How is the documentation regarding restricting access to secrets affected by #625 and this change?

#685 (comment)

wozniakjan pushed a commit that referenced this pull request Sep 25, 2024
* Create rolebinding for .Release.Namespace implicitly

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>

* Operator should be able to list and watch secrets in the release ns (certs)

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>

---------

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>
wozniakjan pushed a commit to wozniakjan/keda-charts that referenced this pull request Nov 28, 2024
* Create rolebinding for .Release.Namespace implicitly

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>

* Operator should be able to list and watch secrets in the release ns (certs)

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>

---------

Signed-off-by: Jirka Kremser <jiri.kremser@gmail.com>
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.

Restricted (minimal) RBAC permission cannot watch and list secrets in Keda namespace in Keda-operator
6 participants