From 3ae8eda440e1e5174d5708c70ddb8ac503283caf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Vl=C3=A9rick?= Date: Wed, 11 Oct 2023 08:57:09 +0200 Subject: [PATCH] review comments addressed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Vlérick --- oci/layout/fixtures/manifest/index.json | 18 +++- oci/layout/oci_delete.go | 10 +-- oci/layout/oci_transport_test.go | 104 ++++++++++-------------- 3 files changed, 65 insertions(+), 67 deletions(-) diff --git a/oci/layout/fixtures/manifest/index.json b/oci/layout/fixtures/manifest/index.json index fd6930cf1c..7e779082ce 100644 --- a/oci/layout/fixtures/manifest/index.json +++ b/oci/layout/fixtures/manifest/index.json @@ -1 +1,17 @@ -{"schemaVersion":2,"manifests":[{"mediaType":"application/vnd.oci.image.manifest.v1+json","digest":"sha256:84afb6189c4d69f2d040c5f1dc4e0a16fed9b539ce9cfb4ac2526ae4e0576cc0","size":496,"annotations":{"org.opencontainers.image.ref.name":"v0.1.1"},"platform":{"architecture":"amd64","os":"linux"}}]} \ No newline at end of file +{ + "schemaVersion": 2, + "manifests": [ + { + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "digest": "sha256:84afb6189c4d69f2d040c5f1dc4e0a16fed9b539ce9cfb4ac2526ae4e0576cc0", + "size": 496, + "annotations": { + "org.opencontainers.image.ref.name": "v0.1.1" + }, + "platform": { + "architecture": "amd64", + "os": "linux" + } + } + ] +} \ No newline at end of file diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index a8285ea097..1265ff2097 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -75,7 +75,7 @@ func (ref ociReference) getBlobsUsedInImageIndex(descriptor *imgspecv1.Descripto } blobsUsedInImageRefIndex := make(map[digest.Digest]int) - err = ref.getBlobsUsedInIndex(blobsUsedInImageRefIndex, index, sharedBlobsDir) + err = ref.addBlobsUsedInIndex(blobsUsedInImageRefIndex, index, sharedBlobsDir) if err != nil { return nil, err } @@ -84,8 +84,8 @@ func (ref ociReference) getBlobsUsedInImageIndex(descriptor *imgspecv1.Descripto return blobsUsedInImageRefIndex, nil } -// Returns a map of digest with the usage count, so a blob that is referenced three times will have 3 in the map -func (ref ociReference) getBlobsUsedInIndex(destination map[digest.Digest]int, index *imgspecv1.Index, sharedBlobsDir string) error { +// Updates a map of digest with the usage count, so a blob that is referenced three times will have 3 in the map +func (ref ociReference) addBlobsUsedInIndex(destination map[digest.Digest]int, index *imgspecv1.Index, sharedBlobsDir string) error { for _, descriptor := range index.Manifests { destination[descriptor.Digest]++ switch descriptor.MediaType { @@ -106,7 +106,7 @@ func (ref ociReference) getBlobsUsedInIndex(destination map[digest.Digest]int, i if err != nil { return err } - err = ref.getBlobsUsedInIndex(destination, index, sharedBlobsDir) + err = ref.addBlobsUsedInIndex(destination, index, sharedBlobsDir) if err != nil { return err } @@ -137,7 +137,7 @@ func (ref ociReference) getBlobsToDelete(blobsUsedByDescriptorToDelete map[diges return nil, err } blobsUsedInRootIndex := make(map[digest.Digest]int) - err = ref.getBlobsUsedInIndex(blobsUsedInRootIndex, rootIndex, sharedBlobsDir) + err = ref.addBlobsUsedInIndex(blobsUsedInRootIndex, rootIndex, sharedBlobsDir) if err != nil { return nil, err } diff --git a/oci/layout/oci_transport_test.go b/oci/layout/oci_transport_test.go index 3595b0d309..8beb52dd19 100644 --- a/oci/layout/oci_transport_test.go +++ b/oci/layout/oci_transport_test.go @@ -17,99 +17,81 @@ func TestGetManifestDescriptor(t *testing.T) { emptyDir := t.TempDir() for _, c := range []struct { - dir, image string - expected *struct { - descriptor *imgspecv1.Descriptor - index int - } // nil if a failure ie expected. errorIs / errorAs allows more specific checks. - errorIs error - errorAs any + dir, image string + expectedDescriptor *imgspecv1.Descriptor // nil if a failure ie expected. errorIs / errorAs allows more specific checks. + expectedIndex int + errorIs error + errorAs any }{ { // Index is missing - dir: emptyDir, - image: "", - expected: nil, + dir: emptyDir, + image: "", + expectedDescriptor: nil, }, { // A valid reference to the only manifest dir: "fixtures/manifest", image: "", - expected: &struct { - descriptor *imgspecv1.Descriptor - index int - }{ - descriptor: &imgspecv1.Descriptor{ - MediaType: "application/vnd.oci.image.manifest.v1+json", - Digest: "sha256:84afb6189c4d69f2d040c5f1dc4e0a16fed9b539ce9cfb4ac2526ae4e0576cc0", - Size: 496, - Annotations: map[string]string{"org.opencontainers.image.ref.name": "v0.1.1"}, - Platform: &imgspecv1.Platform{ - Architecture: "amd64", - OS: "linux", - }, + expectedDescriptor: &imgspecv1.Descriptor{ + MediaType: "application/vnd.oci.image.manifest.v1+json", + Digest: "sha256:84afb6189c4d69f2d040c5f1dc4e0a16fed9b539ce9cfb4ac2526ae4e0576cc0", + Size: 496, + Annotations: map[string]string{"org.opencontainers.image.ref.name": "v0.1.1"}, + Platform: &imgspecv1.Platform{ + Architecture: "amd64", + OS: "linux", }, - index: 0, }, + expectedIndex: 0, }, { // An ambiguous reference to a multi-manifest directory - dir: "fixtures/two_images_manifest", - image: "", - expected: nil, - errorIs: ErrMoreThanOneImage, + dir: "fixtures/two_images_manifest", + image: "", + expectedDescriptor: nil, + errorIs: ErrMoreThanOneImage, }, { // A valid reference in a multi-manifest directory dir: "fixtures/name_lookups", image: "a", - expected: &struct { - descriptor *imgspecv1.Descriptor - index int - }{ - descriptor: &imgspecv1.Descriptor{ - MediaType: "application/vnd.oci.image.manifest.v1+json", - Digest: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", - Size: 1, - Annotations: map[string]string{"org.opencontainers.image.ref.name": "a"}, - }, - index: 0, + expectedDescriptor: &imgspecv1.Descriptor{ + MediaType: "application/vnd.oci.image.manifest.v1+json", + Digest: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + Size: 1, + Annotations: map[string]string{"org.opencontainers.image.ref.name": "a"}, }, + expectedIndex: 0, }, { // A valid reference in a multi-manifest directory dir: "fixtures/name_lookups", image: "b", - expected: &struct { - descriptor *imgspecv1.Descriptor - index int - }{ - descriptor: &imgspecv1.Descriptor{ - MediaType: "application/vnd.oci.image.manifest.v1+json", - Digest: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", - Size: 2, - Annotations: map[string]string{"org.opencontainers.image.ref.name": "b"}, - }, - index: 0, + expectedDescriptor: &imgspecv1.Descriptor{ + MediaType: "application/vnd.oci.image.manifest.v1+json", + Digest: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + Size: 2, + Annotations: map[string]string{"org.opencontainers.image.ref.name": "b"}, }, + expectedIndex: 1, }, { // No entry found - dir: "fixtures/name_lookups", - image: "this-does-not-exist", - expected: nil, - errorAs: &ImageNotFoundError{}, + dir: "fixtures/name_lookups", + image: "this-does-not-exist", + expectedDescriptor: nil, + errorAs: &ImageNotFoundError{}, }, { // Entries with invalid MIME types found - dir: "fixtures/name_lookups", - image: "invalid-mime", - expected: nil, + dir: "fixtures/name_lookups", + image: "invalid-mime", + expectedDescriptor: nil, }, } { ref, err := NewReference(c.dir, c.image) require.NoError(t, err) res, i, err := ref.(ociReference).getManifestDescriptor() - if c.expected != nil { + if c.expectedDescriptor != nil { require.NoError(t, err) - assert.Equal(t, 0, c.expected.index) - assert.Equal(t, *c.expected.descriptor, res) + assert.Equal(t, c.expectedIndex, i) + assert.Equal(t, *c.expectedDescriptor, res) } else { - assert.Equal(t, -1, i) require.Error(t, err) if c.errorIs != nil { assert.ErrorIs(t, err, c.errorIs)