From f438fa7be4eb97b289beff07a68d5c634e413c77 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 29 Aug 2023 01:01:05 -0600 Subject: [PATCH 1/8] Add SECURITY.md file (#579) Signed-off-by: Terry Howe --- SECURITY.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 SECURITY.md diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..ffefe341 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,3 @@ +# Security Policy + +Please follow the [security policy](https://oras.land/docs/community/reporting_security_concerns) to report a security vulnerability or concern. From 03ced8f0a9748f1e5584cb17c93af7412c630aea Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 05:44:33 +0000 Subject: [PATCH 2/8] build(deps): bump actions/checkout from 3 to 4 (#585) --- .github/workflows/build.yml | 2 +- .github/workflows/codeql-analysis.yml | 2 +- .github/workflows/license-checker.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 691d349e..a875a072 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -32,7 +32,7 @@ jobs: fail-fast: true steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Go ${{ matrix.go-version }} environment uses: actions/setup-go@v4 with: diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 2786ed71..10531148 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -39,7 +39,7 @@ jobs: fail-fast: false steps: - name: Checkout repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Go ${{ matrix.go-version }} environment uses: actions/setup-go@v4 with: diff --git a/.github/workflows/license-checker.yml b/.github/workflows/license-checker.yml index 8ff3d6d2..33c39831 100644 --- a/.github/workflows/license-checker.yml +++ b/.github/workflows/license-checker.yml @@ -32,7 +32,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Check license header uses: apache/skywalking-eyes/header@v0.5.0 with: From 688077c231d04f92adb7533ab8b047ad2cc0447c Mon Sep 17 00:00:00 2001 From: "Lixia (Sylvia) Lei" Date: Tue, 5 Sep 2023 22:11:18 +0800 Subject: [PATCH 3/8] build: bump go version (#587) 1. Update go mod version to `1.20` 2. Update go version matrix in workflow config files 3. Update README Resolves: #583 --------- Signed-off-by: Lixia (Sylvia) Lei --- .github/workflows/build.yml | 2 +- .github/workflows/codeql-analysis.yml | 2 +- README.md | 3 +++ go.mod | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a875a072..4e1eac42 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -28,7 +28,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go-version: ['1.19', '1.20'] + go-version: ['1.20', '1.21'] fail-fast: true steps: - name: Checkout diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 10531148..4c8067d4 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -35,7 +35,7 @@ jobs: security-events: write strategy: matrix: - go-version: ['1.19', '1.20'] + go-version: ['1.20', '1.21'] fail-fast: false steps: - name: Checkout repository diff --git a/README.md b/README.md index 50bb3064..70662555 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,9 @@ The ORAS Go library follows [Semantic Versioning](https://semver.org/), where br The version `2` is actively developed in the [`main`](https://github.com/oras-project/oras-go/tree/main) branch with all new features. +> [!Note] +> The `main` branch follows [Go's Security Policy](https://github.com/golang/go/security/policy) and supports the two latest versions of Go (currently `1.20` and `1.21`). + Examples for common use cases can be found below: - [Copy examples](https://pkg.go.dev/oras.land/oras-go/v2#pkg-examples) diff --git a/go.mod b/go.mod index 747de6eb..01ffb2bf 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module oras.land/oras-go/v2 -go 1.19 +go 1.20 require ( github.com/opencontainers/go-digest v1.0.0 From 9b2d3bdfa1805c102ebb2b9c22700f7c812eeec7 Mon Sep 17 00:00:00 2001 From: "Lixia (Sylvia) Lei" Date: Tue, 12 Sep 2023 10:02:21 +0800 Subject: [PATCH 4/8] fix: push an empty index when deleting the last referrer with SkipReferrersGC (#593) Fix: #592 Signed-off-by: Lixia (Sylvia) Lei --- registry/remote/repository.go | 39 +- registry/remote/repository_test.go | 597 ++++++++++++++++++++++++++--- 2 files changed, 572 insertions(+), 64 deletions(-) diff --git a/registry/remote/repository.go b/registry/remote/repository.go index 0f8c6acd..337b4115 100644 --- a/registry/remote/repository.go +++ b/registry/remote/repository.go @@ -1425,26 +1425,25 @@ func (s *manifestStore) indexReferrersForPush(ctx context.Context, desc ocispec. func (s *manifestStore) updateReferrersIndex(ctx context.Context, subject ocispec.Descriptor, change referrerChange) (err error) { referrersTag := buildReferrersTag(subject) - skipDelete := s.repo.SkipReferrersGC - var oldIndexDesc ocispec.Descriptor - var referrers []ocispec.Descriptor + var oldIndexDesc *ocispec.Descriptor + var oldReferrers []ocispec.Descriptor prepare := func() error { // 1. pull the original referrers list using the referrers tag schema - var err error - oldIndexDesc, referrers, err = s.repo.referrersFromIndex(ctx, referrersTag) + indexDesc, referrers, err := s.repo.referrersFromIndex(ctx, referrersTag) if err != nil { if errors.Is(err, errdef.ErrNotFound) { - // no old index found, skip delete - skipDelete = true + // valid case: no old referrers index return nil } return err } + oldIndexDesc = &indexDesc + oldReferrers = referrers return nil } update := func(referrerChanges []referrerChange) error { // 2. apply the referrer changes on the referrers list - updatedReferrers, err := applyReferrerChanges(referrers, referrerChanges) + updatedReferrers, err := applyReferrerChanges(oldReferrers, referrerChanges) if err != nil { if err == errNoReferrerUpdate { return nil @@ -1453,7 +1452,12 @@ func (s *manifestStore) updateReferrersIndex(ctx context.Context, subject ocispe } // 3. push the updated referrers list using referrers tag schema - if len(updatedReferrers) > 0 { + if len(updatedReferrers) > 0 || s.repo.SkipReferrersGC { + // push a new index in either case: + // 1. the referrers list has been updated with a non-zero size + // 2. OR the updated referrers list is empty but referrers GC + // is skipped, in this case an empty index should still be pushed + // as the old index won't get deleted newIndexDesc, newIndex, err := generateIndex(updatedReferrers) if err != nil { return fmt.Errorf("failed to generate referrers index for referrers tag %s: %w", referrersTag, err) @@ -1463,14 +1467,15 @@ func (s *manifestStore) updateReferrersIndex(ctx context.Context, subject ocispe } } - // 4. delete the dangling original referrers index - if !skipDelete { - if err := s.repo.delete(ctx, oldIndexDesc, true); err != nil { - return &ReferrersError{ - Op: opDeleteReferrersIndex, - Err: fmt.Errorf("failed to delete dangling referrers index %s for referrers tag %s: %w", oldIndexDesc.Digest.String(), referrersTag, err), - Subject: subject, - } + // 4. delete the dangling original referrers index, if applicable + if s.repo.SkipReferrersGC || oldIndexDesc == nil { + return nil + } + if err := s.repo.delete(ctx, *oldIndexDesc, true); err != nil { + return &ReferrersError{ + Op: opDeleteReferrersIndex, + Err: fmt.Errorf("failed to delete dangling referrers index %s for referrers tag %s: %w", oldIndexDesc.Digest.String(), referrersTag, err), + Subject: subject, } } return nil diff --git a/registry/remote/repository_test.go b/registry/remote/repository_test.go index fdd5e9ce..3e3ee61a 100644 --- a/registry/remote/repository_test.go +++ b/registry/remote/repository_test.go @@ -3589,7 +3589,7 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { artifactDesc.ArtifactType = artifact.ArtifactType artifactDesc.Annotations = artifact.Annotations - // test pushing artifact with subject + // test pushing artifact with subject, a referrer list should be created index_1 := ocispec.Index{ Versioned: specs.Versioned{ SchemaVersion: 2, // historical value. does not pertain to OCI or docker version @@ -3671,6 +3671,92 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) } + // test pushing artifact with subject when an old empty referrer list exists, + // the referrer list should be updated + emptyIndex := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, + MediaType: ocispec.MediaTypeImageIndex, + } + emptyIndexJSON, err := json.Marshal(emptyIndex) + if err != nil { + t.Error("failed to marshal index", err) + } + emptyIndexDesc := content.NewDescriptorFromBytes(emptyIndex.MediaType, emptyIndexJSON) + var indexDeleted bool + ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+artifactDesc.Digest.String(): + if contentType := r.Header.Get("Content-Type"); contentType != artifactDesc.MediaType { + w.WriteHeader(http.StatusBadRequest) + break + } + buf := bytes.NewBuffer(nil) + if _, err := buf.ReadFrom(r.Body); err != nil { + t.Errorf("fail to read: %v", err) + } + gotManifest = buf.Bytes() + w.Header().Set("Docker-Content-Digest", artifactDesc.Digest.String()) + w.WriteHeader(http.StatusCreated) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest: + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+referrersTag: + w.Write(emptyIndexJSON) + case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+referrersTag: + if contentType := r.Header.Get("Content-Type"); contentType != ocispec.MediaTypeImageIndex { + w.WriteHeader(http.StatusBadRequest) + break + } + buf := bytes.NewBuffer(nil) + if _, err := buf.ReadFrom(r.Body); err != nil { + t.Errorf("fail to read: %v", err) + } + gotReferrerIndex = buf.Bytes() + w.Header().Set("Docker-Content-Digest", indexDesc_1.Digest.String()) + w.WriteHeader(http.StatusCreated) + case r.Method == http.MethodDelete && r.URL.Path == "/v2/test/manifests/"+emptyIndexDesc.Digest.String(): + indexDeleted = true + // no "Docker-Content-Digest" header for manifest deletion + w.WriteHeader(http.StatusAccepted) + default: + t.Errorf("unexpected access: %s %s", r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + } + })) + defer ts.Close() + uri, err = url.Parse(ts.URL) + if err != nil { + t.Fatalf("invalid test http server: %v", err) + } + + ctx = context.Background() + repo, err = NewRepository(uri.Host + "/test") + if err != nil { + t.Fatalf("NewRepository() error = %v", err) + } + repo.PlainHTTP = true + + if state := repo.loadReferrersState(); state != referrersStateUnknown { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnknown) + } + err = repo.Push(ctx, artifactDesc, bytes.NewReader(artifactJSON)) + if err != nil { + t.Fatalf("Manifests.Push() error = %v", err) + } + if !bytes.Equal(gotManifest, artifactJSON) { + t.Errorf("Manifests.Push() = %v, want %v", string(gotManifest), string(artifactJSON)) + } + if !bytes.Equal(gotReferrerIndex, indexJSON_1) { + t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_1)) + } + if !indexDeleted { + t.Errorf("indexDeleted = %v, want %v", indexDeleted, true) + } + if state := repo.loadReferrersState(); state != referrersStateUnsupported { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) + } + // test pushing image manifest with subject, referrer list should be updated manifest := ocispec.Manifest{ MediaType: ocispec.MediaTypeImageManifest, @@ -3702,7 +3788,7 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { t.Fatalf("failed to marshal manifest: %v", err) } indexDesc_2 := content.NewDescriptorFromBytes(index_2.MediaType, indexJSON_2) - var manifestDeleted bool + indexDeleted = false ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+manifestDesc.Digest.String(): @@ -3734,7 +3820,7 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { w.Header().Set("Docker-Content-Digest", indexDesc_2.Digest.String()) w.WriteHeader(http.StatusCreated) case r.Method == http.MethodDelete && r.URL.Path == "/v2/test/manifests/"+indexDesc_1.Digest.String(): - manifestDeleted = true + indexDeleted = true // no "Docker-Content-Digest" header for manifest deletion w.WriteHeader(http.StatusAccepted) default: @@ -3767,14 +3853,14 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { if !bytes.Equal(gotReferrerIndex, indexJSON_2) { t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_2)) } - if !manifestDeleted { - t.Errorf("manifestDeleted = %v, want %v", manifestDeleted, true) + if !indexDeleted { + t.Errorf("indexDeleted = %v, want %v", indexDeleted, true) } if state := repo.loadReferrersState(); state != referrersStateUnsupported { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) } - // test pushing image manifest with subject without cleaning dangling referrers + // test pushing image manifest with subject again, referrers list should not be changed ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+manifestDesc.Digest.String(): @@ -3792,7 +3878,91 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { case r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest: w.WriteHeader(http.StatusNotFound) case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+referrersTag: - w.Write(indexJSON_1) + w.Write(indexJSON_2) + default: + t.Errorf("unexpected access: %s %s", r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + } + })) + defer ts.Close() + uri, err = url.Parse(ts.URL) + if err != nil { + t.Fatalf("invalid test http server: %v", err) + } + + ctx = context.Background() + repo, err = NewRepository(uri.Host + "/test") + if err != nil { + t.Fatalf("NewRepository() error = %v", err) + } + repo.PlainHTTP = true + if state := repo.loadReferrersState(); state != referrersStateUnknown { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnknown) + } + err = repo.Push(ctx, manifestDesc, bytes.NewReader(manifestJSON)) + if err != nil { + t.Fatalf("Manifests.Push() error = %v", err) + } + if !bytes.Equal(gotManifest, manifestJSON) { + t.Errorf("Manifests.Push() = %v, want %v", string(gotManifest), string(manifestJSON)) + } + // referrers list should not be changed + if !bytes.Equal(gotReferrerIndex, indexJSON_2) { + t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_2)) + } + if state := repo.loadReferrersState(); state != referrersStateUnsupported { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) + } + + // push image index with subject, referrer list should be updated + indexManifest := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Subject: &subjectDesc, + ArtifactType: "test/index", + Annotations: map[string]string{"foo": "bar"}, + } + indexManifestJSON, err := json.Marshal(indexManifest) + if err != nil { + t.Fatalf("failed to marshal manifest: %v", err) + } + indexManifestDesc := content.NewDescriptorFromBytes(indexManifest.MediaType, indexManifestJSON) + indexManifestDesc.ArtifactType = indexManifest.ArtifactType + indexManifestDesc.Annotations = indexManifest.Annotations + index_3 := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{ + artifactDesc, + manifestDesc, + indexManifestDesc, + }, + } + indexJSON_3, err := json.Marshal(index_3) + if err != nil { + t.Fatalf("failed to marshal manifest: %v", err) + } + indexDesc_3 := content.NewDescriptorFromBytes(index_3.MediaType, indexJSON_3) + indexDeleted = false + ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+indexManifestDesc.Digest.String(): + if contentType := r.Header.Get("Content-Type"); contentType != indexManifestDesc.MediaType { + w.WriteHeader(http.StatusBadRequest) + break + } + buf := bytes.NewBuffer(nil) + if _, err := buf.ReadFrom(r.Body); err != nil { + t.Errorf("fail to read: %v", err) + } + gotManifest = buf.Bytes() + w.Header().Set("Docker-Content-Digest", indexManifestDesc.Digest.String()) + w.WriteHeader(http.StatusCreated) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest: + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+referrersTag: + w.Write(indexJSON_2) case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+referrersTag: if contentType := r.Header.Get("Content-Type"); contentType != ocispec.MediaTypeImageIndex { w.WriteHeader(http.StatusBadRequest) @@ -3803,10 +3973,10 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { t.Errorf("fail to read: %v", err) } gotReferrerIndex = buf.Bytes() - w.Header().Set("Docker-Content-Digest", indexDesc_2.Digest.String()) + w.Header().Set("Docker-Content-Digest", indexDesc_3.Digest.String()) w.WriteHeader(http.StatusCreated) - case r.Method == http.MethodDelete && r.URL.Path == "/v2/test/manifests/"+indexDesc_1.Digest.String(): - manifestDeleted = true + case r.Method == http.MethodDelete && r.URL.Path == "/v2/test/manifests/"+indexDesc_2.Digest.String(): + indexDeleted = true // no "Docker-Content-Digest" header for manifest deletion w.WriteHeader(http.StatusAccepted) default: @@ -3826,11 +3996,117 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { t.Fatalf("NewRepository() error = %v", err) } repo.PlainHTTP = true + if state := repo.loadReferrersState(); state != referrersStateUnknown { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnknown) + } + err = repo.Push(ctx, indexManifestDesc, bytes.NewReader(indexManifestJSON)) + if err != nil { + t.Fatalf("Manifests.Push() error = %v", err) + } + if !bytes.Equal(gotManifest, indexManifestJSON) { + t.Errorf("Manifests.Push() = %v, want %v", string(gotManifest), string(indexManifestJSON)) + } + if !bytes.Equal(gotReferrerIndex, indexJSON_3) { + t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_3)) + } + if !indexDeleted { + t.Errorf("indexDeleted = %v, want %v", indexDeleted, true) + } + if state := repo.loadReferrersState(); state != referrersStateUnsupported { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) + } +} + +func Test_ManifestStore_Push_ReferrersAPIUnavailable_SkipReferrersGC(t *testing.T) { + // generate test content + subject := []byte(`{"layers":[]}`) + subjectDesc := content.NewDescriptorFromBytes(spec.MediaTypeArtifactManifest, subject) + referrersTag := strings.Replace(subjectDesc.Digest.String(), ":", "-", 1) + manifest := ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Config: ocispec.Descriptor{ + MediaType: "testconfig", + }, + Subject: &subjectDesc, + Annotations: map[string]string{"foo": "bar"}, + } + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatalf("failed to marshal manifest: %v", err) + } + manifestDesc := content.NewDescriptorFromBytes(manifest.MediaType, manifestJSON) + manifestDesc.ArtifactType = manifest.Config.MediaType + manifestDesc.Annotations = manifest.Annotations + index_1 := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{ + manifestDesc, + }, + } + + // test pushing image manifest with subject, a referrers list should be created + indexJSON_1, err := json.Marshal(index_1) + if err != nil { + t.Fatalf("failed to marshal manifest: %v", err) + } + indexDesc_1 := content.NewDescriptorFromBytes(index_1.MediaType, indexJSON_1) + var gotManifest []byte + var gotReferrerIndex []byte + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+manifestDesc.Digest.String(): + if contentType := r.Header.Get("Content-Type"); contentType != manifestDesc.MediaType { + w.WriteHeader(http.StatusBadRequest) + break + } + buf := bytes.NewBuffer(nil) + if _, err := buf.ReadFrom(r.Body); err != nil { + t.Errorf("fail to read: %v", err) + } + gotManifest = buf.Bytes() + w.Header().Set("Docker-Content-Digest", manifestDesc.Digest.String()) + w.WriteHeader(http.StatusCreated) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest: + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+referrersTag: + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+referrersTag: + if contentType := r.Header.Get("Content-Type"); contentType != ocispec.MediaTypeImageIndex { + w.WriteHeader(http.StatusBadRequest) + break + } + buf := bytes.NewBuffer(nil) + if _, err := buf.ReadFrom(r.Body); err != nil { + t.Errorf("fail to read: %v", err) + } + gotReferrerIndex = buf.Bytes() + w.Header().Set("Docker-Content-Digest", indexDesc_1.Digest.String()) + w.WriteHeader(http.StatusCreated) + default: + t.Errorf("unexpected access: %s %s", r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + } + })) + defer ts.Close() + uri, err := url.Parse(ts.URL) + if err != nil { + t.Fatalf("invalid test http server: %v", err) + } + + ctx := context.Background() + repo, err := NewRepository(uri.Host + "/test") + if err != nil { + t.Fatalf("NewRepository() error = %v", err) + } + repo.PlainHTTP = true repo.SkipReferrersGC = true + if state := repo.loadReferrersState(); state != referrersStateUnknown { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnknown) } - manifestDeleted = false err = repo.Push(ctx, manifestDesc, bytes.NewReader(manifestJSON)) if err != nil { t.Fatalf("Manifests.Push() error = %v", err) @@ -3838,17 +4114,25 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { if !bytes.Equal(gotManifest, manifestJSON) { t.Errorf("Manifests.Push() = %v, want %v", string(gotManifest), string(manifestJSON)) } - if !bytes.Equal(gotReferrerIndex, indexJSON_2) { - t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_2)) - } - if manifestDeleted { - t.Errorf("manifestDeleted = %v, want %v", manifestDeleted, false) + if !bytes.Equal(gotReferrerIndex, indexJSON_1) { + t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_1)) } if state := repo.loadReferrersState(); state != referrersStateUnsupported { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) } - // test pushing image manifest with subject again, referrers list should not be changed + // test pushing image manifest with subject when an old empty referrer list exists, + // the referrer list should be updated + emptyIndex := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, + MediaType: ocispec.MediaTypeImageIndex, + } + emptyIndexJSON, err := json.Marshal(emptyIndex) + if err != nil { + t.Error("failed to marshal index", err) + } ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+manifestDesc.Digest.String(): @@ -3860,13 +4144,25 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { if _, err := buf.ReadFrom(r.Body); err != nil { t.Errorf("fail to read: %v", err) } - gotManifest = buf.Bytes() - w.Header().Set("Docker-Content-Digest", manifestDesc.Digest.String()) + gotManifest = buf.Bytes() + w.Header().Set("Docker-Content-Digest", manifestDesc.Digest.String()) + w.WriteHeader(http.StatusCreated) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest: + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+referrersTag: + w.Write(emptyIndexJSON) + case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+referrersTag: + if contentType := r.Header.Get("Content-Type"); contentType != ocispec.MediaTypeImageIndex { + w.WriteHeader(http.StatusBadRequest) + break + } + buf := bytes.NewBuffer(nil) + if _, err := buf.ReadFrom(r.Body); err != nil { + t.Errorf("fail to read: %v", err) + } + gotReferrerIndex = buf.Bytes() + w.Header().Set("Docker-Content-Digest", indexDesc_1.Digest.String()) w.WriteHeader(http.StatusCreated) - case r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest: - w.WriteHeader(http.StatusNotFound) - case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+referrersTag: - w.Write(indexJSON_2) default: t.Errorf("unexpected access: %s %s", r.Method, r.URL) w.WriteHeader(http.StatusNotFound) @@ -3884,6 +4180,8 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { t.Fatalf("NewRepository() error = %v", err) } repo.PlainHTTP = true + repo.SkipReferrersGC = true + if state := repo.loadReferrersState(); state != referrersStateUnknown { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnknown) } @@ -3894,15 +4192,15 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { if !bytes.Equal(gotManifest, manifestJSON) { t.Errorf("Manifests.Push() = %v, want %v", string(gotManifest), string(manifestJSON)) } - // referrers list should not be changed - if !bytes.Equal(gotReferrerIndex, indexJSON_2) { - t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_2)) + if !bytes.Equal(gotReferrerIndex, indexJSON_1) { + t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_1)) } if state := repo.loadReferrersState(); state != referrersStateUnsupported { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) } - // push image index with subject, referrer list should be updated + // push image index with subject, referrer list should be updated, the old + // one should not be deleted indexManifest := ocispec.Index{ MediaType: ocispec.MediaTypeImageIndex, Subject: &subjectDesc, @@ -3916,23 +4214,21 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { indexManifestDesc := content.NewDescriptorFromBytes(indexManifest.MediaType, indexManifestJSON) indexManifestDesc.ArtifactType = indexManifest.ArtifactType indexManifestDesc.Annotations = indexManifest.Annotations - index_3 := ocispec.Index{ + index_2 := ocispec.Index{ Versioned: specs.Versioned{ SchemaVersion: 2, // historical value. does not pertain to OCI or docker version }, MediaType: ocispec.MediaTypeImageIndex, Manifests: []ocispec.Descriptor{ - artifactDesc, manifestDesc, indexManifestDesc, }, } - indexJSON_3, err := json.Marshal(index_3) + indexJSON_2, err := json.Marshal(index_2) if err != nil { t.Fatalf("failed to marshal manifest: %v", err) } - indexDesc_3 := content.NewDescriptorFromBytes(index_3.MediaType, indexJSON_3) - manifestDeleted = false + indexDesc_2 := content.NewDescriptorFromBytes(index_2.MediaType, indexJSON_2) ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+indexManifestDesc.Digest.String(): @@ -3950,7 +4246,7 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { case r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest: w.WriteHeader(http.StatusNotFound) case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+referrersTag: - w.Write(indexJSON_2) + w.Write(indexJSON_1) case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+referrersTag: if contentType := r.Header.Get("Content-Type"); contentType != ocispec.MediaTypeImageIndex { w.WriteHeader(http.StatusBadRequest) @@ -3961,12 +4257,8 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { t.Errorf("fail to read: %v", err) } gotReferrerIndex = buf.Bytes() - w.Header().Set("Docker-Content-Digest", indexDesc_3.Digest.String()) + w.Header().Set("Docker-Content-Digest", indexDesc_2.Digest.String()) w.WriteHeader(http.StatusCreated) - case r.Method == http.MethodDelete && r.URL.Path == "/v2/test/manifests/"+indexDesc_2.Digest.String(): - manifestDeleted = true - // no "Docker-Content-Digest" header for manifest deletion - w.WriteHeader(http.StatusAccepted) default: t.Errorf("unexpected access: %s %s", r.Method, r.URL) w.WriteHeader(http.StatusNotFound) @@ -3984,6 +4276,8 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { t.Fatalf("NewRepository() error = %v", err) } repo.PlainHTTP = true + repo.SkipReferrersGC = true + if state := repo.loadReferrersState(); state != referrersStateUnknown { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnknown) } @@ -3994,11 +4288,8 @@ func Test_ManifestStore_Push_ReferrersAPIUnavailable(t *testing.T) { if !bytes.Equal(gotManifest, indexManifestJSON) { t.Errorf("Manifests.Push() = %v, want %v", string(gotManifest), string(indexManifestJSON)) } - if !bytes.Equal(gotReferrerIndex, indexJSON_3) { - t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_3)) - } - if !manifestDeleted { - t.Errorf("manifestDeleted = %v, want %v", manifestDeleted, true) + if !bytes.Equal(gotReferrerIndex, indexJSON_2) { + t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_2)) } if state := repo.loadReferrersState(); state != referrersStateUnsupported { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) @@ -4317,7 +4608,6 @@ func Test_ManifestStore_Delete_ReferrersAPIUnavailable(t *testing.T) { } indexManifestDesc := content.NewDescriptorFromBytes(indexManifest.MediaType, indexManifestJSON) - // test deleting artifact with subject index_1 := ocispec.Index{ Versioned: specs.Versioned{ SchemaVersion: 2, // historical value. does not pertain to OCI or docker version @@ -4364,6 +4654,7 @@ func Test_ManifestStore_Delete_ReferrersAPIUnavailable(t *testing.T) { } indexDesc_3 := content.NewDescriptorFromBytes(index_3.MediaType, indexJSON_3) + // test deleting artifact with subject, referrers list should be updated manifestDeleted := false indexDeleted := false var gotReferrerIndex []byte @@ -4422,7 +4713,6 @@ func Test_ManifestStore_Delete_ReferrersAPIUnavailable(t *testing.T) { store := repo.Manifests() ctx := context.Background() - // test deleting artifact with subject if state := repo.loadReferrersState(); state != referrersStateUnknown { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnknown) } @@ -4443,7 +4733,7 @@ func Test_ManifestStore_Delete_ReferrersAPIUnavailable(t *testing.T) { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) } - // test deleting manifest with subject + // test deleting manifest with subject, referrers list should be updated manifestDeleted = false indexDeleted = false ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -4517,7 +4807,7 @@ func Test_ManifestStore_Delete_ReferrersAPIUnavailable(t *testing.T) { t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) } - // test deleting index with a subject + // test deleting index with a subject, referrers list should be updated manifestDeleted = false indexDeleted = false ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -4580,6 +4870,219 @@ func Test_ManifestStore_Delete_ReferrersAPIUnavailable(t *testing.T) { } } +func Test_ManifestStore_Delete_ReferrersAPIUnavailable_SkipReferrersGC(t *testing.T) { + // generate test content + subject := []byte(`{"layers":[]}`) + subjectDesc := content.NewDescriptorFromBytes(spec.MediaTypeArtifactManifest, subject) + referrersTag := strings.Replace(subjectDesc.Digest.String(), ":", "-", 1) + + manifest := ocispec.Manifest{ + MediaType: ocispec.MediaTypeImageManifest, + Subject: &subjectDesc, + } + manifestJSON, err := json.Marshal(manifest) + if err != nil { + t.Fatalf("failed to marshal manifest: %v", err) + } + manifestDesc := content.NewDescriptorFromBytes(manifest.MediaType, manifestJSON) + + indexManifest := ocispec.Index{ + MediaType: ocispec.MediaTypeImageIndex, + Subject: &subjectDesc, + } + indexManifestJSON, err := json.Marshal(indexManifest) + if err != nil { + t.Fatalf("failed to marshal manifest: %v", err) + } + indexManifestDesc := content.NewDescriptorFromBytes(indexManifest.MediaType, indexManifestJSON) + + index_1 := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{ + manifestDesc, + indexManifestDesc, + }, + } + indexJSON_1, err := json.Marshal(index_1) + if err != nil { + t.Fatalf("failed to marshal manifest: %v", err) + } + index_2 := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{ + indexManifestDesc, + }, + } + indexJSON_2, err := json.Marshal(index_2) + if err != nil { + t.Fatalf("failed to marshal manifest: %v", err) + } + indexDesc_2 := content.NewDescriptorFromBytes(index_2.MediaType, indexJSON_2) + index_3 := ocispec.Index{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, + MediaType: ocispec.MediaTypeImageIndex, + Manifests: []ocispec.Descriptor{}, + } + indexJSON_3, err := json.Marshal(index_3) + if err != nil { + t.Fatalf("failed to marshal manifest: %v", err) + } + indexDesc_3 := content.NewDescriptorFromBytes(index_3.MediaType, indexJSON_3) + + // test deleting image manifest with subject, referrers list should be updated, + // the old one should not be deleted + manifestDeleted := false + var gotReferrerIndex []byte + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodDelete && r.URL.Path == "/v2/test/manifests/"+manifestDesc.Digest.String(): + manifestDeleted = true + // no "Docker-Content-Digest" header for manifest deletion + w.WriteHeader(http.StatusAccepted) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+manifestDesc.Digest.String(): + if accept := r.Header.Get("Accept"); !strings.Contains(accept, manifestDesc.MediaType) { + t.Errorf("manifest not convertable: %s", accept) + w.WriteHeader(http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", manifestDesc.MediaType) + w.Header().Set("Docker-Content-Digest", manifestDesc.Digest.String()) + if _, err := w.Write(manifestJSON); err != nil { + t.Errorf("failed to write %q: %v", r.URL, err) + } + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest: + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+referrersTag: + w.Write(indexJSON_1) + case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+referrersTag: + if contentType := r.Header.Get("Content-Type"); contentType != ocispec.MediaTypeImageIndex { + w.WriteHeader(http.StatusBadRequest) + break + } + buf := bytes.NewBuffer(nil) + if _, err := buf.ReadFrom(r.Body); err != nil { + t.Errorf("fail to read: %v", err) + } + gotReferrerIndex = buf.Bytes() + w.Header().Set("Docker-Content-Digest", indexDesc_2.Digest.String()) + w.WriteHeader(http.StatusCreated) + default: + t.Errorf("unexpected access: %s %s", r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + } + })) + defer ts.Close() + uri, err := url.Parse(ts.URL) + if err != nil { + t.Fatalf("invalid test http server: %v", err) + } + repo, err := NewRepository(uri.Host + "/test") + if err != nil { + t.Fatalf("NewRepository() error = %v", err) + } + repo.PlainHTTP = true + repo.SkipReferrersGC = true + store := repo.Manifests() + ctx := context.Background() + + if state := repo.loadReferrersState(); state != referrersStateUnknown { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnknown) + } + err = store.Delete(ctx, manifestDesc) + if err != nil { + t.Fatalf("Manifests.Delete() error = %v", err) + } + if !manifestDeleted { + t.Errorf("Manifests.Delete() = %v, want %v", manifestDeleted, true) + } + if !bytes.Equal(gotReferrerIndex, indexJSON_2) { + t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_2)) + } + if state := repo.loadReferrersState(); state != referrersStateUnsupported { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) + } + + // test deleting index with a subject, referrers list should be updated, + // the old one should not be deleted, an empty one should be pushed + manifestDeleted = false + ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodDelete && r.URL.Path == "/v2/test/manifests/"+indexManifestDesc.Digest.String(): + manifestDeleted = true + // no "Docker-Content-Digest" header for manifest deletion + w.WriteHeader(http.StatusAccepted) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+indexManifestDesc.Digest.String(): + if accept := r.Header.Get("Accept"); !strings.Contains(accept, indexManifestDesc.MediaType) { + t.Errorf("manifest not convertable: %s", accept) + w.WriteHeader(http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", indexManifestDesc.MediaType) + w.Header().Set("Docker-Content-Digest", indexManifestDesc.Digest.String()) + if _, err := w.Write(indexManifestJSON); err != nil { + t.Errorf("failed to write %q: %v", r.URL, err) + } + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/referrers/"+zeroDigest: + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodGet && r.URL.Path == "/v2/test/manifests/"+referrersTag: + w.Write(indexJSON_2) + case r.Method == http.MethodPut && r.URL.Path == "/v2/test/manifests/"+referrersTag: + if contentType := r.Header.Get("Content-Type"); contentType != ocispec.MediaTypeImageIndex { + w.WriteHeader(http.StatusBadRequest) + break + } + buf := bytes.NewBuffer(nil) + if _, err := buf.ReadFrom(r.Body); err != nil { + t.Errorf("fail to read: %v", err) + } + gotReferrerIndex = buf.Bytes() + w.Header().Set("Docker-Content-Digest", indexDesc_3.Digest.String()) + w.WriteHeader(http.StatusCreated) + default: + t.Errorf("unexpected access: %s %s", r.Method, r.URL) + w.WriteHeader(http.StatusNotFound) + } + })) + defer ts.Close() + uri, err = url.Parse(ts.URL) + if err != nil { + t.Fatalf("invalid test http server: %v", err) + } + repo, err = NewRepository(uri.Host + "/test") + if err != nil { + t.Fatalf("NewRepository() error = %v", err) + } + repo.PlainHTTP = true + repo.SkipReferrersGC = true + store = repo.Manifests() + ctx = context.Background() + + if state := repo.loadReferrersState(); state != referrersStateUnknown { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnknown) + } + err = store.Delete(ctx, indexManifestDesc) + if err != nil { + t.Fatalf("Manifests.Delete() error = %v", err) + } + if !manifestDeleted { + t.Errorf("Manifests.Delete() = %v, want %v", manifestDeleted, true) + } + if !bytes.Equal(gotReferrerIndex, indexJSON_3) { + t.Errorf("got referrers index = %v, want %v", string(gotReferrerIndex), string(indexJSON_3)) + } + if state := repo.loadReferrersState(); state != referrersStateUnsupported { + t.Errorf("Repository.loadReferrersState() = %v, want %v", state, referrersStateUnsupported) + } +} + func Test_ManifestStore_Delete_ReferrersAPIUnavailable_InconsistentIndex(t *testing.T) { // generate test content subject := []byte(`{"layers":[]}`) From 5b8d3d94b0bf21259f0a63e07f5398b555f98745 Mon Sep 17 00:00:00 2001 From: Kyle Tarplee Date: Mon, 25 Sep 2023 22:43:43 -0400 Subject: [PATCH 5/8] fix: Avoid a copy of sync.Mutex in Repository (#603) Closes #600 Signed-off-by: Kyle M. Tarplee --- registry/remote/repository.go | 32 +++++++++++++++++++----------- registry/remote/repository_test.go | 21 ++++++++++++++++++++ 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/registry/remote/repository.go b/registry/remote/repository.go index 337b4115..fc4f6bf6 100644 --- a/registry/remote/repository.go +++ b/registry/remote/repository.go @@ -147,7 +147,7 @@ type Repository struct { // - https://www.rfc-editor.org/rfc/rfc7234#section-5.5 HandleWarning func(warning Warning) - // NOTE: Must keep fields in sync with newRepositoryWithOptions function. + // NOTE: Must keep fields in sync with clone(). // referrersState represents that if the repository supports Referrers API. // default: referrersStateUnknown @@ -186,16 +186,24 @@ func newRepositoryWithOptions(ref registry.Reference, opts *RepositoryOptions) ( if err := ref.ValidateRepository(); err != nil { return nil, err } + repo := (*Repository)(opts).clone() + repo.Reference = ref + return repo, nil +} + +// clone makes a copy of the Repository being careful not to copy non-copyable fields (sync.Mutex and syncutil.Pool types) +func (r *Repository) clone() *Repository { return &Repository{ - Client: opts.Client, - Reference: ref, - PlainHTTP: opts.PlainHTTP, - SkipReferrersGC: opts.SkipReferrersGC, - ManifestMediaTypes: slices.Clone(opts.ManifestMediaTypes), - TagListPageSize: opts.TagListPageSize, - ReferrerListPageSize: opts.ReferrerListPageSize, - MaxMetadataBytes: opts.MaxMetadataBytes, - }, nil + Client: r.Client, + Reference: r.Reference, + PlainHTTP: r.PlainHTTP, + ManifestMediaTypes: slices.Clone(r.ManifestMediaTypes), + TagListPageSize: r.TagListPageSize, + ReferrerListPageSize: r.ReferrerListPageSize, + MaxMetadataBytes: r.MaxMetadataBytes, + SkipReferrersGC: r.SkipReferrersGC, + HandleWarning: r.HandleWarning, + } } // SetReferrersCapability indicates the Referrers API capability of the remote @@ -803,10 +811,10 @@ func (s *blobStore) Mount(ctx context.Context, desc ocispec.Descriptor, fromRepo // sibling returns a blob store for another repository in the same // registry. func (s *blobStore) sibling(otherRepoName string) *blobStore { - otherRepo := *s.repo + otherRepo := s.repo.clone() otherRepo.Reference.Repository = otherRepoName return &blobStore{ - repo: &otherRepo, + repo: otherRepo, } } diff --git a/registry/remote/repository_test.go b/registry/remote/repository_test.go index 3e3ee61a..b6772cbd 100644 --- a/registry/remote/repository_test.go +++ b/registry/remote/repository_test.go @@ -7447,3 +7447,24 @@ func TestRepository_do(t *testing.T) { t.Errorf("Repository.do() = %v, want %v", gotWarnings, wantWarnings) } } + +func TestRepository_clone(t *testing.T) { + repo, err := NewRepository("localhost:1234/repo/image") + if err != nil { + t.Fatalf("invalid repository: %v", err) + } + + crepo := repo.clone() + + if repo.Reference != crepo.Reference { + t.Fatal("references should be the same") + } + + if !reflect.DeepEqual(&repo.referrersPingLock, &crepo.referrersPingLock) { + t.Fatal("referrersPingLock should be different") + } + + if !reflect.DeepEqual(&repo.referrersMergePool, &crepo.referrersMergePool) { + t.Fatal("referrersMergePool should be different") + } +} From 177d7409f2e0056fecf7c1260d63fa3711088180 Mon Sep 17 00:00:00 2001 From: "Lixia (Sylvia) Lei" Date: Tue, 26 Sep 2023 18:48:40 +0800 Subject: [PATCH 6/8] fix: correctly handle OnCopySkipped (#609) Fix: #552 Signed-off-by: Lixia (Sylvia) Lei --- copy.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/copy.go b/copy.go index e55312dd..ca0f5d6f 100644 --- a/copy.go +++ b/copy.go @@ -393,18 +393,26 @@ func prepareCopy(ctx context.Context, dst Target, dstRef string, proxy *cas.Prox onCopySkipped := opts.OnCopySkipped opts.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error { - if onCopySkipped != nil { - if err := onCopySkipped(ctx, desc); err != nil { - return err - } - } if !content.Equal(desc, root) { + if onCopySkipped != nil { + return onCopySkipped(ctx, desc) + } return nil } - // enforce tagging when root is skipped + + // enforce tagging when the skipped node is root if refPusher, ok := dst.(registry.ReferencePusher); ok { + // NOTE: refPusher tags the node by copying it with the reference, + // so onCopySkipped shouldn't be invoked in this case return copyCachedNodeWithReference(ctx, proxy, refPusher, desc, dstRef) } + + // invoke onCopySkipped before tagging + if onCopySkipped != nil { + if err := onCopySkipped(ctx, desc); err != nil { + return err + } + } return dst.Tag(ctx, root, dstRef) } From 20b1a4871c2789b50bd4eef4a75ae280e3976253 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 8 Oct 2023 01:55:57 +0000 Subject: [PATCH 7/8] build(deps): bump golang.org/x/sync from 0.3.0 to 0.4.0 (#611) --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 01ffb2bf..df53ceb2 100644 --- a/go.mod +++ b/go.mod @@ -5,5 +5,5 @@ go 1.20 require ( github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.0-rc4 - golang.org/x/sync v0.3.0 + golang.org/x/sync v0.4.0 ) diff --git a/go.sum b/go.sum index e391513b..ee56d5ac 100644 --- a/go.sum +++ b/go.sum @@ -2,5 +2,5 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8 github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.0-rc4 h1:oOxKUJWnFC4YGHCCMNql1x4YaDfYBTS5Y4x/Cgeo1E0= github.com/opencontainers/image-spec v1.1.0-rc4/go.mod h1:X4pATf0uXsnn3g5aiGIsVnJBR4mxhKzfwmvK/B2NTm8= -golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E= -golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= +golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ= +golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= From 8cc5c30fbe7b744d5930449780ea7bc3def536a7 Mon Sep 17 00:00:00 2001 From: "Lixia (Sylvia) Lei" Date: Thu, 12 Oct 2023 14:59:39 +0800 Subject: [PATCH 8/8] build: bump `github.com/opencontainers/image-spec` to `v1.1.0-rc5` (#615) 1. Bump `github.com/opencontainers/image-spec` to `v1.1.0-rc5` in go mod 2. Replace "index.json" and "blobs" with corresponding constants in the spec 3. Fix testable examples that broke by the spec change - `MediaType` became a required field in `Descriptor` Resolve: #599 Signed-off-by: Lixia (Sylvia) Lei --- content/oci/oci.go | 14 ++------------ content/oci/oci_test.go | 6 +++--- content/oci/readonlyoci.go | 2 +- content/oci/readonlyoci_test.go | 14 +++++++------- content/oci/readonlystorage.go | 2 +- content/oci/readonlystorage_test.go | 6 +++--- content/oci/storage_test.go | 2 +- go.mod | 2 +- go.sum | 4 ++-- registry/remote/example_test.go | 26 +++++++++++++++++--------- 10 files changed, 38 insertions(+), 40 deletions(-) diff --git a/content/oci/oci.go b/content/oci/oci.go index 97ba6de5..27afde16 100644 --- a/content/oci/oci.go +++ b/content/oci/oci.go @@ -38,16 +38,6 @@ import ( "oras.land/oras-go/v2/internal/resolver" ) -// ociImageIndexFile is the file name of the index -// from the OCI Image Layout Specification. -// Reference: https://github.com/opencontainers/image-spec/blob/v1.1.0-rc4/image-layout.md#indexjson-file -const ociImageIndexFile = "index.json" - -// ociBlobsDir is the name of the blobs directory -// from the OCI Image Layout Specification. -// Reference: https://github.com/opencontainers/image-spec/blob/v1.1.0-rc4/image-layout.md#content -const ociBlobsDir = "blobs" - // Store implements `oras.Target`, and represents a content store // based on file system with the OCI-Image layout. // Reference: https://github.com/opencontainers/image-spec/blob/v1.1.0-rc4/image-layout.md @@ -89,13 +79,13 @@ func NewWithContext(ctx context.Context, root string) (*Store, error) { store := &Store{ AutoSaveIndex: true, root: rootAbs, - indexPath: filepath.Join(rootAbs, ociImageIndexFile), + indexPath: filepath.Join(rootAbs, ocispec.ImageIndexFile), storage: storage, tagResolver: resolver.NewMemory(), graph: graph.NewMemory(), } - if err := ensureDir(filepath.Join(rootAbs, ociBlobsDir)); err != nil { + if err := ensureDir(filepath.Join(rootAbs, ocispec.ImageBlobsDir)); err != nil { return nil, err } if err := store.ensureOCILayoutFile(); err != nil { diff --git a/content/oci/oci_test.go b/content/oci/oci_test.go index 931dabf5..43e57f0a 100644 --- a/content/oci/oci_test.go +++ b/content/oci/oci_test.go @@ -109,7 +109,7 @@ func TestStore_Success(t *testing.T) { } // validate index.json - indexFilePath := filepath.Join(tempDir, ociImageIndexFile) + indexFilePath := filepath.Join(tempDir, "index.json") indexFile, err := os.Open(indexFilePath) if err != nil { t.Errorf("error opening layout file, error = %v", err) @@ -361,7 +361,7 @@ func TestStore_NotExistingRoot(t *testing.T) { } // validate index.json - indexFilePath := filepath.Join(root, ociImageIndexFile) + indexFilePath := filepath.Join(root, "index.json") indexFile, err := os.Open(indexFilePath) if err != nil { t.Errorf("error opening layout file, error = %v", err) @@ -930,7 +930,7 @@ func TestStore_TagByDigest(t *testing.T) { func TestStore_BadIndex(t *testing.T) { tempDir := t.TempDir() content := []byte("whatever") - path := filepath.Join(tempDir, ociImageIndexFile) + path := filepath.Join(tempDir, "index.json") os.WriteFile(path, content, 0666) _, err := New(tempDir) diff --git a/content/oci/readonlyoci.go b/content/oci/readonlyoci.go index eb94f61c..66ca54c9 100644 --- a/content/oci/readonlyoci.go +++ b/content/oci/readonlyoci.go @@ -154,7 +154,7 @@ func validateOCILayout(layout *ocispec.ImageLayout) error { // loadIndexFile reads index.json from s.fsys. func (s *ReadOnlyStore) loadIndexFile(ctx context.Context) error { - indexFile, err := s.fsys.Open(ociImageIndexFile) + indexFile, err := s.fsys.Open(ocispec.ImageIndexFile) if err != nil { return fmt.Errorf("failed to open index file: %w", err) } diff --git a/content/oci/readonlyoci_test.go b/content/oci/readonlyoci_test.go index fdc0cde3..d7b4fd0e 100644 --- a/content/oci/readonlyoci_test.go +++ b/content/oci/readonlyoci_test.go @@ -123,11 +123,11 @@ func TestReadOnlyStore(t *testing.T) { // build fs fsys := fstest.MapFS{} for i, desc := range descs { - path := strings.Join([]string{ociBlobsDir, desc.Digest.Algorithm().String(), desc.Digest.Encoded()}, "/") + path := strings.Join([]string{"blobs", desc.Digest.Algorithm().String(), desc.Digest.Encoded()}, "/") fsys[path] = &fstest.MapFile{Data: blobs[i]} } fsys[ocispec.ImageLayoutFile] = &fstest.MapFile{Data: layoutJSON} - fsys[ociImageIndexFile] = &fstest.MapFile{Data: indexJSON} + fsys["index.json"] = &fstest.MapFile{Data: indexJSON} // test read-only store ctx := context.Background() @@ -507,7 +507,7 @@ func TestReadOnlyStore_TarFS(t *testing.T) { func TestReadOnlyStore_BadIndex(t *testing.T) { content := []byte("whatever") fsys := fstest.MapFS{ - ociImageIndexFile: &fstest.MapFile{Data: content}, + "index.json": &fstest.MapFile{Data: content}, } ctx := context.Background() @@ -603,11 +603,11 @@ func TestReadOnlyStore_Copy_OCIToMemory(t *testing.T) { // build fs fsys := fstest.MapFS{} for i, desc := range descs { - path := strings.Join([]string{ociBlobsDir, desc.Digest.Algorithm().String(), desc.Digest.Encoded()}, "/") + path := strings.Join([]string{"blobs", desc.Digest.Algorithm().String(), desc.Digest.Encoded()}, "/") fsys[path] = &fstest.MapFile{Data: blobs[i]} } fsys[ocispec.ImageLayoutFile] = &fstest.MapFile{Data: layoutJSON} - fsys[ociImageIndexFile] = &fstest.MapFile{Data: indexJSON} + fsys["index.json"] = &fstest.MapFile{Data: indexJSON} // test read-only store ctx := context.Background() @@ -717,11 +717,11 @@ func TestReadOnlyStore_Tags(t *testing.T) { // build fs fsys := fstest.MapFS{} for i, desc := range descs { - path := strings.Join([]string{ociBlobsDir, desc.Digest.Algorithm().String(), desc.Digest.Encoded()}, "/") + path := strings.Join([]string{"blobs", desc.Digest.Algorithm().String(), desc.Digest.Encoded()}, "/") fsys[path] = &fstest.MapFile{Data: blobs[i]} } fsys[ocispec.ImageLayoutFile] = &fstest.MapFile{Data: layoutJSON} - fsys[ociImageIndexFile] = &fstest.MapFile{Data: indexJSON} + fsys["index.json"] = &fstest.MapFile{Data: indexJSON} // test read-only store ctx := context.Background() diff --git a/content/oci/readonlystorage.go b/content/oci/readonlystorage.go index b5e53039..6e319a64 100644 --- a/content/oci/readonlystorage.go +++ b/content/oci/readonlystorage.go @@ -95,5 +95,5 @@ func blobPath(dgst digest.Digest) (string, error) { return "", fmt.Errorf("cannot calculate blob path from invalid digest %s: %w: %v", dgst.String(), errdef.ErrInvalidDigest, err) } - return path.Join(ociBlobsDir, dgst.Algorithm().String(), dgst.Encoded()), nil + return path.Join(ocispec.ImageBlobsDir, dgst.Algorithm().String(), dgst.Encoded()), nil } diff --git a/content/oci/readonlystorage_test.go b/content/oci/readonlystorage_test.go index d7b4f176..b1d1906a 100644 --- a/content/oci/readonlystorage_test.go +++ b/content/oci/readonlystorage_test.go @@ -38,7 +38,7 @@ func TestReadOnlyStorage_Exists(t *testing.T) { dgst := digest.FromBytes(blob) desc := content.NewDescriptorFromBytes("", blob) fsys := fstest.MapFS{ - strings.Join([]string{ociBlobsDir, dgst.Algorithm().String(), dgst.Encoded()}, "/"): {}, + strings.Join([]string{"blobs", dgst.Algorithm().String(), dgst.Encoded()}, "/"): {}, } s := NewStorageFromFS(fsys) ctx := context.Background() @@ -76,7 +76,7 @@ func TestReadOnlyStorage_Fetch(t *testing.T) { dgst := digest.FromBytes(blob) desc := content.NewDescriptorFromBytes("", blob) fsys := fstest.MapFS{ - strings.Join([]string{ociBlobsDir, dgst.Algorithm().String(), dgst.Encoded()}, "/"): { + strings.Join([]string{"blobs", dgst.Algorithm().String(), dgst.Encoded()}, "/"): { Data: blob, }, } @@ -123,7 +123,7 @@ func TestReadOnlyStorage_DirFS(t *testing.T) { dgst := digest.FromBytes(blob) desc := content.NewDescriptorFromBytes("test", blob) // write blob to disk - path := filepath.Join(tempDir, ociBlobsDir, dgst.Algorithm().String(), dgst.Encoded()) + path := filepath.Join(tempDir, "blobs", dgst.Algorithm().String(), dgst.Encoded()) if err := os.MkdirAll(filepath.Dir(path), 0777); err != nil { t.Fatal("error calling Mkdir(), error =", err) } diff --git a/content/oci/storage_test.go b/content/oci/storage_test.go index ac6ae8dd..7e3b1e58 100644 --- a/content/oci/storage_test.go +++ b/content/oci/storage_test.go @@ -289,7 +289,7 @@ func TestStorage_Fetch_ExistingBlobs(t *testing.T) { } tempDir := t.TempDir() - path := filepath.Join(tempDir, ociBlobsDir, dgst.Algorithm().String(), dgst.Encoded()) + path := filepath.Join(tempDir, "blobs", dgst.Algorithm().String(), dgst.Encoded()) if err := os.MkdirAll(filepath.Dir(path), 0777); err != nil { t.Fatal("error calling Mkdir(), error =", err) } diff --git a/go.mod b/go.mod index df53ceb2..57c4c93d 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,6 @@ go 1.20 require ( github.com/opencontainers/go-digest v1.0.0 - github.com/opencontainers/image-spec v1.1.0-rc4 + github.com/opencontainers/image-spec v1.1.0-rc5 golang.org/x/sync v0.4.0 ) diff --git a/go.sum b/go.sum index ee56d5ac..41b71e0d 100644 --- a/go.sum +++ b/go.sum @@ -1,6 +1,6 @@ github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= -github.com/opencontainers/image-spec v1.1.0-rc4 h1:oOxKUJWnFC4YGHCCMNql1x4YaDfYBTS5Y4x/Cgeo1E0= -github.com/opencontainers/image-spec v1.1.0-rc4/go.mod h1:X4pATf0uXsnn3g5aiGIsVnJBR4mxhKzfwmvK/B2NTm8= +github.com/opencontainers/image-spec v1.1.0-rc5 h1:Ygwkfw9bpDvs+c9E34SdgGOj41dX/cbdlwvlWt0pnFI= +github.com/opencontainers/image-spec v1.1.0-rc5/go.mod h1:X4pATf0uXsnn3g5aiGIsVnJBR4mxhKzfwmvK/B2NTm8= golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ= golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= diff --git a/registry/remote/example_test.go b/registry/remote/example_test.go index 35502a94..d2fb529d 100644 --- a/registry/remote/example_test.go +++ b/registry/remote/example_test.go @@ -40,18 +40,20 @@ import ( ) const ( - exampleRepositoryName = "example" - exampleTag = "latest" - exampleConfig = "Example config content" - exampleLayer = "Example layer content" - exampleUploadUUid = "0bc84d80-837c-41d9-824e-1907463c53b3" - ManifestDigest = "sha256:0b696106ecd0654e031f19e0a8cbd1aee4ad457d7c9cea881f07b12a930cd307" - ReferenceManifestDigest = "sha256:6983f495f7ee70d43e571657ae8b39ca3d3ca1b0e77270fd4fbddfb19832a1cf" + _ = ExampleUnplayable + exampleRepositoryName = "example" + exampleTag = "latest" + exampleConfig = "Example config content" + exampleLayer = "Example layer content" + exampleUploadUUid = "0bc84d80-837c-41d9-824e-1907463c53b3" + // For ExampleRepository_Push_artifactReferenceManifest: + ManifestDigest = "sha256:a3f9d449466b9b7194c3a76ca4890d792e11eb4e62e59aa8b4c3cce0a56f129d" + ReferenceManifestDigest = "sha256:2d30397701742b04550891851529abe6b071e4fae920a91897d34612662a3bf6" + // For Example_pushAndIgnoreReferrersIndexError: referrersAPIUnavailableRepositoryName = "no-referrers-api" - referrerDigest = "sha256:21c623eb8ccd273f2702efd74a0abb455dd06a99987f413c2114fb00961ebfe7" + referrerDigest = "sha256:4caba1e18385eb152bd92e9fee1dc01e47c436e594123b3c2833acfcad9883e2" referrersTag = "sha256-c824a9aa7d2e3471306648c6d4baa1abbcb97ff0276181ab4722ca27127cdba0" referrerIndexDigest = "sha256:7baac5147dd58d56fdbaad5a888fa919235a3a90cb71aaa8b56ee5d19f4cd838" - _ = ExampleUnplayable ) var ( @@ -107,8 +109,10 @@ var ( Size: int64(len(exampleManifestWithBlobs))} subjectDescriptor = content.NewDescriptorFromBytes(ocispec.MediaTypeImageManifest, []byte(`{"layers":[]}`)) referrerManifestContent, _ = json.Marshal(ocispec.Manifest{ + Versioned: specs.Versioned{SchemaVersion: 2}, MediaType: ocispec.MediaTypeImageManifest, Subject: &subjectDescriptor, + Config: ocispec.DescriptorEmptyJSON, }) referrerDescriptor = content.NewDescriptorFromBytes(ocispec.MediaTypeImageManifest, referrerManifestContent) referrerIndex, _ = json.Marshal(ocispec.Index{ @@ -304,7 +308,11 @@ func ExampleRepository_Push_artifactReferenceManifest() { // 1. assemble the referenced artifact manifest manifest := ocispec.Manifest{ + Versioned: specs.Versioned{ + SchemaVersion: 2, // historical value. does not pertain to OCI or docker version + }, MediaType: ocispec.MediaTypeImageManifest, + Config: content.NewDescriptorFromBytes(ocispec.MediaTypeImageConfig, []byte("config bytes")), } manifestContent, err := json.Marshal(manifest) if err != nil {