From 1608ffa4659d995e20730e13db2ebebc7b320bd7 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Thu, 18 Jun 2026 21:38:30 +1000 Subject: [PATCH] feat(ecr): implement CreateRepository + DeleteRepository control plane (#388) Add the two missing repository-lifecycle actions, served inline in ECR_Request (like DescribeRepositories) since both return the repository ARN/URI built from the gateway region + internal suffix. CreateRepository validates the name, guards cross-account registryId, rejects an existing repo (RepositoryAlreadyExistsException), and writes the per-account KV meta (predastore bucket stays lazy on first push). DeleteRepository rejects a non-empty repo without force (RepositoryNotEmptyException) and cascades the KV records (meta, policy, tags, manifests). Predastore blob GC is deferred to a separate pass. Adds MetaStore DeleteRepo + ListManifests with the matching daemon-relayed subjects, service methods, and NATS client, plus two new awserrors codes. --- spinifex/awserrors/awserrors.go | 4 + spinifex/daemon/daemon.go | 2 + spinifex/daemon/daemon_handlers_ecr.go | 8 + .../ecr_createdeleterepository_test.go | 194 ++++++++++++++++++ spinifex/gateway/ecr_createrepository.go | 80 ++++++++ spinifex/gateway/ecr_deleterepository.go | 92 +++++++++ spinifex/gateway/ecr_test.go | 6 +- spinifex/gateway/ecrapi.go | 9 + spinifex/handlers/ecr/meta.go | 92 +++++++++ spinifex/handlers/ecr/meta_kv_test.go | 40 ++++ spinifex/handlers/ecr/meta_nats.go | 19 ++ spinifex/handlers/ecr/meta_test.go | 34 +++ spinifex/handlers/ecr/service.go | 20 ++ spinifex/handlers/ecr/service_impl.go | 19 ++ spinifex/handlers/ecr/storage.go | 19 +- 15 files changed, 629 insertions(+), 9 deletions(-) create mode 100644 spinifex/gateway/ecr_createdeleterepository_test.go create mode 100644 spinifex/gateway/ecr_createrepository.go create mode 100644 spinifex/gateway/ecr_deleterepository.go diff --git a/spinifex/awserrors/awserrors.go b/spinifex/awserrors/awserrors.go index cd22e64a7..8191dd8e2 100644 --- a/spinifex/awserrors/awserrors.go +++ b/spinifex/awserrors/awserrors.go @@ -456,6 +456,8 @@ var ( // ECR-specific error codes ErrorRepositoryNotFound = "RepositoryNotFoundException" ErrorRepositoryPolicyNotFound = "RepositoryPolicyNotFoundException" + ErrorRepositoryAlreadyExists = "RepositoryAlreadyExistsException" + ErrorRepositoryNotEmpty = "RepositoryNotEmptyException" // ELBv2-specific error codes ErrorELBv2LoadBalancerNotFound = "LoadBalancerNotFound" @@ -964,6 +966,8 @@ var ErrorLookup = map[string]ErrorMessage{ // ECR error codes ErrorRepositoryNotFound: {HTTPCode: 400, Message: "The repository could not be found. Check the spelling of the specified repository and ensure that you are performing operations on the correct registry."}, ErrorRepositoryPolicyNotFound: {HTTPCode: 400, Message: "The repository and registry do not have an associated repository policy."}, + ErrorRepositoryAlreadyExists: {HTTPCode: 400, Message: "The repository already exists in the registry."}, + ErrorRepositoryNotEmpty: {HTTPCode: 400, Message: "The repository contains images. Either delete the images or use the force option to delete the repository."}, // ELBv2 error codes ErrorELBv2LoadBalancerNotFound: {HTTPCode: 400, Message: "One or more load balancers not found."}, diff --git a/spinifex/daemon/daemon.go b/spinifex/daemon/daemon.go index e526f6b1c..428a0e660 100644 --- a/spinifex/daemon/daemon.go +++ b/spinifex/daemon/daemon.go @@ -965,6 +965,7 @@ func (d *Daemon) subscribeAll() error { natsSub{handlers_ecr.SubjectRepoCreate, d.handleECRRepoCreate, "spinifex-workers"}, natsSub{handlers_ecr.SubjectRepoDescribe, d.handleECRRepoDescribe, "spinifex-workers"}, natsSub{handlers_ecr.SubjectRepoList, d.handleECRRepoList, "spinifex-workers"}, + natsSub{handlers_ecr.SubjectRepoDelete, d.handleECRRepoDelete, "spinifex-workers"}, natsSub{handlers_ecr.SubjectPolicyPut, d.handleECRPolicyPut, "spinifex-workers"}, natsSub{handlers_ecr.SubjectPolicyGet, d.handleECRPolicyGet, "spinifex-workers"}, natsSub{handlers_ecr.SubjectPolicyDelete, d.handleECRPolicyDelete, "spinifex-workers"}, @@ -974,6 +975,7 @@ func (d *Daemon) subscribeAll() error { natsSub{handlers_ecr.SubjectTagDelete, d.handleECRTagDelete, "spinifex-workers"}, natsSub{handlers_ecr.SubjectManifestPut, d.handleECRManifestPut, "spinifex-workers"}, natsSub{handlers_ecr.SubjectManifestDescribe, d.handleECRManifestDescribe, "spinifex-workers"}, + natsSub{handlers_ecr.SubjectManifestList, d.handleECRManifestList, "spinifex-workers"}, natsSub{handlers_ecr.SubjectUploadCreate, d.handleECRUploadCreate, "spinifex-workers"}, natsSub{handlers_ecr.SubjectUploadGet, d.handleECRUploadGet, "spinifex-workers"}, natsSub{handlers_ecr.SubjectUploadUpdate, d.handleECRUploadUpdate, "spinifex-workers"}, diff --git a/spinifex/daemon/daemon_handlers_ecr.go b/spinifex/daemon/daemon_handlers_ecr.go index 2e82a0c93..e7938b0ec 100644 --- a/spinifex/daemon/daemon_handlers_ecr.go +++ b/spinifex/daemon/daemon_handlers_ecr.go @@ -14,6 +14,10 @@ func (d *Daemon) handleECRRepoList(msg *nats.Msg) { handleNATSRequest(msg, d.ecrMetaService.RepoList) } +func (d *Daemon) handleECRRepoDelete(msg *nats.Msg) { + handleNATSRequest(msg, d.ecrMetaService.RepoDelete) +} + func (d *Daemon) handleECRPolicyPut(msg *nats.Msg) { handleNATSRequest(msg, d.ecrMetaService.PolicyPut) } @@ -50,6 +54,10 @@ func (d *Daemon) handleECRManifestDescribe(msg *nats.Msg) { handleNATSRequest(msg, d.ecrMetaService.ManifestDescribe) } +func (d *Daemon) handleECRManifestList(msg *nats.Msg) { + handleNATSRequest(msg, d.ecrMetaService.ManifestList) +} + func (d *Daemon) handleECRUploadCreate(msg *nats.Msg) { handleNATSRequest(msg, d.ecrMetaService.UploadCreate) } diff --git a/spinifex/gateway/ecr_createdeleterepository_test.go b/spinifex/gateway/ecr_createdeleterepository_test.go new file mode 100644 index 000000000..487458d29 --- /dev/null +++ b/spinifex/gateway/ecr_createdeleterepository_test.go @@ -0,0 +1,194 @@ +package gateway + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + handlers_ecr "github.com/mulgadc/spinifex/spinifex/handlers/ecr" + "github.com/mulgadc/spinifex/spinifex/testutil" + "github.com/nats-io/nats.go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// newRepoLifecycleGateway wires the repo create/describe/delete + manifest +// subjects against an embedded KV-backed service, returning a gateway client. +func newRepoLifecycleGateway(t *testing.T) (*GatewayConfig, *nats.Conn) { + t.Helper() + _, nc, js := testutil.StartTestJetStream(t) + svc := handlers_ecr.NewKVMetaService(js) + serveECRMeta(t, nc, handlers_ecr.SubjectRepoCreate, svc.RepoCreate) + serveECRMeta(t, nc, handlers_ecr.SubjectRepoDescribe, svc.RepoDescribe) + serveECRMeta(t, nc, handlers_ecr.SubjectRepoDelete, svc.RepoDelete) + serveECRMeta(t, nc, handlers_ecr.SubjectManifestPut, svc.ManifestPut) + serveECRMeta(t, nc, handlers_ecr.SubjectManifestList, svc.ManifestList) + gw := &GatewayConfig{ + NATSConn: nc, Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, DisableLogging: true, + } + return gw, nc +} + +func ecrLifecycleRequest(t *testing.T, gw *GatewayConfig, handler func(*GatewayConfig, http.ResponseWriter, *http.Request) error, body string) (*httptest.ResponseRecorder, error) { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + ctx := context.WithValue(req.Context(), ctxAccountID, ecrTestAccount) + w := httptest.NewRecorder() + return w, handler(gw, w, req.WithContext(ctx)) +} + +func createRepo(t *testing.T, gw *GatewayConfig, body string) (*httptest.ResponseRecorder, error) { + return ecrLifecycleRequest(t, gw, (*GatewayConfig).handleCreateRepository, body) +} + +func deleteRepo(t *testing.T, gw *GatewayConfig, body string) (*httptest.ResponseRecorder, error) { + return ecrLifecycleRequest(t, gw, (*GatewayConfig).handleDeleteRepository, body) +} + +type repoOut struct { + Repository struct { + RepositoryName string `json:"repositoryName"` + RegistryID string `json:"registryId"` + RepositoryArn string `json:"repositoryArn"` + RepositoryURI string `json:"repositoryUri"` + ImageTagMutability string `json:"imageTagMutability"` + CreatedAt float64 `json:"createdAt"` + } `json:"repository"` +} + +func TestCreateRepository_Happy(t *testing.T) { + gw, _ := newRepoLifecycleGateway(t) + w, err := createRepo(t, gw, `{"repositoryName":"team/app"}`) + require.NoError(t, err) + require.Equal(t, http.StatusOK, w.Code) + + var out repoOut + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &out)) + assert.Equal(t, "team/app", out.Repository.RepositoryName) + assert.Equal(t, ecrTestAccount, out.Repository.RegistryID) + assert.Equal(t, "arn:aws:ecr:"+ecrTestRegion+":"+ecrTestAccount+":repository/team/app", out.Repository.RepositoryArn) + assert.Equal(t, ecrTestAccount+".dkr.ecr."+ecrTestRegion+"."+ecrTestSuffix+"/team/app", out.Repository.RepositoryURI) + assert.Equal(t, "MUTABLE", out.Repository.ImageTagMutability) + assert.Positive(t, out.Repository.CreatedAt) +} + +func TestCreateRepository_Errors(t *testing.T) { + gw, _ := newRepoLifecycleGateway(t) + _, err := createRepo(t, gw, `{"repositoryName":"team/app"}`) + require.NoError(t, err) + + cases := []struct { + name, body, expect string + }{ + {"already exists", `{"repositoryName":"team/app"}`, "RepositoryAlreadyExistsException"}, + {"invalid name", `{"repositoryName":"Team/App"}`, "InvalidParameterValue"}, + {"empty name", `{}`, "InvalidParameterValue"}, + {"cross-account", `{"repositoryName":"team/x","registryId":"999999999999"}`, "AccessDenied"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := createRepo(t, gw, tc.body) + require.Error(t, err) + assert.Equal(t, tc.expect, err.Error()) + }) + } +} + +// noAccountRequest builds a request without the auth-context account ID, which +// both handlers reject with ServerInternal before touching the store. +func noAccountRequest(body string) *http.Request { + return httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) +} + +func TestCreateRepository_NoAccountAndMalformed(t *testing.T) { + gw, _ := newRepoLifecycleGateway(t) + + err := gw.handleCreateRepository(httptest.NewRecorder(), noAccountRequest(`{"repositoryName":"team/app"}`)) + require.Error(t, err) + assert.Equal(t, "ServerInternal", err.Error()) + + _, err = createRepo(t, gw, `{`) + require.Error(t, err) + assert.Equal(t, "InvalidParameterValue", err.Error()) +} + +func TestDeleteRepository_NoAccountAndMalformed(t *testing.T) { + gw, _ := newRepoLifecycleGateway(t) + + err := gw.handleDeleteRepository(httptest.NewRecorder(), noAccountRequest(`{"repositoryName":"team/app"}`)) + require.Error(t, err) + assert.Equal(t, "ServerInternal", err.Error()) + + _, err = deleteRepo(t, gw, `{`) + require.Error(t, err) + assert.Equal(t, "InvalidParameterValue", err.Error()) +} + +func TestDeleteRepository_Happy(t *testing.T) { + gw, _ := newRepoLifecycleGateway(t) + _, err := createRepo(t, gw, `{"repositoryName":"team/app"}`) + require.NoError(t, err) + + w, err := deleteRepo(t, gw, `{"repositoryName":"team/app"}`) + require.NoError(t, err) + require.Equal(t, http.StatusOK, w.Code) + var out repoOut + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &out)) + assert.Equal(t, "team/app", out.Repository.RepositoryName) + + // Gone afterwards. + _, err = deleteRepo(t, gw, `{"repositoryName":"team/app"}`) + require.Error(t, err) + assert.Equal(t, "RepositoryNotFoundException", err.Error()) +} + +func TestDeleteRepository_NotEmpty(t *testing.T) { + gw, nc := newRepoLifecycleGateway(t) + _, err := createRepo(t, gw, `{"repositoryName":"team/app"}`) + require.NoError(t, err) + + // Seed an image (manifest) so the repo is non-empty. + store := handlers_ecr.NewNATSMetaStore(nc) + require.NoError(t, store.PutManifestMeta(ecrTestAccount, "team/app", handlers_ecr.ManifestMeta{ + Digest: "sha256:" + strings.Repeat("a", 64), MediaType: "application/json", Size: 7, PushedAt: time.Now(), + })) + + // Without force -> RepositoryNotEmptyException. + _, err = deleteRepo(t, gw, `{"repositoryName":"team/app"}`) + require.Error(t, err) + assert.Equal(t, "RepositoryNotEmptyException", err.Error()) + + // With force -> deleted. + w, err := deleteRepo(t, gw, `{"repositoryName":"team/app","force":true}`) + require.NoError(t, err) + require.Equal(t, http.StatusOK, w.Code) +} + +func TestDeleteRepository_CrossAccountDenied(t *testing.T) { + gw, _ := newRepoLifecycleGateway(t) + _, err := createRepo(t, gw, `{"repositoryName":"team/app"}`) + require.NoError(t, err) + _, err = deleteRepo(t, gw, `{"repositoryName":"team/app","registryId":"999999999999"}`) + require.Error(t, err) + assert.Equal(t, "AccessDenied", err.Error()) +} + +func TestECRRequest_CreateDeleteDispatched(t *testing.T) { + gw, _ := newRepoLifecycleGateway(t) + + create := setupECRRequest("AmazonEC2ContainerRegistry_V20150921.CreateRepository", `{"repositoryName":"team/app"}`) + create = create.WithContext(context.WithValue(create.Context(), ctxAccountID, ecrTestAccount)) + wc := httptest.NewRecorder() + require.NoError(t, gw.ECR_Request(wc, create)) + assert.Equal(t, http.StatusOK, wc.Code) + + del := setupECRRequest("AmazonEC2ContainerRegistry_V20150921.DeleteRepository", `{"repositoryName":"team/app"}`) + del = del.WithContext(context.WithValue(del.Context(), ctxAccountID, ecrTestAccount)) + wd := httptest.NewRecorder() + require.NoError(t, gw.ECR_Request(wd, del)) + assert.Equal(t, http.StatusOK, wd.Code) +} diff --git a/spinifex/gateway/ecr_createrepository.go b/spinifex/gateway/ecr_createrepository.go new file mode 100644 index 000000000..ee12eb10f --- /dev/null +++ b/spinifex/gateway/ecr_createrepository.go @@ -0,0 +1,80 @@ +package gateway + +import ( + "encoding/json" + "errors" + "io" + "log/slog" + "net/http" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ecr" + "github.com/mulgadc/spinifex/spinifex/awserrors" + gateway_ecrapi "github.com/mulgadc/spinifex/spinifex/gateway/ecrapi" + handlers_ecr "github.com/mulgadc/spinifex/spinifex/handlers/ecr" +) + +// createRepositoryRequest is the camelCase AWS JSON 1.1 input shape. The SDK +// input struct carries locationName tags rather than json tags, so the subset +// honored here is decoded explicitly. Tags, encryption, and scanning config are +// accepted-and-ignored in v1. +type createRepositoryRequest struct { + RepositoryName string `json:"repositoryName"` + RegistryID string `json:"registryId"` + ImageTagMutability string `json:"imageTagMutability"` +} + +// handleCreateRepository provisions an empty repository in the caller account. +// The predastore object bucket stays lazily created on first push; this writes +// only the per-account KV meta record. imageTagMutability is reported as MUTABLE +// in v1 (immutable tags are a separate follow-on). +func (gw *GatewayConfig) handleCreateRepository(w http.ResponseWriter, r *http.Request) error { + accountID, _ := r.Context().Value(ctxAccountID).(string) + if accountID == "" { + slog.Error("CreateRepository: no account ID in auth context") + return errors.New(awserrors.ErrorServerInternal) + } + + body, err := io.ReadAll(r.Body) + if err != nil { + slog.Error("CreateRepository: failed to read body", "err", err) + return errors.New(awserrors.ErrorInvalidParameterValue) + } + var req createRepositoryRequest + if err := json.Unmarshal(body, &req); err != nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if err := handlers_ecr.ValidateRepoName(req.RepositoryName); err != nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if req.RegistryID != "" && req.RegistryID != accountID { + return errors.New(awserrors.ErrorAccessDenied) + } + + store := handlers_ecr.NewNATSMetaStore(gw.NATSConn) + if _, err := store.GetRepo(accountID, req.RepositoryName); err == nil { + return errors.New(awserrors.ErrorRepositoryAlreadyExists) + } else if !errors.Is(err, handlers_ecr.ErrNotFound) { + slog.Error("CreateRepository: get repo failed", "repo", req.RepositoryName, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + + meta := handlers_ecr.RepoMeta{Name: req.RepositoryName, CreatedAt: time.Now().UTC()} + if err := store.PutRepo(accountID, meta); err != nil { + slog.Error("CreateRepository: put repo failed", "repo", req.RepositoryName, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + + gateway_ecrapi.WriteJSONResponse(w, &ecr.CreateRepositoryOutput{ + Repository: &ecr.Repository{ + RegistryId: aws.String(accountID), + RepositoryName: aws.String(req.RepositoryName), + RepositoryArn: aws.String(gw.ecrRepositoryArn(accountID, req.RepositoryName)), + RepositoryUri: aws.String(gw.ecrRepositoryUri(accountID, req.RepositoryName)), + CreatedAt: aws.Time(meta.CreatedAt), + ImageTagMutability: aws.String("MUTABLE"), + }, + }) + return nil +} diff --git a/spinifex/gateway/ecr_deleterepository.go b/spinifex/gateway/ecr_deleterepository.go new file mode 100644 index 000000000..86bdacaa6 --- /dev/null +++ b/spinifex/gateway/ecr_deleterepository.go @@ -0,0 +1,92 @@ +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" + "github.com/mulgadc/spinifex/spinifex/awserrors" + gateway_ecrapi "github.com/mulgadc/spinifex/spinifex/gateway/ecrapi" + handlers_ecr "github.com/mulgadc/spinifex/spinifex/handlers/ecr" +) + +// 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. +func (gw *GatewayConfig) handleDeleteRepository(w http.ResponseWriter, r *http.Request) error { + accountID, _ := r.Context().Value(ctxAccountID).(string) + if accountID == "" { + slog.Error("DeleteRepository: no account ID in auth context") + return errors.New(awserrors.ErrorServerInternal) + } + + body, err := io.ReadAll(r.Body) + if err != nil { + slog.Error("DeleteRepository: failed to read body", "err", err) + return errors.New(awserrors.ErrorInvalidParameterValue) + } + var req deleteRepositoryRequest + if err := json.Unmarshal(body, &req); err != nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if err := handlers_ecr.ValidateRepoName(req.RepositoryName); err != nil { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if req.RegistryID != "" && req.RegistryID != accountID { + return errors.New(awserrors.ErrorAccessDenied) + } + + store := handlers_ecr.NewNATSMetaStore(gw.NATSConn) + meta, err := store.GetRepo(accountID, req.RepositoryName) + if err != nil { + if errors.Is(err, handlers_ecr.ErrNotFound) { + return errors.New(awserrors.ErrorRepositoryNotFound) + } + slog.Error("DeleteRepository: get repo failed", "repo", req.RepositoryName, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + + if !req.Force { + manifests, err := store.ListManifests(accountID, req.RepositoryName) + if err != nil { + slog.Error("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(accountID, req.RepositoryName); err != nil { + if errors.Is(err, handlers_ecr.ErrNotFound) { + return errors.New(awserrors.ErrorRepositoryNotFound) + } + slog.Error("DeleteRepository: delete repo failed", "repo", req.RepositoryName, "err", err) + return errors.New(awserrors.ErrorServerInternal) + } + + gateway_ecrapi.WriteJSONResponse(w, &ecr.DeleteRepositoryOutput{ + Repository: &ecr.Repository{ + RegistryId: aws.String(accountID), + RepositoryName: aws.String(req.RepositoryName), + RepositoryArn: aws.String(gw.ecrRepositoryArn(accountID, req.RepositoryName)), + RepositoryUri: aws.String(gw.ecrRepositoryUri(accountID, req.RepositoryName)), + CreatedAt: aws.Time(meta.CreatedAt), + ImageTagMutability: aws.String("MUTABLE"), + }, + }) + return nil +} diff --git a/spinifex/gateway/ecr_test.go b/spinifex/gateway/ecr_test.go index 78aafc684..141ba6c81 100644 --- a/spinifex/gateway/ecr_test.go +++ b/spinifex/gateway/ecr_test.go @@ -59,11 +59,13 @@ func TestECRRequest_UnknownAction(t *testing.T) { assert.Equal(t, awserrors.ErrorInvalidAction, err.Error()) } -// Every registered action resolves to the 501 stub until its handler lands. +// A registered-but-unimplemented action resolves to the 501 stub until its +// handler lands. CreateRepository/DeleteRepository/DescribeRepositories are now +// served inline, so ListImages stands in as a still-stubbed action. func TestECRRequest_KnownActionNotImplemented(t *testing.T) { gw := &GatewayConfig{DisableLogging: true} err := gw.ECR_Request(httptest.NewRecorder(), - setupECRRequest("AmazonEC2ContainerRegistry_V20150921.CreateRepository", "{}")) + setupECRRequest("AmazonEC2ContainerRegistry_V20150921.ListImages", "{}")) require.Error(t, err) assert.Equal(t, awserrors.ErrorNotImplemented, err.Error()) } diff --git a/spinifex/gateway/ecrapi.go b/spinifex/gateway/ecrapi.go index 68cd36e2b..0942825bf 100644 --- a/spinifex/gateway/ecrapi.go +++ b/spinifex/gateway/ecrapi.go @@ -52,6 +52,15 @@ func (gw *GatewayConfig) ECR_Request(w http.ResponseWriter, r *http.Request) err return gw.handleDescribeRepositories(w, r) } + // CreateRepository and DeleteRepository return the repository ARN/URI, built + // from the gateway region and internal suffix, so they are served inline. + if action == "CreateRepository" { + return gw.handleCreateRepository(w, r) + } + if action == "DeleteRepository" { + return gw.handleDeleteRepository(w, r) + } + accountID, _ := r.Context().Value(ctxAccountID).(string) if accountID == "" { slog.Error("ECR_Request: no account ID in auth context") diff --git a/spinifex/handlers/ecr/meta.go b/spinifex/handlers/ecr/meta.go index 11f12dfb1..20f2b6030 100644 --- a/spinifex/handlers/ecr/meta.go +++ b/spinifex/handlers/ecr/meta.go @@ -59,6 +59,9 @@ type MetaStore interface { PutRepo(accountID string, meta RepoMeta) error GetRepo(accountID, repo string) (RepoMeta, error) ListRepos(accountID string) ([]string, error) + DeleteRepo(accountID, repo string) error + + ListManifests(accountID, repo string) ([]string, error) PutRepoPolicy(accountID, repo string, policyText []byte) error GetRepoPolicy(accountID, repo string) ([]byte, error) @@ -163,6 +166,61 @@ func (s *KVMetaStore) ListRepos(accountID string) ([]string, error) { return repos, nil } +// DeleteRepo removes a repository and cascades its metadata: meta, policy, all +// tags, and all manifest records. Predastore blob garbage collection is out of +// scope here (deferred); only the per-account KV records are removed. Returns +// ErrNotFound when the repository meta is absent. +func (s *KVMetaStore) DeleteRepo(accountID, repo string) error { + kv, err := s.bucket(accountID) + if err != nil { + return err + } + if _, err := kv.Get(KVRepoMetaKey(repo)); err != nil { + return mapKVErr(err) + } + keys, err := kv.Keys() + if err != nil { + if errors.Is(err, nats.ErrNoKeysFound) { + return nil + } + return err + } + tagsPrefix, manifestsPrefix := KVTagsPrefix(repo), KVManifestsPrefix(repo) + metaKey, policyKey := KVRepoMetaKey(repo), KVRepoPolicyKey(repo) + for _, k := range keys { + if k == metaKey || k == policyKey || + strings.HasPrefix(k, tagsPrefix) || strings.HasPrefix(k, manifestsPrefix) { + if err := kv.Delete(k); err != nil { + return mapKVErr(err) + } + } + } + return nil +} + +func (s *KVMetaStore) ListManifests(accountID, repo string) ([]string, error) { + kv, err := s.bucket(accountID) + if err != nil { + return nil, err + } + keys, err := kv.Keys() + if err != nil { + if errors.Is(err, nats.ErrNoKeysFound) { + return nil, nil + } + return nil, err + } + prefix := KVManifestsPrefix(repo) + var digests []string + for _, k := range keys { + if token, ok := strings.CutPrefix(k, prefix); ok { + digests = append(digests, token) + } + } + sort.Strings(digests) + return digests, nil +} + func (s *KVMetaStore) PutRepoPolicy(accountID, repo string, policyText []byte) error { kv, err := s.bucket(accountID) if err != nil { @@ -413,6 +471,40 @@ func (m *MemoryMetaStore) ListRepos(accountID string) ([]string, error) { return out, nil } +func (m *MemoryMetaStore) DeleteRepo(accountID, repo string) error { + m.mu.Lock() + defer m.mu.Unlock() + if _, ok := m.repos[accountID][repo]; !ok { + return ErrNotFound + } + delete(m.repos[accountID], repo) + delete(m.policies[accountID], repo) + for k := range m.tags[accountID] { + if r, _, ok := strings.Cut(k, "|"); ok && r == repo { + delete(m.tags[accountID], k) + } + } + for k := range m.manifests[accountID] { + if r, _, ok := strings.Cut(k, "|"); ok && r == repo { + delete(m.manifests[accountID], k) + } + } + return nil +} + +func (m *MemoryMetaStore) ListManifests(accountID, repo string) ([]string, error) { + m.mu.Lock() + defer m.mu.Unlock() + var out []string + for k := range m.manifests[accountID] { + if r, d, ok := strings.Cut(k, "|"); ok && r == repo { + out = append(out, d) + } + } + sort.Strings(out) + return out, nil +} + func (m *MemoryMetaStore) PutRepoPolicy(accountID, repo string, policyText []byte) error { m.mu.Lock() defer m.mu.Unlock() diff --git a/spinifex/handlers/ecr/meta_kv_test.go b/spinifex/handlers/ecr/meta_kv_test.go index b5d7894ed..9e74591c9 100644 --- a/spinifex/handlers/ecr/meta_kv_test.go +++ b/spinifex/handlers/ecr/meta_kv_test.go @@ -79,6 +79,46 @@ func TestKVMetaStore_FullCycle(t *testing.T) { assert.Empty(t, empty) } +// TestKVMetaStore_DeleteRepo proves the cascade removes a repo's meta, policy, +// tags, and manifests, leaves a same-prefix nested repo intact, and reports +// ErrNotFound for an absent repo. +func TestKVMetaStore_DeleteRepo(t *testing.T) { + _, _, js := testutil.StartTestJetStream(t) + store := NewKVMetaStore(js) + const acct = "000000000000" + const dig = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + require.NoError(t, store.PutRepo(acct, RepoMeta{Name: "team/app", CreatedAt: time.Now()})) + require.NoError(t, store.PutRepo(acct, RepoMeta{Name: "team/app/sub", CreatedAt: time.Now()})) + require.NoError(t, store.PutTag(acct, "team/app", "v1", dig)) + require.NoError(t, store.PutManifestMeta(acct, "team/app", ManifestMeta{Digest: dig, MediaType: "x", Size: 1})) + require.NoError(t, store.PutRepoPolicy(acct, "team/app", []byte(`{}`))) + + manifests, err := store.ListManifests(acct, "team/app") + require.NoError(t, err) + assert.Len(t, manifests, 1) + + require.NoError(t, store.DeleteRepo(acct, "team/app")) + + _, err = store.GetRepo(acct, "team/app") + assert.ErrorIs(t, err, ErrNotFound) + tags, err := store.ListTags(acct, "team/app") + require.NoError(t, err) + assert.Empty(t, tags) + manifests, err = store.ListManifests(acct, "team/app") + require.NoError(t, err) + assert.Empty(t, manifests) + _, err = store.GetRepoPolicy(acct, "team/app") + assert.ErrorIs(t, err, ErrNotFound) + + // The same-prefix nested repo must survive. + got, err := store.GetRepo(acct, "team/app/sub") + require.NoError(t, err) + assert.Equal(t, "team/app/sub", got.Name) + + assert.ErrorIs(t, store.DeleteRepo(acct, "team/ghost"), ErrNotFound) +} + // TestKVMetaStore_RepoPolicy exercises the policy passthrough against the // JetStream-backed store and proves the policy key shares the repo bucket // without being mistaken for a repository by ListRepos. diff --git a/spinifex/handlers/ecr/meta_nats.go b/spinifex/handlers/ecr/meta_nats.go index a08a540bd..d81dd52a2 100644 --- a/spinifex/handlers/ecr/meta_nats.go +++ b/spinifex/handlers/ecr/meta_nats.go @@ -48,6 +48,25 @@ func (s *NATSMetaStore) ListRepos(accountID string) ([]string, error) { return resp.Repos, nil } +func (s *NATSMetaStore) DeleteRepo(accountID, repo string) error { + resp, err := utils.NATSRequest[RepoDeleteResponse](s.conn, SubjectRepoDelete, &RepoDeleteRequest{Repo: repo}, metaRequestTimeout, accountID) + if err != nil { + return err + } + if !resp.Found { + return ErrNotFound + } + return nil +} + +func (s *NATSMetaStore) ListManifests(accountID, repo string) ([]string, error) { + resp, err := utils.NATSRequest[ManifestListResponse](s.conn, SubjectManifestList, &ManifestListRequest{Repo: repo}, metaRequestTimeout, accountID) + if err != nil { + return nil, err + } + return resp.Digests, nil +} + func (s *NATSMetaStore) PutRepoPolicy(accountID, repo string, policyText []byte) error { _, err := utils.NATSRequest[PolicyPutResponse](s.conn, SubjectPolicyPut, &PolicyPutRequest{Repo: repo, PolicyText: policyText}, metaRequestTimeout, accountID) return err diff --git a/spinifex/handlers/ecr/meta_test.go b/spinifex/handlers/ecr/meta_test.go index 7fef8ea0c..09d6d98ff 100644 --- a/spinifex/handlers/ecr/meta_test.go +++ b/spinifex/handlers/ecr/meta_test.go @@ -115,6 +115,40 @@ func TestTrimSuffixMeta(t *testing.T) { assert.Equal(t, "team/app", trimSuffixMeta("repos/team/app/meta")) } +func TestMemoryMetaStore_DeleteRepoAndListManifests(t *testing.T) { + m := NewMemoryMetaStore() + const acct = "000000000000" + + require.NoError(t, m.PutRepo(acct, RepoMeta{Name: "team/app", CreatedAt: time.Now()})) + require.NoError(t, m.PutRepo(acct, RepoMeta{Name: "team/web", CreatedAt: time.Now()})) + require.NoError(t, m.PutTag(acct, "team/app", "v1", "sha256:aaa")) + require.NoError(t, m.PutManifestMeta(acct, "team/app", ManifestMeta{Digest: "sha256:ccc", MediaType: "x", Size: 3})) + require.NoError(t, m.PutRepoPolicy(acct, "team/app", []byte(`{}`))) + + digs, err := m.ListManifests(acct, "team/app") + require.NoError(t, err) + assert.Equal(t, []string{"sha256:ccc"}, digs) + + require.NoError(t, m.DeleteRepo(acct, "team/app")) + _, err = m.GetRepo(acct, "team/app") + assert.ErrorIs(t, err, ErrNotFound) + tags, err := m.ListTags(acct, "team/app") + require.NoError(t, err) + assert.Empty(t, tags) + digs, err = m.ListManifests(acct, "team/app") + require.NoError(t, err) + assert.Empty(t, digs) + _, err = m.GetRepoPolicy(acct, "team/app") + assert.ErrorIs(t, err, ErrNotFound) + + // Other repo untouched. + got, err := m.GetRepo(acct, "team/web") + require.NoError(t, err) + assert.Equal(t, "team/web", got.Name) + + assert.ErrorIs(t, m.DeleteRepo(acct, "team/ghost"), ErrNotFound) +} + func TestMemoryMetaStore_RepoPolicy(t *testing.T) { m := NewMemoryMetaStore() const acct = "000000000000" diff --git a/spinifex/handlers/ecr/service.go b/spinifex/handlers/ecr/service.go index 22492d6d4..03b0dc113 100644 --- a/spinifex/handlers/ecr/service.go +++ b/spinifex/handlers/ecr/service.go @@ -7,6 +7,7 @@ const ( SubjectRepoCreate = "ecr.repo.create" SubjectRepoDescribe = "ecr.repo.describe" SubjectRepoList = "ecr.repo.list" + SubjectRepoDelete = "ecr.repo.delete" SubjectPolicyPut = "ecr.policy.put" SubjectPolicyGet = "ecr.policy.get" @@ -19,6 +20,7 @@ const ( SubjectManifestPut = "ecr.manifest.put" SubjectManifestDescribe = "ecr.manifest.describe" + SubjectManifestList = "ecr.manifest.list" SubjectUploadCreate = "ecr.upload.create" SubjectUploadGet = "ecr.upload.get" @@ -51,6 +53,14 @@ type RepoListResponse struct { Repos []string `json:"repos"` } +type RepoDeleteRequest struct { + Repo string `json:"repo"` +} + +type RepoDeleteResponse struct { + Found bool `json:"found"` +} + type PolicyPutRequest struct { Repo string `json:"repo"` PolicyText []byte `json:"policyText"` @@ -128,6 +138,14 @@ type ManifestDescribeResponse struct { Meta ManifestMeta `json:"meta"` } +type ManifestListRequest struct { + Repo string `json:"repo"` +} + +type ManifestListResponse struct { + Digests []string `json:"digests"` +} + type UploadCreateRequest struct { UploadID string `json:"uploadID"` State UploadState `json:"state"` @@ -175,6 +193,7 @@ type MetaService interface { RepoCreate(req *RepoCreateRequest, accountID string) (*RepoCreateResponse, error) RepoDescribe(req *RepoDescribeRequest, accountID string) (*RepoDescribeResponse, error) RepoList(req *RepoListRequest, accountID string) (*RepoListResponse, error) + RepoDelete(req *RepoDeleteRequest, accountID string) (*RepoDeleteResponse, error) PolicyPut(req *PolicyPutRequest, accountID string) (*PolicyPutResponse, error) PolicyGet(req *PolicyGetRequest, accountID string) (*PolicyGetResponse, error) @@ -187,6 +206,7 @@ type MetaService interface { ManifestPut(req *ManifestPutRequest, accountID string) (*ManifestPutResponse, error) ManifestDescribe(req *ManifestDescribeRequest, accountID string) (*ManifestDescribeResponse, error) + ManifestList(req *ManifestListRequest, accountID string) (*ManifestListResponse, error) UploadCreate(req *UploadCreateRequest, accountID string) (*UploadCreateResponse, error) UploadGet(req *UploadGetRequest, accountID string) (*UploadGetResponse, error) diff --git a/spinifex/handlers/ecr/service_impl.go b/spinifex/handlers/ecr/service_impl.go index c6a87a06b..e7c85f52b 100644 --- a/spinifex/handlers/ecr/service_impl.go +++ b/spinifex/handlers/ecr/service_impl.go @@ -52,6 +52,17 @@ func (s *MetaServiceImpl) RepoList(_ *RepoListRequest, accountID string) (*RepoL return &RepoListResponse{Repos: repos}, nil } +func (s *MetaServiceImpl) RepoDelete(req *RepoDeleteRequest, accountID string) (*RepoDeleteResponse, error) { + err := s.store.DeleteRepo(accountID, req.Repo) + if errors.Is(err, ErrNotFound) { + return &RepoDeleteResponse{Found: false}, nil + } + if err != nil { + return nil, err + } + return &RepoDeleteResponse{Found: true}, nil +} + func (s *MetaServiceImpl) PolicyPut(req *PolicyPutRequest, accountID string) (*PolicyPutResponse, error) { if err := s.store.PutRepoPolicy(accountID, req.Repo, req.PolicyText); err != nil { return nil, err @@ -125,6 +136,14 @@ func (s *MetaServiceImpl) ManifestPut(req *ManifestPutRequest, accountID string) return &ManifestPutResponse{}, nil } +func (s *MetaServiceImpl) ManifestList(req *ManifestListRequest, accountID string) (*ManifestListResponse, error) { + digests, err := s.store.ListManifests(accountID, req.Repo) + if err != nil { + return nil, err + } + return &ManifestListResponse{Digests: digests}, nil +} + func (s *MetaServiceImpl) ManifestDescribe(req *ManifestDescribeRequest, accountID string) (*ManifestDescribeResponse, error) { meta, err := s.store.GetManifestMeta(accountID, req.Repo, req.Digest) if errors.Is(err, ErrNotFound) { diff --git a/spinifex/handlers/ecr/storage.go b/spinifex/handlers/ecr/storage.go index 5c9d2c024..f5cd2eb62 100644 --- a/spinifex/handlers/ecr/storage.go +++ b/spinifex/handlers/ecr/storage.go @@ -89,13 +89,14 @@ func KVAccountBucket(accountID string) string { // repos/{name}/manifests/{digest} // uploads/{uuid} const ( - kvRepoMetaKeyFmt = "repos/%s/meta" - kvRepoPolicyKeyFmt = "repos/%s/policy" - kvTagsPrefixFmt = "repos/%s/tags/" - kvTagKeyFmt = "repos/%s/tags/%s" - kvManifestKeyFmt = "repos/%s/manifests/%s" - kvReposPrefix = "repos/" - kvUploadKeyFmt = "uploads/%s" + kvRepoMetaKeyFmt = "repos/%s/meta" + kvRepoPolicyKeyFmt = "repos/%s/policy" + kvTagsPrefixFmt = "repos/%s/tags/" + kvTagKeyFmt = "repos/%s/tags/%s" + kvManifestKeyFmt = "repos/%s/manifests/%s" + kvManifestsPrefixFmt = "repos/%s/manifests/" + kvReposPrefix = "repos/" + kvUploadKeyFmt = "uploads/%s" ) // KVRepoMetaKey returns the KV key for a repository's meta record. @@ -116,6 +117,10 @@ func KVManifestKey(repo, digest string) string { return fmt.Sprintf(kvManifestKeyFmt, repo, DigestToken(digest)) } +// KVManifestsPrefix returns the KV key prefix enumerating a repository's +// manifest metadata records. +func KVManifestsPrefix(repo string) string { return fmt.Sprintf(kvManifestsPrefixFmt, repo) } + // KVReposPrefix is the prefix under which every repo meta key lives. Used by // the catalog listing to enumerate account repositories. const KVReposPrefix = kvReposPrefix