From 7b2b155d94ad2ba249694cd9171aa632f5fc6d06 Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Tue, 12 May 2026 17:23:51 -0400 Subject: [PATCH 01/10] Let downstream approval flows reuse validated apply admission 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 --- internal/registry/api/router/v0.go | 55 ++++++++++++- internal/registry/registry_app.go | 55 +++++++++++-- .../service/deployment/coordinator.go | 22 ++++++ .../service/deployment/coordinator_test.go | 13 ++- pkg/api/v0/apply.go | 3 +- pkg/registry/resource/apply.go | 30 ++++++- pkg/registry/resource/apply_test.go | 79 +++++++++++++++++++ pkg/registry/resource/core.go | 60 ++++++++++++++ pkg/registry/resource/handler.go | 2 + pkg/types/types.go | 51 +++++++++++- 10 files changed, 353 insertions(+), 17 deletions(-) diff --git a/internal/registry/api/router/v0.go b/internal/registry/api/router/v0.go index 3d407212..a6905029 100644 --- a/internal/registry/api/router/v0.go +++ b/internal/registry/api/router/v0.go @@ -21,6 +21,7 @@ import ( "github.com/agentregistry-dev/agentregistry/pkg/importer" "github.com/agentregistry-dev/agentregistry/pkg/registry/resource" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" + "github.com/agentregistry-dev/agentregistry/pkg/types" "github.com/danielgtaylor/huma/v2" ) @@ -80,6 +81,22 @@ type RouteOptions struct { // Optional callback for integration-owned route registration. ExtraRoutes func(api huma.API, pathPrefix string) + + // ApplyInterceptor optionally accepts a validated apply before + // production Upsert. Nil preserves normal direct writes. + ApplyInterceptor resource.ApplyInterceptor + + // ResolverWrapper decorates the shared ResourceRef resolver before + // resource and apply routes are registered. + ResolverWrapper func(v1alpha1.ResolverFunc) v1alpha1.ResolverFunc + + // ExtraResourceRoutes registers adjacent routes with access to the same + // v1alpha1 stores and hooks used by /v0/apply. + ExtraResourceRoutes func(api huma.API, pathPrefix string, ctx types.ResourceRouteContext) + + // ImportAuthorizers overrides PerKindHooks.Authorizers for /v0/import. + // Nil preserves the regular authorizer map. + ImportAuthorizers map[string]func(ctx context.Context, in resource.AuthorizeInput) error } // RegisterRoutes registers all API routes under /v0. Required @@ -116,6 +133,9 @@ func RegisterRoutes( opts.DeploymentCoordinator, opts.PerKindHooks, opts.RegistryValidator, + opts.ApplyInterceptor, + opts.ResolverWrapper, + opts.ExtraResourceRoutes, ) // POST /v0/import — runs decoded manifests through the enrichment @@ -123,10 +143,14 @@ func RegisterRoutes( // Authorizers wires the same per-kind RBAC the regular apply path // uses; without it the import endpoint would be a write-bypass. if opts.Importer != nil { + importAuthorizers := opts.PerKindHooks.Authorizers + if opts.ImportAuthorizers != nil { + importAuthorizers = opts.ImportAuthorizers + } importpipeline.Register(api, importpipeline.Config{ BasePrefix: pathPrefix, Importer: opts.Importer, - Authorizers: opts.PerKindHooks.Authorizers, + Authorizers: importAuthorizers, }) } @@ -157,8 +181,14 @@ func registerKindRoutes( coord *deploymentsvc.Coordinator, perKind crud.PerKindHooks, registryValidator v1alpha1.RegistryValidatorFunc, + applyInterceptor resource.ApplyInterceptor, + resolverWrapper func(v1alpha1.ResolverFunc) v1alpha1.ResolverFunc, + extraResourceRoutes func(api huma.API, pathPrefix string, ctx types.ResourceRouteContext), ) { resolver := internaldb.NewResolver(stores) + if resolverWrapper != nil { + resolver = resolverWrapper(resolver) + } if registryValidator == nil { registryValidator = registries.Dispatcher } @@ -212,7 +242,7 @@ func registerKindRoutes( // same per-kind hook table populated above, so Deployment reconciliation // and any caller-supplied PostUpsert/PostDelete fire identically on // the batch path. - resource.RegisterApply(api, resource.ApplyConfig{ + applyCfg := resource.ApplyConfig{ BasePrefix: basePrefix, Stores: stores, Resolver: resolver, @@ -221,5 +251,24 @@ func registerKindRoutes( PostUpserts: perKind.PostUpserts, PostDeletes: perKind.PostDeletes, InitialFinalizers: perKind.InitialFinalizers, - }) + ApplyInterceptor: applyInterceptor, + } + productionApplyCfg := applyCfg + productionApplyCfg.ApplyInterceptor = nil + resource.RegisterApply(api, applyCfg) + + if extraResourceRoutes != nil { + opaqueStores := make(map[string]any, len(stores)) + for kind, store := range stores { + opaqueStores[kind] = store + } + extraResourceRoutes(api, basePrefix, types.ResourceRouteContext{ + Stores: opaqueStores, + Resolver: resolver, + RegistryValidator: registryValidator, + Apply: func(ctx context.Context, obj v1alpha1.Object, dryRun bool) arv0.ApplyResult { + return resource.ApplyObject(ctx, productionApplyCfg, obj, dryRun) + }, + }) + } } diff --git a/internal/registry/registry_app.go b/internal/registry/registry_app.go index a1120d74..cd3368af 100644 --- a/internal/registry/registry_app.go +++ b/internal/registry/registry_app.go @@ -242,11 +242,15 @@ func buildRouteOptions( adapters map[string]types.DeploymentAdapter, ) *router.RouteOptions { routeOpts := &router.RouteOptions{ - ExtraRoutes: options.ExtraRoutes, - Stores: stores, - Importer: importer, - PerKindHooks: crudPerKindHooks(options), - RegistryValidator: options.RegistryValidator, + ExtraRoutes: options.ExtraRoutes, + Stores: stores, + Importer: importer, + PerKindHooks: crudPerKindHooks(options), + RegistryValidator: options.RegistryValidator, + ApplyInterceptor: adaptApplyInterceptor(options.ApplyInterceptor), + ResolverWrapper: options.ResolverWrapper, + ExtraResourceRoutes: options.ExtraResourceRoutes, + ImportAuthorizers: adaptAuthorizers(options.ImportAuthorizers), } if stores != nil { @@ -260,6 +264,47 @@ func buildRouteOptions( return routeOpts } +func adaptAuthorizers(in map[string]types.Authorizer) map[string]func(context.Context, resource.AuthorizeInput) error { + if len(in) == 0 { + return nil + } + out := make(map[string]func(context.Context, resource.AuthorizeInput) error, len(in)) + for kind, fn := range in { + f := fn + out[kind] = func(ctx context.Context, in resource.AuthorizeInput) error { + return f(ctx, types.AuthorizeInput{ + Verb: in.Verb, Kind: in.Kind, Namespace: in.Namespace, + Name: in.Name, Tag: in.Tag, + }) + } + } + return out +} + +func adaptApplyInterceptor(fn types.ApplyInterceptor) resource.ApplyInterceptor { + if fn == nil { + return nil + } + return func(ctx context.Context, in resource.ApplyInterceptorInput) (resource.ApplyInterceptorResult, error) { + out, err := fn(ctx, types.ApplyInterceptorInput{ + Kind: in.Kind, + Namespace: in.Namespace, + Name: in.Name, + Tag: in.Tag, + Object: in.Object, + Store: in.Store, + }) + if err != nil { + return resource.ApplyInterceptorResult{}, err + } + return resource.ApplyInterceptorResult{ + Handled: out.Handled, + Status: out.Status, + Tag: out.Tag, + }, nil + } +} + // crudPerKindHooks adapts the AppOptions per-kind authorizer + // list-filter maps (which use the public pkg/types signatures) into // the internal crud.PerKindHooks struct (which uses the diff --git a/internal/registry/service/deployment/coordinator.go b/internal/registry/service/deployment/coordinator.go index afc23723..d36dc823 100644 --- a/internal/registry/service/deployment/coordinator.go +++ b/internal/registry/service/deployment/coordinator.go @@ -95,10 +95,16 @@ func (c *Coordinator) Apply(ctx context.Context, deployment *v1alpha1.Deployment target, err := c.resolveTarget(ctx, deployment) if err != nil { + if errors.Is(err, v1alpha1.ErrDanglingRef) { + return c.persistReferencePending(ctx, deployment, err) + } return err } runtime, err := c.resolveRuntime(ctx, deployment) if err != nil { + if errors.Is(err, v1alpha1.ErrDanglingRef) { + return c.persistReferencePending(ctx, deployment, err) + } return err } @@ -124,6 +130,22 @@ func (c *Coordinator) Apply(ctx context.Context, deployment *v1alpha1.Deployment return c.persistApplyResult(ctx, deployment, result) } +func (c *Coordinator) persistReferencePending(ctx context.Context, deployment *v1alpha1.Deployment, cause error) error { + message := "referenced resource is not available yet" + if cause != nil { + message = cause.Error() + } + return c.persistApplyResult(ctx, deployment, &types.ApplyResult{ + Conditions: []v1alpha1.Condition{{ + Type: "Ready", + Status: v1alpha1.ConditionFalse, + Reason: "ReferencePending", + Message: message, + ObservedGeneration: deployment.Metadata.Generation, + }}, + }) +} + // Remove tears down a Deployment's runtime resources via the adapter and // merges the resulting Removed condition into the row's status. Called // after the row's DeletionTimestamp is set (soft-delete path) or when diff --git a/internal/registry/service/deployment/coordinator_test.go b/internal/registry/service/deployment/coordinator_test.go index 5346ec34..dadd3c9c 100644 --- a/internal/registry/service/deployment/coordinator_test.go +++ b/internal/registry/service/deployment/coordinator_test.go @@ -168,8 +168,17 @@ func TestCoordinator_DanglingTargetRef(t *testing.T) { }) err := coord.Apply(ctx, deployment) - require.Error(t, err) - require.ErrorIs(t, err, v1alpha1.ErrDanglingRef) + require.NoError(t, err) + + raw, err := stores[v1alpha1.KindDeployment].Get(ctx, deployment.Metadata.Namespace, deployment.Metadata.Name, "") + require.NoError(t, err) + var status v1alpha1.Status + require.NoError(t, v1alpha1.UnmarshalStatusFromStorage(raw.Status, &status)) + ready := status.GetCondition("Ready") + require.NotNil(t, ready) + require.Equal(t, v1alpha1.ConditionFalse, ready.Status) + require.Equal(t, "ReferencePending", ready.Reason) + require.Contains(t, ready.Message, "does-not-exist") } func TestCoordinator_Discover_ReturnsAdapterResults(t *testing.T) { diff --git a/pkg/api/v0/apply.go b/pkg/api/v0/apply.go index 44cfd0f9..34a73c3d 100644 --- a/pkg/api/v0/apply.go +++ b/pkg/api/v0/apply.go @@ -9,7 +9,7 @@ type ApplyResult struct { Namespace string `json:"namespace,omitempty"` Name string `json:"name"` Tag string `json:"tag,omitempty"` - // Status is one of: created, configured, unchanged, deleted, + // Status is one of: created, configured, unchanged, staged, deleted, // dry-run, failed. Matches kubectl-style apply output. Status string `json:"status"` // Generation is the server-managed generation after the apply. @@ -26,6 +26,7 @@ const ( ApplyStatusCreated = "created" ApplyStatusConfigured = "configured" ApplyStatusUnchanged = "unchanged" + ApplyStatusStaged = "staged" ApplyStatusDeleted = "deleted" ApplyStatusDryRun = "dry-run" ApplyStatusFailed = "failed" diff --git a/pkg/registry/resource/apply.go b/pkg/registry/resource/apply.go index d8a0c973..a3d98d5e 100644 --- a/pkg/registry/resource/apply.go +++ b/pkg/registry/resource/apply.go @@ -63,6 +63,10 @@ type ApplyConfig struct { // InitialFinalizers mirrors resource.Config.InitialFinalizers per kind. InitialFinalizers map[string]func(obj v1alpha1.Object) []string + + // ApplyInterceptor optionally accepts a validated apply before + // production Upsert. Nil preserves normal direct-write behavior. + ApplyInterceptor ApplyInterceptor } // applyInput receives a raw multi-doc YAML stream. RawBody keeps bytes @@ -150,6 +154,14 @@ func runApplyBatch(ctx context.Context, cfg ApplyConfig, scheme *v1alpha1.Scheme return out } +// ApplyObject runs one already-decoded object through the same production +// apply path used by POST /v0/apply. Downstream routes can call this to replay +// a previously accepted object without duplicating validation, authz, +// persistence, or post-upsert behavior. +func ApplyObject(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun bool) arv0.ApplyResult { + return applyOne(ctx, cfg, obj, dryRun) +} + // applyOne runs a single document through the shared apply pipeline. // Never errors; encodes any failure into the returned ApplyResult. func applyOne(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun bool) arv0.ApplyResult { @@ -170,6 +182,7 @@ func applyOne(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun RegistryValidator: cfg.RegistryValidator, PostUpsert: cfg.PostUpserts[obj.GetKind()], InitialFinalizers: cfg.InitialFinalizers[obj.GetKind()], + ApplyInterceptor: cfg.ApplyInterceptor, }, dryRun) if ae != nil { return failResult(res, ae) @@ -182,12 +195,21 @@ func applyOne(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun // Map the Store outcome onto the wire status. Tagged-artifact creates // surface as created, same-tag replacements as configured, and exact // re-applies as unchanged. - switch up.Outcome { - case v1alpha1store.UpsertCreated: + switch { + case up.Intercepted: + res.Status = up.InterceptStatus + if res.Status == "" { + res.Status = arv0.ApplyStatusStaged + } + if up.InterceptTag != "" { + res.Tag = up.InterceptTag + } + return res + case up.Outcome == v1alpha1store.UpsertCreated: res.Status = arv0.ApplyStatusCreated - case v1alpha1store.UpsertReplaced: + case up.Outcome == v1alpha1store.UpsertReplaced: res.Status = arv0.ApplyStatusConfigured - case v1alpha1store.UpsertNoOp: + case up.Outcome == v1alpha1store.UpsertNoOp: res.Status = arv0.ApplyStatusUnchanged } if up.Tag != "" { diff --git a/pkg/registry/resource/apply_test.go b/pkg/registry/resource/apply_test.go index cb3c1945..9aabfd59 100644 --- a/pkg/registry/resource/apply_test.go +++ b/pkg/registry/resource/apply_test.go @@ -116,6 +116,85 @@ spec: require.Contains(t, out.Results[1].Error, "unknown or unconfigured kind") } +func TestRegisterApply_ApplyInterceptorCanHandleBeforeProductionUpsert(t *testing.T) { + pool := v1alpha1store.NewTestPool(t) + agents := v1alpha1store.NewStore(pool, "v1alpha1.agents") + + var intercepted resource.ApplyInterceptorInput + postUpsertCalled := false + _, api := humatest.New(t) + resource.RegisterApply(api, resource.ApplyConfig{ + BasePrefix: "/v0", + Stores: map[string]*v1alpha1store.Store{ + v1alpha1.KindAgent: agents, + }, + PostUpserts: map[string]func(context.Context, v1alpha1.Object) error{ + v1alpha1.KindAgent: func(context.Context, v1alpha1.Object) error { + postUpsertCalled = true + return nil + }, + }, + ApplyInterceptor: func(ctx context.Context, in resource.ApplyInterceptorInput) (resource.ApplyInterceptorResult, error) { + intercepted = in + return resource.ApplyInterceptorResult{Handled: true, Tag: in.Tag}, nil + }, + }) + + yaml := []byte(`apiVersion: ar.dev/v1alpha1 +kind: Agent +metadata: + namespace: default + name: staged-agent +spec: + title: Staged Agent +`) + resp := api.Post("/v0/apply", "Content-Type: application/yaml", strings.NewReader(string(yaml))) + require.Equal(t, http.StatusOK, resp.Code, resp.Body.String()) + + var out struct { + Results []arv0.ApplyResult `json:"results"` + } + require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &out)) + require.Len(t, out.Results, 1) + require.Equal(t, arv0.ApplyStatusStaged, out.Results[0].Status) + require.Equal(t, v1alpha1store.DefaultTag(), out.Results[0].Tag) + require.False(t, postUpsertCalled, "intercepted applies must not fire production side effects") + require.Equal(t, v1alpha1.KindAgent, intercepted.Kind) + require.Equal(t, "default", intercepted.Namespace) + require.Equal(t, "staged-agent", intercepted.Name) + require.Equal(t, v1alpha1store.DefaultTag(), intercepted.Tag) + require.Same(t, agents, intercepted.Store) + + _, err := agents.Get(t.Context(), "default", "staged-agent", v1alpha1store.DefaultTag()) + require.ErrorIs(t, err, pkgdb.ErrNotFound) +} + +func TestApplyObject_ReusesProductionApplyPath(t *testing.T) { + pool := v1alpha1store.NewTestPool(t) + agents := v1alpha1store.NewStore(pool, "v1alpha1.agents") + + obj := &v1alpha1.Agent{ + TypeMeta: v1alpha1.TypeMeta{APIVersion: v1alpha1.GroupVersion, Kind: v1alpha1.KindAgent}, + Metadata: v1alpha1.ObjectMeta{ + Namespace: "default", + Name: "replayed-agent", + Tag: "stable", + }, + Spec: v1alpha1.AgentSpec{Title: "Replayed Agent"}, + } + res := resource.ApplyObject(t.Context(), resource.ApplyConfig{ + Stores: map[string]*v1alpha1store.Store{ + v1alpha1.KindAgent: agents, + }, + }, obj, false) + require.Equal(t, arv0.ApplyStatusCreated, res.Status) + require.Equal(t, "stable", res.Tag) + + row, err := agents.Get(t.Context(), "default", "replayed-agent", "stable") + require.NoError(t, err) + require.Equal(t, "stable", row.Metadata.Tag) +} + func TestRegisterApply_MutableObjectResultsDoNotExposeVersion(t *testing.T) { pool := v1alpha1store.NewTestPool(t) runtimes := v1alpha1store.NewMutableObjectStore(pool, "v1alpha1.runtimes") diff --git a/pkg/registry/resource/core.go b/pkg/registry/resource/core.go index 77316143..fc086590 100644 --- a/pkg/registry/resource/core.go +++ b/pkg/registry/resource/core.go @@ -20,6 +20,7 @@ type applyOpts struct { RegistryValidator v1alpha1.RegistryValidatorFunc PostUpsert func(ctx context.Context, obj v1alpha1.Object) error InitialFinalizers func(obj v1alpha1.Object) []string + ApplyInterceptor ApplyInterceptor } // upsertResult is the outcome of a successful applyCore call. @@ -33,6 +34,43 @@ type upsertResult struct { Generation int64 // UID is the server-managed row identity after production upsert. UID string + // Intercepted reports that a downstream hook handled the apply before + // production upsert. No production PostUpsert hook has run. + Intercepted bool + // InterceptStatus is the ApplyResult status to report when Intercepted + // is true. Empty defaults to ApplyStatusStaged at the batch layer. + InterceptStatus string + // InterceptTag is the optional tag to report when Intercepted is true. + InterceptTag string +} + +// ApplyInterceptor can accept a validated apply request before the object is +// written to the production Store. Downstream builds use this as a neutral +// admission seam for workflows that persist the object somewhere else first. +// +// The hook runs after authorization, validation, reference resolution, and +// registry validation, but before Store.Upsert and PostUpsert. Returning +// Handled=true short-circuits the production upsert and skips PostUpsert. +type ApplyInterceptor func(ctx context.Context, in ApplyInterceptorInput) (ApplyInterceptorResult, error) + +// ApplyInterceptorInput describes the apply request seen by ApplyInterceptor. +// Store is the production store the object would otherwise be written to. +type ApplyInterceptorInput struct { + Kind string + Namespace string + Name string + Tag string + Object v1alpha1.Object + Store *v1alpha1store.Store +} + +// ApplyInterceptorResult reports whether the hook handled the apply. +// Status is copied into ApplyResult.Status when Handled is true; leave it empty +// to use the generic "staged" status. +type ApplyInterceptorResult struct { + Handled bool + Status string + Tag string } // applyStage tags which step of the pipeline produced an error so @@ -45,6 +83,7 @@ const ( stageValidation applyStage = "validation" stageRefs applyStage = "refs" stageRegistries applyStage = "registries" + stageAdmission applyStage = "admission" stageMarshal applyStage = "marshal" stageUpsert applyStage = "upsert" stagePostUpsert applyStage = "post-upsert" @@ -121,6 +160,27 @@ func applyCore( return upsertResult{}, nil } + if opts.ApplyInterceptor != nil { + intercept, err := opts.ApplyInterceptor(ctx, ApplyInterceptorInput{ + Kind: kind, + Namespace: meta.Namespace, + Name: meta.Name, + Tag: meta.Tag, + Object: obj, + Store: store, + }) + if err != nil { + return upsertResult{}, &applyError{Stage: stageAdmission, Err: err} + } + if intercept.Handled { + return upsertResult{ + Intercepted: true, + InterceptStatus: intercept.Status, + InterceptTag: intercept.Tag, + }, nil + } + } + upsertOpts := v1alpha1store.UpsertOpts{} if opts.InitialFinalizers != nil { upsertOpts.InitialFinalizers = opts.InitialFinalizers(obj) diff --git a/pkg/registry/resource/handler.go b/pkg/registry/resource/handler.go index 0b59f5f6..b1666f84 100644 --- a/pkg/registry/resource/handler.go +++ b/pkg/registry/resource/handler.go @@ -608,6 +608,8 @@ func mapApplyErrorToHuma(ae *applyError, kind, ns, name, tag string) error { return huma.Error400BadRequest("refs: " + ae.Err.Error()) case stageRegistries: return huma.Error400BadRequest("registries: " + ae.Err.Error()) + case stageAdmission: + return ae.Err case stageMarshal: return huma.Error400BadRequest("marshal spec: " + ae.Err.Error()) case stageUpsert: diff --git a/pkg/types/types.go b/pkg/types/types.go index f3e0e257..84b627fd 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -15,11 +15,11 @@ import ( "context" "net/http" - "github.com/danielgtaylor/huma/v2" - + "github.com/agentregistry-dev/agentregistry/pkg/api/v0" "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" "github.com/agentregistry-dev/agentregistry/pkg/registry/auth" "github.com/agentregistry-dev/agentregistry/pkg/registry/database" + "github.com/danielgtaylor/huma/v2" ) // DatabaseFactory is a function type that creates a store implementation. @@ -67,6 +67,37 @@ type PostUpsert func(ctx context.Context, obj v1alpha1.Object) error // batch's per-doc delete hook. type PostDelete func(ctx context.Context, obj v1alpha1.Object) error +// ApplyInterceptor can accept a validated apply before the object reaches +// production storage. Store is intentionally opaque to keep pkg/types free of +// registry/store implementation imports; integrations that need it can type +// assert against the concrete store package they already depend on. +type ApplyInterceptor func(ctx context.Context, in ApplyInterceptorInput) (ApplyInterceptorResult, error) + +type ApplyInterceptorInput struct { + Kind string + Namespace string + Name string + Tag string + Object v1alpha1.Object + Store any +} + +type ApplyInterceptorResult struct { + Handled bool + Status string + Tag string +} + +// ResourceRouteContext exposes the finalized v1alpha1 route wiring to +// downstream integrations that need adjacent routes against the same stores +// and hooks as /v0/apply. +type ResourceRouteContext struct { + Stores map[string]any + Resolver v1alpha1.ResolverFunc + RegistryValidator v1alpha1.RegistryValidatorFunc + Apply func(ctx context.Context, obj v1alpha1.Object, dryRun bool) v0.ApplyResult +} + // Auditor receives audit events for state changes that the OSS layer // considers significant. The default OSS implementation is a no-op; // downstream builds plug in a real audit sink via NewStore options. @@ -152,6 +183,18 @@ type AppOptions struct { // PostDeletes mirror PostUpserts on the delete path. PostDeletes map[string]PostDelete + // ApplyInterceptor optionally accepts a validated apply before the row + // reaches production storage. Nil preserves normal direct writes. + ApplyInterceptor ApplyInterceptor + + // ImportAuthorizers optionally overrides Authorizers for /v0/import. + // Nil makes import use the regular Authorizers map. + ImportAuthorizers map[string]Authorizer + + // ResolverWrapper decorates the shared ResourceRef resolver before route + // registration. Nil preserves the default store-backed resolver. + ResolverWrapper func(v1alpha1.ResolverFunc) v1alpha1.ResolverFunc + // V1Alpha1StoreTables registers additional v1alpha1 kinds with their // backing PostgreSQL tables. Downstream builds that add their own // Scheme kinds should populate this so the shared /v0/apply, @@ -198,6 +241,10 @@ type AppOptions struct { // routes. ExtraRoutes func(api huma.API, pathPrefix string) + // ExtraResourceRoutes is like ExtraRoutes, but runs after the v1alpha1 + // resource route context has been finalized. + ExtraResourceRoutes func(api huma.API, pathPrefix string, ctx ResourceRouteContext) + // HTTPServerFactory is an optional function to create a server that // adds new API routes. HTTPServerFactory HTTPServerFactory From 47874afc97b36d62009b110b81a0a2fdd3ec0052 Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Tue, 12 May 2026 18:16:55 -0400 Subject: [PATCH 02/10] Mark approval admission seams as temporary KRT bridge 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 --- internal/registry/api/router/v0.go | 4 ++++ pkg/registry/resource/core.go | 4 ++++ pkg/types/types.go | 12 ++++++++++++ 3 files changed, 20 insertions(+) diff --git a/internal/registry/api/router/v0.go b/internal/registry/api/router/v0.go index a6905029..0aa24c98 100644 --- a/internal/registry/api/router/v0.go +++ b/internal/registry/api/router/v0.go @@ -84,14 +84,18 @@ type RouteOptions struct { // ApplyInterceptor optionally accepts a validated apply before // production Upsert. Nil preserves normal direct writes. + // TODO(krt): temporary synchronous-handler bridge; remove when KRT owns + // admission/staging. ApplyInterceptor resource.ApplyInterceptor // ResolverWrapper decorates the shared ResourceRef resolver before // resource and apply routes are registered. + // TODO(krt): temporary bridge for pending staged refs during HTTP apply. ResolverWrapper func(v1alpha1.ResolverFunc) v1alpha1.ResolverFunc // ExtraResourceRoutes registers adjacent routes with access to the same // v1alpha1 stores and hooks used by /v0/apply. + // TODO(krt): temporary bridge for downstream synchronous approval routes. ExtraResourceRoutes func(api huma.API, pathPrefix string, ctx types.ResourceRouteContext) // ImportAuthorizers overrides PerKindHooks.Authorizers for /v0/import. diff --git a/pkg/registry/resource/core.go b/pkg/registry/resource/core.go index fc086590..255fe999 100644 --- a/pkg/registry/resource/core.go +++ b/pkg/registry/resource/core.go @@ -48,6 +48,10 @@ type upsertResult struct { // written to the production Store. Downstream builds use this as a neutral // admission seam for workflows that persist the object somewhere else first. // +// TODO(krt): this is a synchronous-handler bridge for the pre-KRT apply path. +// Remove or collapse it when reconciler-owned admission/staging becomes the +// production architecture. +// // The hook runs after authorization, validation, reference resolution, and // registry validation, but before Store.Upsert and PostUpsert. Returning // Handled=true short-circuits the production upsert and skips PostUpsert. diff --git a/pkg/types/types.go b/pkg/types/types.go index 84b627fd..ab713121 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -71,6 +71,10 @@ type PostDelete func(ctx context.Context, obj v1alpha1.Object) error // production storage. Store is intentionally opaque to keep pkg/types free of // registry/store implementation imports; integrations that need it can type // assert against the concrete store package they already depend on. +// +// TODO(krt): this belongs to the synchronous handler architecture. Prefer a +// reconciler-owned admission/staging model when KRT becomes the write path, and +// delete this bridge once no downstream route depends on it. type ApplyInterceptor func(ctx context.Context, in ApplyInterceptorInput) (ApplyInterceptorResult, error) type ApplyInterceptorInput struct { @@ -91,6 +95,10 @@ type ApplyInterceptorResult struct { // ResourceRouteContext exposes the finalized v1alpha1 route wiring to // downstream integrations that need adjacent routes against the same stores // and hooks as /v0/apply. +// +// TODO(krt): this is a temporary way for downstream synchronous routes to reuse +// production apply wiring. KRT should make this unnecessary by owning the +// staging-to-production transition outside HTTP route callbacks. type ResourceRouteContext struct { Stores map[string]any Resolver v1alpha1.ResolverFunc @@ -185,6 +193,8 @@ type AppOptions struct { // ApplyInterceptor optionally accepts a validated apply before the row // reaches production storage. Nil preserves normal direct writes. + // TODO(krt): temporary synchronous-handler bridge; remove with reconciler + // admission/staging. ApplyInterceptor ApplyInterceptor // ImportAuthorizers optionally overrides Authorizers for /v0/import. @@ -193,6 +203,7 @@ type AppOptions struct { // ResolverWrapper decorates the shared ResourceRef resolver before route // registration. Nil preserves the default store-backed resolver. + // TODO(krt): temporary bridge for pending staged refs in HTTP apply. ResolverWrapper func(v1alpha1.ResolverFunc) v1alpha1.ResolverFunc // V1Alpha1StoreTables registers additional v1alpha1 kinds with their @@ -243,6 +254,7 @@ type AppOptions struct { // ExtraResourceRoutes is like ExtraRoutes, but runs after the v1alpha1 // resource route context has been finalized. + // TODO(krt): temporary bridge for downstream synchronous approval routes. ExtraResourceRoutes func(api huma.API, pathPrefix string, ctx ResourceRouteContext) // HTTPServerFactory is an optional function to create a server that From 82a6408fcd229a7c8b171f302f8a442300f2ead8 Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Tue, 12 May 2026 18:21:14 -0400 Subject: [PATCH 03/10] Preserve generated import formatting in approval seam 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. --- pkg/types/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/types/types.go b/pkg/types/types.go index ab713121..1abc5277 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -15,7 +15,7 @@ import ( "context" "net/http" - "github.com/agentregistry-dev/agentregistry/pkg/api/v0" + v0 "github.com/agentregistry-dev/agentregistry/pkg/api/v0" "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" "github.com/agentregistry-dev/agentregistry/pkg/registry/auth" "github.com/agentregistry-dev/agentregistry/pkg/registry/database" From 6db5c0a0f44a2eb9c66e0e3c4dba2cfcb5d3a8c3 Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Wed, 13 May 2026 02:13:36 -0400 Subject: [PATCH 04/10] Route import through shared admission 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 --- .../v0/importpipeline/importpipeline.go | 61 ++------ .../v0/importpipeline/importpipeline_test.go | 25 +++- internal/registry/api/router/v0.go | 39 +++-- internal/registry/registry_app.go | 32 +--- pkg/importer/importer.go | 138 +++++++----------- pkg/registry/resource/apply.go | 27 +++- pkg/registry/resource/apply_test.go | 24 +-- pkg/registry/resource/core.go | 83 +++++++---- pkg/types/types.go | 25 ++-- 9 files changed, 210 insertions(+), 244 deletions(-) diff --git a/internal/registry/api/handlers/v0/importpipeline/importpipeline.go b/internal/registry/api/handlers/v0/importpipeline/importpipeline.go index 174b3d8f..eab03c13 100644 --- a/internal/registry/api/handlers/v0/importpipeline/importpipeline.go +++ b/internal/registry/api/handlers/v0/importpipeline/importpipeline.go @@ -1,25 +1,20 @@ // Package importpipeline owns POST /v0/import — the multi-doc YAML // import endpoint that runs each decoded document through the -// pre-constructed importer.Importer (validation + scanner enrichment -// + Upsert) and returns per-document results. +// pre-constructed importer.Importer (scanner enrichment + shared +// apply/admission) and returns per-document results. // // Distinct from the per-kind CRUD bindings in v1alpha1crud and from -// the in-package POST /v0/apply (pkg/registry/resource): apply -// short-circuits to plain Upsert without scanner runs, while -// importpipeline always passes through the importer's enrichment -// pipeline so scanner annotations + findings rows land alongside -// the persisted spec. +// the in-package POST /v0/apply (pkg/registry/resource): import adds +// scanner enrichment, then persists through the same apply/admission path. package importpipeline import ( "context" - "fmt" "net/http" "strings" "github.com/danielgtaylor/huma/v2" - "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" "github.com/agentregistry-dev/agentregistry/pkg/importer" "github.com/agentregistry-dev/agentregistry/pkg/registry/resource" ) @@ -31,17 +26,10 @@ import ( type Config struct { BasePrefix string Importer *importer.Importer - // Authorizers is the same per-kind authz map the regular apply - // pipeline consults. When set, every decoded document is - // authorized before Upsert via importer.Options.PerObjectAuthorize; - // a deny on any kind makes that doc fail with Status=failed - // without aborting the rest of the batch (matches the - // per-doc-failure pattern in pkg/registry/resource/apply.go). - // - // Without this map, POST /v0/import is a write-bypass for any - // kind the Importer accepts — denied users could create / replace - // rows by routing writes through this endpoint. - Authorizers map[string]func(ctx context.Context, in resource.AuthorizeInput) error + // ApplyConfig is the same apply/admission config used by /v0/apply, with + // Source set to import by the router. Importer adds scanner enrichment via + // Prepare, then delegates persistence to resource.ApplyObject. + ApplyConfig resource.ApplyConfig } // importInput is the HTTP input for POST /import. RawBody carries @@ -79,10 +67,11 @@ func Register(api huma.API, cfg Config) { Summary: "Import v1alpha1 resources (validate, optionally enrich, upsert)", }, func(ctx context.Context, in *importInput) (*importOutput, error) { opts := importer.Options{ - Namespace: in.Namespace, - Enrich: in.Enrich, - DryRun: in.DryRun, - ScannedBy: firstNonEmpty(in.ScannedBy, "importer-http"), + Namespace: in.Namespace, + Enrich: in.Enrich, + DryRun: in.DryRun, + ScannedBy: firstNonEmpty(in.ScannedBy, "importer-http"), + ApplyConfig: cfg.ApplyConfig, } if s := strings.TrimSpace(in.WhichScans); s != "" { for name := range strings.SplitSeq(s, ",") { @@ -92,30 +81,6 @@ func Register(api huma.API, cfg Config) { } } } - if len(cfg.Authorizers) > 0 { - authorizers := cfg.Authorizers - opts.PerObjectAuthorize = func(ctx context.Context, obj v1alpha1.Object) error { - kind := obj.GetKind() - authz, ok := authorizers[kind] - // Defense-in-depth: when the caller has wired any - // Authorizers, a kind without an entry must DENY - // rather than silently allow. Downstream boot guards - // can ensure every OSS BuiltinKinds entry has an - // authorizer, so this only fires for extension kinds - // the operator added without - // updating the import config — fail closed there. - if !ok || authz == nil { - return huma.Error403Forbidden(fmt.Sprintf("import: no authorizer wired for kind %q", kind)) - } - meta := obj.GetMetadata() - return authz(ctx, resource.AuthorizeInput{ - Verb: "apply", Kind: kind, - Namespace: meta.Namespace, Name: meta.Name, Tag: meta.Tag, - Object: obj, - }) - } - } - out := &importOutput{} out.Body.Results = cfg.Importer.ImportBytes(ctx, "", in.RawBody, opts) return out, nil diff --git a/internal/registry/api/handlers/v0/importpipeline/importpipeline_test.go b/internal/registry/api/handlers/v0/importpipeline/importpipeline_test.go index 346becbe..bda69c13 100644 --- a/internal/registry/api/handlers/v0/importpipeline/importpipeline_test.go +++ b/internal/registry/api/handlers/v0/importpipeline/importpipeline_test.go @@ -60,6 +60,9 @@ func newImportTestServer(t *testing.T, scanners ...importer.Scanner) (*v1alpha1s importpipeline.Register(api, importpipeline.Config{ BasePrefix: "/v0", Importer: imp, + ApplyConfig: resource.ApplyConfig{ + Stores: stores, + }, }) return agents, findings, api } @@ -222,9 +225,12 @@ func TestRegisterImport_PerDocAuthorize(t *testing.T) { _, api := humatest.New(t) importpipeline.Register(api, importpipeline.Config{ - BasePrefix: "/v0", - Importer: imp, - Authorizers: authorizers, + BasePrefix: "/v0", + Importer: imp, + ApplyConfig: resource.ApplyConfig{ + Stores: stores, + Authorizers: authorizers, + }, }) yaml := `apiVersion: ar.dev/v1alpha1 @@ -258,11 +264,11 @@ spec: require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &out)) require.Len(t, out.Results, 2) - // Denied doc → failed; error mentions authorize so operators can + // Denied doc → failed; error mentions forbidden so operators can // distinguish from validation failures. require.Equal(t, "secret", out.Results[0].Name) require.Equal(t, "failed", out.Results[0].Status) - require.Contains(t, out.Results[0].Error, "authorize") + require.Contains(t, out.Results[0].Error, "forbidden") // Allowed doc → created. require.Equal(t, "ok", out.Results[1].Name) @@ -303,9 +309,12 @@ func TestRegisterImport_DeniesKindWithNoAuthorizer(t *testing.T) { _, api := humatest.New(t) importpipeline.Register(api, importpipeline.Config{ - BasePrefix: "/v0", - Importer: imp, - Authorizers: authorizers, + BasePrefix: "/v0", + Importer: imp, + ApplyConfig: resource.ApplyConfig{ + Stores: stores, + Authorizers: authorizers, + }, }) yaml := `apiVersion: ar.dev/v1alpha1 diff --git a/internal/registry/api/router/v0.go b/internal/registry/api/router/v0.go index 0aa24c98..e4c48b69 100644 --- a/internal/registry/api/router/v0.go +++ b/internal/registry/api/router/v0.go @@ -82,11 +82,11 @@ type RouteOptions struct { // Optional callback for integration-owned route registration. ExtraRoutes func(api huma.API, pathPrefix string) - // ApplyInterceptor optionally accepts a validated apply before - // production Upsert. Nil preserves normal direct writes. + // Admission optionally accepts a validated write before production + // Upsert. Nil preserves normal direct writes. // TODO(krt): temporary synchronous-handler bridge; remove when KRT owns // admission/staging. - ApplyInterceptor resource.ApplyInterceptor + Admission resource.AdmissionFunc // ResolverWrapper decorates the shared ResourceRef resolver before // resource and apply routes are registered. @@ -97,10 +97,6 @@ type RouteOptions struct { // v1alpha1 stores and hooks used by /v0/apply. // TODO(krt): temporary bridge for downstream synchronous approval routes. ExtraResourceRoutes func(api huma.API, pathPrefix string, ctx types.ResourceRouteContext) - - // ImportAuthorizers overrides PerKindHooks.Authorizers for /v0/import. - // Nil preserves the regular authorizer map. - ImportAuthorizers map[string]func(ctx context.Context, in resource.AuthorizeInput) error } // RegisterRoutes registers all API routes under /v0. Required @@ -130,31 +126,29 @@ func RegisterRoutes( // v1alpha1 generic routes. Cross-kind dangling-ref detection uses // a Store-backed resolver. Deployment reconciliation hooks plug in // when the coordinator is supplied. - registerKindRoutes( + applyCfg := registerKindRoutes( api, pathPrefix, opts.Stores, opts.DeploymentCoordinator, opts.PerKindHooks, opts.RegistryValidator, - opts.ApplyInterceptor, + opts.Admission, opts.ResolverWrapper, opts.ExtraResourceRoutes, ) - // POST /v0/import — runs decoded manifests through the enrichment - // pipeline (validate + scanners + findings-write) before Upsert. - // Authorizers wires the same per-kind RBAC the regular apply path - // uses; without it the import endpoint would be a write-bypass. + // POST /v0/import — runs decoded manifests through scanner enrichment + // before persisting via the same source-aware apply/admission pipeline as + // /v0/apply. That keeps per-kind authz and downstream admission policy in + // one write path instead of route-specific overrides. if opts.Importer != nil { - importAuthorizers := opts.PerKindHooks.Authorizers - if opts.ImportAuthorizers != nil { - importAuthorizers = opts.ImportAuthorizers - } + importApplyCfg := applyCfg + importApplyCfg.Source = resource.ApplySourceImport importpipeline.Register(api, importpipeline.Config{ BasePrefix: pathPrefix, Importer: opts.Importer, - Authorizers: importAuthorizers, + ApplyConfig: importApplyCfg, }) } @@ -185,10 +179,10 @@ func registerKindRoutes( coord *deploymentsvc.Coordinator, perKind crud.PerKindHooks, registryValidator v1alpha1.RegistryValidatorFunc, - applyInterceptor resource.ApplyInterceptor, + admission resource.AdmissionFunc, resolverWrapper func(v1alpha1.ResolverFunc) v1alpha1.ResolverFunc, extraResourceRoutes func(api huma.API, pathPrefix string, ctx types.ResourceRouteContext), -) { +) resource.ApplyConfig { resolver := internaldb.NewResolver(stores) if resolverWrapper != nil { resolver = resolverWrapper(resolver) @@ -255,10 +249,10 @@ func registerKindRoutes( PostUpserts: perKind.PostUpserts, PostDeletes: perKind.PostDeletes, InitialFinalizers: perKind.InitialFinalizers, - ApplyInterceptor: applyInterceptor, + Admission: admission, } productionApplyCfg := applyCfg - productionApplyCfg.ApplyInterceptor = nil + productionApplyCfg.Admission = nil resource.RegisterApply(api, applyCfg) if extraResourceRoutes != nil { @@ -275,4 +269,5 @@ func registerKindRoutes( }, }) } + return applyCfg } diff --git a/internal/registry/registry_app.go b/internal/registry/registry_app.go index cd3368af..06118c49 100644 --- a/internal/registry/registry_app.go +++ b/internal/registry/registry_app.go @@ -247,10 +247,9 @@ func buildRouteOptions( Importer: importer, PerKindHooks: crudPerKindHooks(options), RegistryValidator: options.RegistryValidator, - ApplyInterceptor: adaptApplyInterceptor(options.ApplyInterceptor), + Admission: adaptAdmission(options.Admission), ResolverWrapper: options.ResolverWrapper, ExtraResourceRoutes: options.ExtraResourceRoutes, - ImportAuthorizers: adaptAuthorizers(options.ImportAuthorizers), } if stores != nil { @@ -264,29 +263,14 @@ func buildRouteOptions( return routeOpts } -func adaptAuthorizers(in map[string]types.Authorizer) map[string]func(context.Context, resource.AuthorizeInput) error { - if len(in) == 0 { - return nil - } - out := make(map[string]func(context.Context, resource.AuthorizeInput) error, len(in)) - for kind, fn := range in { - f := fn - out[kind] = func(ctx context.Context, in resource.AuthorizeInput) error { - return f(ctx, types.AuthorizeInput{ - Verb: in.Verb, Kind: in.Kind, Namespace: in.Namespace, - Name: in.Name, Tag: in.Tag, - }) - } - } - return out -} - -func adaptApplyInterceptor(fn types.ApplyInterceptor) resource.ApplyInterceptor { +func adaptAdmission(fn types.Admission) resource.AdmissionFunc { if fn == nil { return nil } - return func(ctx context.Context, in resource.ApplyInterceptorInput) (resource.ApplyInterceptorResult, error) { - out, err := fn(ctx, types.ApplyInterceptorInput{ + return func(ctx context.Context, in resource.AdmissionInput) (resource.AdmissionDecision, error) { + out, err := fn(ctx, types.AdmissionInput{ + Source: string(in.Source), + Verb: in.Verb, Kind: in.Kind, Namespace: in.Namespace, Name: in.Name, @@ -295,9 +279,9 @@ func adaptApplyInterceptor(fn types.ApplyInterceptor) resource.ApplyInterceptor Store: in.Store, }) if err != nil { - return resource.ApplyInterceptorResult{}, err + return resource.AdmissionDecision{}, err } - return resource.ApplyInterceptorResult{ + return resource.AdmissionDecision{ Handled: out.Handled, Status: out.Status, Tag: out.Tag, diff --git a/pkg/importer/importer.go b/pkg/importer/importer.go index 01e9b07e..9a4e195b 100644 --- a/pkg/importer/importer.go +++ b/pkg/importer/importer.go @@ -13,7 +13,9 @@ import ( "strings" "time" + arv0 "github.com/agentregistry-dev/agentregistry/pkg/api/v0" "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" + "github.com/agentregistry-dev/agentregistry/pkg/registry/resource" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" ) @@ -47,23 +49,12 @@ type Options struct { // "importer-cli" when blank. ScannedBy string - // PerObjectAuthorize, when non-nil, is invoked once per decoded - // object after validation + ref/registry/remote-URL checks but - // BEFORE Upsert. A non-nil error fails the per-doc - // ImportResult with Status=ImportStatusFailed; the rest of the - // stream still runs. - // - // HTTP callers (POST /v0/import) wire this from the same - // PerKindHooks.Authorizers map the regular apply path consults so - // the import surface enforces the same per-kind RBAC. Nil is the - // non-HTTP default (admin context, no per-call gate). - // - // Object identity is fully populated by the time this fires — - // metadata.namespace has been defaulted, validation has passed, - // labels/annotations from scanners are NOT yet applied (those run - // only on enrich + after authz, so an authz failure can't leak - // scanner-derived state). - PerObjectAuthorize func(ctx context.Context, obj v1alpha1.Object) error + // ApplyConfig, when set, is the shared apply/admission pipeline used for + // persistence. HTTP callers pass the same config as /v0/apply with + // Source=import so per-kind authz, downstream admission, finalizers, and + // post-upsert hooks are not reimplemented by import. When Stores is nil, + // the importer falls back to its own server boot dependencies. + ApplyConfig resource.ApplyConfig } // ImportResult is the per-object outcome of Importer.Import. One @@ -82,8 +73,8 @@ type ImportResult struct { Namespace string `json:"namespace,omitempty"` Name string `json:"name,omitempty"` - // Status is one of "created" | "updated" | "unchanged" | "failed" - // | "dry-run". Matches the apply-handler vocabulary. + // Status is one of "created" | "updated" | "unchanged" | "staged" + // | "failed" | "dry-run". Matches the apply-handler vocabulary. Status string `json:"status"` // EnrichmentStatus is "skipped" (Enrich=false or no supporting @@ -107,6 +98,7 @@ const ( ImportStatusCreated = "created" ImportStatusUpdated = "updated" ImportStatusUnchanged = "unchanged" + ImportStatusStaged = "staged" ImportStatusFailed = "failed" ImportStatusDryRun = "dry-run" @@ -274,9 +266,9 @@ func (i *Importer) importStream(ctx context.Context, source string, data []byte, return out } -// importOne runs one decoded object through validate → enrich → -// upsert → findings-write. Never errors; failures come back as the -// ImportResult. +// importOne runs one decoded object through the shared apply pipeline. Import +// contributes scanner enrichment through ApplyConfig.Prepare, then writes +// findings only after production apply succeeds. func (i *Importer) importOne(ctx context.Context, source string, obj v1alpha1.Object, opts Options) ImportResult { meta := obj.GetMetadata() kind := obj.GetKind() @@ -300,78 +292,56 @@ func (i *Importer) importOne(ctx context.Context, source string, obj v1alpha1.Ob res.Namespace = meta.Namespace } - store, ok := i.stores[kind] - if !ok || store == nil { - res.Status = ImportStatusFailed - res.Error = fmt.Sprintf("unknown or unconfigured kind %q", kind) - return res + var pendingFindings map[string][]Finding + applyCfg := i.applyConfig(opts) + applyCfg.Source = resource.ApplySourceImport + applyCfg.Prepare = func(ctx context.Context, obj v1alpha1.Object) error { + if !opts.Enrich { + return nil + } + pendingFindings, res.EnrichmentStatus, res.EnrichmentErrors = i.runScanners(ctx, obj, opts) + return nil } - if v1alpha1.IsTaggedArtifactKind(kind) && meta.Tag == "" { - meta.Tag = v1alpha1store.DefaultTag() - obj.SetMetadata(*meta) - res.Tag = meta.Tag - } + applyRes := resource.ApplyObject(ctx, applyCfg, obj, opts.DryRun) + res.Namespace = applyRes.Namespace + res.Name = applyRes.Name + res.Tag = applyRes.Tag + res.Status = importStatusFromApplyStatus(applyRes.Status) + res.Error = applyRes.Error - if err := v1alpha1.ValidateObject(obj); err != nil { - res.Status = ImportStatusFailed - res.Error = "validation: " + err.Error() - return res - } - if err := v1alpha1.ResolveObjectRefs(ctx, obj, i.resolver); err != nil { - res.Status = ImportStatusFailed - res.Error = "refs: " + err.Error() - return res - } - if err := v1alpha1.ValidateObjectRegistries(ctx, obj, i.registryValidator); err != nil { - res.Status = ImportStatusFailed - res.Error = "registries: " + err.Error() - return res - } - // Per-object authz gate. Mirrors the apply pipeline's Authorize - // call (pkg/registry/resource/apply.go:prepareApplyDoc). Wired by - // the HTTP /v0/import handler from PerKindHooks.Authorizers so - // callers without role grants for a kind can't bypass per-kind - // RBAC by routing writes through the import endpoint. - if opts.PerObjectAuthorize != nil { - if err := opts.PerObjectAuthorize(ctx, obj); err != nil { - res.Status = ImportStatusFailed - res.Error = "authorize: " + err.Error() - return res - } + if res.Status != ImportStatusFailed && res.Status != ImportStatusDryRun && res.Status != ImportStatusStaged { + i.writeFindings(ctx, obj, opts, pendingFindings, &res, applyRes.Tag) } + return res +} - // Enrichment: mutate obj's annotations/labels in place, accumulate - // per-source findings to write after Upsert. Scanners run against - // the fully-populated object so they see user-authored labels too. - var pendingFindings map[string][]Finding - if opts.Enrich { - pendingFindings, res.EnrichmentStatus, res.EnrichmentErrors = i.runScanners(ctx, obj, opts) +func (i *Importer) applyConfig(opts Options) resource.ApplyConfig { + if opts.ApplyConfig.Stores != nil { + return opts.ApplyConfig } - - if opts.DryRun { - res.Status = ImportStatusDryRun - return res + return resource.ApplyConfig{ + Stores: i.stores, + Resolver: i.resolver, + RegistryValidator: i.registryValidator, } +} - up, err := store.Upsert(ctx, obj) - if err != nil { - res.Status = ImportStatusFailed - res.Error = "upsert: " + err.Error() - return res - } - switch up.Outcome { - case v1alpha1store.UpsertCreated: - res.Status = ImportStatusCreated - case v1alpha1store.UpsertReplaced: - res.Status = ImportStatusUpdated +func importStatusFromApplyStatus(status string) string { + switch status { + case arv0.ApplyStatusCreated: + return ImportStatusCreated + case arv0.ApplyStatusConfigured: + return ImportStatusUpdated + case arv0.ApplyStatusUnchanged: + return ImportStatusUnchanged + case arv0.ApplyStatusDryRun: + return ImportStatusDryRun + case arv0.ApplyStatusStaged: + return ImportStatusStaged default: - res.Status = ImportStatusUnchanged + return ImportStatusFailed } - res.Tag = up.Tag - - i.writeFindings(ctx, obj, opts, pendingFindings, &res, up.Tag) - return res } // writeFindings persists per-scanner findings after a successful Upsert. diff --git a/pkg/registry/resource/apply.go b/pkg/registry/resource/apply.go index a3d98d5e..0337c9d1 100644 --- a/pkg/registry/resource/apply.go +++ b/pkg/registry/resource/apply.go @@ -64,9 +64,18 @@ type ApplyConfig struct { // InitialFinalizers mirrors resource.Config.InitialFinalizers per kind. InitialFinalizers map[string]func(obj v1alpha1.Object) []string - // ApplyInterceptor optionally accepts a validated apply before - // production Upsert. Nil preserves normal direct-write behavior. - ApplyInterceptor ApplyInterceptor + // Source labels the producer of objects entering this apply pipeline. + // Empty defaults to ApplySourceApply. + Source ApplySource + + // Admission optionally accepts a validated write before production Upsert. + // Nil preserves normal direct-write behavior. + Admission AdmissionFunc + + // Prepare optionally mutates an object after validation/admission and + // before Upsert. Import uses this to merge scanner output while still + // persisting through the shared apply path. + Prepare func(ctx context.Context, obj v1alpha1.Object) error } // applyInput receives a raw multi-doc YAML stream. RawBody keeps bytes @@ -182,7 +191,9 @@ func applyOne(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun RegistryValidator: cfg.RegistryValidator, PostUpsert: cfg.PostUpserts[obj.GetKind()], InitialFinalizers: cfg.InitialFinalizers[obj.GetKind()], - ApplyInterceptor: cfg.ApplyInterceptor, + Admission: cfg.Admission, + Source: cfg.Source, + Prepare: cfg.Prepare, }, dryRun) if ae != nil { return failResult(res, ae) @@ -196,13 +207,13 @@ func applyOne(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun // surface as created, same-tag replacements as configured, and exact // re-applies as unchanged. switch { - case up.Intercepted: - res.Status = up.InterceptStatus + case up.Admitted: + res.Status = up.AdmitStatus if res.Status == "" { res.Status = arv0.ApplyStatusStaged } - if up.InterceptTag != "" { - res.Tag = up.InterceptTag + if up.AdmitTag != "" { + res.Tag = up.AdmitTag } return res case up.Outcome == v1alpha1store.UpsertCreated: diff --git a/pkg/registry/resource/apply_test.go b/pkg/registry/resource/apply_test.go index 9aabfd59..bfaf4907 100644 --- a/pkg/registry/resource/apply_test.go +++ b/pkg/registry/resource/apply_test.go @@ -116,11 +116,11 @@ spec: require.Contains(t, out.Results[1].Error, "unknown or unconfigured kind") } -func TestRegisterApply_ApplyInterceptorCanHandleBeforeProductionUpsert(t *testing.T) { +func TestRegisterApply_AdmissionCanHandleBeforeProductionUpsert(t *testing.T) { pool := v1alpha1store.NewTestPool(t) agents := v1alpha1store.NewStore(pool, "v1alpha1.agents") - var intercepted resource.ApplyInterceptorInput + var admitted resource.AdmissionInput postUpsertCalled := false _, api := humatest.New(t) resource.RegisterApply(api, resource.ApplyConfig{ @@ -134,9 +134,9 @@ func TestRegisterApply_ApplyInterceptorCanHandleBeforeProductionUpsert(t *testin return nil }, }, - ApplyInterceptor: func(ctx context.Context, in resource.ApplyInterceptorInput) (resource.ApplyInterceptorResult, error) { - intercepted = in - return resource.ApplyInterceptorResult{Handled: true, Tag: in.Tag}, nil + Admission: func(ctx context.Context, in resource.AdmissionInput) (resource.AdmissionDecision, error) { + admitted = in + return resource.AdmissionDecision{Handled: true, Tag: in.Tag}, nil }, }) @@ -158,12 +158,14 @@ spec: require.Len(t, out.Results, 1) require.Equal(t, arv0.ApplyStatusStaged, out.Results[0].Status) require.Equal(t, v1alpha1store.DefaultTag(), out.Results[0].Tag) - require.False(t, postUpsertCalled, "intercepted applies must not fire production side effects") - require.Equal(t, v1alpha1.KindAgent, intercepted.Kind) - require.Equal(t, "default", intercepted.Namespace) - require.Equal(t, "staged-agent", intercepted.Name) - require.Equal(t, v1alpha1store.DefaultTag(), intercepted.Tag) - require.Same(t, agents, intercepted.Store) + require.False(t, postUpsertCalled, "admitted applies must not fire production side effects") + require.Equal(t, resource.ApplySourceApply, admitted.Source) + require.Equal(t, "apply", admitted.Verb) + require.Equal(t, v1alpha1.KindAgent, admitted.Kind) + require.Equal(t, "default", admitted.Namespace) + require.Equal(t, "staged-agent", admitted.Name) + require.Equal(t, v1alpha1store.DefaultTag(), admitted.Tag) + require.Same(t, agents, admitted.Store) _, err := agents.Get(t.Context(), "default", "staged-agent", v1alpha1store.DefaultTag()) require.ErrorIs(t, err, pkgdb.ErrNotFound) diff --git a/pkg/registry/resource/core.go b/pkg/registry/resource/core.go index 255fe999..cd35d1fc 100644 --- a/pkg/registry/resource/core.go +++ b/pkg/registry/resource/core.go @@ -20,7 +20,9 @@ type applyOpts struct { RegistryValidator v1alpha1.RegistryValidatorFunc PostUpsert func(ctx context.Context, obj v1alpha1.Object) error InitialFinalizers func(obj v1alpha1.Object) []string - ApplyInterceptor ApplyInterceptor + Admission AdmissionFunc + Source ApplySource + Prepare func(ctx context.Context, obj v1alpha1.Object) error } // upsertResult is the outcome of a successful applyCore call. @@ -34,19 +36,29 @@ type upsertResult struct { Generation int64 // UID is the server-managed row identity after production upsert. UID string - // Intercepted reports that a downstream hook handled the apply before + // Admitted reports that a downstream hook handled the apply before // production upsert. No production PostUpsert hook has run. - Intercepted bool - // InterceptStatus is the ApplyResult status to report when Intercepted + Admitted bool + // AdmitStatus is the ApplyResult status to report when Admitted // is true. Empty defaults to ApplyStatusStaged at the batch layer. - InterceptStatus string - // InterceptTag is the optional tag to report when Intercepted is true. - InterceptTag string + AdmitStatus string + // AdmitTag is the optional tag to report when Admitted is true. + AdmitTag string } -// ApplyInterceptor can accept a validated apply request before the object is -// written to the production Store. Downstream builds use this as a neutral -// admission seam for workflows that persist the object somewhere else first. +// ApplySource identifies the route or subsystem that produced an object for +// the shared apply pipeline. +type ApplySource string + +const ( + ApplySourceApply ApplySource = "apply" + ApplySourceImport ApplySource = "import" +) + +// AdmissionFunc can accept or reject a validated apply request before the +// object is written to the production Store. Downstream builds use this as a +// neutral admission seam for workflows that persist the object somewhere else +// first. // // TODO(krt): this is a synchronous-handler bridge for the pre-KRT apply path. // Remove or collapse it when reconciler-owned admission/staging becomes the @@ -55,11 +67,13 @@ type upsertResult struct { // The hook runs after authorization, validation, reference resolution, and // registry validation, but before Store.Upsert and PostUpsert. Returning // Handled=true short-circuits the production upsert and skips PostUpsert. -type ApplyInterceptor func(ctx context.Context, in ApplyInterceptorInput) (ApplyInterceptorResult, error) +type AdmissionFunc func(ctx context.Context, in AdmissionInput) (AdmissionDecision, error) -// ApplyInterceptorInput describes the apply request seen by ApplyInterceptor. -// Store is the production store the object would otherwise be written to. -type ApplyInterceptorInput struct { +// AdmissionInput describes the write request seen by AdmissionFunc. Store is +// the production store the object would otherwise be written to. +type AdmissionInput struct { + Source ApplySource + Verb string Kind string Namespace string Name string @@ -68,10 +82,10 @@ type ApplyInterceptorInput struct { Store *v1alpha1store.Store } -// ApplyInterceptorResult reports whether the hook handled the apply. -// Status is copied into ApplyResult.Status when Handled is true; leave it empty -// to use the generic "staged" status. -type ApplyInterceptorResult struct { +// AdmissionDecision reports whether the hook handled the apply. Status is +// copied into ApplyResult.Status when Handled is true; leave it empty to use +// the generic "staged" status. +type AdmissionDecision struct { Handled bool Status string Tag string @@ -88,6 +102,7 @@ const ( stageRefs applyStage = "refs" stageRegistries applyStage = "registries" stageAdmission applyStage = "admission" + stagePrepare applyStage = "prepare" stageMarshal applyStage = "marshal" stageUpsert applyStage = "upsert" stagePostUpsert applyStage = "post-upsert" @@ -160,12 +175,14 @@ func applyCore( return upsertResult{}, &applyError{Stage: stageRegistries, Err: err} } - if dryRun { - return upsertResult{}, nil - } - - if opts.ApplyInterceptor != nil { - intercept, err := opts.ApplyInterceptor(ctx, ApplyInterceptorInput{ + if !dryRun && opts.Admission != nil { + source := opts.Source + if source == "" { + source = ApplySourceApply + } + decision, err := opts.Admission(ctx, AdmissionInput{ + Source: source, + Verb: "apply", Kind: kind, Namespace: meta.Namespace, Name: meta.Name, @@ -176,15 +193,25 @@ func applyCore( if err != nil { return upsertResult{}, &applyError{Stage: stageAdmission, Err: err} } - if intercept.Handled { + if decision.Handled { return upsertResult{ - Intercepted: true, - InterceptStatus: intercept.Status, - InterceptTag: intercept.Tag, + Admitted: true, + AdmitStatus: decision.Status, + AdmitTag: decision.Tag, }, nil } } + if opts.Prepare != nil { + if err := opts.Prepare(ctx, obj); err != nil { + return upsertResult{}, &applyError{Stage: stagePrepare, Err: err} + } + } + + if dryRun { + return upsertResult{}, nil + } + upsertOpts := v1alpha1store.UpsertOpts{} if opts.InitialFinalizers != nil { upsertOpts.InitialFinalizers = opts.InitialFinalizers(obj) diff --git a/pkg/types/types.go b/pkg/types/types.go index 1abc5277..da44043a 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -67,7 +67,12 @@ type PostUpsert func(ctx context.Context, obj v1alpha1.Object) error // batch's per-doc delete hook. type PostDelete func(ctx context.Context, obj v1alpha1.Object) error -// ApplyInterceptor can accept a validated apply before the object reaches +const ( + AdmissionSourceApply = "apply" + AdmissionSourceImport = "import" +) + +// Admission can accept or reject a validated write before the object reaches // production storage. Store is intentionally opaque to keep pkg/types free of // registry/store implementation imports; integrations that need it can type // assert against the concrete store package they already depend on. @@ -75,9 +80,11 @@ type PostDelete func(ctx context.Context, obj v1alpha1.Object) error // TODO(krt): this belongs to the synchronous handler architecture. Prefer a // reconciler-owned admission/staging model when KRT becomes the write path, and // delete this bridge once no downstream route depends on it. -type ApplyInterceptor func(ctx context.Context, in ApplyInterceptorInput) (ApplyInterceptorResult, error) +type Admission func(ctx context.Context, in AdmissionInput) (AdmissionDecision, error) -type ApplyInterceptorInput struct { +type AdmissionInput struct { + Source string + Verb string Kind string Namespace string Name string @@ -86,7 +93,7 @@ type ApplyInterceptorInput struct { Store any } -type ApplyInterceptorResult struct { +type AdmissionDecision struct { Handled bool Status string Tag string @@ -191,15 +198,11 @@ type AppOptions struct { // PostDeletes mirror PostUpserts on the delete path. PostDeletes map[string]PostDelete - // ApplyInterceptor optionally accepts a validated apply before the row - // reaches production storage. Nil preserves normal direct writes. + // Admission optionally accepts a validated write before the row reaches + // production storage. Nil preserves normal direct writes. // TODO(krt): temporary synchronous-handler bridge; remove with reconciler // admission/staging. - ApplyInterceptor ApplyInterceptor - - // ImportAuthorizers optionally overrides Authorizers for /v0/import. - // Nil makes import use the regular Authorizers map. - ImportAuthorizers map[string]Authorizer + Admission Admission // ResolverWrapper decorates the shared ResourceRef resolver before route // registration. Nil preserves the default store-backed resolver. From 5563854ab43cf10fffd4810268826ed2e319f292 Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Wed, 13 May 2026 10:17:05 -0400 Subject: [PATCH 05/10] Clarify admission adapter boundary 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 --- internal/registry/registry_app.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/registry/registry_app.go b/internal/registry/registry_app.go index 06118c49..99b15aab 100644 --- a/internal/registry/registry_app.go +++ b/internal/registry/registry_app.go @@ -267,6 +267,11 @@ func adaptAdmission(fn types.Admission) resource.AdmissionFunc { if fn == nil { return nil } + // AppOptions exposes the public pkg/types admission contract so downstream + // integrations do not need to import the internal resource package. The + // route layer still needs resource.AdmissionFunc, so this adapter keeps the + // composition root as the only place that translates between the two + // field-compatible shapes. return func(ctx context.Context, in resource.AdmissionInput) (resource.AdmissionDecision, error) { out, err := fn(ctx, types.AdmissionInput{ Source: string(in.Source), From 9b1395d6cd28157d49ff44db1c9ea16b3c8f9637 Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Wed, 13 May 2026 10:24:14 -0400 Subject: [PATCH 06/10] Use one admission contract 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 --- internal/registry/api/router/v0.go | 6 ++-- internal/registry/registry_app.go | 33 +----------------- pkg/importer/importer.go | 3 +- pkg/registry/resource/apply.go | 7 ++-- pkg/registry/resource/apply_test.go | 9 ++--- pkg/registry/resource/core.go | 54 +++-------------------------- pkg/types/types.go | 6 ++-- 7 files changed, 23 insertions(+), 95 deletions(-) diff --git a/internal/registry/api/router/v0.go b/internal/registry/api/router/v0.go index e4c48b69..e2248f2d 100644 --- a/internal/registry/api/router/v0.go +++ b/internal/registry/api/router/v0.go @@ -86,7 +86,7 @@ type RouteOptions struct { // Upsert. Nil preserves normal direct writes. // TODO(krt): temporary synchronous-handler bridge; remove when KRT owns // admission/staging. - Admission resource.AdmissionFunc + Admission types.Admission // ResolverWrapper decorates the shared ResourceRef resolver before // resource and apply routes are registered. @@ -144,7 +144,7 @@ func RegisterRoutes( // one write path instead of route-specific overrides. if opts.Importer != nil { importApplyCfg := applyCfg - importApplyCfg.Source = resource.ApplySourceImport + importApplyCfg.Source = types.AdmissionSourceImport importpipeline.Register(api, importpipeline.Config{ BasePrefix: pathPrefix, Importer: opts.Importer, @@ -179,7 +179,7 @@ func registerKindRoutes( coord *deploymentsvc.Coordinator, perKind crud.PerKindHooks, registryValidator v1alpha1.RegistryValidatorFunc, - admission resource.AdmissionFunc, + admission types.Admission, resolverWrapper func(v1alpha1.ResolverFunc) v1alpha1.ResolverFunc, extraResourceRoutes func(api huma.API, pathPrefix string, ctx types.ResourceRouteContext), ) resource.ApplyConfig { diff --git a/internal/registry/registry_app.go b/internal/registry/registry_app.go index 99b15aab..52eb06ea 100644 --- a/internal/registry/registry_app.go +++ b/internal/registry/registry_app.go @@ -247,7 +247,7 @@ func buildRouteOptions( Importer: importer, PerKindHooks: crudPerKindHooks(options), RegistryValidator: options.RegistryValidator, - Admission: adaptAdmission(options.Admission), + Admission: options.Admission, ResolverWrapper: options.ResolverWrapper, ExtraResourceRoutes: options.ExtraResourceRoutes, } @@ -263,37 +263,6 @@ func buildRouteOptions( return routeOpts } -func adaptAdmission(fn types.Admission) resource.AdmissionFunc { - if fn == nil { - return nil - } - // AppOptions exposes the public pkg/types admission contract so downstream - // integrations do not need to import the internal resource package. The - // route layer still needs resource.AdmissionFunc, so this adapter keeps the - // composition root as the only place that translates between the two - // field-compatible shapes. - return func(ctx context.Context, in resource.AdmissionInput) (resource.AdmissionDecision, error) { - out, err := fn(ctx, types.AdmissionInput{ - Source: string(in.Source), - Verb: in.Verb, - Kind: in.Kind, - Namespace: in.Namespace, - Name: in.Name, - Tag: in.Tag, - Object: in.Object, - Store: in.Store, - }) - if err != nil { - return resource.AdmissionDecision{}, err - } - return resource.AdmissionDecision{ - Handled: out.Handled, - Status: out.Status, - Tag: out.Tag, - }, nil - } -} - // crudPerKindHooks adapts the AppOptions per-kind authorizer + // list-filter maps (which use the public pkg/types signatures) into // the internal crud.PerKindHooks struct (which uses the diff --git a/pkg/importer/importer.go b/pkg/importer/importer.go index 9a4e195b..3d6106a4 100644 --- a/pkg/importer/importer.go +++ b/pkg/importer/importer.go @@ -17,6 +17,7 @@ import ( "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" "github.com/agentregistry-dev/agentregistry/pkg/registry/resource" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" + "github.com/agentregistry-dev/agentregistry/pkg/types" ) // Options controls one invocation of Importer.Import. @@ -294,7 +295,7 @@ func (i *Importer) importOne(ctx context.Context, source string, obj v1alpha1.Ob var pendingFindings map[string][]Finding applyCfg := i.applyConfig(opts) - applyCfg.Source = resource.ApplySourceImport + applyCfg.Source = types.AdmissionSourceImport applyCfg.Prepare = func(ctx context.Context, obj v1alpha1.Object) error { if !opts.Enrich { return nil diff --git a/pkg/registry/resource/apply.go b/pkg/registry/resource/apply.go index 0337c9d1..2e847059 100644 --- a/pkg/registry/resource/apply.go +++ b/pkg/registry/resource/apply.go @@ -12,6 +12,7 @@ import ( "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" pkgdb "github.com/agentregistry-dev/agentregistry/pkg/registry/database" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" + "github.com/agentregistry-dev/agentregistry/pkg/types" ) // ApplyConfig is the per-server configuration for the multi-doc apply @@ -65,12 +66,12 @@ type ApplyConfig struct { InitialFinalizers map[string]func(obj v1alpha1.Object) []string // Source labels the producer of objects entering this apply pipeline. - // Empty defaults to ApplySourceApply. - Source ApplySource + // Empty defaults to types.AdmissionSourceApply. + Source string // Admission optionally accepts a validated write before production Upsert. // Nil preserves normal direct-write behavior. - Admission AdmissionFunc + Admission types.Admission // Prepare optionally mutates an object after validation/admission and // before Upsert. Import uses this to merge scanner output while still diff --git a/pkg/registry/resource/apply_test.go b/pkg/registry/resource/apply_test.go index bfaf4907..a0be966a 100644 --- a/pkg/registry/resource/apply_test.go +++ b/pkg/registry/resource/apply_test.go @@ -17,6 +17,7 @@ import ( pkgdb "github.com/agentregistry-dev/agentregistry/pkg/registry/database" "github.com/agentregistry-dev/agentregistry/pkg/registry/resource" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" + "github.com/agentregistry-dev/agentregistry/pkg/types" ) func TestRegisterApply_MultiDocRoundTrip(t *testing.T) { @@ -120,7 +121,7 @@ func TestRegisterApply_AdmissionCanHandleBeforeProductionUpsert(t *testing.T) { pool := v1alpha1store.NewTestPool(t) agents := v1alpha1store.NewStore(pool, "v1alpha1.agents") - var admitted resource.AdmissionInput + var admitted types.AdmissionInput postUpsertCalled := false _, api := humatest.New(t) resource.RegisterApply(api, resource.ApplyConfig{ @@ -134,9 +135,9 @@ func TestRegisterApply_AdmissionCanHandleBeforeProductionUpsert(t *testing.T) { return nil }, }, - Admission: func(ctx context.Context, in resource.AdmissionInput) (resource.AdmissionDecision, error) { + Admission: func(ctx context.Context, in types.AdmissionInput) (types.AdmissionDecision, error) { admitted = in - return resource.AdmissionDecision{Handled: true, Tag: in.Tag}, nil + return types.AdmissionDecision{Handled: true, Tag: in.Tag}, nil }, }) @@ -159,7 +160,7 @@ spec: require.Equal(t, arv0.ApplyStatusStaged, out.Results[0].Status) require.Equal(t, v1alpha1store.DefaultTag(), out.Results[0].Tag) require.False(t, postUpsertCalled, "admitted applies must not fire production side effects") - require.Equal(t, resource.ApplySourceApply, admitted.Source) + require.Equal(t, types.AdmissionSourceApply, admitted.Source) require.Equal(t, "apply", admitted.Verb) require.Equal(t, v1alpha1.KindAgent, admitted.Kind) require.Equal(t, "default", admitted.Namespace) diff --git a/pkg/registry/resource/core.go b/pkg/registry/resource/core.go index cd35d1fc..30b9b573 100644 --- a/pkg/registry/resource/core.go +++ b/pkg/registry/resource/core.go @@ -7,6 +7,7 @@ import ( "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" pkgdb "github.com/agentregistry-dev/agentregistry/pkg/registry/database" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" + "github.com/agentregistry-dev/agentregistry/pkg/types" ) // applyOpts threads the per-kind dependencies into the apply pipeline. @@ -20,8 +21,8 @@ type applyOpts struct { RegistryValidator v1alpha1.RegistryValidatorFunc PostUpsert func(ctx context.Context, obj v1alpha1.Object) error InitialFinalizers func(obj v1alpha1.Object) []string - Admission AdmissionFunc - Source ApplySource + Admission types.Admission + Source string Prepare func(ctx context.Context, obj v1alpha1.Object) error } @@ -46,51 +47,6 @@ type upsertResult struct { AdmitTag string } -// ApplySource identifies the route or subsystem that produced an object for -// the shared apply pipeline. -type ApplySource string - -const ( - ApplySourceApply ApplySource = "apply" - ApplySourceImport ApplySource = "import" -) - -// AdmissionFunc can accept or reject a validated apply request before the -// object is written to the production Store. Downstream builds use this as a -// neutral admission seam for workflows that persist the object somewhere else -// first. -// -// TODO(krt): this is a synchronous-handler bridge for the pre-KRT apply path. -// Remove or collapse it when reconciler-owned admission/staging becomes the -// production architecture. -// -// The hook runs after authorization, validation, reference resolution, and -// registry validation, but before Store.Upsert and PostUpsert. Returning -// Handled=true short-circuits the production upsert and skips PostUpsert. -type AdmissionFunc func(ctx context.Context, in AdmissionInput) (AdmissionDecision, error) - -// AdmissionInput describes the write request seen by AdmissionFunc. Store is -// the production store the object would otherwise be written to. -type AdmissionInput struct { - Source ApplySource - Verb string - Kind string - Namespace string - Name string - Tag string - Object v1alpha1.Object - Store *v1alpha1store.Store -} - -// AdmissionDecision reports whether the hook handled the apply. Status is -// copied into ApplyResult.Status when Handled is true; leave it empty to use -// the generic "staged" status. -type AdmissionDecision struct { - Handled bool - Status string - Tag string -} - // applyStage tags which step of the pipeline produced an error so // callers can shape their error response (huma 4xx vs ApplyResult.Error) // without re-classifying the underlying err. @@ -178,9 +134,9 @@ func applyCore( if !dryRun && opts.Admission != nil { source := opts.Source if source == "" { - source = ApplySourceApply + source = types.AdmissionSourceApply } - decision, err := opts.Admission(ctx, AdmissionInput{ + decision, err := opts.Admission(ctx, types.AdmissionInput{ Source: source, Verb: "apply", Kind: kind, diff --git a/pkg/types/types.go b/pkg/types/types.go index da44043a..13ed0896 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -73,9 +73,9 @@ const ( ) // Admission can accept or reject a validated write before the object reaches -// production storage. Store is intentionally opaque to keep pkg/types free of -// registry/store implementation imports; integrations that need it can type -// assert against the concrete store package they already depend on. +// production storage. Store is intentionally any-typed so downstream +// integrations can use this public contract without forcing pkg/types to +// depend on the concrete registry store package. // // TODO(krt): this belongs to the synchronous handler architecture. Prefer a // reconciler-owned admission/staging model when KRT becomes the write path, and From a2f0477555f4f4576eea7015a990994bd02fcdd6 Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Wed, 13 May 2026 10:46:14 -0400 Subject: [PATCH 07/10] Make admission own apply writes 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 --- internal/registry/api/router/v0.go | 4 +- pkg/registry/resource/apply.go | 40 ++------ pkg/registry/resource/apply_test.go | 6 +- pkg/registry/resource/core.go | 152 ++++++++++++++-------------- pkg/types/types.go | 37 +++---- 5 files changed, 112 insertions(+), 127 deletions(-) diff --git a/internal/registry/api/router/v0.go b/internal/registry/api/router/v0.go index e2248f2d..2fbcd847 100644 --- a/internal/registry/api/router/v0.go +++ b/internal/registry/api/router/v0.go @@ -82,8 +82,8 @@ type RouteOptions struct { // Optional callback for integration-owned route registration. ExtraRoutes func(api huma.API, pathPrefix string) - // Admission optionally accepts a validated write before production - // Upsert. Nil preserves normal direct writes. + // Admission optionally owns the final apply write. Nil preserves OSS + // production writes through resource.ProductionAdmission. // TODO(krt): temporary synchronous-handler bridge; remove when KRT owns // admission/staging. Admission types.Admission diff --git a/pkg/registry/resource/apply.go b/pkg/registry/resource/apply.go index 2e847059..e3758d5c 100644 --- a/pkg/registry/resource/apply.go +++ b/pkg/registry/resource/apply.go @@ -69,12 +69,12 @@ type ApplyConfig struct { // Empty defaults to types.AdmissionSourceApply. Source string - // Admission optionally accepts a validated write before production Upsert. - // Nil preserves normal direct-write behavior. + // Admission optionally owns the final apply write. Nil uses + // ProductionAdmission, which writes to the configured production Store. Admission types.Admission - // Prepare optionally mutates an object after validation/admission and - // before Upsert. Import uses this to merge scanner output while still + // Prepare optionally mutates an object after validation and before + // admission. Import uses this to merge scanner output while still // persisting through the shared apply path. Prepare func(ctx context.Context, obj v1alpha1.Object) error } @@ -186,7 +186,7 @@ func applyOne(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun return failResult(res, ae) } - up, ae := applyCore(ctx, store, obj, applyOpts{ + admitted, ae := applyCore(ctx, store, obj, applyOpts{ Authorize: batchAuthorize(cfg, obj.GetKind()), Resolver: cfg.Resolver, RegistryValidator: cfg.RegistryValidator, @@ -200,34 +200,12 @@ func applyOne(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun return failResult(res, ae) } - if dryRun { - res.Status = arv0.ApplyStatusDryRun - return res - } - // Map the Store outcome onto the wire status. Tagged-artifact creates - // surface as created, same-tag replacements as configured, and exact - // re-applies as unchanged. - switch { - case up.Admitted: - res.Status = up.AdmitStatus - if res.Status == "" { - res.Status = arv0.ApplyStatusStaged - } - if up.AdmitTag != "" { - res.Tag = up.AdmitTag - } - return res - case up.Outcome == v1alpha1store.UpsertCreated: - res.Status = arv0.ApplyStatusCreated - case up.Outcome == v1alpha1store.UpsertReplaced: - res.Status = arv0.ApplyStatusConfigured - case up.Outcome == v1alpha1store.UpsertNoOp: + res.Status = admitted.Status + if res.Status == "" { res.Status = arv0.ApplyStatusUnchanged } - if up.Tag != "" { - res.Tag = up.Tag - } - res.Generation = up.Generation + res.Tag = admitted.Tag + res.Generation = admitted.Generation return res } diff --git a/pkg/registry/resource/apply_test.go b/pkg/registry/resource/apply_test.go index a0be966a..14cace14 100644 --- a/pkg/registry/resource/apply_test.go +++ b/pkg/registry/resource/apply_test.go @@ -117,7 +117,7 @@ spec: require.Contains(t, out.Results[1].Error, "unknown or unconfigured kind") } -func TestRegisterApply_AdmissionCanHandleBeforeProductionUpsert(t *testing.T) { +func TestRegisterApply_AdmissionCanStageInsteadOfProductionUpsert(t *testing.T) { pool := v1alpha1store.NewTestPool(t) agents := v1alpha1store.NewStore(pool, "v1alpha1.agents") @@ -135,9 +135,9 @@ func TestRegisterApply_AdmissionCanHandleBeforeProductionUpsert(t *testing.T) { return nil }, }, - Admission: func(ctx context.Context, in types.AdmissionInput) (types.AdmissionDecision, error) { + Admission: func(ctx context.Context, in types.AdmissionInput) (types.AdmissionResult, error) { admitted = in - return types.AdmissionDecision{Handled: true, Tag: in.Tag}, nil + return types.AdmissionResult{Status: arv0.ApplyStatusStaged, Tag: in.Tag}, nil }, }) diff --git a/pkg/registry/resource/core.go b/pkg/registry/resource/core.go index 30b9b573..f550c9dd 100644 --- a/pkg/registry/resource/core.go +++ b/pkg/registry/resource/core.go @@ -4,6 +4,7 @@ import ( "context" "errors" + arv0 "github.com/agentregistry-dev/agentregistry/pkg/api/v0" "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" pkgdb "github.com/agentregistry-dev/agentregistry/pkg/registry/database" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" @@ -26,27 +27,6 @@ type applyOpts struct { Prepare func(ctx context.Context, obj v1alpha1.Object) error } -// upsertResult is the outcome of a successful applyCore call. -type upsertResult struct { - // Outcome categorises what the underlying Store.Upsert did. Callers - // map this onto their wire status (ApplyStatusCreated, etc.). - Outcome v1alpha1store.UpsertOutcome - // Tag is the content tag after apply for tagged artifact stores. - Tag string - // Generation is the internal row generation after apply. - Generation int64 - // UID is the server-managed row identity after production upsert. - UID string - // Admitted reports that a downstream hook handled the apply before - // production upsert. No production PostUpsert hook has run. - Admitted bool - // AdmitStatus is the ApplyResult status to report when Admitted - // is true. Empty defaults to ApplyStatusStaged at the batch layer. - AdmitStatus string - // AdmitTag is the optional tag to report when Admitted is true. - AdmitTag string -} - // applyStage tags which step of the pipeline produced an error so // callers can shape their error response (huma 4xx vs ApplyResult.Error) // without re-classifying the underlying err. @@ -86,18 +66,18 @@ func (e *applyError) Error() string { // already-decoded, metadata-stamped object: // // canonicalize metadata → authorize → validate → resolve refs → -// validate registries → marshal spec → Store.Upsert → PostUpsert +// validate registries → prepare → admission // -// dryRun=true skips Upsert + PostUpsert; everything else still runs so -// clients get the same 400-class error surface they would on a real -// apply. Returns a stage-tagged applyError on failure; nil otherwise. +// The admission implementation owns the final write result. The OSS default +// ProductionAdmission maps dry-runs to ApplyStatusDryRun and real writes to +// Store.Upsert + PostUpsert. Returns a stage-tagged applyError on failure. func applyCore( ctx context.Context, store *v1alpha1store.Store, obj v1alpha1.Object, opts applyOpts, dryRun bool, -) (upsertResult, *applyError) { +) (types.AdmissionResult, *applyError) { meta := obj.GetMetadata() kind := obj.GetKind() @@ -117,85 +97,109 @@ func applyCore( Namespace: meta.Namespace, Name: meta.Name, Tag: meta.Tag, Object: obj, }); err != nil { - return upsertResult{}, &applyError{Stage: stageAuth, Err: err} + return types.AdmissionResult{}, &applyError{Stage: stageAuth, Err: err} } } if err := v1alpha1.ValidateObject(obj); err != nil { - return upsertResult{}, &applyError{Stage: stageValidation, Err: err} + return types.AdmissionResult{}, &applyError{Stage: stageValidation, Err: err} } if err := v1alpha1.ResolveObjectRefs(ctx, obj, opts.Resolver); err != nil { - return upsertResult{}, &applyError{Stage: stageRefs, Err: err} + return types.AdmissionResult{}, &applyError{Stage: stageRefs, Err: err} } if err := v1alpha1.ValidateObjectRegistries(ctx, obj, opts.RegistryValidator); err != nil { - return upsertResult{}, &applyError{Stage: stageRegistries, Err: err} + return types.AdmissionResult{}, &applyError{Stage: stageRegistries, Err: err} } - if !dryRun && opts.Admission != nil { - source := opts.Source - if source == "" { - source = types.AdmissionSourceApply - } - decision, err := opts.Admission(ctx, types.AdmissionInput{ - Source: source, - Verb: "apply", - Kind: kind, - Namespace: meta.Namespace, - Name: meta.Name, - Tag: meta.Tag, - Object: obj, - Store: store, - }) - if err != nil { - return upsertResult{}, &applyError{Stage: stageAdmission, Err: err} - } - if decision.Handled { - return upsertResult{ - Admitted: true, - AdmitStatus: decision.Status, - AdmitTag: decision.Tag, - }, nil + if opts.Prepare != nil { + if err := opts.Prepare(ctx, obj); err != nil { + return types.AdmissionResult{}, &applyError{Stage: stagePrepare, Err: err} } } - if opts.Prepare != nil { - if err := opts.Prepare(ctx, obj); err != nil { - return upsertResult{}, &applyError{Stage: stagePrepare, Err: err} + source := opts.Source + if source == "" { + source = types.AdmissionSourceApply + } + admission := opts.Admission + if admission == nil { + admission = ProductionAdmission + } + result, err := admission(ctx, types.AdmissionInput{ + Source: source, + Verb: "apply", + DryRun: dryRun, + Kind: kind, + Namespace: meta.Namespace, + Name: meta.Name, + Tag: meta.Tag, + Object: obj, + Store: store, + PostUpsert: opts.PostUpsert, + InitialFinalizers: opts.InitialFinalizers, + }) + if err != nil { + if ae, ok := err.(*applyError); ok { + return types.AdmissionResult{}, ae } + return types.AdmissionResult{}, &applyError{Stage: stageAdmission, Err: err} } + return result, nil +} - if dryRun { - return upsertResult{}, nil +// ProductionAdmission is the OSS admission implementation: dry-runs stop after +// validation, and real writes upsert the object into the production store and +// run the per-kind post-upsert hook. +func ProductionAdmission(ctx context.Context, in types.AdmissionInput) (types.AdmissionResult, error) { + if in.DryRun { + return types.AdmissionResult{Status: arv0.ApplyStatusDryRun, Tag: in.Tag}, nil + } + store, ok := in.Store.(*v1alpha1store.Store) + if !ok || store == nil { + return types.AdmissionResult{}, errors.New("production store is required") } upsertOpts := v1alpha1store.UpsertOpts{} - if opts.InitialFinalizers != nil { - upsertOpts.InitialFinalizers = opts.InitialFinalizers(obj) + if in.InitialFinalizers != nil { + upsertOpts.InitialFinalizers = in.InitialFinalizers(in.Object) } - up, err := store.Upsert(ctx, obj, upsertOpts) + up, err := store.Upsert(ctx, in.Object, upsertOpts) if err != nil { - return upsertResult{}, &applyError{ + return types.AdmissionResult{}, &applyError{ Stage: stageUpsert, Err: err, Terminating: errors.Is(err, v1alpha1store.ErrTerminating), } } - res := upsertResult{ - Outcome: up.Outcome, - Tag: up.Tag, - Generation: up.Generation, - UID: up.UID, - } - if opts.PostUpsert != nil { + if in.PostUpsert != nil { + meta := in.Object.GetMetadata() meta.Generation = up.Generation meta.UID = up.UID - obj.SetMetadata(*meta) - if err := opts.PostUpsert(ctx, obj); err != nil { - return res, &applyError{Stage: stagePostUpsert, Err: err} + in.Object.SetMetadata(*meta) + if err := in.PostUpsert(ctx, in.Object); err != nil { + return types.AdmissionResult{}, &applyError{Stage: stagePostUpsert, Err: err} } } - return res, nil + + return types.AdmissionResult{ + Status: applyStatusFromUpsert(up.Outcome), + Tag: up.Tag, + Generation: up.Generation, + }, nil +} + +func applyStatusFromUpsert(outcome v1alpha1store.UpsertOutcome) string { + switch outcome { + case v1alpha1store.UpsertCreated: + return arv0.ApplyStatusCreated + case v1alpha1store.UpsertReplaced: + return arv0.ApplyStatusConfigured + case v1alpha1store.UpsertNoOp: + return arv0.ApplyStatusUnchanged + default: + return arv0.ApplyStatusUnchanged + } } // deleteOpts threads the per-kind dependencies into deleteCore. As with diff --git a/pkg/types/types.go b/pkg/types/types.go index 13ed0896..475f2272 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -72,31 +72,34 @@ const ( AdmissionSourceImport = "import" ) -// Admission can accept or reject a validated write before the object reaches -// production storage. Store is intentionally any-typed so downstream -// integrations can use this public contract without forcing pkg/types to -// depend on the concrete registry store package. +// Admission owns the final write decision for an apply request after authz, +// validation, reference resolution, and registry checks have passed. The OSS +// default writes to production; downstream integrations can wrap that behavior +// to stage, reject, or otherwise route the write. // // TODO(krt): this belongs to the synchronous handler architecture. Prefer a // reconciler-owned admission/staging model when KRT becomes the write path, and // delete this bridge once no downstream route depends on it. -type Admission func(ctx context.Context, in AdmissionInput) (AdmissionDecision, error) +type Admission func(ctx context.Context, in AdmissionInput) (AdmissionResult, error) type AdmissionInput struct { - Source string - Verb string - Kind string - Namespace string - Name string - Tag string - Object v1alpha1.Object - Store any + Source string + Verb string + DryRun bool + Kind string + Namespace string + Name string + Tag string + Object v1alpha1.Object + Store any + PostUpsert PostUpsert + InitialFinalizers func(v1alpha1.Object) []string } -type AdmissionDecision struct { - Handled bool - Status string - Tag string +type AdmissionResult struct { + Status string + Tag string + Generation int64 } // ResourceRouteContext exposes the finalized v1alpha1 route wiring to From 16d6174b6a996a653dc69bc9814173211d6fde5b Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Wed, 13 May 2026 13:32:33 -0400 Subject: [PATCH 08/10] Name production admission replay explicitly 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 --- internal/registry/api/router/v0.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/registry/api/router/v0.go b/internal/registry/api/router/v0.go index 2fbcd847..158b61d4 100644 --- a/internal/registry/api/router/v0.go +++ b/internal/registry/api/router/v0.go @@ -252,7 +252,7 @@ func registerKindRoutes( Admission: admission, } productionApplyCfg := applyCfg - productionApplyCfg.Admission = nil + productionApplyCfg.Admission = resource.ProductionAdmission resource.RegisterApply(api, applyCfg) if extraResourceRoutes != nil { From 48edcd748ad4447bd86fcb41917cedd882bc0f79 Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Tue, 19 May 2026 11:09:45 -0400 Subject: [PATCH 09/10] Expose delete admission for approval staging 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 ./... --- internal/client/client_integration_test.go | 4 +- .../registry/api/handlers/v0/crud/crud.go | 3 + .../handlers/v0/crud/deployment_hooks_test.go | 1 + internal/registry/api/router/v0.go | 16 ++++- internal/registry/registry_app.go | 1 + pkg/registry/resource/apply.go | 56 +++++++-------- pkg/registry/resource/apply_test.go | 62 +++++++++++++++++ pkg/registry/resource/core.go | 69 +++++++++++++++---- pkg/registry/resource/handler.go | 16 ++++- pkg/registry/resource/handler_test.go | 45 ++++++++++++ pkg/types/types.go | 35 ++++++++++ 11 files changed, 259 insertions(+), 49 deletions(-) diff --git a/internal/client/client_integration_test.go b/internal/client/client_integration_test.go index c69be435..38e68b4e 100644 --- a/internal/client/client_integration_test.go +++ b/internal/client/client_integration_test.go @@ -31,7 +31,7 @@ func TestClient_V1Alpha1RoundTrip(t *testing.T) { mux := http.NewServeMux() api := humago.New(mux, huma.DefaultConfig("test", "v1")) - crud.Register(api, "/v0", stores, nil, nil, crud.PerKindHooks{}) + crud.Register(api, "/v0", stores, nil, nil, crud.PerKindHooks{}, nil) resource.RegisterApply(api, resource.ApplyConfig{ BasePrefix: "/v0", Stores: stores, @@ -143,7 +143,7 @@ func TestClient_V1Alpha1_NotFound(t *testing.T) { mux := http.NewServeMux() api := humago.New(mux, huma.DefaultConfig("test", "v1")) - crud.Register(api, "/v0", stores, nil, nil, crud.PerKindHooks{}) + crud.Register(api, "/v0", stores, nil, nil, crud.PerKindHooks{}, nil) ts := httptest.NewServer(mux) defer ts.Close() diff --git a/internal/registry/api/handlers/v0/crud/crud.go b/internal/registry/api/handlers/v0/crud/crud.go index b033246a..dd95a968 100644 --- a/internal/registry/api/handlers/v0/crud/crud.go +++ b/internal/registry/api/handlers/v0/crud/crud.go @@ -24,6 +24,7 @@ import ( "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" "github.com/agentregistry-dev/agentregistry/pkg/registry/resource" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" + "github.com/agentregistry-dev/agentregistry/pkg/types" ) // PerKindHooks groups optional, per-kind callbacks layered on top of @@ -71,6 +72,7 @@ func Register( resolver v1alpha1.ResolverFunc, registryValidator v1alpha1.RegistryValidatorFunc, perKind PerKindHooks, + deleteAdmission types.DeleteAdmission, ) { cfgFor := func(kind string) (resource.Config, bool) { store, ok := stores[kind] @@ -87,6 +89,7 @@ func Register( ListFilter: perKind.ListFilters[kind], PostUpsert: perKind.PostUpserts[kind], PostDelete: perKind.PostDeletes[kind], + DeleteAdmission: deleteAdmission, InitialFinalizers: perKind.InitialFinalizers[kind], }, true } diff --git a/internal/registry/api/handlers/v0/crud/deployment_hooks_test.go b/internal/registry/api/handlers/v0/crud/deployment_hooks_test.go index a4fb685e..5fd4e916 100644 --- a/internal/registry/api/handlers/v0/crud/deployment_hooks_test.go +++ b/internal/registry/api/handlers/v0/crud/deployment_hooks_test.go @@ -75,6 +75,7 @@ func seedDeploymentFixtures(t *testing.T) (humatest.TestAPI, map[string]*v1alpha }, }, }, + nil, ) deploymentlogs.Register(api, deploymentlogs.Config{ BasePrefix: "/v0", diff --git a/internal/registry/api/router/v0.go b/internal/registry/api/router/v0.go index 158b61d4..ea91ac54 100644 --- a/internal/registry/api/router/v0.go +++ b/internal/registry/api/router/v0.go @@ -88,6 +88,12 @@ type RouteOptions struct { // admission/staging. Admission types.Admission + // DeleteAdmission optionally owns the final delete. Nil preserves OSS + // production deletes through resource.ProductionDeleteAdmission. + // TODO(krt): temporary synchronous-handler bridge; remove when KRT owns + // admission/staging. + DeleteAdmission types.DeleteAdmission + // ResolverWrapper decorates the shared ResourceRef resolver before // resource and apply routes are registered. // TODO(krt): temporary bridge for pending staged refs during HTTP apply. @@ -134,6 +140,7 @@ func RegisterRoutes( opts.PerKindHooks, opts.RegistryValidator, opts.Admission, + opts.DeleteAdmission, opts.ResolverWrapper, opts.ExtraResourceRoutes, ) @@ -180,6 +187,7 @@ func registerKindRoutes( perKind crud.PerKindHooks, registryValidator v1alpha1.RegistryValidatorFunc, admission types.Admission, + deleteAdmission types.DeleteAdmission, resolverWrapper func(v1alpha1.ResolverFunc) v1alpha1.ResolverFunc, extraResourceRoutes func(api huma.API, pathPrefix string, ctx types.ResourceRouteContext), ) resource.ApplyConfig { @@ -223,7 +231,7 @@ func registerKindRoutes( // Per-kind CRUD endpoints — one call per built-in kind, hidden // inside crud.Register. - crud.Register(api, basePrefix, stores, resolver, registryValidator, perKind) + crud.Register(api, basePrefix, stores, resolver, registryValidator, perKind, deleteAdmission) // Deployment-specific endpoints: logs stream (cancel is subsumed // by DesiredState=undeployed + DELETE in the v1alpha1 lifecycle). @@ -250,9 +258,12 @@ func registerKindRoutes( PostDeletes: perKind.PostDeletes, InitialFinalizers: perKind.InitialFinalizers, Admission: admission, + DeleteAdmission: deleteAdmission, } productionApplyCfg := applyCfg productionApplyCfg.Admission = resource.ProductionAdmission + productionDeleteCfg := applyCfg + productionDeleteCfg.DeleteAdmission = resource.ProductionDeleteAdmission resource.RegisterApply(api, applyCfg) if extraResourceRoutes != nil { @@ -267,6 +278,9 @@ func registerKindRoutes( Apply: func(ctx context.Context, obj v1alpha1.Object, dryRun bool) arv0.ApplyResult { return resource.ApplyObject(ctx, productionApplyCfg, obj, dryRun) }, + Delete: func(ctx context.Context, obj v1alpha1.Object, dryRun bool) arv0.ApplyResult { + return resource.DeleteObject(ctx, productionDeleteCfg, obj, dryRun) + }, }) } return applyCfg diff --git a/internal/registry/registry_app.go b/internal/registry/registry_app.go index 52eb06ea..d6b8eec0 100644 --- a/internal/registry/registry_app.go +++ b/internal/registry/registry_app.go @@ -248,6 +248,7 @@ func buildRouteOptions( PerKindHooks: crudPerKindHooks(options), RegistryValidator: options.RegistryValidator, Admission: options.Admission, + DeleteAdmission: options.DeleteAdmission, ResolverWrapper: options.ResolverWrapper, ExtraResourceRoutes: options.ExtraResourceRoutes, } diff --git a/pkg/registry/resource/apply.go b/pkg/registry/resource/apply.go index e3758d5c..9f8d9474 100644 --- a/pkg/registry/resource/apply.go +++ b/pkg/registry/resource/apply.go @@ -2,7 +2,6 @@ package resource import ( "context" - "errors" "fmt" "net/http" @@ -10,7 +9,6 @@ import ( arv0 "github.com/agentregistry-dev/agentregistry/pkg/api/v0" "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" - pkgdb "github.com/agentregistry-dev/agentregistry/pkg/registry/database" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" "github.com/agentregistry-dev/agentregistry/pkg/types" ) @@ -73,6 +71,11 @@ type ApplyConfig struct { // ProductionAdmission, which writes to the configured production Store. Admission types.Admission + // DeleteAdmission optionally owns the final delete. Nil uses + // ProductionDeleteAdmission, which deletes from the configured production + // Store and runs the per-kind PostDelete hook. + DeleteAdmission types.DeleteAdmission + // Prepare optionally mutates an object after validation and before // admission. Import uses this to merge scanner output while still // persisting through the shared apply path. @@ -172,6 +175,12 @@ func ApplyObject(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryR return applyOne(ctx, cfg, obj, dryRun) } +// DeleteObject runs one already-decoded object through the same production +// delete path used by DELETE /v0/apply. +func DeleteObject(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun bool) arv0.ApplyResult { + return deleteOne(ctx, cfg, obj, dryRun) +} + // applyOne runs a single document through the shared apply pipeline. // Never errors; encodes any failure into the returned ApplyResult. func applyOne(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun bool) arv0.ApplyResult { @@ -231,38 +240,21 @@ func deleteOne(ctx context.Context, cfg ApplyConfig, obj v1alpha1.Object, dryRun return failResult(res, ae) } - if dryRun { - res.Status = arv0.ApplyStatusDryRun - return res - } - - authz := batchAuthorize(cfg, obj.GetKind()) - if authz != nil { - if err := authz(ctx, AuthorizeInput{ - Verb: "delete", Kind: obj.GetKind(), - Namespace: meta.Namespace, Name: meta.Name, Tag: meta.Tag, - Object: obj, - }); err != nil { - return failResult(res, &applyError{Stage: stageAuth, Err: err}) - } - } - - err := store.DeleteByRef(ctx, meta.Namespace, meta.Name, meta.Tag) - if err != nil { - return failResult(res, &applyError{ - Stage: stageDelete, - Err: err, - NotFound: errors.Is(err, pkgdb.ErrNotFound), - }) + admitted, ae := deleteCore(ctx, store, obj.GetKind(), meta.Namespace, meta.Name, meta.Tag, deleteOpts{ + Authorize: batchAuthorize(cfg, obj.GetKind()), + PostDelete: cfg.PostDeletes[obj.GetKind()], + PreDeleteObject: obj, + DeleteAdmission: cfg.DeleteAdmission, + Source: cfg.Source, + }, dryRun) + if ae != nil { + return failResult(res, ae) } - - res.Tag = meta.Tag - if hook := cfg.PostDeletes[obj.GetKind()]; hook != nil { - if err := hook(ctx, obj); err != nil { - return failResult(res, &applyError{Stage: stagePostDelete, Err: err}) - } + res.Status = admitted.Status + if res.Status == "" { + res.Status = arv0.ApplyStatusDeleted } - res.Status = arv0.ApplyStatusDeleted + res.Tag = admitted.Tag return res } diff --git a/pkg/registry/resource/apply_test.go b/pkg/registry/resource/apply_test.go index 4e76af9a..7479dcb1 100644 --- a/pkg/registry/resource/apply_test.go +++ b/pkg/registry/resource/apply_test.go @@ -175,6 +175,68 @@ spec: require.ErrorIs(t, err, pkgdb.ErrNotFound) } +func TestRegisterApply_DeleteAdmissionCanStageInsteadOfProductionDelete(t *testing.T) { + pool := v1alpha1store.NewTestPool(t) + agents := v1alpha1store.NewStore(pool, "v1alpha1.agents") + _, err := agents.Upsert(t.Context(), &v1alpha1.Agent{ + Metadata: v1alpha1.ObjectMeta{Namespace: "default", Name: "staged-delete", Tag: "stable"}, + Spec: v1alpha1.AgentSpec{Title: "Staged Delete"}, + }) + require.NoError(t, err) + + var admitted types.DeleteAdmissionInput + postDeleteCalled := false + _, api := humatest.New(t) + resource.RegisterApply(api, resource.ApplyConfig{ + BasePrefix: "/v0", + Stores: map[string]*v1alpha1store.Store{ + v1alpha1.KindAgent: agents, + }, + PostDeletes: map[string]func(context.Context, v1alpha1.Object) error{ + v1alpha1.KindAgent: func(context.Context, v1alpha1.Object) error { + postDeleteCalled = true + return nil + }, + }, + DeleteAdmission: func(ctx context.Context, in types.DeleteAdmissionInput) (types.DeleteAdmissionResult, error) { + admitted = in + return types.DeleteAdmissionResult{Status: arv0.ApplyStatusStaged, Tag: in.Tag}, nil + }, + }) + + yaml := []byte(`apiVersion: ar.dev/v1alpha1 +kind: Agent +metadata: + namespace: default + name: staged-delete + tag: stable +`) + resp := api.Do(http.MethodDelete, "/v0/apply", "Content-Type: application/yaml", strings.NewReader(string(yaml))) + require.Equal(t, http.StatusOK, resp.Code, resp.Body.String()) + + var out struct { + Results []arv0.ApplyResult `json:"results"` + } + require.NoError(t, json.Unmarshal(resp.Body.Bytes(), &out)) + require.Len(t, out.Results, 1) + require.Equal(t, arv0.ApplyStatusStaged, out.Results[0].Status) + require.Equal(t, "stable", out.Results[0].Tag) + require.False(t, postDeleteCalled, "staged deletes must not fire production side effects") + require.Equal(t, types.AdmissionSourceDelete, admitted.Source) + require.Equal(t, "delete", admitted.Verb) + require.Equal(t, v1alpha1.KindAgent, admitted.Kind) + require.Equal(t, "default", admitted.Namespace) + require.Equal(t, "staged-delete", admitted.Name) + require.Equal(t, "stable", admitted.Tag) + require.NotNil(t, admitted.Object) + require.NotNil(t, admitted.PostDelete) + require.Same(t, agents, admitted.Store) + + row, err := agents.Get(t.Context(), "default", "staged-delete", "stable") + require.NoError(t, err) + require.Equal(t, "stable", row.Metadata.Tag) +} + func TestApplyObject_ReusesProductionApplyPath(t *testing.T) { pool := v1alpha1store.NewTestPool(t) agents := v1alpha1store.NewStore(pool, "v1alpha1.agents") diff --git a/pkg/registry/resource/core.go b/pkg/registry/resource/core.go index f550c9dd..5787f334 100644 --- a/pkg/registry/resource/core.go +++ b/pkg/registry/resource/core.go @@ -211,11 +211,15 @@ type deleteOpts struct { Authorize func(ctx context.Context, in AuthorizeInput) error PostDelete func(ctx context.Context, obj v1alpha1.Object) error PreDeleteObject v1alpha1.Object + DeleteAdmission types.DeleteAdmission + Source string + Force bool } -// deleteCore runs Authorize → Store.Delete → PostDelete for a single resource. -// Validation is intentionally skipped — deleting a row should not require its -// spec to validate. +// deleteCore runs Authorize → delete admission for a single resource. +// Validation is intentionally skipped — deleting a row should not require +// its spec to validate. The OSS default admission performs Store.DeleteByRef +// + PostDelete; downstream implementations may stage or reject the delete. // // Returns NotFound=true on the missing-row case so callers can map it // to 404 (single PUT) or "not found" Result (batch). @@ -224,29 +228,70 @@ func deleteCore( store *v1alpha1store.Store, kind, namespace, name, tag string, opts deleteOpts, -) *applyError { + dryRun bool, +) (types.DeleteAdmissionResult, *applyError) { if opts.Authorize != nil { if err := opts.Authorize(ctx, AuthorizeInput{ Verb: "delete", Kind: kind, Namespace: namespace, Name: name, Tag: tag, Object: opts.PreDeleteObject, }); err != nil { - return &applyError{Stage: stageAuth, Err: err} + return types.DeleteAdmissionResult{}, &applyError{Stage: stageAuth, Err: err} + } + } + + source := opts.Source + if source == "" { + source = types.AdmissionSourceDelete + } + admission := opts.DeleteAdmission + if admission == nil { + admission = ProductionDeleteAdmission + } + result, err := admission(ctx, types.DeleteAdmissionInput{ + Source: source, + Verb: "delete", + DryRun: dryRun, + Kind: kind, + Namespace: namespace, + Name: name, + Tag: tag, + Object: opts.PreDeleteObject, + Store: store, + PostDelete: opts.PostDelete, + Force: opts.Force, + }) + if err != nil { + if ae, ok := err.(*applyError); ok { + return types.DeleteAdmissionResult{}, ae } + return types.DeleteAdmissionResult{}, &applyError{Stage: stageAdmission, Err: err} } + return result, nil +} - if err := store.Delete(ctx, namespace, name, tag); err != nil { - return &applyError{ +// ProductionDeleteAdmission is the OSS delete admission implementation. It +// removes the selected production row(s) and then runs the per-kind post-delete +// hook when present. +func ProductionDeleteAdmission(ctx context.Context, in types.DeleteAdmissionInput) (types.DeleteAdmissionResult, error) { + if in.DryRun { + return types.DeleteAdmissionResult{Status: arv0.ApplyStatusDryRun, Tag: in.Tag}, nil + } + store, ok := in.Store.(*v1alpha1store.Store) + if !ok || store == nil { + return types.DeleteAdmissionResult{}, errors.New("production store is required") + } + if err := store.DeleteByRef(ctx, in.Namespace, in.Name, in.Tag); err != nil { + return types.DeleteAdmissionResult{}, &applyError{ Stage: stageDelete, Err: err, NotFound: errors.Is(err, pkgdb.ErrNotFound), } } - - if opts.PostDelete != nil && opts.PreDeleteObject != nil { - if err := opts.PostDelete(ctx, opts.PreDeleteObject); err != nil { - return &applyError{Stage: stagePostDelete, Err: err} + if in.PostDelete != nil && in.Object != nil { + if err := in.PostDelete(ctx, in.Object); err != nil { + return types.DeleteAdmissionResult{}, &applyError{Stage: stagePostDelete, Err: err} } } - return nil + return types.DeleteAdmissionResult{Status: arv0.ApplyStatusDeleted, Tag: in.Tag}, nil } diff --git a/pkg/registry/resource/handler.go b/pkg/registry/resource/handler.go index efe8bbdc..eb0f1c30 100644 --- a/pkg/registry/resource/handler.go +++ b/pkg/registry/resource/handler.go @@ -31,6 +31,7 @@ import ( "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" pkgdb "github.com/agentregistry-dev/agentregistry/pkg/registry/database" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" + "github.com/agentregistry-dev/agentregistry/pkg/types" ) // unescapePath URL-decodes a path segment captured by Huma. Resource @@ -112,6 +113,11 @@ type Config struct { // and writes the terminal Removed condition. PostDelete func(ctx context.Context, obj v1alpha1.Object) error + // DeleteAdmission optionally owns the final delete after authz. Nil uses + // ProductionDeleteAdmission, which deletes from the configured Store and + // runs PostDelete. + DeleteAdmission types.DeleteAdmission + // InitialFinalizers, when non-nil, seeds finalizers atomically on create. // Updates preserve existing finalizers. InitialFinalizers func(obj v1alpha1.Object) []string @@ -573,9 +579,13 @@ func runDeleteLatest[T v1alpha1.Object](ctx context.Context, cfg Config, newObj dopts := deleteOpts{Authorize: cfg.Authorize} if cfg.PostDelete != nil && !force { dopts.PostDelete = cfg.PostDelete + } + if cfg.DeleteAdmission != nil || dopts.PostDelete != nil { dopts.PreDeleteObject = obj } - if ae := deleteCore(ctx, cfg.Store, kind, ns, name, "", dopts); ae != nil { + dopts.DeleteAdmission = cfg.DeleteAdmission + dopts.Force = force + if _, ae := deleteCore(ctx, cfg.Store, kind, ns, name, "", dopts, false); ae != nil { return nil, mapApplyErrorToHuma(ae, kind, ns, name, "") } return &deleteOutput{}, nil @@ -614,7 +624,9 @@ func runDelete[T v1alpha1.Object](ctx context.Context, cfg Config, newObj func() if runHook { dopts.PostDelete = cfg.PostDelete } - if ae := deleteCore(ctx, cfg.Store, kind, ns, name, tag, dopts); ae != nil { + dopts.DeleteAdmission = cfg.DeleteAdmission + dopts.Force = force + if _, ae := deleteCore(ctx, cfg.Store, kind, ns, name, tag, dopts, false); ae != nil { return nil, mapApplyErrorToHuma(ae, kind, ns, name, tag) } return &deleteOutput{}, nil diff --git a/pkg/registry/resource/handler_test.go b/pkg/registry/resource/handler_test.go index b7285b62..c60e0183 100644 --- a/pkg/registry/resource/handler_test.go +++ b/pkg/registry/resource/handler_test.go @@ -20,6 +20,7 @@ import ( "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" "github.com/agentregistry-dev/agentregistry/pkg/registry/resource" "github.com/agentregistry-dev/agentregistry/pkg/registry/v1alpha1store" + "github.com/agentregistry-dev/agentregistry/pkg/types" ) // registerAgent wires the generic resource handler for *v1alpha1.Agent and @@ -224,6 +225,50 @@ func TestResourceRegister_DeleteTaggedPassesTagToAuthorizer(t *testing.T) { require.Equal(t, "stable", seen.Tag) } +func TestResourceRegister_DeleteAdmissionCanStageTaggedDelete(t *testing.T) { + pool := v1alpha1store.NewTestPool(t) + store := v1alpha1store.NewStore(pool, "v1alpha1.agents") + _, err := store.Upsert(t.Context(), &v1alpha1.Agent{ + Metadata: v1alpha1.ObjectMeta{Namespace: "default", Name: "alice", Tag: "stable"}, + Spec: v1alpha1.AgentSpec{Title: "Stable Alice"}, + }) + require.NoError(t, err) + + var admitted types.DeleteAdmissionInput + postDeleteCalled := false + _, api := humatest.New(t) + resource.Register[*v1alpha1.Agent](api, resource.Config{ + Kind: v1alpha1.KindAgent, + BasePrefix: "/v0", + Store: store, + PostDelete: func(context.Context, v1alpha1.Object) error { + postDeleteCalled = true + return nil + }, + DeleteAdmission: func(ctx context.Context, in types.DeleteAdmissionInput) (types.DeleteAdmissionResult, error) { + admitted = in + return types.DeleteAdmissionResult{Status: arv0.ApplyStatusStaged, Tag: in.Tag}, nil + }, + }, func() *v1alpha1.Agent { return &v1alpha1.Agent{} }) + + resp := api.Delete("/v0/agents/alice/stable") + require.Equal(t, http.StatusNoContent, resp.Code, resp.Body.String()) + require.False(t, postDeleteCalled, "staged deletes must not fire production side effects") + require.Equal(t, types.AdmissionSourceDelete, admitted.Source) + require.Equal(t, "delete", admitted.Verb) + require.Equal(t, v1alpha1.KindAgent, admitted.Kind) + require.Equal(t, "default", admitted.Namespace) + require.Equal(t, "alice", admitted.Name) + require.Equal(t, "stable", admitted.Tag) + require.NotNil(t, admitted.Object) + require.NotNil(t, admitted.PostDelete) + require.Same(t, store, admitted.Store) + + row, err := store.Get(t.Context(), "default", "alice", "stable") + require.NoError(t, err) + require.Equal(t, "stable", row.Metadata.Tag) +} + func TestResourceRegister_AgentNamespaceIsolation(t *testing.T) { pool := v1alpha1store.NewTestPool(t) store := v1alpha1store.NewStore(pool, "v1alpha1.agents") diff --git a/pkg/types/types.go b/pkg/types/types.go index 475f2272..1525afc6 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -69,6 +69,7 @@ type PostDelete func(ctx context.Context, obj v1alpha1.Object) error const ( AdmissionSourceApply = "apply" + AdmissionSourceDelete = "delete" AdmissionSourceImport = "import" ) @@ -102,6 +103,33 @@ type AdmissionResult struct { Generation int64 } +// DeleteAdmission owns the final delete decision after authz has passed. The +// OSS default deletes from production; downstream integrations can stage, +// reject, or otherwise route the delete before production storage is touched. +// +// TODO(krt): temporary synchronous-handler bridge; remove with reconciler +// admission/staging. +type DeleteAdmission func(ctx context.Context, in DeleteAdmissionInput) (DeleteAdmissionResult, error) + +type DeleteAdmissionInput struct { + Source string + Verb string + DryRun bool + Kind string + Namespace string + Name string + Tag string + Object v1alpha1.Object + Store any + PostDelete PostDelete + Force bool +} + +type DeleteAdmissionResult struct { + Status string + Tag string +} + // ResourceRouteContext exposes the finalized v1alpha1 route wiring to // downstream integrations that need adjacent routes against the same stores // and hooks as /v0/apply. @@ -114,6 +142,7 @@ type ResourceRouteContext struct { Resolver v1alpha1.ResolverFunc RegistryValidator v1alpha1.RegistryValidatorFunc Apply func(ctx context.Context, obj v1alpha1.Object, dryRun bool) v0.ApplyResult + Delete func(ctx context.Context, obj v1alpha1.Object, dryRun bool) v0.ApplyResult } // Auditor receives audit events for state changes that the OSS layer @@ -207,6 +236,12 @@ type AppOptions struct { // admission/staging. Admission Admission + // DeleteAdmission optionally accepts an authorized delete before the row is + // removed from production storage. Nil preserves normal direct deletes. + // TODO(krt): temporary synchronous-handler bridge; remove with reconciler + // admission/staging. + DeleteAdmission DeleteAdmission + // ResolverWrapper decorates the shared ResourceRef resolver before route // registration. Nil preserves the default store-backed resolver. // TODO(krt): temporary bridge for pending staged refs in HTTP apply. From b1d2aa8a290a3279137eec555441cf64b0f92611 Mon Sep 17 00:00:00 2001 From: Scott Weiss Date: Tue, 19 May 2026 16:24:53 -0400 Subject: [PATCH 10/10] Keep type imports verify-clean 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 --- pkg/types/types.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/types/types.go b/pkg/types/types.go index c0afd1a6..7199d50d 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -15,11 +15,12 @@ import ( "context" "net/http" + "github.com/danielgtaylor/huma/v2" + v0 "github.com/agentregistry-dev/agentregistry/pkg/api/v0" "github.com/agentregistry-dev/agentregistry/pkg/api/v1alpha1" "github.com/agentregistry-dev/agentregistry/pkg/registry/auth" "github.com/agentregistry-dev/agentregistry/pkg/registry/database" - "github.com/danielgtaylor/huma/v2" ) // DatabaseFactory is a function type that creates a store implementation.