feat: migrating to validating admission policies for our validating logic (1/)#708
Open
michaelawyu (michaelawyu) wants to merge 4 commits into
Open
feat: migrating to validating admission policies for our validating logic (1/)#708michaelawyu (michaelawyu) wants to merge 4 commits into
michaelawyu (michaelawyu) wants to merge 4 commits into
Conversation
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Member
Author
|
Please note that for this PR the manager is not enabled in the hub agent yet. To control the PR size, additional tests will be submitted as a separate PR. |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces the first slice of work to move KubeFleet's validation logic from validating webhooks to Kubernetes ValidatingAdmissionPolicies (VAP). It adds a startup-time policy manager that reconciles a configurable set of generated VAPs and their bindings, plus two initial generators (deny pod/replicaset writes outside reserved namespaces, deny serviceaccount/token writes in reserved namespaces). It also exports the namespace prefix constants from pkg/utils.
Changes:
- New
pkg/admissionpolicymanagerpackage: manager that create-or-updates and garbage-collects VAPs labeled by Fleet, with two generators (pods/replicasets, serviceaccounts/tokenrequests). - Integration test suite (
envtest) validating the manager produces the expected VAP/VAPB objects (effects deferred to E2E). - Exports
KubePrefix,FleetPrefix,FleetMemberNamespacePrefixfrompkg/utils/common.gofor reuse in the generator configs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/utils/common.go | Exports previously unexported namespace prefix constants so the admission policy generator configs can reuse them. |
| pkg/admissionpolicymanager/manager.go | Core policy manager: reflection-based generator selection, create-or-update with retry, GC of orphan policies/bindings via label selector. |
| pkg/admissionpolicymanager/configs.go | Top-level configuration struct exposing per-generator config and a default. |
| pkg/admissionpolicymanager/podsnreplicasets.go | Generator producing a VAP that denies CREATE of pods/replicasets outside reserved namespaces. |
| pkg/admissionpolicymanager/svcaccountsntokenreqs.go | Generator producing a VAP that denies serviceaccount CRUD and tokenrequest CREATE in reserved namespaces, with user/group whitelist. |
| pkg/admissionpolicymanager/suite_test.go | Ginkgo/envtest suite bootstrap; starts the policy manager once. |
| pkg/admissionpolicymanager/manager_integration_test.go | Asserts the expected VAP and VAPB objects exist with correct spec/labels. |
Comment on lines
+102
to
+132
| // Exempt whitelisted users from this admission policy. | ||
| for _, username := range g.WhitelistedUsernames { | ||
| celExprAccSegs = append(celExprAccSegs, fmt.Sprintf(`request.userInfo.username == "%s"`, username)) | ||
| } | ||
| // Exempt whitelisted user groups from this admission policy. | ||
| for _, userGroup := range g.WhitelistedUserGroups { | ||
| celExprAccSegs = append(celExprAccSegs, fmt.Sprintf(`"%s" in request.userInfo.groups`, userGroup)) | ||
| } | ||
| // Exempt requests from the Kubernetes scheduler, any of the nodes, and (esp.) the | ||
| // Kubernetes controller manager from this admission policy. | ||
| // | ||
| // Important: the Kubernetes controller manager, when deployed with the option | ||
| // --use-service-account-credentials=true, creates a service account token for many of its controllers | ||
| // and uses those tokens to authenticate to the Kubernetes API server. It retrieves a token | ||
| // via the TokenRequest API; failure to exempt this scenario may lead to critical errors. | ||
| celExprAccSegs = append(celExprAccSegs, fmt.Sprintf(`request.userInfo.username == "%s"`, kubeSchedulerUserName)) | ||
| celExprAccSegs = append(celExprAccSegs, fmt.Sprintf(`request.userInfo.username == "%s"`, kubeControllerManagerUserName)) | ||
| celExprAccSegs = append(celExprAccSegs, fmt.Sprintf(`"%s" in request.userInfo.groups`, kubeNodeUserGroup)) | ||
| // Exempt requests from admin users from this admission policy. | ||
| celExprAccSegs = append(celExprAccSegs, fmt.Sprintf(`"%s" in request.userInfo.groups`, adminUserGroup)) | ||
|
|
||
| celExprAcc := strings.Join(celExprAccSegs, " || ") | ||
|
|
||
| celExprNSSegs := []string{} | ||
| for _, prefix := range g.ReservedNamespacePrefixes { | ||
| celExprNSSegs = append(celExprNSSegs, fmt.Sprintf(`object.metadata.namespace.startsWith("%s")`, prefix)) | ||
| } | ||
| celExprNS := strings.Join(celExprNSSegs, " || ") | ||
|
|
||
| celExpr := fmt.Sprintf("!(%s) || (%s)", celExprNS, celExprAcc) | ||
|
|
Member
Author
There was a problem hiding this comment.
Note: this restriction is added in consistency with our existing webhook setup. system:masters is supposed to have unlimited access to all resources BTW.
Comment on lines
+119
to
+121
| // The error message has been set to match with the checks in some of the E2E tests | ||
| // in our existing release pipeline. | ||
| Message: "creating pods and replicas is disallowed in the fleet hub cluster", |
Member
Author
There was a problem hiding this comment.
N/A: as mentioned, the error msg is explicitly kept in the same style as some of the existing checks for releasing purposes.
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
Signed-off-by: michaelawyu <chenyu1@microsoft.com>
This file contains hidden or 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
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
This PR is the first of many following PRs to migrate to validating admission policies for applicable validation logic.
Specifically, this PR:
Releated to #707.
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer