feat: Role & Role binding webhook #396
Conversation
| // See https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#label-selector-and-annotation-conventions | ||
| fleetPrefix = "placement.karavel.io/" | ||
| // Non-prefixed labels/annotations are reserved for end-users | ||
| FleetPrefix = "placement.karavel.io/" |
There was a problem hiding this comment.
Hi Arvind! I left a comment about this in another PR, PTAL 🙏
| @@ -292,7 +307,7 @@ func (r *Reconciler) syncRole(ctx context.Context, mc *fleetv1alpha1.MemberClust | |||
| currentRole.Rules = expectedRole.Rules | |||
There was a problem hiding this comment.
Hi Arvind! This is actually about line 304, I know it's not a part of PR, and sorry for pointing it out here, but technically speaking we probably shouldn't use cmp outside test code as the package itself recommends.
| case createMemberClusterGVK(): | ||
| klog.V(2).InfoS("handling Member cluster resource", "GVK", createMemberClusterGVK()) | ||
| response = v.handleMemberCluster(ctx, req) | ||
| case createRoleGVK(): |
There was a problem hiding this comment.
Hi Arvind! For this one, I am wondering: we are already blocking all requests to Fleet namespaces from non-whitelisted users, i.e., they cannot create anything (incl. roles/rolebindings) in these destinations -> would be this more general rule already cover the cases here?
There was a problem hiding this comment.
Makes sense closing this PR and will open a PR to address roles and role bindings along with all other resources in an other PR
…zure#396) * do not register handler for the same GVR Signed-off-by: Wei Weng <Wei.Weng@microsoft.com> --------- Signed-off-by: Wei Weng <Wei.Weng@microsoft.com> Co-authored-by: Wei Weng <Wei.Weng@microsoft.com>
2 participants
Description of your changes
Fixes #
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer