From 86afb94e00541ff575d2d4b9b3f4e5369321d968 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Fri, 16 Jul 2021 11:38:42 +0200
Subject: [PATCH] refine dangling filters

As discussed in github.com/containers/podman/issues/10832 the definition
of a "dangling" image in Podman has historically been incorrect.  While
the Docker docs describe a dangling image as an image without a tag, and
Podman implemented the filters as such, Docker actually implemented the
filters for images without a tag and without children.

Refine the dangling filters and hence `IsDangling()` to only return true
if an image is untagged and has no children.

Also correct the comments of `IsIntermediate()`.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 libimage/filters.go    | 10 +++++++---
 libimage/image.go      | 34 ++++++++++++++++++----------------
 libimage/image_test.go |  4 +++-
 3 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/libimage/filters.go b/libimage/filters.go
index eae18fd9c..0cc5cc311 100644
--- a/libimage/filters.go
+++ b/libimage/filters.go
@@ -88,7 +88,7 @@ func (r *Runtime) compileImageFilters(ctx context.Context, filters []string) ([]
 			if err != nil {
 				return nil, errors.Wrapf(err, "non-boolean value %q for dangling filter", value)
 			}
-			filterFuncs = append(filterFuncs, filterDangling(dangling))
+			filterFuncs = append(filterFuncs, filterDangling(ctx, dangling))
 
 		case "id":
 			filterFuncs = append(filterFuncs, filterID(value))
@@ -201,9 +201,13 @@ func filterContainers(value bool) filterFunc {
 }
 
 // filterDangling creates a dangling filter for matching the specified value.
-func filterDangling(value bool) filterFunc {
+func filterDangling(ctx context.Context, value bool) filterFunc {
 	return func(img *Image) (bool, error) {
-		return img.IsDangling() == value, nil
+		isDangling, err := img.IsDangling(ctx)
+		if err != nil {
+			return false, err
+		}
+		return isDangling == value, nil
 	}
 }
 
diff --git a/libimage/image.go b/libimage/image.go
index 19b929dc7..2ac513455 100644
--- a/libimage/image.go
+++ b/libimage/image.go
@@ -121,24 +121,29 @@ func (i *Image) IsReadOnly() bool {
 	return i.storageImage.ReadOnly
 }
 
-// IsDangling returns true if the image is dangling.  An image is considered
-// dangling if no names are associated with it in the containers storage.
-func (i *Image) IsDangling() bool {
-	return len(i.Names()) == 0
+// IsDangling returns true if the image is dangling, that is an untagged image
+// without children.
+func (i *Image) IsDangling(ctx context.Context) (bool, error) {
+	if len(i.Names()) > 0 {
+		return false, nil
+	}
+	children, err := i.getChildren(ctx, false)
+	if err != nil {
+		return false, err
+	}
+	return len(children) == 0, nil
 }
 
 // IsIntermediate returns true if the image is an intermediate image, that is
-// a dangling image without children.
+// an untagged image with children.
 func (i *Image) IsIntermediate(ctx context.Context) (bool, error) {
-	// If the image has tags, it's not an intermediate one.
-	if !i.IsDangling() {
+	if len(i.Names()) > 0 {
 		return false, nil
 	}
 	children, err := i.getChildren(ctx, false)
 	if err != nil {
 		return false, err
 	}
-	// No tags, no children -> intermediate!
 	return len(children) != 0, nil
 }
 
@@ -420,19 +425,16 @@ func (i *Image) remove(ctx context.Context, rmMap map[string]*RemoveImageReport,
 		return nil
 	}
 
-	if !parent.IsDangling() {
-		return nil
-	}
-
-	// If the image has siblings, we don't remove the parent.
-	hasSiblings, err := parent.HasChildren(ctx)
+	// Only remove the parent if it's dangling, that is being untagged and
+	// without children.
+	danglingParent, err := parent.IsDangling(ctx)
 	if err != nil {
 		// See Podman commit fd9dd7065d44: we need to
 		// be tolerant toward corrupted images.
 		logrus.Warnf("error determining if an image is a parent: %v, ignoring the error", err)
-		hasSiblings = false
+		danglingParent = false
 	}
-	if hasSiblings {
+	if !danglingParent {
 		return nil
 	}
 
diff --git a/libimage/image_test.go b/libimage/image_test.go
index af37fcb85..3e27fa98b 100644
--- a/libimage/image_test.go
+++ b/libimage/image_test.go
@@ -64,7 +64,9 @@ func TestImageFunctions(t *testing.T) {
 
 	// Below mostly smoke tests.
 	require.False(t, image.IsReadOnly())
-	require.False(t, image.IsDangling())
+	isDangling, err := image.IsDangling(ctx)
+	require.NoError(t, err)
+	require.False(t, isDangling)
 
 	isIntermediate, err := image.IsIntermediate(ctx)
 	require.NoError(t, err)