From 40e7b14adb9747c2d460a89bfb8e7981b2e4379d Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Fri, 2 Oct 2026 08:53:09 +1000 Subject: [PATCH] refactor: compose ecr lifecycle preview actions --- .../ecr/awsapi/lifecycle_preview_actions.go | 64 +++++++++++++++++ spinifex/gateway/ecr_image_test.go | 16 +++-- spinifex/gateway/ecr_lifecyclepreview.go | 68 ------------------- spinifex/gateway/ecr_lifecyclepreview_test.go | 12 ++++ spinifex/gateway/ecrapi.go | 22 ++++-- .../gateway/ecrapi_authz_completeness_test.go | 5 ++ spinifex/gateway/gateway.go | 4 ++ spinifex/gateway/operations.go | 2 +- spinifex/services/awsgw/awsgw.go | 4 +- 9 files changed, 114 insertions(+), 83 deletions(-) create mode 100644 spinifex/domains/ecr/awsapi/lifecycle_preview_actions.go delete mode 100644 spinifex/gateway/ecr_lifecyclepreview.go diff --git a/spinifex/domains/ecr/awsapi/lifecycle_preview_actions.go b/spinifex/domains/ecr/awsapi/lifecycle_preview_actions.go new file mode 100644 index 000000000..0d9c2e486 --- /dev/null +++ b/spinifex/domains/ecr/awsapi/lifecycle_preview_actions.go @@ -0,0 +1,64 @@ +package awsapi + +import ( + "context" + "errors" + + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" +) + +// LifecyclePreviewActionService composes the policy-store and image-catalog +// capabilities used by the ECR lifecycle-preview actions. It deliberately does +// not share RegistryActionService: lifecycle previews have a different storage +// dependency and should not make the OCI registry a broad ECR control-plane +// service. +type LifecyclePreviewActionService struct { + policies LifecyclePolicyStore + catalog ImageCatalog +} + +// NewLifecyclePreviewActionService binds the policy and image capabilities +// consumed by the synchronous lifecycle-preview action pair. +func NewLifecyclePreviewActionService(policies LifecyclePolicyStore, catalog ImageCatalog) *LifecyclePreviewActionService { + return &LifecyclePreviewActionService{policies: policies, catalog: catalog} +} + +var lifecyclePreviewActionNames = []string{ + "GetLifecyclePolicyPreview", + "StartLifecyclePolicyPreview", +} + +// LifecyclePreviewActionNames returns the lifecycle-preview actions served by +// LifecyclePreviewActionService. The copy keeps the coverage inventory from +// changing the service's dispatch set. +func LifecyclePreviewActionNames() []string { + return append([]string(nil), lifecyclePreviewActionNames...) +} + +// IsLifecyclePreviewAction reports whether action is handled by the composed +// lifecycle-preview capability. +func IsLifecyclePreviewAction(action string) bool { + for _, name := range lifecyclePreviewActionNames { + if action == name { + return true + } + } + return false +} + +// Execute handles a lifecycle-preview ECR JSON action using the caller account +// established by gateway authentication and policy enforcement. +func (s *LifecyclePreviewActionService) Execute(ctx context.Context, action, accountID string, body []byte) (any, error) { + if s == nil { + return nil, errors.New(awserrors.ErrorServerInternal) + } + + switch action { + case "StartLifecyclePolicyPreview": + return StartLifecyclePolicyPreview(ctx, s.policies, s.catalog, accountID, body) + case "GetLifecyclePolicyPreview": + return GetLifecyclePolicyPreview(ctx, s.policies, s.catalog, accountID, body) + default: + return nil, errors.New(awserrors.ErrorInvalidAction) + } +} diff --git a/spinifex/gateway/ecr_image_test.go b/spinifex/gateway/ecr_image_test.go index 17d5ba4e8..2745f4471 100644 --- a/spinifex/gateway/ecr_image_test.go +++ b/spinifex/gateway/ecr_image_test.go @@ -23,14 +23,16 @@ import ( // object store + memory meta) and its composed ECR JSON image-action service. func newImageGateway(t *testing.T) *GatewayConfig { t.Helper() - reg := ecrregistry.NewRegistry(objectstore.NewMemoryObjectStore(), handlers_ecr.NewMemoryMetaStore(), ecrTestAccount) + meta := handlers_ecr.NewMemoryMetaStore() + reg := ecrregistry.NewRegistry(objectstore.NewMemoryObjectStore(), meta, ecrTestAccount) return &GatewayConfig{ - ECRRegistry: reg, - ECRRegistryActions: awsapi.NewRegistryActionService(reg, reg, reg, reg), - Region: ecrTestRegion, - InternalSuffix: ecrTestSuffix, - DisableLogging: true, - IAMService: allowAllIAMService(), + ECRRegistry: reg, + ECRRegistryActions: awsapi.NewRegistryActionService(reg, reg, reg, reg), + ECRLifecyclePreview: awsapi.NewLifecyclePreviewActionService(meta, reg), + Region: ecrTestRegion, + InternalSuffix: ecrTestSuffix, + DisableLogging: true, + IAMService: allowAllIAMService(), } } diff --git a/spinifex/gateway/ecr_lifecyclepreview.go b/spinifex/gateway/ecr_lifecyclepreview.go deleted file mode 100644 index 04a3f884a..000000000 --- a/spinifex/gateway/ecr_lifecyclepreview.go +++ /dev/null @@ -1,68 +0,0 @@ -package gateway - -import ( - "errors" - "io" - "log/slog" - "net/http" - - handlers_ecr "github.com/mulgadc/spinifex/spinifex/domains/ecr" - awsapi "github.com/mulgadc/spinifex/spinifex/domains/ecr/awsapi" - "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" -) - -// ecrLifecyclePreviewAccount obtains the authenticated account and the -// registry capability still required by the transitional lifecycle-preview -// adapters. Registry-backed image actions use RegistryActionService instead. -func (gw *GatewayConfig) ecrLifecyclePreviewAccount(r *http.Request) (string, error) { - accountID, _ := r.Context().Value(ctxAccountID).(string) - if accountID == "" { - slog.ErrorContext(r.Context(), "ECR lifecycle preview: no account ID in auth context") - return "", errors.New(awserrors.ErrorServerInternal) - } - if gw.ECRRegistry == nil { - slog.ErrorContext(r.Context(), "ECR lifecycle preview: OCI registry not configured") - return "", errors.New(awserrors.ErrorServerInternal) - } - return accountID, nil -} - -// handleStartLifecyclePolicyPreview adapts the authenticated HTTP request to -// the ECR action. Lifecycle evaluation and AWS response semantics live in the -// ECR adapter; gateway supplies the policy store and image catalog. -func (gw *GatewayConfig) handleStartLifecyclePolicyPreview(w http.ResponseWriter, r *http.Request) error { - accountID, err := gw.ecrLifecyclePreviewAccount(r) - if err != nil { - return err - } - body, err := io.ReadAll(r.Body) - if err != nil { - return awsapi.MalformedBodyError() - } - output, err := awsapi.StartLifecyclePolicyPreview(r.Context(), handlers_ecr.NewNATSMetaStore(gw.NATSConn), gw.ECRRegistry, accountID, body) - if err != nil { - return err - } - awsapi.WriteJSONResponse(w, output) - return nil -} - -// handleGetLifecyclePolicyPreview adapts the authenticated HTTP request to the -// ECR action. Lifecycle evaluation and AWS response projection live in the -// ECR adapter; gateway supplies the policy store and image catalog. -func (gw *GatewayConfig) handleGetLifecyclePolicyPreview(w http.ResponseWriter, r *http.Request) error { - accountID, err := gw.ecrLifecyclePreviewAccount(r) - if err != nil { - return err - } - body, err := io.ReadAll(r.Body) - if err != nil { - return awsapi.MalformedBodyError() - } - output, err := awsapi.GetLifecyclePolicyPreview(r.Context(), handlers_ecr.NewNATSMetaStore(gw.NATSConn), gw.ECRRegistry, accountID, body) - if err != nil { - return err - } - awsapi.WriteJSONResponse(w, output) - return nil -} diff --git a/spinifex/gateway/ecr_lifecyclepreview_test.go b/spinifex/gateway/ecr_lifecyclepreview_test.go index 17423fc9c..6202a2aec 100644 --- a/spinifex/gateway/ecr_lifecyclepreview_test.go +++ b/spinifex/gateway/ecr_lifecyclepreview_test.go @@ -89,6 +89,18 @@ func TestLifecyclePreview_Errors(t *testing.T) { assert.Equal(t, "AccessDenied", err.Error()) } +func TestLifecyclePreview_MissingComposition(t *testing.T) { + gw := &GatewayConfig{ + Region: ecrTestRegion, + DisableLogging: true, + IAMService: allowAllIAMService(), + } + + _, err := callImage(t, gw, "StartLifecyclePolicyPreview", previewBody("team/app", previewExpireOldest)) + require.Error(t, err) + assert.Equal(t, "ServerInternal", err.Error()) +} + func strconvQuotePreview(s string) string { b, _ := json.Marshal(s) return string(b) diff --git a/spinifex/gateway/ecrapi.go b/spinifex/gateway/ecrapi.go index 93425c8aa..f51265d21 100644 --- a/spinifex/gateway/ecrapi.go +++ b/spinifex/gateway/ecrapi.go @@ -19,12 +19,10 @@ type ecrInlineHandler func(*GatewayConfig, http.ResponseWriter, *http.Request) e // Keeping dispatch and coverage on the same map prevents the generated // operation report from classifying an inline implementation as a stub. var ecrInlineActions = map[string]ecrInlineHandler{ - "GetAuthorizationToken": (*GatewayConfig).handleGetAuthorizationToken, - "DescribeRepositories": (*GatewayConfig).handleDescribeRepositories, - "CreateRepository": (*GatewayConfig).handleCreateRepository, - "DeleteRepository": (*GatewayConfig).handleDeleteRepository, - "StartLifecyclePolicyPreview": (*GatewayConfig).handleStartLifecyclePolicyPreview, - "GetLifecyclePolicyPreview": (*GatewayConfig).handleGetLifecyclePolicyPreview, + "GetAuthorizationToken": (*GatewayConfig).handleGetAuthorizationToken, + "DescribeRepositories": (*GatewayConfig).handleDescribeRepositories, + "CreateRepository": (*GatewayConfig).handleCreateRepository, + "DeleteRepository": (*GatewayConfig).handleDeleteRepository, } // ecrActionFromTarget extracts the action suffix from an X-Amz-Target header. @@ -87,6 +85,18 @@ func (gw *GatewayConfig) ECR_Request(w http.ResponseWriter, r *http.Request) err awsapi.WriteJSONResponse(w, output) return nil } + if awsapi.IsLifecyclePreviewAction(action) { + if gw.ECRLifecyclePreview == nil { + slog.Error("ECR lifecycle-preview action: capabilities not configured", "action", action) + return errors.New(awserrors.ErrorServerInternal) + } + output, err := gw.ECRLifecyclePreview.Execute(r.Context(), action, accountID, body) + if err != nil { + return err + } + awsapi.WriteJSONResponse(w, output) + return nil + } if inline, ok := ecrInlineActions[action]; ok { // The inline handlers read r.Body themselves, so it is rewound over the diff --git a/spinifex/gateway/ecrapi_authz_completeness_test.go b/spinifex/gateway/ecrapi_authz_completeness_test.go index f7190a598..813a4b770 100644 --- a/spinifex/gateway/ecrapi_authz_completeness_test.go +++ b/spinifex/gateway/ecrapi_authz_completeness_test.go @@ -41,4 +41,9 @@ func TestECRComposedActionsAreInTheNamespace(t *testing.T) { assert.True(t, ok, "composed ECR registry action %q is not in awsapi.Actions, so ECR_Request rejects it as InvalidAction", action) } + for _, action := range awsapi.LifecyclePreviewActionNames() { + _, ok := awsapi.Actions[action] + assert.True(t, ok, + "composed ECR lifecycle-preview action %q is not in awsapi.Actions, so ECR_Request rejects it as InvalidAction", action) + } } diff --git a/spinifex/gateway/gateway.go b/spinifex/gateway/gateway.go index 7bb30852e..5eed1d5fb 100644 --- a/spinifex/gateway/gateway.go +++ b/spinifex/gateway/gateway.go @@ -164,6 +164,10 @@ type GatewayConfig struct { // composed from the OCI registry at startup; nil is a composition fault for // those actions rather than a fallback to the generic NotImplemented table. ECRRegistryActions *awsapi.RegistryActionService + // ECRLifecyclePreview serves the lifecycle-preview pair. Its distinct + // policy-store and image-catalog composition prevents the OCI registry from + // becoming a broad ECR control-plane service. + ECRLifecyclePreview *awsapi.LifecyclePreviewActionService // ECRTokenIssuer mints GetAuthorizationToken JWTs; ECRTokenVerifier validates // them on /v2/*. Both nil disables the auth bridge (registry mounts open, as // in unit tests of unrelated routes). diff --git a/spinifex/gateway/operations.go b/spinifex/gateway/operations.go index 0a125c966..6f1e68492 100644 --- a/spinifex/gateway/operations.go +++ b/spinifex/gateway/operations.go @@ -23,7 +23,7 @@ type ServiceOperationInventory struct { // dispatch S3 operations and delegates that REST surface to Predastore. func AWSOperationInventory() map[string]ServiceOperationInventory { ecrInline := mapKeys(ecrInlineActions) - ecrComposed := union(ecrInline, awsapi.RegistryActionNames()) + ecrComposed := union(union(ecrInline, awsapi.RegistryActionNames()), awsapi.LifecyclePreviewActionNames()) ecrRegistered := union(mapKeys(awsapi.Actions), ecrComposed) ecrStubbed := without(awsapi.StubbedActionNames(), ecrComposed) return map[string]ServiceOperationInventory{ diff --git a/spinifex/services/awsgw/awsgw.go b/spinifex/services/awsgw/awsgw.go index 1a30d1959..2d2ca4be3 100644 --- a/spinifex/services/awsgw/awsgw.go +++ b/spinifex/services/awsgw/awsgw.go @@ -271,7 +271,8 @@ func launchService(config *config.ClusterConfig) error { nodeConfig.Predastore.AccessKey, nodeConfig.Predastore.SecretKey, ) - ecrRegistry := ecrregistry.NewRegistry(objStore, ecr.NewNATSMetaStore(natsConn), config.Bootstrap.AccountID) + ecrMeta := ecr.NewNATSMetaStore(natsConn) + ecrRegistry := ecrregistry.NewRegistry(objStore, ecrMeta, config.Bootstrap.AccountID) // Lifecycle expiry sweep applies each repo's stored lifecycle policy and // deletes the expired set via the registry GC path. It runs here (not the @@ -454,6 +455,7 @@ func launchService(config *config.ClusterConfig) error { Commit: commit, ECRRegistry: ecrRegistry, ECRRegistryActions: awsapi.NewRegistryActionService(ecrRegistry, ecrRegistry, ecrRegistry, ecrRegistry), + ECRLifecyclePreview: awsapi.NewLifecyclePreviewActionService(ecrMeta, ecrRegistry), ECRTokenIssuer: ecrauth.NewIssuer(signingKey, ecrAudience), ECRTokenVerifier: ecrauth.NewVerifier(verifyKeys, ecrAudience), BedrockCredentials: bedrockCredentials,