feat(iam): resolve PolicyBinding subject uid from name#653
Open
ecv wants to merge 2 commits into
Open
Conversation
Add a mutating/defaulting webhook on PolicyBinding that resolves a subject's metadata.uid from its name when the uid is omitted. The webhook looks up the named User, Group, or ServiceAccount and stamps its current uid into the subject before the validating CEL rule runs. This makes Group/User/ServiceAccount bindings declaratively committable: a Group and its PolicyBinding can be applied together by GitOps without an out-of-band render-after-reconcile step to read the uid back. It also centralizes the imperative uid read-back that internal controllers already perform. The instance-pinning guarantee is preserved. The webhook stamps the current uid, so a delete+recreate of a same-named subject yields a different uid and the binding no longer matches until re-applied. System groups (name starting with "system:") have no backing object and are left without a uid, matching the existing CEL exemption. Subjects that already carry a uid are untouched. Closes #652 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Adding chainsaw now… |
Add a Chainsaw test exercising the PolicyBinding mutating webhook against a running Milo API server: - A name-only Group subject (no uid) gets the Group's metadata.uid stamped into the stored binding. - A subject naming a non-existent Group is rejected at admission. - A system group subject is admitted without a uid. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
This lets us greatly simplify https://github.com/datum-cloud/infra/pull/2706 |
Contributor
Author
|
bump |
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
1 participant
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.
Summary
Closes #652.
Adds a mutating/defaulting webhook on
PolicyBindingthat resolves a subject'smetadata.uidfrom itsnamewhen the uid is omitted. For each subject with no uid, the webhook looks up the namedUser,Group, orServiceAccountand stamps its current uid into the subject — before the validating CEL rule runs, so the existingSubjectvalidation passes unchanged.This implements Option 1 from the issue (least disruptive): the uid stays immutable in the stored spec; admission just fills it from the name.
Why
A
Group'smetadata.uidis cluster-assigned at create time and unknown at commit time, which made Group/User bindings non-declarative — every caller had to read the uid back imperatively (internal controllers do; external GitOps consumers shipped render-after-reconcile scripts). Now aGroup+ itsPolicyBindingcan be committed and applied together.Behavior
User(cluster-scoped, by name),ServiceAccount(cluster-scoped, by name), andGroup(namespaced, by name + namespace).namestarting withsystem:) — no backing object, matching the existing CEL exemption.Groupsubject omits its namespace.Instance-pinning preserved
The webhook stamps the current uid, so a delete+recreate of a same-named subject yields a different uid and the binding no longer matches until re-applied — same guarantee the uid-on-spec requirement provided, without pushing the read-back onto every caller.
Changes
internal/webhooks/iam/v1alpha1/policybinding_webhook.go—PolicyBindingMutator+SetupPolicyBindingWebhooksWithManager.cmd/milo/controller-manager/controllermanager.go— wire the webhook into the controller manager.config/webhook/manifests.yaml— generated mutating webhook config (mpolicybinding.iam.miloapis.com, CREATE/UPDATE).config/controller-manager/overlays/core-control-plane/rbac/role.yaml— generated; grantsget;list;watchonserviceaccounts(users/groups already present).Taskfile.yaml— include./internal/webhooks/...in the RBAC generation paths so webhook+kubebuilder:rbacmarkers are honored.Testing
internal/webhooks/iam/v1alpha1/policybinding_webhook_test.go— 8 subtests (User/Group/ServiceAccount resolution, multi-subject, skip-when-set, skip system group, not-found error, missing-namespace error).go test ./internal/webhooks/iam/...passes.test/iam/policybinding-subject-uid-resolution/— name-only Group subject gets its uid resolved; non-existent Group is rejected at admission; system group admitted without a uid. Test document validates withchainsaw lint.go build ./...clean.🤖 Generated with Claude Code