From 4c4708de68afcc88b8175e899540e39fbab19de6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 04:34:23 +0200 Subject: [PATCH 01/12] Factor out a repeated check into assertRefNameIsMissing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... and use slices.Contains instead of a manual loop. Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete_test.go | 36 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/oci/layout/oci_delete_test.go b/oci/layout/oci_delete_test.go index 8c88b4ffe..73fee985d 100644 --- a/oci/layout/oci_delete_test.go +++ b/oci/layout/oci_delete_test.go @@ -7,6 +7,7 @@ import ( "io/fs" "os" "path/filepath" + "slices" "testing" "github.com/containers/image/v5/types" @@ -92,6 +93,14 @@ func TestReferenceDeleteImage_sharedBlobDir(t *testing.T) { require.Equal(t, 0, len(index.Manifests)) } +func assertRefNameIsMissing(t *testing.T, index *imgspecv1.Index, refName string) { + if slices.ContainsFunc(index.Manifests, func(desc imgspecv1.Descriptor) bool { + return desc.Annotations[imgspecv1.AnnotationRefName] == refName + }) { + assert.Failf(t, "index still contains refName %q after deletion", refName) + } +} + func TestReferenceDeleteImage_multipleImages(t *testing.T) { tmpDir := loadFixture(t, "delete_image_multiple_images") @@ -118,14 +127,7 @@ func TestReferenceDeleteImage_multipleImages(t *testing.T) { require.NoError(t, err) require.Equal(t, 6, len(index.Manifests)) // .. Check that the image is not in the index anymore - for _, descriptor := range index.Manifests { - switch descriptor.Annotations[imgspecv1.AnnotationRefName] { - case "3.17.5": - assert.Fail(t, "image still present in the index after deletion") - default: - continue - } - } + assertRefNameIsMissing(t, index, "3.17.5") } func TestReferenceDeleteImage_multipleImages_blobsUsedByOtherImages(t *testing.T) { @@ -154,14 +156,7 @@ func TestReferenceDeleteImage_multipleImages_blobsUsedByOtherImages(t *testing.T require.NoError(t, err) require.Equal(t, 6, len(index.Manifests)) // .. Check that the image is not in the index anymore - for _, descriptor := range index.Manifests { - switch descriptor.Annotations[imgspecv1.AnnotationRefName] { - case "1.0.0": - assert.Fail(t, "image still present in the index after deletion") - default: - continue - } - } + assertRefNameIsMissing(t, index, "1.0.0") } func TestReferenceDeleteImage_multipleImages_imageDoesNotExist(t *testing.T) { @@ -214,14 +209,7 @@ func TestReferenceDeleteImage_multipleImages_nestedIndexImage(t *testing.T) { require.NoError(t, err) require.Equal(t, 6, len(index.Manifests)) // .. Check that the image is not in the index anymore - for _, descriptor := range index.Manifests { - switch descriptor.Annotations[imgspecv1.AnnotationRefName] { - case "3.16.7": - assert.Fail(t, "image still present in the index after deletion") - default: - continue - } - } + assertRefNameIsMissing(t, index, "3.16.7") } func TestReferenceDeleteImage_multipleImages_nestedIndexImage_refWithSameContent(t *testing.T) { From cec387ecb3f54e2c334d589a625e9db49a235aaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 04:48:11 +0200 Subject: [PATCH 02/12] Turn getBlobsUsedInManifest into addBlobsUsedInManifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... so that we don't always create a new map. Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index bcf257df6..c78aa67e3 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -59,7 +59,8 @@ func (ref ociReference) getBlobsUsedInSingleImage(descriptor *imgspecv1.Descript if err != nil { return nil, err } - blobsUsedInManifest := ref.getBlobsUsedInManifest(manifest) + blobsUsedInManifest := make(map[digest.Digest]int) + ref.addBlobsUsedInManifest(blobsUsedInManifest, manifest) blobsUsedInManifest[descriptor.Digest]++ // Add the current manifest to the list of blobs used by this reference return blobsUsedInManifest, nil @@ -95,9 +96,7 @@ func (ref ociReference) addBlobsUsedInIndex(destination map[digest.Digest]int, i if err != nil { return err } - for digest, count := range ref.getBlobsUsedInManifest(manifest) { - destination[digest] += count - } + ref.addBlobsUsedInManifest(destination, manifest) case imgspecv1.MediaTypeImageIndex: blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) if err != nil { @@ -119,15 +118,11 @@ func (ref ociReference) addBlobsUsedInIndex(destination map[digest.Digest]int, i return nil } -func (ref ociReference) getBlobsUsedInManifest(manifest *imgspecv1.Manifest) map[digest.Digest]int { - blobsUsedInManifest := make(map[digest.Digest]int, 0) - - blobsUsedInManifest[manifest.Config.Digest]++ +func (ref ociReference) addBlobsUsedInManifest(destination map[digest.Digest]int, manifest *imgspecv1.Manifest) { + destination[manifest.Config.Digest]++ for _, layer := range manifest.Layers { - blobsUsedInManifest[layer.Digest]++ + destination[layer.Digest]++ } - - return blobsUsedInManifest } // This takes in a map of the digest and their usage count in the manifest to be deleted From b56cd59dc1b4e788f1f6db6f39e2d38655cea6f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 04:55:40 +0200 Subject: [PATCH 03/12] Move reading the manifest into addBlobsUsedInManifest to avoid duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index c78aa67e3..1c7e619e8 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -55,12 +55,10 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex } func (ref ociReference) getBlobsUsedInSingleImage(descriptor *imgspecv1.Descriptor, sharedBlobsDir string) (map[digest.Digest]int, error) { - manifest, err := ref.getManifest(descriptor, sharedBlobsDir) - if err != nil { + blobsUsedInManifest := make(map[digest.Digest]int) + if err := ref.addBlobsUsedInManifest(blobsUsedInManifest, descriptor, sharedBlobsDir); err != nil { return nil, err } - blobsUsedInManifest := make(map[digest.Digest]int) - ref.addBlobsUsedInManifest(blobsUsedInManifest, manifest) blobsUsedInManifest[descriptor.Digest]++ // Add the current manifest to the list of blobs used by this reference return blobsUsedInManifest, nil @@ -92,11 +90,9 @@ func (ref ociReference) addBlobsUsedInIndex(destination map[digest.Digest]int, i destination[descriptor.Digest]++ switch descriptor.MediaType { case imgspecv1.MediaTypeImageManifest: - manifest, err := ref.getManifest(&descriptor, sharedBlobsDir) - if err != nil { + if err := ref.addBlobsUsedInManifest(destination, &descriptor, sharedBlobsDir); err != nil { return err } - ref.addBlobsUsedInManifest(destination, manifest) case imgspecv1.MediaTypeImageIndex: blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) if err != nil { @@ -118,11 +114,17 @@ func (ref ociReference) addBlobsUsedInIndex(destination map[digest.Digest]int, i return nil } -func (ref ociReference) addBlobsUsedInManifest(destination map[digest.Digest]int, manifest *imgspecv1.Manifest) { +func (ref ociReference) addBlobsUsedInManifest(destination map[digest.Digest]int, descriptor *imgspecv1.Descriptor, sharedBlobsDir string) error { + manifest, err := ref.getManifest(descriptor, sharedBlobsDir) + if err != nil { + return err + } + destination[manifest.Config.Digest]++ for _, layer := range manifest.Layers { destination[layer.Digest]++ } + return nil } // This takes in a map of the digest and their usage count in the manifest to be deleted From 0a66550e5a546f259beda3c9915b26ea66a69018 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 05:00:31 +0200 Subject: [PATCH 04/12] Factor out addBlobsUsedInNestedIndex to avoid duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index 1c7e619e8..f93994ebb 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -65,18 +65,8 @@ func (ref ociReference) getBlobsUsedInSingleImage(descriptor *imgspecv1.Descript } func (ref ociReference) getBlobsUsedInImageIndex(descriptor *imgspecv1.Descriptor, sharedBlobsDir string) (map[digest.Digest]int, error) { - blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) - if err != nil { - return nil, err - } - index, err := parseIndex(blobPath) - if err != nil { - return nil, err - } - blobsUsedInImageRefIndex := make(map[digest.Digest]int) - err = ref.addBlobsUsedInIndex(blobsUsedInImageRefIndex, index, sharedBlobsDir) - if err != nil { + if err := ref.addBlobsUsedInNestedIndex(blobsUsedInImageRefIndex, descriptor, sharedBlobsDir); err != nil { return nil, err } blobsUsedInImageRefIndex[descriptor.Digest]++ // Add the nested index in the list of blobs used by this reference @@ -84,6 +74,18 @@ func (ref ociReference) getBlobsUsedInImageIndex(descriptor *imgspecv1.Descripto return blobsUsedInImageRefIndex, nil } +func (ref ociReference) addBlobsUsedInNestedIndex(destination map[digest.Digest]int, descriptor *imgspecv1.Descriptor, sharedBlobsDir string) error { + blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) + if err != nil { + return err + } + index, err := parseIndex(blobPath) + if err != nil { + return err + } + return ref.addBlobsUsedInIndex(destination, index, sharedBlobsDir) +} + // 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 { @@ -94,16 +96,7 @@ func (ref ociReference) addBlobsUsedInIndex(destination map[digest.Digest]int, i return err } case imgspecv1.MediaTypeImageIndex: - blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) - if err != nil { - return err - } - index, err := parseIndex(blobPath) - if err != nil { - return err - } - err = ref.addBlobsUsedInIndex(destination, index, sharedBlobsDir) - if err != nil { + if err := ref.addBlobsUsedInNestedIndex(destination, &descriptor, sharedBlobsDir); err != nil { return err } default: From 1be4384f4bdbe0c188ebe986ab09b673d000fcaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 05:02:59 +0200 Subject: [PATCH 05/12] Inline getBlobsUsedInImageIndex into the only user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index f93994ebb..cdb965697 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -33,7 +33,11 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex case imgspecv1.MediaTypeImageManifest: blobsUsedByImage, err = ref.getBlobsUsedInSingleImage(&descriptor, sharedBlobsDir) case imgspecv1.MediaTypeImageIndex: - blobsUsedByImage, err = ref.getBlobsUsedInImageIndex(&descriptor, sharedBlobsDir) + blobsUsedByImage = make(map[digest.Digest]int) + if err := ref.addBlobsUsedInNestedIndex(blobsUsedByImage, &descriptor, sharedBlobsDir); err != nil { + return err + } + blobsUsedByImage[descriptor.Digest]++ // Add the nested index in the list of blobs used by this reference default: return fmt.Errorf("unsupported mediaType in index: %q", descriptor.MediaType) } @@ -64,16 +68,6 @@ func (ref ociReference) getBlobsUsedInSingleImage(descriptor *imgspecv1.Descript return blobsUsedInManifest, nil } -func (ref ociReference) getBlobsUsedInImageIndex(descriptor *imgspecv1.Descriptor, sharedBlobsDir string) (map[digest.Digest]int, error) { - blobsUsedInImageRefIndex := make(map[digest.Digest]int) - if err := ref.addBlobsUsedInNestedIndex(blobsUsedInImageRefIndex, descriptor, sharedBlobsDir); err != nil { - return nil, err - } - blobsUsedInImageRefIndex[descriptor.Digest]++ // Add the nested index in the list of blobs used by this reference - - return blobsUsedInImageRefIndex, nil -} - func (ref ociReference) addBlobsUsedInNestedIndex(destination map[digest.Digest]int, descriptor *imgspecv1.Descriptor, sharedBlobsDir string) error { blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) if err != nil { From 8c5f25f720ea4b77b45c26bb9afd3c5556f14528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 05:05:56 +0200 Subject: [PATCH 06/12] Inline getBlobsUsedInSingleImage into its only caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index cdb965697..0f0f06034 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -31,7 +31,11 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex switch descriptor.MediaType { case imgspecv1.MediaTypeImageManifest: - blobsUsedByImage, err = ref.getBlobsUsedInSingleImage(&descriptor, sharedBlobsDir) + blobsUsedByImage = make(map[digest.Digest]int) + if err := ref.addBlobsUsedInManifest(blobsUsedByImage, &descriptor, sharedBlobsDir); err != nil { + return err + } + blobsUsedByImage[descriptor.Digest]++ // Add the current manifest to the list of blobs used by this reference case imgspecv1.MediaTypeImageIndex: blobsUsedByImage = make(map[digest.Digest]int) if err := ref.addBlobsUsedInNestedIndex(blobsUsedByImage, &descriptor, sharedBlobsDir); err != nil { @@ -41,9 +45,6 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex default: return fmt.Errorf("unsupported mediaType in index: %q", descriptor.MediaType) } - if err != nil { - return err - } blobsToDelete, err := ref.getBlobsToDelete(blobsUsedByImage, sharedBlobsDir) if err != nil { @@ -58,16 +59,6 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex return ref.deleteReferenceFromIndex(descriptorIndex) } -func (ref ociReference) getBlobsUsedInSingleImage(descriptor *imgspecv1.Descriptor, sharedBlobsDir string) (map[digest.Digest]int, error) { - blobsUsedInManifest := make(map[digest.Digest]int) - if err := ref.addBlobsUsedInManifest(blobsUsedInManifest, descriptor, sharedBlobsDir); err != nil { - return nil, err - } - blobsUsedInManifest[descriptor.Digest]++ // Add the current manifest to the list of blobs used by this reference - - return blobsUsedInManifest, nil -} - func (ref ociReference) addBlobsUsedInNestedIndex(destination map[digest.Digest]int, descriptor *imgspecv1.Descriptor, sharedBlobsDir string) error { blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) if err != nil { From 446f2508e0a69d83e125ecd16878ab5350c9acb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 05:08:53 +0200 Subject: [PATCH 07/12] Share duplicate code in DeleteImage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index 0f0f06034..16f7f245b 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -27,21 +27,17 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex return err } - var blobsUsedByImage map[digest.Digest]int - + blobsUsedByImage := make(map[digest.Digest]int) + blobsUsedByImage[descriptor.Digest]++ // Add the current object to the list of blobs used by this reference switch descriptor.MediaType { case imgspecv1.MediaTypeImageManifest: - blobsUsedByImage = make(map[digest.Digest]int) if err := ref.addBlobsUsedInManifest(blobsUsedByImage, &descriptor, sharedBlobsDir); err != nil { return err } - blobsUsedByImage[descriptor.Digest]++ // Add the current manifest to the list of blobs used by this reference case imgspecv1.MediaTypeImageIndex: - blobsUsedByImage = make(map[digest.Digest]int) if err := ref.addBlobsUsedInNestedIndex(blobsUsedByImage, &descriptor, sharedBlobsDir); err != nil { return err } - blobsUsedByImage[descriptor.Digest]++ // Add the nested index in the list of blobs used by this reference default: return fmt.Errorf("unsupported mediaType in index: %q", descriptor.MediaType) } From 057fee0ae299bb3f360082224d675caabbf84b67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 05:17:04 +0200 Subject: [PATCH 08/12] Factor out countBlobsForDescriptor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... instead of maintaining two copies. Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 47 +++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index 16f7f245b..4edda7cef 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -28,18 +28,8 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex } blobsUsedByImage := make(map[digest.Digest]int) - blobsUsedByImage[descriptor.Digest]++ // Add the current object to the list of blobs used by this reference - switch descriptor.MediaType { - case imgspecv1.MediaTypeImageManifest: - if err := ref.addBlobsUsedInManifest(blobsUsedByImage, &descriptor, sharedBlobsDir); err != nil { - return err - } - case imgspecv1.MediaTypeImageIndex: - if err := ref.addBlobsUsedInNestedIndex(blobsUsedByImage, &descriptor, sharedBlobsDir); err != nil { - return err - } - default: - return fmt.Errorf("unsupported mediaType in index: %q", descriptor.MediaType) + if err := ref.countBlobsForDescriptor(blobsUsedByImage, &descriptor, sharedBlobsDir); err != nil { + return err } blobsToDelete, err := ref.getBlobsToDelete(blobsUsedByImage, sharedBlobsDir) @@ -55,6 +45,24 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex return ref.deleteReferenceFromIndex(descriptorIndex) } +// countBlobsForDescriptor updates dest with usage counts of blobs required for descriptor, INCLUDING descriptor itself. +func (ref ociReference) countBlobsForDescriptor(dest map[digest.Digest]int, descriptor *imgspecv1.Descriptor, sharedBlobsDir string) error { + dest[descriptor.Digest]++ + switch descriptor.MediaType { + case imgspecv1.MediaTypeImageManifest: + if err := ref.addBlobsUsedInManifest(dest, descriptor, sharedBlobsDir); err != nil { + return err + } + case imgspecv1.MediaTypeImageIndex: + if err := ref.addBlobsUsedInNestedIndex(dest, descriptor, sharedBlobsDir); err != nil { + return err + } + default: + return fmt.Errorf("unsupported mediaType in index: %q", descriptor.MediaType) + } + return nil +} + func (ref ociReference) addBlobsUsedInNestedIndex(destination map[digest.Digest]int, descriptor *imgspecv1.Descriptor, sharedBlobsDir string) error { blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) if err != nil { @@ -70,21 +78,10 @@ func (ref ociReference) addBlobsUsedInNestedIndex(destination map[digest.Digest] // 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 { - case imgspecv1.MediaTypeImageManifest: - if err := ref.addBlobsUsedInManifest(destination, &descriptor, sharedBlobsDir); err != nil { - return err - } - case imgspecv1.MediaTypeImageIndex: - if err := ref.addBlobsUsedInNestedIndex(destination, &descriptor, sharedBlobsDir); err != nil { - return err - } - default: - return fmt.Errorf("unsupported mediaType in index: %q", descriptor.MediaType) + if err := ref.countBlobsForDescriptor(destination, &descriptor, sharedBlobsDir); err != nil { + return err } } - return nil } From cd276d268d6847da0581dfac3f5ecc4f579c9b66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 05:20:42 +0200 Subject: [PATCH 09/12] Inline addBlobsUsedInNestedIndex and addBlobsUsedInManifest into the only caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 43 +++++++++++++++------------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index 4edda7cef..f645a541d 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -50,11 +50,25 @@ func (ref ociReference) countBlobsForDescriptor(dest map[digest.Digest]int, desc dest[descriptor.Digest]++ switch descriptor.MediaType { case imgspecv1.MediaTypeImageManifest: - if err := ref.addBlobsUsedInManifest(dest, descriptor, sharedBlobsDir); err != nil { + manifest, err := ref.getManifest(descriptor, sharedBlobsDir) + if err != nil { return err } + + dest[manifest.Config.Digest]++ + for _, layer := range manifest.Layers { + dest[layer.Digest]++ + } case imgspecv1.MediaTypeImageIndex: - if err := ref.addBlobsUsedInNestedIndex(dest, descriptor, sharedBlobsDir); err != nil { + blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) + if err != nil { + return err + } + index, err := parseIndex(blobPath) + if err != nil { + return err + } + if err := ref.addBlobsUsedInIndex(dest, index, sharedBlobsDir); err != nil { return err } default: @@ -63,18 +77,6 @@ func (ref ociReference) countBlobsForDescriptor(dest map[digest.Digest]int, desc return nil } -func (ref ociReference) addBlobsUsedInNestedIndex(destination map[digest.Digest]int, descriptor *imgspecv1.Descriptor, sharedBlobsDir string) error { - blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) - if err != nil { - return err - } - index, err := parseIndex(blobPath) - if err != nil { - return err - } - return ref.addBlobsUsedInIndex(destination, index, sharedBlobsDir) -} - // 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 { @@ -85,19 +87,6 @@ func (ref ociReference) addBlobsUsedInIndex(destination map[digest.Digest]int, i return nil } -func (ref ociReference) addBlobsUsedInManifest(destination map[digest.Digest]int, descriptor *imgspecv1.Descriptor, sharedBlobsDir string) error { - manifest, err := ref.getManifest(descriptor, sharedBlobsDir) - if err != nil { - return err - } - - destination[manifest.Config.Digest]++ - for _, layer := range manifest.Layers { - destination[layer.Digest]++ - } - return nil -} - // This takes in a map of the digest and their usage count in the manifest to be deleted // It will compare it to the digest usage in the root index, and return a set of the blobs that can be safely deleted func (ref ociReference) getBlobsToDelete(blobsUsedByDescriptorToDelete map[digest.Digest]int, sharedBlobsDir string) (*set.Set[digest.Digest], error) { From a34ad07f8c1aa897a710475a0ef00a5a5fcc6bf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 05:23:58 +0200 Subject: [PATCH 10/12] Inline getManifest into the only caller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index f645a541d..fc19278e6 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -50,11 +50,14 @@ func (ref ociReference) countBlobsForDescriptor(dest map[digest.Digest]int, desc dest[descriptor.Digest]++ switch descriptor.MediaType { case imgspecv1.MediaTypeImageManifest: - manifest, err := ref.getManifest(descriptor, sharedBlobsDir) + blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) + if err != nil { + return err + } + manifest, err := parseJSON[imgspecv1.Manifest](blobPath) if err != nil { return err } - dest[manifest.Config.Digest]++ for _, layer := range manifest.Layers { dest[layer.Digest]++ @@ -181,17 +184,3 @@ func saveJSON(path string, content any) error { return json.NewEncoder(file).Encode(content) } - -func (ref ociReference) getManifest(descriptor *imgspecv1.Descriptor, sharedBlobsDir string) (*imgspecv1.Manifest, error) { - manifestPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) - if err != nil { - return nil, err - } - - manifest, err := parseJSON[imgspecv1.Manifest](manifestPath) - if err != nil { - return nil, err - } - - return manifest, nil -} From 009ce7cfea0d1eab568f766285695771d8a6bb10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 05:27:24 +0200 Subject: [PATCH 11/12] Move shared code outside of the switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index fc19278e6..df305077e 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -47,13 +47,14 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex // countBlobsForDescriptor updates dest with usage counts of blobs required for descriptor, INCLUDING descriptor itself. func (ref ociReference) countBlobsForDescriptor(dest map[digest.Digest]int, descriptor *imgspecv1.Descriptor, sharedBlobsDir string) error { + blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) + if err != nil { + return err + } + dest[descriptor.Digest]++ switch descriptor.MediaType { case imgspecv1.MediaTypeImageManifest: - blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) - if err != nil { - return err - } manifest, err := parseJSON[imgspecv1.Manifest](blobPath) if err != nil { return err @@ -63,10 +64,6 @@ func (ref ociReference) countBlobsForDescriptor(dest map[digest.Digest]int, desc dest[layer.Digest]++ } case imgspecv1.MediaTypeImageIndex: - blobPath, err := ref.blobPath(descriptor.Digest, sharedBlobsDir) - if err != nil { - return err - } index, err := parseIndex(blobPath) if err != nil { return err From f957a9e07035960b3358ef4c2229844bf676d5cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Fri, 23 Aug 2024 05:28:48 +0200 Subject: [PATCH 12/12] Rename addBlobsUsedInIndex to countBlobsReferencedByIndex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ... and document it a bit better Should not change behavior. Signed-off-by: Miloslav Trmač --- oci/layout/oci_delete.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/oci/layout/oci_delete.go b/oci/layout/oci_delete.go index df305077e..08366a7e2 100644 --- a/oci/layout/oci_delete.go +++ b/oci/layout/oci_delete.go @@ -68,7 +68,7 @@ func (ref ociReference) countBlobsForDescriptor(dest map[digest.Digest]int, desc if err != nil { return err } - if err := ref.addBlobsUsedInIndex(dest, index, sharedBlobsDir); err != nil { + if err := ref.countBlobsReferencedByIndex(dest, index, sharedBlobsDir); err != nil { return err } default: @@ -77,8 +77,8 @@ func (ref ociReference) countBlobsForDescriptor(dest map[digest.Digest]int, desc return nil } -// 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 { +// countBlobsReferencedByIndex updates dest with usage counts of blobs required for index, EXCLUDING the index itself. +func (ref ociReference) countBlobsReferencedByIndex(destination map[digest.Digest]int, index *imgspecv1.Index, sharedBlobsDir string) error { for _, descriptor := range index.Manifests { if err := ref.countBlobsForDescriptor(destination, &descriptor, sharedBlobsDir); err != nil { return err @@ -95,7 +95,7 @@ func (ref ociReference) getBlobsToDelete(blobsUsedByDescriptorToDelete map[diges return nil, err } blobsUsedInRootIndex := make(map[digest.Digest]int) - err = ref.addBlobsUsedInIndex(blobsUsedInRootIndex, rootIndex, sharedBlobsDir) + err = ref.countBlobsReferencedByIndex(blobsUsedInRootIndex, rootIndex, sharedBlobsDir) if err != nil { return nil, err }