feat: Webhook for Role/RoleBinding #425
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Arvindthiru
force-pushed
the
NSResourceWebhook
branch
from
61e367a
to
dd7a4f3
Compare
michaelawyu
reviewed
Jul 11, 2023
pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go
Outdated
Show resolved
Hide resolved
pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go
Outdated
Show resolved
Hide resolved
pkg/webhook/fleetresourcehandler/fleetresourcehandler_webhook.go
Outdated
Show resolved
Hide resolved
| return slices.Contains(whiteListedUsers, userInfo.Username) || slices.Contains(userInfo.Groups, mastersGroup) | ||
| } | ||
|
|
||
| // IsUserAuthenticatedServiceAccount returns true if user is a valid/authenticated service account. |
There was a problem hiding this comment.
I have a hunch that this should always be allowed for any resources
michaelawyu
reviewed
Jul 12, 2023
Arvindthiru
force-pushed
the
NSResourceWebhook
branch
from
a301431
to
5bc80df
Compare
michaelawyu
reviewed
Jul 14, 2023
| return true | ||
| // ValidateUserForFleetCRD checks to see if user is not allowed to modify fleet CRDs. | ||
| func ValidateUserForFleetCRD(group string, namespacedName types.NamespacedName, whiteListedUsers []string, userInfo authenticationv1.UserInfo) admission.Response { | ||
| if !checkCRDGroup(group) && isMasterGroupUserOrWhiteListedUser(whiteListedUsers, userInfo) { |
There was a problem hiding this comment.
Hi Arvind! Should this be an OR term?
There was a problem hiding this comment.
I've updated the condition to only allow master group users and whitelisted users to modfiy fleet CRDs. We allow any user to modify other CRDs created by the user
michaelawyu
reviewed
Jul 14, 2023
Arvindthiru
force-pushed
the
NSResourceWebhook
branch
from
703899b
to
e1cb88f
Compare
| return admission.Denied(fmt.Sprintf("failed to validate user: %s in groups: %v to modify fleet CRD: %s", req.UserInfo.Username, req.UserInfo.Groups, crd.Name)) | ||
| } | ||
| return admission.Allowed(fmt.Sprintf("user: %s in groups: %v is allowed to modify CRD: %s", req.UserInfo.Username, req.UserInfo.Groups, crd.Name)) | ||
| return validation.ValidateUserForFleetCRD(group, types.NamespacedName{Name: crd.Name, Namespace: crd.Namespace}, v.whiteListedUsers, req.UserInfo) |
There was a problem hiding this comment.
CRD has no namespace
|
|
||
| // ValidateUserForResource checks to see if user is allowed to modify argued fleet resource. | ||
| func ValidateUserForResource(resKind string, namespacedName types.NamespacedName, whiteListedUsers []string, userInfo authenticationv1.UserInfo) admission.Response { | ||
| if isMasterGroupUserOrWhiteListedUser(whiteListedUsers, userInfo) || isUserAuthenticatedServiceAccount(userInfo) { |
There was a problem hiding this comment.
looks like resKind is not used in the validation?
ryanzhang-oss
approved these changes
Jul 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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