From 57737af09e66e5018f612af13d70ae0ad6577fb7 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Fri, 19 Jun 2026 12:38:15 +1000 Subject: [PATCH] feat(ecr): require repository to exist before push (#400) The OCI /v2 registry auto-created a repository on first push (ensureRepo). AWS ECR does not: a push or PutImage to a repository that was never created with CreateRepository is rejected. Replace ensureRepo with requireRepo, which returns OCI NAME_UNKNOWN on the data plane and RepositoryNotFoundException on the JSON PutImage path. Enforcement sits at the single push choke point (startUpload + StoreManifest). --- spinifex/gateway/ecr/registry.go | 25 ++++++++++----- spinifex/gateway/ecr/registry_image.go | 2 +- spinifex/gateway/ecr/registry_image_test.go | 1 + spinifex/gateway/ecr/registry_test.go | 33 ++++++++++++++++++++ spinifex/gateway/ecr/registry_upload_test.go | 2 ++ spinifex/gateway/ecr_image.go | 2 ++ spinifex/gateway/ecr_image_test.go | 24 ++++++++++++++ tests/e2e/ecr/crane_skopeo_test.go | 22 +++++++++++++ 8 files changed, 103 insertions(+), 8 deletions(-) diff --git a/spinifex/gateway/ecr/registry.go b/spinifex/gateway/ecr/registry.go index 9bafc6092..40763d2bc 100644 --- a/spinifex/gateway/ecr/registry.go +++ b/spinifex/gateway/ecr/registry.go @@ -237,8 +237,13 @@ func (reg *Registry) getBlob(w http.ResponseWriter, _, digest string) { // to 201 when the digest already lives in the account pool; otherwise it opens a // new chunked upload (202) with a fresh running sha256 state. func (reg *Registry) startUpload(w http.ResponseWriter, r *http.Request, name string) { - if err := reg.ensureRepo(name); err != nil { - reg.internal(w, "ensure repo", err) + if err := reg.requireRepo(name); err != nil { + var mErr *ManifestStoreError + if errors.As(err, &mErr) { + WriteError(w, mErr.Status, mErr.Code, mErr.Msg) + return + } + reg.internal(w, "require repo", err) return } @@ -731,13 +736,19 @@ func (reg *Registry) ensureBucket() error { return nil } -func (reg *Registry) ensureRepo(name string) error { - if _, err := reg.Meta.GetRepo(reg.AccountID, name); err == nil { +// requireRepo enforces that the repository exists before a push. ECR does not +// auto-create repositories: a push to a repo that was never created with +// CreateRepository is rejected with OCI NAME_UNKNOWN (the JSON PutImage path +// maps this to RepositoryNotFoundException). +func (reg *Registry) requireRepo(name string) error { + _, err := reg.Meta.GetRepo(reg.AccountID, name) + if err == nil { return nil - } else if !errors.Is(err, ecr.ErrNotFound) { - return err } - return reg.Meta.PutRepo(reg.AccountID, ecr.RepoMeta{Name: name, CreatedAt: time.Now().UTC()}) + if errors.Is(err, ecr.ErrNotFound) { + return &ManifestStoreError{http.StatusNotFound, "NAME_UNKNOWN", fmt.Sprintf("repository %q does not exist", name)} + } + return err } func (reg *Registry) blobExists(digest string) bool { diff --git a/spinifex/gateway/ecr/registry_image.go b/spinifex/gateway/ecr/registry_image.go index bb1d7fbca..d7b747499 100644 --- a/spinifex/gateway/ecr/registry_image.go +++ b/spinifex/gateway/ecr/registry_image.go @@ -52,7 +52,7 @@ type ImageRecord struct { // *ManifestStoreError carrying the OCI code. func (reg *Registry) StoreManifest(account, repo, ref, contentType string, body []byte) (string, error) { scoped := reg.forAccount(account) - if err := scoped.ensureRepo(repo); err != nil { + if err := scoped.requireRepo(repo); err != nil { return "", err } if len(body) > maxManifestBytes { diff --git a/spinifex/gateway/ecr/registry_image_test.go b/spinifex/gateway/ecr/registry_image_test.go index e396c9c05..7537fb958 100644 --- a/spinifex/gateway/ecr/registry_image_test.go +++ b/spinifex/gateway/ecr/registry_image_test.go @@ -159,6 +159,7 @@ func TestListImages_KVDigestForm(t *testing.T) { _, _, js := testutil.StartTestJetStream(t) reg := NewRegistry(objectstore.NewMemoryObjectStore(), ecr.NewKVMetaStore(js), testAccount) repo := "team/app" + require.NoError(t, reg.Meta.PutRepo(testAccount, ecr.RepoMeta{Name: repo})) manifest := []byte(`{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","layers":[]}`) digest, err := reg.StoreManifest(testAccount, repo, "v1", mediaTypeDockerManifest, manifest) diff --git a/spinifex/gateway/ecr/registry_test.go b/spinifex/gateway/ecr/registry_test.go index b0ccef3e0..72984500e 100644 --- a/spinifex/gateway/ecr/registry_test.go +++ b/spinifex/gateway/ecr/registry_test.go @@ -43,9 +43,17 @@ func do(reg *Registry, method, path string, body []byte, hdr map[string]string) return w } +// seedRepo creates the repository metadata so push handlers — which require an +// existing repository — accept writes. Idempotent. +func seedRepo(t *testing.T, reg *Registry, repo string) { + t.Helper() + require.NoError(t, reg.Meta.PutRepo(testAccount, ecr.RepoMeta{Name: repo})) +} + // pushBlob runs the monolithic upload-start + PUT?digest round-trip. func pushBlob(t *testing.T, reg *Registry, repo string, data []byte) string { t.Helper() + seedRepo(t, reg, repo) w := do(reg, http.MethodPost, "/v2/"+repo+"/blobs/uploads/", nil, nil) require.Equal(t, http.StatusAccepted, w.Code) loc := w.Header().Get("Location") @@ -88,6 +96,7 @@ func TestBlobGet_MalformedDigest(t *testing.T) { func TestBlobUpload_Chunked(t *testing.T) { reg := newTestRegistry() + seedRepo(t, reg, "r/repo") c1 := []byte("first-chunk-") c2 := []byte("second-chunk") full := append(append([]byte{}, c1...), c2...) @@ -114,6 +123,7 @@ func TestBlobUpload_Chunked(t *testing.T) { func TestBlobUpload_DigestMismatch(t *testing.T) { reg := newTestRegistry() + seedRepo(t, reg, "r/repo") w := do(reg, http.MethodPost, "/v2/r/repo/blobs/uploads/", nil, nil) loc := w.Header().Get("Location") w = do(reg, http.MethodPut, loc+"?digest="+digestOf([]byte("wrong")), []byte("actual"), nil) @@ -123,6 +133,7 @@ func TestBlobUpload_DigestMismatch(t *testing.T) { func TestBlobUpload_Cancel(t *testing.T) { reg := newTestRegistry() + seedRepo(t, reg, "r/repo") w := do(reg, http.MethodPost, "/v2/r/repo/blobs/uploads/", nil, nil) loc := w.Header().Get("Location") @@ -138,6 +149,7 @@ func TestBlobMount_HitAndMiss(t *testing.T) { reg := newTestRegistry() data := []byte("shared-layer") dg := pushBlob(t, reg, "team/source", data) + seedRepo(t, reg, "team/dest") // Mount hit: blob already in account pool -> 201 without upload. w := do(reg, http.MethodPost, @@ -185,6 +197,7 @@ func TestManifest_PutGetHead_ImageManifest(t *testing.T) { func TestManifest_Put_MissingBlob(t *testing.T) { reg := newTestRegistry() repo := "team/app" + seedRepo(t, reg, repo) manifest := fmt.Appendf(nil, `{"mediaType":"%s","config":{"digest":"%s"},"layers":[]}`, mediaTypeDockerManifest, digestOf([]byte("nope"))) @@ -238,6 +251,26 @@ func TestManifest_Get_Unknown(t *testing.T) { assertCode(t, w, "MANIFEST_UNKNOWN") } +// TestStartUpload_RepoNotCreated asserts a blob upload to a repository that was +// never created is rejected up front — ECR does not auto-create on push. +func TestStartUpload_RepoNotCreated(t *testing.T) { + reg := newTestRegistry() + w := do(reg, http.MethodPost, "/v2/team/ghost/blobs/uploads/", nil, nil) + assert.Equal(t, http.StatusNotFound, w.Code) + assertCode(t, w, "NAME_UNKNOWN") +} + +// TestPutManifest_RepoNotCreated asserts a manifest PUT to an uncreated +// repository is rejected with NAME_UNKNOWN. +func TestPutManifest_RepoNotCreated(t *testing.T) { + reg := newTestRegistry() + manifest := fmt.Appendf(nil, `{"mediaType":"%s","layers":[]}`, mediaTypeDockerManifest) + w := do(reg, http.MethodPut, "/v2/team/ghost/manifests/v1", manifest, + map[string]string{"Content-Type": mediaTypeDockerManifest}) + assert.Equal(t, http.StatusNotFound, w.Code) + assertCode(t, w, "NAME_UNKNOWN") +} + func TestManifest_ImageIndex_ChildValidation(t *testing.T) { reg := newTestRegistry() repo := "team/app" diff --git a/spinifex/gateway/ecr/registry_upload_test.go b/spinifex/gateway/ecr/registry_upload_test.go index 1d64199c4..cd537ff64 100644 --- a/spinifex/gateway/ecr/registry_upload_test.go +++ b/spinifex/gateway/ecr/registry_upload_test.go @@ -35,6 +35,7 @@ func TestFinishUpload_StoreFailure_CleansUp(t *testing.T) { store := &blobPutFailStore{MemoryObjectStore: mem} meta := ecr.NewMemoryMetaStore() reg := NewRegistry(store, meta, testAccount) + seedRepo(t, reg, "team/app") w := do(reg, http.MethodPost, "/v2/team/app/blobs/uploads/", nil, nil) require.Equal(t, http.StatusAccepted, w.Code) @@ -115,6 +116,7 @@ func TestPatchUpload_ConcurrentNoCorruption(t *testing.T) { for iter := range 25 { reg := newTestRegistry() repo := "team/race" + seedRepo(t, reg, repo) w := do(reg, http.MethodPost, "/v2/"+repo+"/blobs/uploads/", nil, nil) require.Equal(t, http.StatusAccepted, w.Code) diff --git a/spinifex/gateway/ecr_image.go b/spinifex/gateway/ecr_image.go index 0fb1c761c..e3fccbd68 100644 --- a/spinifex/gateway/ecr_image.go +++ b/spinifex/gateway/ecr_image.go @@ -388,6 +388,8 @@ func mapStoreManifestError(err error, repo string) error { return errors.New(awserrors.ErrorLayersNotFound) case "TAG_IMMUTABLE": return errors.New(awserrors.ErrorImageTagAlreadyExists) + case "NAME_UNKNOWN": + return errors.New(awserrors.ErrorRepositoryNotFound) default: return errors.New(awserrors.ErrorInvalidParameterValue) } diff --git a/spinifex/gateway/ecr_image_test.go b/spinifex/gateway/ecr_image_test.go index 4cd47e5f3..883adb1d9 100644 --- a/spinifex/gateway/ecr_image_test.go +++ b/spinifex/gateway/ecr_image_test.go @@ -26,10 +26,18 @@ func newImageGateway(t *testing.T) *GatewayConfig { return &GatewayConfig{ECRRegistry: reg, Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, DisableLogging: true} } +// seedGatewayRepo creates the repository metadata so push/PutImage handlers — +// which require an existing repository — accept writes. Idempotent. +func seedGatewayRepo(t *testing.T, gw *GatewayConfig, repo string) { + t.Helper() + require.NoError(t, gw.ECRRegistry.Meta.PutRepo(ecrTestAccount, handlers_ecr.RepoMeta{Name: repo})) +} + // seedTaggedImage stores a layerless manifest under repo:tag, returning its // digest. A unique manifest per tag yields a unique digest. func seedTaggedImage(t *testing.T, gw *GatewayConfig, repo, tag string) string { t.Helper() + seedGatewayRepo(t, gw, repo) manifest := fmt.Appendf(nil, `{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","layers":[],"annotations":{"tag":"%s"}}`, tag) digest, err := gw.ECRRegistry.StoreManifest(ecrTestAccount, repo, tag, "application/vnd.docker.distribution.manifest.v2+json", manifest) @@ -143,6 +151,7 @@ func TestBatchGetImage_CapExceeded(t *testing.T) { func TestPutImage_HappyAndMissingManifest(t *testing.T) { gw := newImageGateway(t) + seedGatewayRepo(t, gw, "team/app") manifest := `{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","layers":[]}` body, _ := json.Marshal(map[string]string{ "repositoryName": "team/app", @@ -171,6 +180,21 @@ func TestPutImage_HappyAndMissingManifest(t *testing.T) { assert.Equal(t, "InvalidParameterValue", err.Error()) } +// TestPutImage_RepoNotCreated asserts the JSON PutImage path rejects an uncreated +// repository with RepositoryNotFoundException (the AWS twin of OCI NAME_UNKNOWN). +func TestPutImage_RepoNotCreated(t *testing.T) { + gw := newImageGateway(t) + body, _ := json.Marshal(map[string]string{ + "repositoryName": "team/ghost", + "imageManifest": `{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","layers":[]}`, + "imageManifestMediaType": "application/vnd.docker.distribution.manifest.v2+json", + "imageTag": "v1", + }) + _, err := callImage(t, gw, (*GatewayConfig).handlePutImage, string(body)) + require.Error(t, err) + assert.Equal(t, "RepositoryNotFoundException", err.Error()) +} + func TestBatchDeleteImage_Partial(t *testing.T) { gw := newImageGateway(t) digest := seedTaggedImage(t, gw, "team/app", "v1") diff --git a/tests/e2e/ecr/crane_skopeo_test.go b/tests/e2e/ecr/crane_skopeo_test.go index 3eb5fb40a..6f224c7f2 100644 --- a/tests/e2e/ecr/crane_skopeo_test.go +++ b/tests/e2e/ecr/crane_skopeo_test.go @@ -79,6 +79,28 @@ func TestCranePushPull(t *testing.T) { require.NoErrorf(t, err, "crane pull: %s", out) } +// TestPushToUncreatedRepoRejected asserts the registry does not auto-create a +// repository on push: a crane append to a repo that was never created with +// CreateRepository must fail with NAME_UNKNOWN. +func TestPushToUncreatedRepoRejected(t *testing.T) { + f := requireECRFixture(t) + host := harness.ECRRegistryHost(f.Account) + harness.RequireRegistryResolves(t, host) + + dockerConfig := filepath.Join(f.TmpDir, "uncreated") + require.NoError(t, os.MkdirAll(dockerConfig, 0o700)) + crane := harness.NewCrane(t, dockerConfig) + craneLogin(t, crane, host, harness.ECRGetLoginPassword(t, f.AWS)) + + // Deliberately NOT created: no CreateECRRepository call. + ref := harness.ECRRepositoryURI(f.Account, uniqueRepo("uncreated")) + ":v1" + layer := writeLayerTar(t, f.TmpDir) + + out, err := crane.Run(ociCmdTimeout, "append", "-f", layer, "-t", ref) + require.Errorf(t, err, "push to uncreated repo should fail; got success: %s", out) + assert.Contains(t, out, "NAME_UNKNOWN", "expected NAME_UNKNOWN rejection, got: %s", out) +} + // TestSkopeoCopy seeds an image with crane, then `skopeo copy` retags it within // the registry — exercising skopeo's Bearer auth on both read and write. func TestSkopeoCopy(t *testing.T) {