Add generic apply admission seam for approval flows#488
Merged
Conversation
The enterprise approval process needs to accept tagged-artifact creates before production side effects while still replaying approved objects through the normal apply path. This adds a neutral apply interceptor and resource route context rather than enterprise-specific approval logic in OSS. Constraint: Approval must not reintroduce metadata.version or fork production apply semantics Rejected: Direct enterprise store upsert on approval | bypasses validation, authz, finalizers, audit, and post-upsert hooks Confidence: high Scope-risk: moderate Directive: Keep ApplyObject aligned with POST /v0/apply; approval replay depends on this being the production path Tested: rtk go test -tags=integration ./pkg/registry/resource ./internal/registry/service/deployment ./internal/registry/api/router ./internal/registry Tested: rtk go test ./pkg/api/v0 ./pkg/types ./pkg/registry/resource ./internal/registry/service/deployment ./internal/registry/api/router ./internal/registry Not-tested: full repository test suite
The approval seam is useful for the current synchronous handler architecture, but prior KRT notes make it clear that reconciler-owned admission and promotion should eventually replace these callbacks. Mark the OSS hook surfaces so reviewers treat them as an incremental bridge rather than durable architecture. Constraint: KRT migration is a separate follow-up track and should port primitives, not replay the old branch wholesale Confidence: high Scope-risk: narrow Directive: Remove these hooks once reconciler admission/staging owns the approval transition Tested: rtk go test ./pkg/types ./pkg/registry/resource ./internal/registry/api/router Not-tested: full repository suite
The approval seam branch introduced references to both public API generations, and the repository formatter requires the v0 import to be explicitly aliased. Committing the formatter's output keeps verify and golangci-lint aligned on clean checkouts. Constraint: CI runs goimports/gci through make verify and golangci-lint. Rejected: Leave the implicit import for readability | CI rewrites it and fails the branch. Confidence: high Scope-risk: narrow Tested: rtk go test -tags=integration ./pkg/registry/resource ./internal/registry/service/deployment ./internal/registry/api/router ./internal/registry Not-tested: Full rtk make verify before commit, because verify is designed to fail while this generated formatter diff is still unstaged.
Import had grown a route-specific authorizer override that duplicated apply behavior and made enterprise approval policy hard to reason about. This makes apply source-aware, gives import a Prepare hook for scanner enrichment, and routes both /v0/apply and /v0/import through the same authz/admission/upsert path. Constraint: Enterprise approval must keep import admin-only for tagged artifacts without special OSS import override plumbing Rejected: Keep ImportAuthorizers override | preserves a second write policy surface Confidence: high Scope-risk: moderate Directive: Add new write sources to AdmissionInput instead of adding route-specific authz overrides Tested: rtk go test ./pkg/registry/resource ./pkg/importer ./internal/registry/api/handlers/v0/importpipeline ./internal/registry/api/router ./internal/registry Tested: rtk go test -tags=integration ./pkg/registry/resource ./pkg/importer ./internal/registry/api/handlers/v0/importpipeline ./internal/registry/api/router ./internal/registry Not-tested: full make verify before commit because the target enforces a clean git diff
The adapter exists only to translate the public pkg/types admission shape into the internal resource route shape. Calling that out near the function makes the source-aware admission wiring easier to review without re-reading the package boundary. Confidence: high Scope-risk: narrow Tested: git diff --check Not-tested: full test suite; comment-only change
The previous source-aware admission slice had both pkg/types and resource admission shapes, which required a field-copy adapter in the composition root. The route and apply layers now consume the public types.Admission contract directly, leaving source selection as data instead of a wrapper boundary. Constraint: Enterprise needs a public app-level admission hook, but OSS resource routes should not duplicate the same callback shape Rejected: Keep adaptAdmission with a clearer comment | still preserves two nearly identical contracts Confidence: high Scope-risk: moderate Directive: Keep admission source policy on types.AdmissionInput; do not reintroduce resource-local admission mirrors unless the public contract changes materially Tested: rtk go test ./pkg/registry/resource ./pkg/importer ./internal/registry/api/handlers/v0/importpipeline ./internal/registry/api/router ./internal/registry Tested: rtk go test -tags=integration ./pkg/registry/resource ./pkg/importer ./internal/registry/api/handlers/v0/importpipeline ./internal/registry/api/router ./internal/registry Not-tested: full make verify before commit because the target enforces a clean git diff
The previous admission hook was still a pre-production escape hatch: applyCore owned the production write and had to special-case handled admission results. This moves the production write behind a default OSS ProductionAdmission implementation, so every apply ends by calling one admission function that returns the final apply result. Constraint: Enterprise approval needs to stage some writes while all other writes keep the normal OSS production behavior Rejected: Keep Handled on AdmissionResult | preserves split ownership between admission and applyCore Confidence: high Scope-risk: moderate Directive: Admission implementations should return the final apply status; do not reintroduce handled/intercepted branches in applyCore Tested: rtk go test ./pkg/registry/resource ./pkg/importer ./internal/registry/api/handlers/v0/importpipeline ./internal/registry/api/router ./internal/registry Tested: rtk go test -tags=integration ./pkg/registry/resource ./pkg/importer ./internal/registry/api/handlers/v0/importpipeline ./internal/registry/api/router ./internal/registry Not-tested: full make verify before commit because the target enforces a clean git diff
Approval replay should read as an explicit production apply path, not as nil admission with hidden default behavior. Set the replay config to resource.ProductionAdmission directly while keeping the public apply endpoint on the configured admission wrapper. Confidence: high Scope-risk: narrow Directive: Keep production replay explicit; nil admission should remain a default, not a semantic signal in route wiring Tested: rtk go test ./pkg/registry/resource ./internal/registry/api/router ./internal/registry Not-tested: full make verify for one-line route wiring cleanup
xytian315
reviewed
May 18, 2026
Comment on lines
+98
to
+100
| if errors.Is(err, v1alpha1.ErrDanglingRef) { | ||
| return c.persistReferencePending(ctx, deployment, err) | ||
| } |
Collaborator
There was a problem hiding this comment.
Question: What happens here if targetRef/runtimeRef is genuinely missing (typo / deleted resource)? Should we split into ErrPendingRef vs ErrDanglingRef, so a typo'd ref still fails loudly?
Contributor
Author
There was a problem hiding this comment.
typo still fails earlier: applyCore runs ResolveObjectRefs before persistence, so bad target/runtime ref does not write Deployment row
Deletes were the remaining write path that still went straight from authorization into production storage. This adds a dedicated delete-admission hook beside apply admission so downstream approval can stage destructive requests without changing OSS production behavior. Constraint: Keep nil hooks on the existing OSS delete path. Rejected: Overload apply admission for deletes | delete has different inputs and post-delete behavior. Confidence: high Scope-risk: moderate Directive: Keep create/update admission and delete admission separate unless the shared contract is redesigned end to end. Tested: go test -tags=integration ./pkg/registry/resource ./internal/registry/api/handlers/v0/crud ./internal/client Tested: go test ./...
Main removed the legacy import pipeline while the approval branch added admission wiring around the v1alpha1 router. The resolution keeps main's deletion and preserves the generic apply/delete admission hooks plus downstream resource route context. Constraint: PR #488 was merge-conflicting against origin/main Rejected: Reintroduce /v0/import wiring | main deleted the importer surface and approval does not need it Confidence: high Scope-risk: moderate Tested: go test -tags=integration ./pkg/registry/resource ./internal/registry/api/handlers/v0/crud ./internal/client
The main merge left pkg/types import grouping in a state that gci rewrites during verify. Committing the formatter result keeps lint-go and verify from failing on generated formatting drift. Constraint: PR #488 CI failed on gci import grouping after the main merge Confidence: high Scope-risk: narrow Tested: source ~/.zshrc >/dev/null 2>&1 && make verify reached UI generation after applying this gci rewrite
timflannagan
approved these changes
May 19, 2026
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
3 participants
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
Adds a neutral OSS apply admission seam that downstream builds can use to accept a validated apply before production storage side effects. The seam runs after auth, object validation, reference validation, and registry validation, but before
Store.UpsertandPostUpsert.Key pieces:
ApplyInterceptorandApplyObjectso downstream approval replay can reuse the same production apply path.AppOptions.ExtraResourceRoutes.Ready=False/ReferencePendingstatus instead of failing the stored Deployment apply.This intentionally avoids enterprise approval names and does not reintroduce public
metadata.version,targetRef.version, deployment version identity, or integer-version compatibility.Change Type
/kind feature
Changelog
Additional Notes
Stack base for the enterprise native approval flow PR.
KRT note:
ApplyInterceptor,ResolverWrapper, andExtraResourceRoutesare marked in code as temporary synchronous-handler bridges. They should be removed or collapsed when KRT/reconciler-owned admission and staging becomes the production write architecture.Verified:
rtk go test -tags=integration ./pkg/registry/resource ./internal/registry/service/deployment ./internal/registry/api/router ./internal/registryrtk go test ./pkg/api/v0 ./pkg/types ./pkg/registry/resource ./internal/registry/service/deployment ./internal/registry/api/router ./internal/registryrtk go test ./pkg/types ./pkg/registry/resource ./internal/registry/api/router