-
Notifications
You must be signed in to change notification settings - Fork 315
chore: kyverno to assign roles to users from user group #9482
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
base: main
Are you sure you want to change the base?
Conversation
🤖 Gemini AI Assistant AvailableHi @Omeramsc! I'm here to help with your pull request. You can interact with me using the following commands: Available Commands
How to Use
PermissionsOnly OWNER, MEMBER, or COLLABORATOR users can trigger my responses. This ensures secure and appropriate usage. This message was automatically added to help you get started with the Gemini AI assistant. Feel free to delete this comment if you don't need assistance. |
|
🤖 Hi @Omeramsc, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
...flux-support-nslister-access/generate-support-nslister-clusterrolebinding-clusterpolicy.yaml
Outdated
Show resolved
Hide resolved
filariow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for implementing this. This PR is updating dev/stg and prod environments at the same time. We should split it in two: one for dev/stg and a follow up one for prod
130eec3 to
30bd863
Compare
sadlerap
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Policy looks good, but I think we may want some chainsaw tests to ensure future changes to this policy doesn't break our use-cases.
a2142d1 to
4c6f051
Compare
...s/development/konflux-rbac/konflux-support-nslister-access/.chainsaw-test/chainsaw-test.yaml
Outdated
Show resolved
Hide resolved
| subjects: "{{ userSubjects }}" | ||
| roleRef: | ||
| kind: ClusterRole | ||
| name: view |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we should use this one or the everyone-can-view. @hugares may I ask your feedback here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the plan was to give kflux permissions to the support people. Either konflux-contributor-user-actions or konflux-viewer-user-actions. All people in konflux development teams already have the view and view-everything clusterrole, those 2 are not what is needed so namespace lister return the NS in the list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to konflux-viewer-user-actions, I believe this should be good enough, isn't it?
4c6f051 to
9ee9e2a
Compare
|
I'm not sure about the chainsaw tests; they fail as it seems Kyverno on CI cannot generate cluster-scoped resource. the other policies on infra check namespaced resources. is there a solution we can develop for it? should we create manual tests? @filariow |
87bcc0e to
f0e7168
Compare
...s/development/konflux-rbac/konflux-support-nslister-access/.chainsaw-test/chainsaw-test.yaml
Show resolved
Hide resolved
1ad9704 to
92bed8f
Compare
Add a new policy that watches the konflux-support Group and automatically creates/updates a ClusterRoleBinding with individual User subjects. This enables the namespace-lister service to grant access based on users in the group. This change reflects on dev and stg envs only Signed-off-by: Omer Turner <[email protected]>
92bed8f to
e84f397
Compare
|
@Omeramsc: The following test failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
filariow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: filariow, Omeramsc 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 |
|
overall the PR looks good to me. However e2e tests are failing with Do we have the Group CRD as part of e2e tests? |
Add a new policy that watches the konflux-support Group and automatically creates/updates a ClusterRoleBinding with individual User subjects.
This enables the namespace-lister service to grant access based on users in the group.