feat: Webhook for member cluster #393
Conversation
Arvindthiru
changed the title
| } | ||
|
|
||
| // ValidateUserForFleetCR checks to see if user is authenticated to make a request to modify Fleet CRs. | ||
| func ValidateUserForFleetCR(ctx context.Context, client client.Client, whiteListedUsers []string, userInfo authenticationv1.UserInfo) bool { |
There was a problem hiding this comment.
is this used anywhere?
There was a problem hiding this comment.
is the whiteList only useful fro FleetCR?
There was a problem hiding this comment.
Using whiteList for CRD check as well
pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go
Outdated
Show resolved
Hide resolved
| var memberClusterList fleetv1alpha1.MemberClusterList | ||
| if err := client.List(ctx, &memberClusterList); err != nil { | ||
| klog.V(2).ErrorS(err, "failed to list member clusters") | ||
| return false |
There was a problem hiding this comment.
is this list from the cache?
There was a problem hiding this comment.
This client is from the manager that's constructed in hub agent's main.go where we don't pass a NewClient func, since we are not passing it looks like we get new delegating client which uses the cache on reads https://github.com/kubernetes-sigs/controller-runtime/blob/main/pkg/manager/manager.go#L141
Seems like we are using a different version of controller runtime but this seems to be true for both versions
I can see why it would be concern if we read stale data from the cache and reject certain requests from the member agents, but even if the webhook rejects the request once the member agent will retry in the next reconcile and in the meantime I'm assuming the cache will be populated with new data. If that's not the case we may need to use a new client here which doesn't use the cache
There was a problem hiding this comment.
we probably should upgrade our controller-runtime now.
I looked at the code, I think we are reading it from cache.
2 participants
Description of your changes
Fixes #
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
UT and E2E tests
Special notes for your reviewer