feat(iam): re-home audit-log-querier grant to a standalone activity role (replaces #677)#679
Draft
ecv wants to merge 1 commit into
Draft
feat(iam): re-home audit-log-querier grant to a standalone activity role (replaces #677)#679ecv wants to merge 1 commit into
ecv wants to merge 1 commit into
Conversation
Replaces #677. That PR added a partial Role manifest (only spec.inheritedRoles) for the existing iam-user-self-manage role to the activity overlay, relying on server-side apply to merge it into the core role. That does not hold under the actual GitOps setup: both the core role (via milo-core-control-plane-crds) and the partial overlay (via milo-activity-policies) are applied by Flux's kustomize-controller, which uses a single shared field manager (kustomize-controller) for every Kustomization. With one shared manager, each apply re-declares its full field set, so the two Kustomizations prune each other's fields on every reconcile -- the role flip-flops between "has includedPermissions, no audit inheritance" and "has audit inheritance, no permissions". The map-type of inheritedRoles only helps across distinct field managers, which Flux does not provide here. This instead delivers the self-audit capability without mutating the shared core role: - New standalone Role activity-self-audit-log-querier in the activity service overlay, inheriting activity.miloapis.com-audit-log-querier. It is a distinct object with a single owner -- no cross-Kustomization field contention. - The user webhook grants it per-user, scoped to the user's own User resource (mirroring the existing user-self-manage PolicyBinding), so self-audit access stays self-scoped. A static Group binding to system:authenticated was rejected because only resourceKind=User is available for a group subject, which would grant every user access to all users' audit logs. - The grant is gated on a configurable role name (empty -> skip), so the core control plane carries zero activity coupling when activity is not deployed -- preserving the dependency-decoupling goal of #676. - The user controller attaches an owner reference to the binding for GC on user deletion, guarded by IsNotFound so it is inert when the activity overlay is absent. Refs: #676 Replaces: #677 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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.
Summary
Replaces #677. Closes #676.
Delivers the "users can query their own audit logs" capability from the activity service overlay without mutating the shared
iam-user-self-manageRole — keeping the foundational milo control plane free of any activity dependency.Why #677 doesn't work
#677 added a partial
Role/iam-user-self-managemanifest (onlyspec.inheritedRoles) to the activity overlay and relied on server-side apply to merge that single entry into the core role.That assumption fails under the actual GitOps setup (datum-cloud/infra):
milo-core-control-plane-crdsFlux Kustomization (./crd/overlays/core-control-plane→../../../roles).milo-activity-policies(./services/activity).kustomize-controllerserver-side-applies every Kustomization under a single shared field manager (kustomize-controller) — there is no per-Kustomization manager and no field-manager override on the Kustomization spec.With one shared manager, each apply re-declares its full field set, so SSA prunes any field that manager owned but no longer lists. The two Kustomizations therefore prune each other's fields on every reconcile (
interval: 1heach). The role flip-flops:milo-core-control-plane-crdsincludedPermissionspresent, audit inheritance gonemilo-activity-policiesinheritedRolesbeinglistType=maponly enables clean merge across distinct field managers owning distinct keys — which Flux does not provide here. (The companion infra PR's premise thatinheritedRolesis "an atomic list" is also incorrect — it is map-type — but the shared-field-manager problem stands regardless.)What this PR does instead
config/services/activity/roles/activity-self-audit-log-querier.yaml— a standaloneRolethat inheritsactivity.miloapis.com-audit-log-querier. A distinct object with a single owner → no cross-Kustomization field contention.config/services/activity/kustomization.yaml— wires the role into the activity overlay.user_webhook.go): grants the role per-user, scoped to the user's ownUserresource, mirroring the existinguser-self-managePolicyBinding, so self-audit access stays self-scoped. Gated on a configurable role name (empty → skip), so the core control plane carries zero activity coupling when activity is not deployed.user_controller.go): attaches an owner reference to the binding for GC on user deletion,IsNotFound-guarded so it is inert when the activity overlay is absent.Why not a static group binding
A single
PolicyBindingtosystem:authenticated(likeorganization-creator-policy) needs no per-user code — but onlyresourceKind: Useris available for aGroupsubject, which would grant every user query access to all users' audit logs. Rejected as a scope regression.Validation
go buildon edited packages → clean.task generate:code→ no manifest diff (no new API types/markers).Coordination
🤖 Generated with Claude Code