From 367cf5b9137105ebe44c1484524545c210d152fb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Philippe=20Vl=C3=A9rick?= <pvlerick@gmail.com>
Date: Fri, 23 Jun 2023 13:32:38 +0200
Subject: [PATCH] Image deletion: support for empty ref.image
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Philippe Vlérick <pvlerick@gmail.com>
---
 oci/layout/oci_transport.go      | 17 +++++++------
 oci/layout/oci_transport_test.go | 42 ++++++++++++++++++++------------
 2 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/oci/layout/oci_transport.go b/oci/layout/oci_transport.go
index a8263bf22f..af7148eea0 100644
--- a/oci/layout/oci_transport.go
+++ b/oci/layout/oci_transport.go
@@ -204,23 +204,23 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) {
 	return imgspecv1.Descriptor{}, ImageNotFoundError{ref}
 }
 
-func (ref ociReference) getManifest(descriptor imgspecv1.Descriptor) (*imgspecv1.Manifest, error) {
+func (ref ociReference) getManifest(descriptor imgspecv1.Descriptor) (imgspecv1.Manifest, error) {
 	manifestPath, err := ref.blobPath(descriptor.Digest, "")
 	if err != nil {
-		return nil, err
+		return imgspecv1.Manifest{}, err
 	}
 
 	manifestJSON, err := os.Open(manifestPath)
 	if err != nil {
-		return nil, err
+		return imgspecv1.Manifest{}, err
 	}
 	defer manifestJSON.Close()
 
 	manifest := &imgspecv1.Manifest{}
 	if err := json.NewDecoder(manifestJSON).Decode(manifest); err != nil {
-		return nil, err
+		return imgspecv1.Manifest{}, err
 	}
-	return manifest, nil
+	return *manifest, nil
 }
 
 // LoadManifestDescriptor loads the manifest descriptor to be used to retrieve the image name
@@ -247,6 +247,7 @@ func (ref ociReference) NewImageDestination(ctx context.Context, sys *types.Syst
 
 // DeleteImage deletes the named image from the registry, if supported.
 func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContext) error {
+
 	// Get the manifest for the image
 	descriptor, err := ref.getManifestDescriptor()
 	if err != nil {
@@ -276,10 +277,10 @@ func (ref ociReference) DeleteImage(ctx context.Context, sys *types.SystemContex
 		return err
 	}
 
-	newManifests := make([]imgspecv1.Descriptor, len(index.Manifests)-1)
-	for i, v := range index.Manifests {
+	newManifests := make([]imgspecv1.Descriptor, 0, len(index.Manifests)-1)
+	for _, v := range index.Manifests {
 		if v.Annotations[imgspecv1.AnnotationRefName] != ref.image {
-			newManifests[i] = v
+			newManifests = append(newManifests, v)
 		}
 	}
 	index.Manifests = newManifests
diff --git a/oci/layout/oci_transport_test.go b/oci/layout/oci_transport_test.go
index 8cb59c8d36..10fe5d96dd 100644
--- a/oci/layout/oci_transport_test.go
+++ b/oci/layout/oci_transport_test.go
@@ -445,9 +445,36 @@ func TestReferenceNewImageDestination(t *testing.T) {
 
 func TestReferenceDeleteImage(t *testing.T) {
 	ref, _, blobs := refToTempOCI_withDescriptorAndConfigAndLayers(t)
+
 	err := ref.DeleteImage(context.Background(), nil)
+	assert.NoError(t, err)
+
 	// Check that all blobs were deleted
+	for _, v := range *blobs {
+		_, err = os.Stat(v)
+		require.True(t, os.IsNotExist(err))
+	}
+	// Check that the index doesn't contain the reference anymore
+	ociRef, ok := ref.(ociReference)
+	require.True(t, ok)
+	index, err := ociRef.getIndex()
+	assert.NoError(t, err)
+	for _, v := range index.Manifests {
+		if v.Annotations[imgspecv1.AnnotationRefName] == ociRef.image {
+			assert.Fail(t, "image still present in the index after deletion")
+		}
+	}
+}
+
+func TestReferenceDeleteImage_emptyImageName(t *testing.T) {
+	_, tmpDir, blobs := refToTempOCI_withDescriptorAndConfigAndLayers(t)
+	ref, err := NewReference(tmpDir, "")
+	require.NoError(t, err)
+
+	err = ref.DeleteImage(context.Background(), nil)
 	assert.NoError(t, err)
+
+	// Check that all blobs were deleted
 	for _, v := range *blobs {
 		_, err = os.Stat(v)
 		require.True(t, os.IsNotExist(err))
@@ -476,21 +503,6 @@ func TestReferenceDeleteImage_someLayersAreReferencedByOtherImages(t *testing.T)
 	t.Skip("not implemented yet")
 }
 
-// TODO not sure if it's possible to check if a container is running with that image...
-func TestReferenceDeleteImage_imageIsUsed(t *testing.T) {
-	t.Skip("not implemented yet")
-}
-
-// TODO Kind of a weird case, but getManifestDescriptor handles this case for some reason
-func TestReferenceDeleteImage_emptyImageName(t *testing.T) {
-	t.Skip("not implemented yet")
-	// _, tmpDir, _ := refToTempOCI_withDescriptorAndConfigAndLayers(t)
-	// ref, err := NewReference(tmpDir, "")
-	// assert.NoError(t, err)
-	// err = ref.DeleteImage(context.Background(), nil)
-	// assert.Error(t, err)
-}
-
 func TestReferenceOCILayoutPath(t *testing.T) {
 	ref, tmpDir := refToTempOCI(t)
 	ociRef, ok := ref.(ociReference)