From 7a5159366d25a45f42441c83aef0ed4a1ea30e9c Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Thu, 1 Oct 2026 15:35:39 +1000 Subject: [PATCH] refactor: move ecr tag mutability action to domain --- spinifex/domains/ecr/awsapi/handler.go | 2 +- spinifex/domains/ecr/awsapi/tag_mutability.go | 67 +++++++++++++++ .../domains/ecr/awsapi/tag_mutability_test.go | 65 +++++++++++++++ spinifex/gateway/ecr_tagmutability.go | 82 ------------------- spinifex/gateway/ecr_tagmutability_test.go | 18 ++-- spinifex/gateway/ecr_test.go | 4 +- spinifex/gateway/ecrapi.go | 1 - 7 files changed, 140 insertions(+), 99 deletions(-) create mode 100644 spinifex/domains/ecr/awsapi/tag_mutability.go create mode 100644 spinifex/domains/ecr/awsapi/tag_mutability_test.go delete mode 100644 spinifex/gateway/ecr_tagmutability.go diff --git a/spinifex/domains/ecr/awsapi/handler.go b/spinifex/domains/ecr/awsapi/handler.go index f36277fee..250eaa227 100644 --- a/spinifex/domains/ecr/awsapi/handler.go +++ b/spinifex/domains/ecr/awsapi/handler.go @@ -59,7 +59,7 @@ var Actions = map[string]Handler{ "DeleteRepository": NotImplemented, "DescribeRepositories": NotImplemented, "ListRepositories": NotImplemented, - "PutImageTagMutability": NotImplemented, + "PutImageTagMutability": PutImageTagMutability, // Images / layers. "BatchGetImage": NotImplemented, diff --git a/spinifex/domains/ecr/awsapi/tag_mutability.go b/spinifex/domains/ecr/awsapi/tag_mutability.go new file mode 100644 index 000000000..c11114499 --- /dev/null +++ b/spinifex/domains/ecr/awsapi/tag_mutability.go @@ -0,0 +1,67 @@ +package awsapi + +import ( + "context" + "encoding/json" + "errors" + "log/slog" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ecr" + handlers_ecr "github.com/mulgadc/spinifex/spinifex/domains/ecr" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + "github.com/nats-io/nats.go" +) + +// putImageTagMutabilityRequest is the camelCase AWS JSON 1.1 input shape. +type putImageTagMutabilityRequest struct { + RepositoryName string `json:"repositoryName"` + RegistryID string `json:"registryId"` + ImageTagMutability string `json:"imageTagMutability"` +} + +// PutImageTagMutability changes a repository between MUTABLE and IMMUTABLE. +// Unlike CreateRepository, the mutability field is required. The persisted +// value is enforced by the ECR registry when it stores a manifest. +func PutImageTagMutability(ctx context.Context, nc *nats.Conn, accountID string, body []byte) (any, error) { + var req putImageTagMutabilityRequest + if err := json.Unmarshal(body, &req); err != nil { + return nil, MalformedBodyError() + } + if err := ValidateRepositoryName(req.RepositoryName); err != nil { + return nil, err + } + if req.RegistryID != "" && req.RegistryID != accountID { + return nil, errors.New(awserrors.ErrorAccessDenied) + } + switch req.ImageTagMutability { + case handlers_ecr.TagMutabilityMutable, handlers_ecr.TagMutabilityImmutable: + case "": + return nil, RequiredParameterError("imageTagMutability") + default: + return nil, EnumValueError("imageTagMutability", req.ImageTagMutability, + handlers_ecr.TagMutabilityMutable, handlers_ecr.TagMutabilityImmutable) + } + + store := handlers_ecr.NewNATSMetaStore(nc) + meta, err := store.GetRepo(ctx, accountID, req.RepositoryName) + if err != nil { + if errors.Is(err, handlers_ecr.ErrNotFound) { + return nil, errors.New(awserrors.ErrorRepositoryNotFound) + } + slog.ErrorContext(ctx, "ECR PutImageTagMutability: get repository failed", "repository", req.RepositoryName, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + meta.ImageTagMutability = req.ImageTagMutability + if err := store.PutRepo(ctx, accountID, meta); err != nil { + slog.ErrorContext(ctx, "ECR PutImageTagMutability: put repository failed", "repository", req.RepositoryName, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + return &ecr.PutImageTagMutabilityOutput{ + RegistryId: aws.String(accountID), + RepositoryName: aws.String(req.RepositoryName), + ImageTagMutability: aws.String(meta.TagMutability()), + }, nil +} diff --git a/spinifex/domains/ecr/awsapi/tag_mutability_test.go b/spinifex/domains/ecr/awsapi/tag_mutability_test.go new file mode 100644 index 000000000..0f020f775 --- /dev/null +++ b/spinifex/domains/ecr/awsapi/tag_mutability_test.go @@ -0,0 +1,65 @@ +package awsapi + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go/service/ecr" + handlers_ecr "github.com/mulgadc/spinifex/spinifex/domains/ecr" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPutImageTagMutability_UpdatesRepository(t *testing.T) { + nc := newRepositoryActionTestConn(t) + seedRepositoryForAction(t, nc, "team/app") + + out, err := PutImageTagMutability(context.Background(), nc, repositoryActionTestAccount, + []byte(`{"repositoryName":"team/app","imageTagMutability":"IMMUTABLE"}`)) + require.NoError(t, err) + response, ok := out.(*ecr.PutImageTagMutabilityOutput) + require.True(t, ok) + assert.Equal(t, "IMMUTABLE", *response.ImageTagMutability) + + meta, err := handlers_ecr.NewNATSMetaStore(nc).GetRepo(context.Background(), repositoryActionTestAccount, "team/app") + require.NoError(t, err) + assert.Equal(t, handlers_ecr.TagMutabilityImmutable, meta.ImageTagMutability) +} + +func TestPutImageTagMutability_ValidatesRequest(t *testing.T) { + nc := newRepositoryActionTestConn(t) + seedRepositoryForAction(t, nc, "team/app") + + cases := []struct { + name string + body string + code string + }{ + {"malformed", `{`, awserrors.ErrorInvalidParameterValue}, + {"missing value", `{"repositoryName":"team/app"}`, awserrors.ErrorInvalidParameterValue}, + {"invalid value", `{"repositoryName":"team/app","imageTagMutability":"NOPE"}`, awserrors.ErrorInvalidParameterValue}, + {"cross account", `{"repositoryName":"team/app","registryId":"999999999999","imageTagMutability":"IMMUTABLE"}`, awserrors.ErrorAccessDenied}, + {"missing repository", `{"repositoryName":"team/missing","imageTagMutability":"IMMUTABLE"}`, awserrors.ErrorRepositoryNotFound}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := PutImageTagMutability(context.Background(), nc, repositoryActionTestAccount, []byte(tc.body)) + require.Error(t, err) + assert.Equal(t, tc.code, awserrors.ValidErrorCodeFromError(err)) + }) + } +} + +func TestActions_PutImageTagMutabilityDispatched(t *testing.T) { + handler, ok := Actions["PutImageTagMutability"] + require.True(t, ok) + nc := newRepositoryActionTestConn(t) + seedRepositoryForAction(t, nc, "team/app") + + out, err := handler(context.Background(), nc, repositoryActionTestAccount, + []byte(`{"repositoryName":"team/app","imageTagMutability":"IMMUTABLE"}`)) + require.NoError(t, err) + _, ok = out.(*ecr.PutImageTagMutabilityOutput) + assert.True(t, ok) +} diff --git a/spinifex/gateway/ecr_tagmutability.go b/spinifex/gateway/ecr_tagmutability.go deleted file mode 100644 index 85523b15d..000000000 --- a/spinifex/gateway/ecr_tagmutability.go +++ /dev/null @@ -1,82 +0,0 @@ -package gateway - -import ( - "encoding/json" - "errors" - "io" - "log/slog" - "net/http" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ecr" - 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" -) - -// putImageTagMutabilityRequest is the camelCase AWS JSON 1.1 input shape. -type putImageTagMutabilityRequest struct { - RepositoryName string `json:"repositoryName"` - RegistryID string `json:"registryId"` - ImageTagMutability string `json:"imageTagMutability"` -} - -// handlePutImageTagMutability flips a repository between MUTABLE and IMMUTABLE. -// It read-modify-writes the per-account RepoMeta record; the value is enforced -// on push by Registry.StoreManifest. -func (gw *GatewayConfig) handlePutImageTagMutability(w http.ResponseWriter, r *http.Request) error { - ctx := r.Context() - accountID, _ := ctx.Value(ctxAccountID).(string) - if accountID == "" { - slog.ErrorContext(ctx, "PutImageTagMutability: no account ID in auth context") - return errors.New(awserrors.ErrorServerInternal) - } - - body, err := io.ReadAll(r.Body) - if err != nil { - slog.ErrorContext(ctx, "PutImageTagMutability: failed to read body", "err", err) - return awsapi.MalformedBodyError() - } - var req putImageTagMutabilityRequest - if err := json.Unmarshal(body, &req); err != nil { - return awsapi.MalformedBodyError() - } - if err := awsapi.ValidateRepositoryName(req.RepositoryName); err != nil { - return err - } - if req.RegistryID != "" && req.RegistryID != accountID { - return errors.New(awserrors.ErrorAccessDenied) - } - // imageTagMutability is required here (unlike CreateRepository's default). - switch req.ImageTagMutability { - case handlers_ecr.TagMutabilityMutable, handlers_ecr.TagMutabilityImmutable: - case "": - return awsapi.RequiredParameterError("imageTagMutability") - default: - return awsapi.EnumValueError("imageTagMutability", req.ImageTagMutability, - handlers_ecr.TagMutabilityMutable, handlers_ecr.TagMutabilityImmutable) - } - - store := handlers_ecr.NewNATSMetaStore(gw.NATSConn) - meta, err := store.GetRepo(ctx, accountID, req.RepositoryName) - if err != nil { - if errors.Is(err, handlers_ecr.ErrNotFound) { - return errors.New(awserrors.ErrorRepositoryNotFound) - } - slog.ErrorContext(ctx, "PutImageTagMutability: get repo failed", "repo", req.RepositoryName, "err", err) - return errors.New(awserrors.ErrorServerInternal) - } - - meta.ImageTagMutability = req.ImageTagMutability - if err := store.PutRepo(ctx, accountID, meta); err != nil { - slog.ErrorContext(ctx, "PutImageTagMutability: put repo failed", "repo", req.RepositoryName, "err", err) - return errors.New(awserrors.ErrorServerInternal) - } - - awsapi.WriteJSONResponse(w, &ecr.PutImageTagMutabilityOutput{ - RegistryId: aws.String(accountID), - RepositoryName: aws.String(req.RepositoryName), - ImageTagMutability: aws.String(meta.TagMutability()), - }) - return nil -} diff --git a/spinifex/gateway/ecr_tagmutability_test.go b/spinifex/gateway/ecr_tagmutability_test.go index d369c9ecf..fdd7c8b58 100644 --- a/spinifex/gateway/ecr_tagmutability_test.go +++ b/spinifex/gateway/ecr_tagmutability_test.go @@ -13,7 +13,11 @@ import ( ) func putTagMutability(t *testing.T, gw *GatewayConfig, body string) (*httptest.ResponseRecorder, error) { - return ecrLifecycleRequest(t, gw, (*GatewayConfig).handlePutImageTagMutability, body) + t.Helper() + req := setupECRRequest("AmazonEC2ContainerRegistry_V20150921.PutImageTagMutability", body) + ctx := context.WithValue(req.Context(), ctxAccountID, ecrTestAccount) + w := httptest.NewRecorder() + return w, gw.ECR_Request(w, req.WithContext(ctx)) } type tagMutabilityOut struct { @@ -97,18 +101,6 @@ func TestPutImageTagMutability_Errors(t *testing.T) { } } -func TestPutImageTagMutability_NoAccountAndMalformed(t *testing.T) { - gw, _ := newRepoLifecycleGateway(t) - - err := gw.handlePutImageTagMutability(httptest.NewRecorder(), noAccountRequest(`{"repositoryName":"team/app","imageTagMutability":"IMMUTABLE"}`)) - require.Error(t, err) - assert.Equal(t, "ServerInternal", err.Error()) - - _, err = putTagMutability(t, gw, `{`) - require.Error(t, err) - assert.Equal(t, "InvalidParameterValue", awserrors.ValidErrorCodeFromError(err)) -} - func TestECRRequest_PutImageTagMutabilityDispatched(t *testing.T) { gw, _ := newRepoLifecycleGateway(t) _, err := createRepo(t, gw, `{"repositoryName":"team/app"}`) diff --git a/spinifex/gateway/ecr_test.go b/spinifex/gateway/ecr_test.go index 6c17aaafb..b0ae74d48 100644 --- a/spinifex/gateway/ecr_test.go +++ b/spinifex/gateway/ecr_test.go @@ -68,8 +68,8 @@ func TestECRRequest_UnknownAction(t *testing.T) { } // A registered-but-unimplemented action resolves to the 501 stub until its -// handler lands. The repo/image actions are now served inline, so -// ListRepositories stands in as a still-stubbed action. +// handler lands. Repository and image actions may be direct domain handlers or +// gateway adapters, so ListRepositories stands in as a still-stubbed action. func TestECRRequest_KnownActionNotImplemented(t *testing.T) { gw := &GatewayConfig{DisableLogging: true, IAMService: allowAllIAMService()} err := gw.ECR_Request(httptest.NewRecorder(), diff --git a/spinifex/gateway/ecrapi.go b/spinifex/gateway/ecrapi.go index 6f51e409d..b819e3ada 100644 --- a/spinifex/gateway/ecrapi.go +++ b/spinifex/gateway/ecrapi.go @@ -23,7 +23,6 @@ var ecrInlineActions = map[string]ecrInlineHandler{ "DescribeRepositories": (*GatewayConfig).handleDescribeRepositories, "CreateRepository": (*GatewayConfig).handleCreateRepository, "DeleteRepository": (*GatewayConfig).handleDeleteRepository, - "PutImageTagMutability": (*GatewayConfig).handlePutImageTagMutability, "ListImages": (*GatewayConfig).handleListImages, "DescribeImages": (*GatewayConfig).handleDescribeImages, "BatchGetImage": (*GatewayConfig).handleBatchGetImage,