Merge pull request #7007 from lohitkolluri/e2e/private-registry-pull-push-5965

e2e: add private registry pull/push regression test

e2e/.gitignore

@@ -0,0 +1,5 @@
+# Generated by gen-certs.sh at setup time
+testdata/registry/certs/ca.crt
+testdata/registry/certs/ca.key
+testdata/registry/certs/tlsregistry.crt
+testdata/registry/certs/tlsregistry.key

e2e/compose-env.yaml

@@ -3,9 +3,35 @@ services:
   registry:
     image: 'registry:3'
 
+  privateregistry:
+    build:
+      context: ./testdata/registry
+    environment:
+      - REGISTRY_HTTP_ADDR=0.0.0.0:5001
+      - REGISTRY_HTTP_DEBUG_ADDR=0.0.0.0:5002
+      - REGISTRY_AUTH=htpasswd
+      - REGISTRY_AUTH_HTPASSWD_REALM=Registry Realm
+      - REGISTRY_AUTH_HTPASSWD_PATH=/auth/htpasswd
+
+  tlsregistry:
+    build:
+      context: ./testdata/registry
+    environment:
+      - REGISTRY_HTTP_ADDR=0.0.0.0:5003
+      - REGISTRY_HTTP_DEBUG_ADDR=0.0.0.0:5004
+      - REGISTRY_AUTH=htpasswd
+      - REGISTRY_AUTH_HTPASSWD_REALM=Registry Realm
+      - REGISTRY_AUTH_HTPASSWD_PATH=/auth/htpasswd
+      - REGISTRY_HTTP_TLS_CERTIFICATE=/certs/tlsregistry.crt
+      - REGISTRY_HTTP_TLS_KEY=/certs/tlsregistry.key
+
   engine:
-      image: 'docker:${ENGINE_VERSION:-29}-dind'
+      build:
+        context: ./testdata
+        dockerfile: engine/Dockerfile
+        args:
+          ENGINE_VERSION: ${ENGINE_VERSION:-29}
       privileged: true
-      command: ['--insecure-registry=registry:5000', '--experimental']
+      command: ['--insecure-registry=registry:5000', '--insecure-registry=privateregistry:5001', '--experimental']
       environment:
         - DOCKER_TLS_CERTDIR=

e2e/container/run_test.go

@@ -209,7 +209,11 @@ func TestProcessTermination(t *testing.T) {
 
 	assert.NilError(t, result.Cmd.Process.Signal(syscall.SIGTERM))
 
-	icmd.WaitOnCmd(time.Second*10, result).Assert(t, icmd.Expected{
+	// Use a generous timeout (20s) because when run through SSH connhelper,
+	// the Docker engine may take longer to close the attach stream after
+	// the container exits. This is a known timing difference across engine
+	// versions (e.g. engine 25 over SSH connhelper).
+	icmd.WaitOnCmd(time.Second*20, result).Assert(t, icmd.Expected{
 		ExitCode: 0,
 	})
 }

e2e/image/private_test.go

@@ -0,0 +1,126 @@
+package image
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/docker/cli/e2e/internal/fixtures"
+	"gotest.tools/v3/assert"
+	"gotest.tools/v3/icmd"
+)
+
+// Regression test for https://github.com/docker/cli/issues/5963
+func TestPullPushPrivateRepository(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name           string
+		registryPrefix string
+		tagSuffix      string
+	}{
+		{name: "insecure", registryPrefix: "privateregistry:5001", tagSuffix: "private"},
+		{name: "tls", registryPrefix: "tlsregistry:5003", tagSuffix: "tls"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			dir := fixtures.SetupConfigFile(t)
+			t.Cleanup(dir.Remove)
+			emptyConfigDir := t.TempDir()
+
+			sourceImage := fixtures.AlpineImage
+			privateImage := tc.registryPrefix + "/private/alpine:test-" + tc.tagSuffix + "-pull-push"
+
+			runWithPrivateRegistryRetry(t,
+				icmd.Command("docker", "pull", sourceImage),
+			).Assert(t, icmd.Success)
+			t.Cleanup(func() {
+				icmd.RunCommand("docker", "image", "rm", "-f", privateImage).Assert(t, icmd.Success)
+			})
+
+			icmd.RunCommand("docker", "tag", sourceImage, privateImage).Assert(t, icmd.Success)
+
+			pushNoAuth := runWithPrivateRegistryRetry(t,
+				icmd.Command("docker", "push", privateImage),
+				fixtures.WithConfig(emptyConfigDir),
+			)
+			pushNoAuth.Assert(t, icmd.Expected{ExitCode: 1})
+			assertAuthDenied(t, pushNoAuth)
+
+			pushWithAuth := runWithPrivateRegistryRetry(t,
+				icmd.Command("docker", "push", privateImage),
+				fixtures.WithConfig(dir.Path()),
+			)
+			pushWithAuth.Assert(t, icmd.Success)
+			// Docker omits the tag in the "push refers to repository" line; strip it before asserting.
+			privateRepo := privateImage[:strings.LastIndex(privateImage, ":")]
+			assert.Check(t, strings.Contains(pushWithAuth.Combined(), "The push refers to repository ["+privateRepo+"]"), pushWithAuth.Combined())
+
+			icmd.RunCommand("docker", "image", "rm", "-f", privateImage).Assert(t, icmd.Success)
+
+			pullNoAuth := runWithPrivateRegistryRetry(t,
+				icmd.Command("docker", "pull", privateImage),
+				fixtures.WithConfig(emptyConfigDir),
+			)
+			pullNoAuth.Assert(t, icmd.Expected{ExitCode: 1})
+			assertAuthDenied(t, pullNoAuth)
+
+			pullWithAuth := runWithPrivateRegistryRetry(t,
+				icmd.Command("docker", "pull", privateImage),
+				fixtures.WithConfig(dir.Path()),
+			)
+			pullWithAuth.Assert(t, icmd.Success)
+			assert.Check(t, strings.Contains(pullWithAuth.Combined(), privateImage), pullWithAuth.Combined())
+		})
+	}
+}
+
+func assertAuthDenied(t *testing.T, result *icmd.Result) {
+	t.Helper()
+	output := result.Combined()
+	if isPrivateRegistryTransient(output) {
+		t.Fatalf("private registry unavailable while expecting auth failure: %s", output)
+	}
+
+	assert.Assert(t,
+		strings.Contains(output, "requested access to the resource is denied") ||
+			strings.Contains(output, "no basic auth credentials") ||
+			strings.Contains(output, "unauthorized") ||
+			strings.Contains(output, "authentication required"),
+		output,
+	)
+}
+
+func runWithPrivateRegistryRetry(t *testing.T, cmd icmd.Cmd, opts ...icmd.CmdOp) *icmd.Result {
+	t.Helper()
+
+	deadline := time.Now().Add(90 * time.Second)
+	for {
+		result := icmd.RunCmd(cmd, opts...)
+		output := result.Combined()
+		if isPrivateRegistryTransient(output) {
+			if time.Now().Before(deadline) {
+				t.Logf("waiting for private registry availability: %s", output)
+				time.Sleep(500 * time.Millisecond)
+				continue
+			}
+		}
+		return result
+	}
+}
+
+func isPrivateRegistryTransient(output string) bool {
+	return strings.Contains(output, "lookup privateregistry") ||
+		strings.Contains(output, "lookup tlsregistry") ||
+		strings.Contains(output, "lookup registry") ||
+		strings.Contains(output, "no such host") ||
+		strings.Contains(output, "server misbehaving") ||
+		strings.Contains(output, "Temporary failure in name resolution") ||
+		strings.Contains(output, "connection refused") ||
+		strings.Contains(output, "i/o timeout") ||
+		strings.Contains(output, "TLS handshake timeout") ||
+		strings.Contains(output, "context deadline exceeded") ||
+		strings.Contains(output, "connection reset by peer") ||
+		strings.Contains(output, "unexpected EOF")
+}

e2e/internal/fixtures/fixtures.go

@@ -23,6 +23,12 @@ func SetupConfigFile(t *testing.T) fs.Dir {
 	"auths": {
 		"registry:5000": {
 			"auth": "ZWlhaXM6cGFzc3dvcmQK"
+		},
+		"privateregistry:5001": {
+			"auth": "ZTJlOnBhc3N3b3Jk"
+		},
+		"tlsregistry:5003": {
+			"auth": "ZTJlOnBhc3N3b3Jk"
 		}
 	}}`), fs.WithDir("trust", fs.WithDir("private")))
 	return *dir

e2e/testdata/Dockerfile.connhelper-ssh

@@ -12,6 +12,9 @@ RUN apk --no-cache add openssl openssh-client openssh-server shadow && \
   useradd --create-home --shell /bin/sh --password $(head -c32 /dev/urandom | base64) penguin && \
   usermod -aG docker penguin && \
   ssh-keygen -A
+# Trust the tlsregistry CA so dockerd connects without --insecure-registry.
+COPY registry/certs/ca.crt /usr/local/share/ca-certificates/tlsregistry-ca.crt
+RUN update-ca-certificates
 # workaround: ssh session excludes /usr/local/bin from $PATH
 RUN  ln -s /usr/local/bin/docker /usr/bin/docker
 COPY ./connhelper-ssh/entrypoint.sh /

e2e/testdata/engine/Dockerfile

@@ -0,0 +1,6 @@
+ARG ENGINE_VERSION
+FROM docker:${ENGINE_VERSION}-dind
+
+# Trust the tlsregistry CA so dockerd connects without --insecure-registry.
+COPY registry/certs/ca.crt /usr/local/share/ca-certificates/tlsregistry-ca.crt
+RUN update-ca-certificates

e2e/testdata/registry/Dockerfile

@@ -0,0 +1,3 @@
+FROM registry:3
+COPY auth /auth
+COPY certs /certs

e2e/testdata/registry/auth/htpasswd

@@ -0,0 +1 @@
+e2e:$2y$05$DxRBsGSy61vZsBgNVxwUh.UtZmlg3wZHMxYcHYAlupY7r1xbIiuoq

e2e/testdata/registry/certs/gen-certs.sh

@@ -0,0 +1,33 @@
+#!/bin/sh
+set -eu
+
+# Regenerate test certificates for the TLS-enabled private registry.
+# Run this from the repository root or from e2e/testdata/registry/certs/.
+
+cd "$(dirname "$0")"
+
+# --- CA ---
+openssl genrsa -out ca.key 2048
+openssl req -new -x509 -days 3650 \
+	-key ca.key \
+	-subj '/CN=Test CA (TLS Registry)' \
+	-out ca.crt
+
+# --- Server cert for tlsregistry (signed by CA) ---
+cat > openssl-tlsregistry.cnf <<-EOF
+	[v3_req]
+	subjectAltName=DNS:tlsregistry
+EOF
+openssl genrsa -out tlsregistry.key 2048
+openssl req -new \
+	-key tlsregistry.key \
+	-subj '/CN=tlsregistry' \
+	-out tlsregistry.csr
+openssl x509 -req -days 3650 \
+	-in tlsregistry.csr \
+	-CA ca.crt -CAkey ca.key \
+	-CAcreateserial \
+	-out tlsregistry.crt \
+	-extfile openssl-tlsregistry.cnf \
+	-extensions v3_req
+rm -f tlsregistry.csr ca.srl openssl-tlsregistry.cnf

scripts/test/e2e/run

@@ -26,8 +26,55 @@ setup() {
         export TEST_CONNHELPER_SSH_ID_RSA_PUB
         file="${file}:./e2e/compose-env.connhelper-ssh.yaml"
     fi
+    # Generate TLS certificates for the TLS-enabled private registry.
+    # The certs are baked into the tlsregistry and engine container images,
+    # so they must exist on disk before docker compose up --build.
+    # gen-certs.sh handles its own directory navigation.
+    certdir=e2e/testdata/registry/certs
+    missing=0
+    for f in ca.crt ca.key tlsregistry.crt tlsregistry.key; do
+        if [ ! -f "${certdir}/${f}" ]; then
+            missing=1
+            break
+        fi
+    done
+    if [ "$missing" -eq 1 ]; then
+        sh e2e/testdata/registry/certs/gen-certs.sh
+    fi
+
     COMPOSE_PROJECT_NAME=$project COMPOSE_FILE=$file docker compose up --build -d >&2
 
+    # Ensure supporting services exist before running tests. If one fails to start,
+    # fail fast and surface logs instead of waiting on downstream DNS timeouts.
+    local deadline=$((SECONDS + 120))
+    while [ $SECONDS -lt $deadline ]; do
+        local ok=1
+        for svc in registry privateregistry tlsregistry engine; do
+            cid="$(COMPOSE_PROJECT_NAME=$project COMPOSE_FILE=$file docker compose ps -q "$svc" 2>/dev/null || true)"
+            if [ -z "$cid" ]; then
+                ok=0
+                break
+            fi
+            if ! docker inspect -f '{{.State.Running}}' "$cid" 2>/dev/null | grep -q true; then
+                ok=0
+                break
+            fi
+        done
+        if [ "$ok" -eq 1 ]; then
+            break
+        fi
+        sleep 1
+    done
+    if [ $SECONDS -ge $deadline ]; then
+        echo "Timed out waiting for e2e services to start" >&2
+        COMPOSE_PROJECT_NAME=$project COMPOSE_FILE=$file docker compose ps >&2 || true
+        for svc in registry privateregistry tlsregistry engine; do
+            echo "--- logs: $svc ---" >&2
+            COMPOSE_PROJECT_NAME=$project COMPOSE_FILE=$file docker compose logs --no-color --tail=200 "$svc" >&2 || true
+        done
+        exit 1
+    fi
+
     local network="${project}_default"
     # TODO: only run if inside a container
     docker network connect "$network" "$(hostname)"