Repository: linuxkit/linuxkit (language: go)

Issue:
ReferenceExpand causes image lookup failures with containerd image store
<!--
If you are reporting a new issue, make sure that we do not have any duplicates
already open. You can ensure this by searching the issue list for this
repository. If there is a duplicate, please close your issue and add a comment
to the existing issue instead.

If you suspect your issue is a bug, please edit your issue description to
include the BUG REPORT INFORMATION shown below. If you fail to provide this
information within 7 days, we cannot debug your issue and will close it. We
will, however, reopen it if you later provide the information.

For more information about reporting issues, see
https://github.com/linuxkit/linuxkit/blob/master/CONTRIBUTING.md#reporting-other-issues

---------------------------------------------------
BUG REPORT INFORMATION
---------------------------------------------------
Use the commands below to provide key information from your environment:
You do NOT have to include this information if this is a FEATURE REQUEST
-->

**Description**

We use the [aws-nitro-enclaves-cli](https://github.com/aws/aws-nitro-enclaves-cli) to build Nitro Enclaves images. The Nitro CLI uses LinuxKit to actually build the EIF files.

After upgrading systems to newer versions of Docker (26.1.3) and enabling the containerd image store, we started seeing build failures. We build a local docker image and pass the image reference to the Nitro CLI (which then itself passes it to `linuxkit` with the `--docker` flag).

The failures we are seeing are:
> Linuxkit reported an error while creating the customer ramfs: "Add init containers:\nProcess init image: docker.io/bastion.io/signer-nitro-enclave:local\nImage docker.io/bastion.io/foo:local not found in local cache, pulling\ntime=\"2024-06-19T00:46:13Z\" level=fatal msg=\"Failed to build init tarball from docker.io/bastion.io/foo:local: Could not pull image docker.io/bastion.io/foo:local: error getting manifest for trusted image docker.io/bastion.io/foo:local: GET https://index.docker.io/v2/bastion.io/foo/manifests/local: UNAUTHORIZED: authentication required; [map[Action:pull Class: Name:bastion.io/foo Type:repository]]\"\n"

While investigating the failures, I tried to reproduce the issue and came to the realization that [util.ReferenceExpand](https://github.com/linuxkit/linuxkit/blob/4f89f4f67e392ffa8c8bab63dfaf31746e3c11a0/src/cmd/linuxkit/util/reference.go#L23-L42) generated bad references in certain scenarios.

The root cause is that `ReferenceExpand` will always append `docker.io/`, even if the image is tagged with a registry domain. This worked for some reason with earlier Docker engine versions, but with newer versions the prepended `docker.io/` will cause the local image to not be found (and will cause Docker to try and pull the image from the wrong registery).

If we re-tag the local image as `bastion/foo:local` and pass that to LinuxKit, everything works fine:
```
INFO[0000] Add init containers:                         
INFO[0000] Process init image: docker.io/bastion/foo:local 
DEBU[0000] image tar: docker.io/bastion/foo:local rootfs/ 
DEBU[0000] docker inspect image: docker.io/bastion/foo:local 
DEBU[0000] docker inspect image: docker.io/bastion/foo:local 
DEBU[0000] docker inspect image: docker.io/bastion/foo:local...Done 
DEBU[0000] docker create: docker.io/bastion/foo:local 
DEBU[0000] docker create: docker.io/bastion/foo:local...Done 
DEBU[0000] docker export: 0ed87efeff945cdabb534e4f453d92038d18d711b4d89e0468ce20e200000000 

```

**Steps to reproduce the issue:**

1. Build or tag a docker image with a non-docker registry URL: `docker tag alpine:latest bastion.io/alpine:latest`
2. Create a YAML config file that references an init image with a non-docker registry:
    ```
    prefix: rootfs/
    init:
      - bastion/alpine:latest
    files:
      - path: rootfs/dev
        directory: true
        mode: "0755"
    ```
3. Run LinuxKit with `--docker`: `linuxkit -v build --name customer-initrd.img --docker --format kernel+initrd --no-sbom conf.yaml`
4. Observe failures trying to local the image locally. 

**Describe the results you received:**

```
Error: failed to build init tarball from docker.io/bastion.io/alpine:latest: Could not pull image docker.io/bastion.io/alpine:latest: error getting manifest for trusted image docker.io/bastion.io/alpine:latest: GET https://index.docker.io/v2/bastion.io/alpine/manifests/latest: UNAUTHORIZED: authentication required; [map[Action:pull Class: Name:bastion.io/alpine Type:repository]]
2024/06/19 04:04:14 error during command execution: failed to build init tarball from docker.io/bastion.io/alpine:latest: Could not pull image docker.io/bastion.io/alpine:latest: error getting manifest for trusted image docker.io/bastion.io/alpine:latest: GET https://index.docker.io/v2/bastion.io/alpine/manifests/latest: UNAUTHORIZED: authentication required; [map[Action:pull Class: Name:bastion.io/alpine Type:repository]]
```

**Describe the results you expected:**

I expected the image to be found locally from the Docker image store.

**Additional information you deem important (e.g. issue happens only occasionally):**

I believe the solution is to simply avoid prepending the `doker.io/` prefix if the image is already prepended with a domain.



Discussion / hints:
I was able to reproduce the issue by testing with this simple Go program:

```
package main

import (
	"github.com/containerd/containerd/reference"
	"github.com/linuxkit/linuxkit/src/cmd/linuxkit/docker"
	"github.com/linuxkit/linuxkit/src/cmd/linuxkit/util"
	log "github.com/sirupsen/logrus"
)

func main() {
	// Create a new client
	cli, err := docker.Client()
	if err != nil {
		log.Fatal(err)
	}

	imageName := util.ReferenceExpand("bastion.io/foo:local")
	ref, err := reference.Parse(imageName)
	if err != nil {
		log.Errorf("could not resolve references for image %s: %v", imageName, err)
	}

	// Check if the image is in the cache
	err = docker.HasImage(&ref)
	if err != nil {
		log.Fatal(err)
	}

	// Inspect the image
	inspect, err := docker.InspectImage(cli, &ref)
	if err != nil {
		log.Fatal(err)
	}

	log.Println(inspect)
}
```

Fix patch:
diff --git a/src/cmd/linuxkit/util/reference.go b/src/cmd/linuxkit/util/reference.go
index 7ed89c3499..04139b54a6 100644
--- a/src/cmd/linuxkit/util/reference.go
+++ b/src/cmd/linuxkit/util/reference.go
@@ -32,7 +32,12 @@ func ReferenceExpand(ref string, options ...ReferenceOption) string {
 	case 1:
 		ret = "docker.io/library/" + ref
 	case 2:
-		ret = "docker.io/" + ref
+		// If the first part is not a domain, assume it is a DockerHub user/org.
+		// This logic is copied from moby:
+		// https://github.com/moby/moby/blob/e7347f8a8c2fd3d2abd34b638d6fc8c18b0278d1/registry/search.go#L148C29-L149C71
+		if !strings.Contains(parts[0], ".") && !strings.Contains(parts[0], ":") && parts[0] != "localhost" {
+			ret = "docker.io/" + ref
+		}
 	}
 
 	if opts.withTag && !strings.Contains(ret, ":") {


Test patch:
diff --git a/src/cmd/linuxkit/util/reference_test.go b/src/cmd/linuxkit/util/reference_test.go
new file mode 100644
index 0000000000..b96232e49b
--- /dev/null
+++ b/src/cmd/linuxkit/util/reference_test.go
@@ -0,0 +1,59 @@
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestReferenceExpand(t *testing.T) {
+	tests := []struct {
+		name    string
+		ref     string
+		options []ReferenceOption
+		want    string
+	}{
+		{
+			"basic image name should expand to docker.io/library image",
+			"redis",
+			nil,
+			"docker.io/library/redis",
+		},
+		{
+			"image name with user/org should expand to docker.io image",
+			"foo/bar",
+			nil,
+			"docker.io/foo/bar",
+		},
+		{
+			"custom registry image name should not expand",
+			"myregistry.io/foo",
+			nil,
+			"myregistry.io/foo",
+		},
+		{
+			"image name with more than three parts should not expand",
+			"foo/bar/baz",
+			nil,
+			"foo/bar/baz",
+		},
+		{
+			"with tag should add latest if image does not have tag",
+			"redis",
+			[]ReferenceOption{ReferenceWithTag()},
+			"docker.io/library/redis:latest",
+		},
+		{
+			"with tag should not add latest if image already has tag",
+			"redis:alpine",
+			[]ReferenceOption{ReferenceWithTag()},
+			"docker.io/library/redis:alpine",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ReferenceExpand(tt.ref, tt.options...)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}