From a6b209b7b3627a016d9dfa55e1a12545d14e013d Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Thu, 1 Oct 2026 15:26:31 +1000 Subject: [PATCH] refactor: move ecr delete action to domain --- .../domains/ecr/awsapi/delete_repository.go | 71 +++++++++++++++++++ .../ecr/awsapi/delete_repository_test.go | 64 +++++++++++++++++ .../ecr/awsapi/repository_action_test.go | 3 + spinifex/gateway/ecr_deleterepository.go | 62 ++-------------- 4 files changed, 144 insertions(+), 56 deletions(-) create mode 100644 spinifex/domains/ecr/awsapi/delete_repository.go create mode 100644 spinifex/domains/ecr/awsapi/delete_repository_test.go diff --git a/spinifex/domains/ecr/awsapi/delete_repository.go b/spinifex/domains/ecr/awsapi/delete_repository.go new file mode 100644 index 000000000..c9b5c0585 --- /dev/null +++ b/spinifex/domains/ecr/awsapi/delete_repository.go @@ -0,0 +1,71 @@ +package awsapi + +import ( + "context" + "encoding/json" + "errors" + "log/slog" + + "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" +) + +// deleteRepositoryRequest is the camelCase AWS JSON 1.1 input shape. force +// permits deletion when a repository still contains images. +type deleteRepositoryRequest struct { + RepositoryName string `json:"repositoryName"` + RegistryID string `json:"registryId"` + Force bool `json:"force"` +} + +// DeleteRepository removes an account-scoped ECR repository. Without force, +// a repository with image manifests returns RepositoryNotEmptyException. The +// metadata service cascades repository records; object blob reclamation is +// intentionally deferred to the separate garbage-collection path. +func DeleteRepository(ctx context.Context, nc *nats.Conn, endpoint RepositoryEndpoint, accountID string, body []byte) (*ecr.DeleteRepositoryOutput, error) { + var req deleteRepositoryRequest + 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) + } + + 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 DeleteRepository: get repository failed", "repository", req.RepositoryName, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + if !req.Force { + manifests, err := store.ListManifests(ctx, accountID, req.RepositoryName) + if err != nil { + slog.ErrorContext(ctx, "ECR DeleteRepository: list manifests failed", "repository", req.RepositoryName, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + if len(manifests) > 0 { + return nil, errors.New(awserrors.ErrorRepositoryNotEmpty) + } + } + + if err := store.DeleteRepo(ctx, accountID, req.RepositoryName); err != nil { + if errors.Is(err, handlers_ecr.ErrNotFound) { + return nil, errors.New(awserrors.ErrorRepositoryNotFound) + } + slog.ErrorContext(ctx, "ECR DeleteRepository: delete repository failed", "repository", req.RepositoryName, "err", err) + return nil, errors.New(awserrors.ErrorServerInternal) + } + + return &ecr.DeleteRepositoryOutput{ + Repository: endpoint.RepositoryFromMeta(accountID, req.RepositoryName, meta), + }, nil +} diff --git a/spinifex/domains/ecr/awsapi/delete_repository_test.go b/spinifex/domains/ecr/awsapi/delete_repository_test.go new file mode 100644 index 000000000..8da4bc30e --- /dev/null +++ b/spinifex/domains/ecr/awsapi/delete_repository_test.go @@ -0,0 +1,64 @@ +package awsapi + +import ( + "context" + "strings" + "testing" + "time" + + 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 TestDeleteRepository_RemovesEmptyRepository(t *testing.T) { + nc := newRepositoryActionTestConn(t) + seedRepositoryForAction(t, nc, "team/app") + + out, err := DeleteRepository(context.Background(), nc, repositoryActionEndpoint(), repositoryActionTestAccount, []byte(`{"repositoryName":"team/app"}`)) + require.NoError(t, err) + require.NotNil(t, out.Repository) + assert.Equal(t, "team/app", *out.Repository.RepositoryName) + + _, err = handlers_ecr.NewNATSMetaStore(nc).GetRepo(context.Background(), repositoryActionTestAccount, "team/app") + require.ErrorIs(t, err, handlers_ecr.ErrNotFound) +} + +func TestDeleteRepository_ProtectsNonEmptyRepositoryUnlessForced(t *testing.T) { + nc := newRepositoryActionTestConn(t) + seedRepositoryForAction(t, nc, "team/app") + store := handlers_ecr.NewNATSMetaStore(nc) + require.NoError(t, store.PutManifestMeta(context.Background(), repositoryActionTestAccount, "team/app", handlers_ecr.ManifestMeta{ + Digest: "sha256:" + strings.Repeat("a", 64), MediaType: "application/json", Size: 7, PushedAt: time.Now(), + })) + + _, err := DeleteRepository(context.Background(), nc, repositoryActionEndpoint(), repositoryActionTestAccount, []byte(`{"repositoryName":"team/app"}`)) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorRepositoryNotEmpty, awserrors.ValidErrorCodeFromError(err)) + + out, err := DeleteRepository(context.Background(), nc, repositoryActionEndpoint(), repositoryActionTestAccount, []byte(`{"repositoryName":"team/app","force":true}`)) + require.NoError(t, err) + assert.Equal(t, "team/app", *out.Repository.RepositoryName) +} + +func TestDeleteRepository_ValidatesRequest(t *testing.T) { + nc := newRepositoryActionTestConn(t) + cases := []struct { + name string + body string + code string + }{ + {"malformed", `{`, awserrors.ErrorInvalidParameterValue}, + {"missing name", `{}`, awserrors.ErrorInvalidParameterValue}, + {"cross account", `{"repositoryName":"team/app","registryId":"999999999999"}`, awserrors.ErrorAccessDenied}, + {"missing repository", `{"repositoryName":"team/missing"}`, awserrors.ErrorRepositoryNotFound}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := DeleteRepository(context.Background(), nc, repositoryActionEndpoint(), repositoryActionTestAccount, []byte(tc.body)) + require.Error(t, err) + assert.Equal(t, tc.code, awserrors.ValidErrorCodeFromError(err)) + }) + } +} diff --git a/spinifex/domains/ecr/awsapi/repository_action_test.go b/spinifex/domains/ecr/awsapi/repository_action_test.go index b706a9150..beb7efe5e 100644 --- a/spinifex/domains/ecr/awsapi/repository_action_test.go +++ b/spinifex/domains/ecr/awsapi/repository_action_test.go @@ -27,7 +27,10 @@ func newRepositoryActionTestConn(t *testing.T) *nats.Conn { svc := handlers_ecr.NewKVMetaService(js) serveRepositoryActionMeta(t, nc, handlers_ecr.SubjectRepoCreate, svc.RepoCreate) serveRepositoryActionMeta(t, nc, handlers_ecr.SubjectRepoDescribe, svc.RepoDescribe) + serveRepositoryActionMeta(t, nc, handlers_ecr.SubjectRepoDelete, svc.RepoDelete) serveRepositoryActionMeta(t, nc, handlers_ecr.SubjectRepoList, svc.RepoList) + serveRepositoryActionMeta(t, nc, handlers_ecr.SubjectManifestPut, svc.ManifestPut) + serveRepositoryActionMeta(t, nc, handlers_ecr.SubjectManifestList, svc.ManifestList) return nc } diff --git a/spinifex/gateway/ecr_deleterepository.go b/spinifex/gateway/ecr_deleterepository.go index 4adac803e..f6fd4b9f4 100644 --- a/spinifex/gateway/ecr_deleterepository.go +++ b/spinifex/gateway/ecr_deleterepository.go @@ -1,30 +1,18 @@ package gateway import ( - "encoding/json" "errors" "io" "log/slog" "net/http" - "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" ) -// deleteRepositoryRequest is the camelCase AWS JSON 1.1 input shape. force=true -// allows deleting a repository that still contains images. -type deleteRepositoryRequest struct { - RepositoryName string `json:"repositoryName"` - RegistryID string `json:"registryId"` - Force bool `json:"force"` -} - -// handleDeleteRepository removes a repository in the caller account. Without -// force, a repository that still holds images is rejected with -// RepositoryNotEmptyException. The KV metadata (meta, policy, tags, manifests) -// is cascaded; predastore blob reclamation is deferred to a separate GC pass. +// handleDeleteRepository adapts the authenticated HTTP request to the ECR AWS +// action. ECR lifecycle rules and metadata operations belong to the domain +// adapter; gateway retains HTTP and auth-context handling. func (gw *GatewayConfig) handleDeleteRepository(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() accountID, _ := ctx.Value(ctxAccountID).(string) @@ -38,48 +26,10 @@ func (gw *GatewayConfig) handleDeleteRepository(w http.ResponseWriter, r *http.R slog.ErrorContext(ctx, "DeleteRepository: failed to read body", "err", err) return awsapi.MalformedBodyError() } - var req deleteRepositoryRequest - if err := json.Unmarshal(body, &req); err != nil { - return awsapi.MalformedBodyError() - } - if err := awsapi.ValidateRepositoryName(req.RepositoryName); err != nil { + output, err := awsapi.DeleteRepository(ctx, gw.NATSConn, gw.ecrRepositoryEndpoint(), accountID, body) + if err != nil { return err } - if req.RegistryID != "" && req.RegistryID != accountID { - return errors.New(awserrors.ErrorAccessDenied) - } - - 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, "DeleteRepository: get repo failed", "repo", req.RepositoryName, "err", err) - return errors.New(awserrors.ErrorServerInternal) - } - - if !req.Force { - manifests, err := store.ListManifests(ctx, accountID, req.RepositoryName) - if err != nil { - slog.ErrorContext(ctx, "DeleteRepository: list manifests failed", "repo", req.RepositoryName, "err", err) - return errors.New(awserrors.ErrorServerInternal) - } - if len(manifests) > 0 { - return errors.New(awserrors.ErrorRepositoryNotEmpty) - } - } - - if err := store.DeleteRepo(ctx, accountID, req.RepositoryName); err != nil { - if errors.Is(err, handlers_ecr.ErrNotFound) { - return errors.New(awserrors.ErrorRepositoryNotFound) - } - slog.ErrorContext(ctx, "DeleteRepository: delete repo failed", "repo", req.RepositoryName, "err", err) - return errors.New(awserrors.ErrorServerInternal) - } - - awsapi.WriteJSONResponse(w, &ecr.DeleteRepositoryOutput{ - Repository: gw.ecrRepositoryEndpoint().RepositoryFromMeta(accountID, req.RepositoryName, meta), - }) + awsapi.WriteJSONResponse(w, output) return nil }