libimage/layer_tree: if parent is empty and a manifest list then ignore check.

[flouthoc]

Layer tree expectes to form a relation between child and parent instances, however it expects an instance from manifest list which is empty, following expectation is not possible and will always resuilt in error.

Closes: containers/podman#19860

[NO NEW TESTS NEEDED]
Image without layer cant be built in libimage, and podman save automatically malforms such image so no such external image can be loaded.

[flouthoc]

@edsantiago Could you PTAL at this patch and confirm if patch works. Meanwhile I am checking someway to test this bug in CI here.

I was trying to add a test for this but seems like podman save or podman load automatically corrects image with no layer and there is no way in libimage to build such an image.

I expected following test to failed but it passes due to above reason

package libimage

import (
	"context"
	"os"
	"testing"

	"github.com/stretchr/testify/require"
)

func TestListWithEmptyLayerImage(t *testing.T) {
	// Make sure that loading images does not leave any artifacts in TMPDIR
	// behind (containers/podman/issues/14287).
	/*tmpdir := t.TempDir()
	os.Setenv("TMPDIR", tmpdir)
	defer func() {
		dir, err := os.ReadDir(tmpdir)
		require.NoError(t, err)
		require.Len(t, dir, 0)
		os.Unsetenv("TMPDIR")
	}()*/

	runtime, cleanup := testNewRuntime(t)
	defer cleanup()
	ctx := context.Background()
	loadOptions := &LoadOptions{}
	loadOptions.Writer = os.Stdout

	list, err := runtime.CreateManifestList("m")
	require.NoError(t, err)
	require.NotNil(t, list)

	for _, test := range []struct {
		input       string
		expectError bool
		numImages   int
		names       []string
	}{
		// OCI ARCHIVE
		{"testdata/empty_image.tar", false, 1, []string{"localhost/i:latest"}},
	} {
		loadedImages, err := runtime.Load(ctx, test.input, loadOptions)
		if test.expectError {
			require.Error(t, err, test.input)
			continue
		}
		require.NoError(t, err, test.input)
		require.Equal(t, test.names, loadedImages, test.input)

		// Make sure that all returned names exist as images in the
		// local containers storage.
		ids := []string{} // later used for image removal
		names := [][]string{}
		for _, name := range loadedImages {
			image, resolvedName, err := runtime.LookupImage(name, nil)
			require.NoError(t, err, test.input)
			require.Equal(t, name, resolvedName, test.input)
			ids = append(ids, image.ID())
			names = append(names, image.Names())
		}
	}
	listedImages, err := runtime.ListImages(ctx, nil, &ListImagesOptions{})
	require.NoError(t, err)
	require.Len(t, listedImages, 2, "expected 2 images")
}

[edsantiago]

@flouthoc I applied the patch below to my podman tree, ran the reproducer in #19860, and see no difference.

But I'm not sure I understood what you're asking me to do. If I misunderstood, I apologize, and could you please clarify?

diff --git a/vendor/github.com/containers/common/libimage/layer_tree.go b/vendor/github.com/containers/common/libimage/layer_tree.go
index e6b012f90..c59b512b0 100644
--- a/vendor/github.com/containers/common/libimage/layer_tree.go
+++ b/vendor/github.com/containers/common/libimage/layer_tree.go
@@ -289,6 +289,10 @@ func (t *layerTree) parent(ctx context.Context, child *Image) (*Image, error) {
 			if childID == empty.ID() {
 				continue
 			}
+			_, err := empty.IsManifestList(ctx)
+			if err != nil {
+				return nil, err
+			}
 			emptyOCI, err := t.toOCI(ctx, empty)
 			if err != nil {
 				if ErrorIsImageUnknown(err) {

[flouthoc]

@edsantiago Sorry my bad 😅 , I was testing the CI and left one line uncommented, its a bit different from the patch which you have shared. Could you please take a look and try the patch again ?

[edsantiago]

With this patch, podman images works again. Thank you. podman rmi i still emits one scary warning, down from two:

$ bin/podman rmi i
WARN[0000] Failed to determine if an image is a parent: choosing image instance: no image found in manifest list for architecture amd64, variant "", OS linux, ignoring the error 
Untagged: localhost/i:latest
Deleted: 14b232baa7c4aaf49147d27c97edd018b5bd3e906f151dba198b065a672aa5e6

[vrothberg]

See the two lint failures of above.

[flouthoc]

I had to remove the test because of this issue #1651 (comment), test was not able correctly test what I was expecting it to test.

[vrothberg]

WARN[0000] Failed to determine if an image is a parent: choosing image instance: no image found in manifest list for architecture amd64, variant "", OS linux, ignoring the error

Can be get rid of this warning, @flouthoc ?

[flouthoc]

@edsantiago Ah the check for empty manifest list had to be added on all places. it should be fixed now.

[vrothberg]

Since we don't have a test here, please open a vendor PR in Podman along with a regression test.

[flouthoc]

@vrothberg Created PR containers/podman#20016

[rhatdan]

LGTM

[vrothberg]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [vrothberg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment