feat: authenticate docker on PullImage (#2446)

* feat: authenticate docker on PullImage

I'm using k3s and specifically its LoadImages function to avoid hassle
for images not published to a public repository. Through this, I noticed
we were accidentally only authenticating docker pulls when launching a
container.

This authenticates the same way, regardless of if you are launching a
container or pulling its image. This also improves the LoadImages test
as before, the pod could have not started due to an image problem, yet
still pass the test.

Signed-off-by: Adrian Cole <adrian@tetrate.io>

* early quit on terminated

Signed-off-by: Adrian Cole <adrian@tetrate.io>

---------

Signed-off-by: Adrian Cole <adrian@tetrate.io>

docker.go

@@ -1027,20 +1027,6 @@ func (p *DockerProvider) CreateContainer(ctx context.Context, req ContainerReque
 			pullOpt := types.ImagePullOptions{
 				Platform: req.ImagePlatform, // may be empty
 			}
-
-			registry, imageAuth, err := DockerImageAuth(ctx, imageName)
-			if err != nil {
-				p.Logger.Printf("Failed to get image auth for %s. Setting empty credentials for the image: %s. Error is:%s", registry, imageName, err)
-			} else {
-				// see https://github.com/docker/docs/blob/e8e1204f914767128814dca0ea008644709c117f/engine/api/sdk/examples.md?plain=1#L649-L657
-				encodedJSON, err := json.Marshal(imageAuth)
-				if err != nil {
-					p.Logger.Printf("Failed to marshal image auth. Setting empty credentials for the image: %s. Error is:%s", imageName, err)
-				} else {
-					pullOpt.RegistryAuth = base64.URLEncoding.EncodeToString(encodedJSON)
-				}
-			}
-
 			if err := p.attemptToPullImage(ctx, imageName, pullOpt); err != nil {
 				return nil, err
 			}
@@ -1245,10 +1231,20 @@ func (p *DockerProvider) ReuseOrCreateContainer(ctx context.Context, req Contain
 // attemptToPullImage tries to pull the image while respecting the ctx cancellations.
 // Besides, if the image cannot be pulled due to ErrorNotFound then no need to retry but terminate immediately.
 func (p *DockerProvider) attemptToPullImage(ctx context.Context, tag string, pullOpt types.ImagePullOptions) error {
-	var (
-		err  error
-		pull io.ReadCloser
-	)
+	registry, imageAuth, err := DockerImageAuth(ctx, tag)
+	if err != nil {
+		p.Logger.Printf("Failed to get image auth for %s. Setting empty credentials for the image: %s. Error is:%s", registry, tag, err)
+	} else {
+		// see https://github.com/docker/docs/blob/e8e1204f914767128814dca0ea008644709c117f/engine/api/sdk/examples.md?plain=1#L649-L657
+		encodedJSON, err := json.Marshal(imageAuth)
+		if err != nil {
+			p.Logger.Printf("Failed to marshal image auth. Setting empty credentials for the image: %s. Error is:%s", tag, err)
+		} else {
+			pullOpt.RegistryAuth = base64.URLEncoding.EncodeToString(encodedJSON)
+		}
+	}
+
+	var pull io.ReadCloser
 	err = backoff.Retry(func() error {
 		pull, err = p.client.ImagePull(ctx, tag, pullOpt)
 		if err != nil {

modules/k3s/k3s_test.go

@@ -2,11 +2,13 @@ package k3s_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kwait "k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"
 
@@ -16,7 +18,9 @@ import (
 )
 
 func Test_LoadImages(t *testing.T) {
-	ctx := context.Background()
+	// Give up to three minutes to run this test
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(3*time.Minute))
+	defer cancel()
 
 	k3sContainer, err := k3s.RunContainer(ctx,
 		testcontainers.WithImage("docker.io/rancher/k3s:v1.27.1-k3s1"),
@@ -53,20 +57,20 @@ func Test_LoadImages(t *testing.T) {
 	}
 
 	// ensure nginx image is available locally
-	err = provider.PullImage(context.Background(), "nginx")
+	err = provider.PullImage(ctx, "nginx")
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	t.Run("Test load image not available", func(t *testing.T) {
-		err := k3sContainer.LoadImages(context.Background(), "fake.registry/fake:non-existing")
+		err := k3sContainer.LoadImages(ctx, "fake.registry/fake:non-existing")
 		if err == nil {
 			t.Fatal("should had failed")
 		}
 	})
 
 	t.Run("Test load image in cluster", func(t *testing.T) {
-		err := k3sContainer.LoadImages(context.Background(), "nginx")
+		err := k3sContainer.LoadImages(ctx, "nginx")
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -90,23 +94,44 @@ func Test_LoadImages(t *testing.T) {
 			},
 		}
 
-		_, err = k8s.CoreV1().Pods("default").Create(context.Background(), pod, metav1.CreateOptions{})
+		_, err = k8s.CoreV1().Pods("default").Create(ctx, pod, metav1.CreateOptions{})
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		err = kwait.PollUntilContextCancel(ctx, time.Second, true, func(ctx context.Context) (bool, error) {
+			state, err := getTestPodState(ctx, k8s)
+			if err != nil {
+				return false, err
+			}
+			if state.Terminated != nil {
+				return false, fmt.Errorf("pod terminated: %v", state.Terminated)
+			}
+			return state.Running != nil, nil
+		})
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		time.Sleep(1 * time.Second)
-		pod, err = k8s.CoreV1().Pods("default").Get(context.Background(), "test-pod", metav1.GetOptions{})
+		state, err := getTestPodState(ctx, k8s)
 		if err != nil {
 			t.Fatal(err)
 		}
-		waiting := pod.Status.ContainerStatuses[0].State.Waiting
-		if waiting != nil && waiting.Reason == "ErrImageNeverPull" {
-			t.Fatal("Image was not loaded")
+		if state.Running == nil {
+			t.Fatalf("Unexpected status %v", state)
 		}
 	})
 }
 
+func getTestPodState(ctx context.Context, k8s *kubernetes.Clientset) (state corev1.ContainerState, err error) {
+	var pod *corev1.Pod
+	pod, err = k8s.CoreV1().Pods("default").Get(ctx, "test-pod", metav1.GetOptions{})
+	if err != nil || len(pod.Status.ContainerStatuses) == 0 {
+		return
+	}
+	return pod.Status.ContainerStatuses[0].State, nil
+}
+
 func Test_APIServerReady(t *testing.T) {
 	ctx := context.Background()
 
@@ -169,7 +194,7 @@ func Test_WithManifestOption(t *testing.T) {
 	k3sContainer, err := k3s.RunContainer(ctx,
 		testcontainers.WithImage("docker.io/rancher/k3s:v1.27.1-k3s1"),
 		k3s.WithManifest("nginx-manifest.yaml"),
-		testcontainers.WithWaitStrategy(wait.ForExec([]string{"kubectl", "wait", "pod", "nginx","--for=condition=Ready"})),
+		testcontainers.WithWaitStrategy(wait.ForExec([]string{"kubectl", "wait", "pod", "nginx", "--for=condition=Ready"})),
 	)
 	if err != nil {
 		t.Fatal(err)