fix: support for ephemeral container mutation (#3560)

Signed-off-by: Brandt Keller <[email protected]>

Author: brandtkeller

Diff for: packages/zarf-agent/manifests/webhook.yaml

@@ -44,6 +44,7 @@ webhooks:
           - "v1"
         resources:
           - "pods"
+          - "pods/ephemeralcontainers"
     admissionReviewVersions:
       - "v1"
       - "v1beta1"

Diff for: site/src/content/docs/contribute/testing.mdx

@@ -105,8 +105,6 @@ When adding new tests, there are several requirements that must be followed, inc
 ```go
 func TestFooBarBaz(t *testing.T) {
     t.Log("E2E: Enter useful description here")
-    e2e.setup(t)
-    defer e2e.teardown(t)
 
     ...
 }
@@ -119,8 +117,6 @@ The end-to-end tests are run sequentially and the naming convention is set inten
 - 00-19 tests run prior to `zarf init` (cluster not initialized).
 
 :::note
-Tests 20+ should call `e2e.setupWithCluster(t)` instead of `e2e.setup(t)`.
-
 Due to resource constraints in public GitHub runners, K8s tests are only performed on Linux.
 :::
 

Diff for: src/internal/agent/hooks/pods.go

@@ -66,6 +66,10 @@ func mutatePod(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Clu
 		return nil, fmt.Errorf(lang.AgentErrParsePod, err)
 	}
 
+	if r.SubResource != "" {
+		return mutatePodSubresource(ctx, r, cluster)
+	}
+
 	if pod.Labels != nil && pod.Labels["zarf-agent"] == "patched" {
 		// We've already played with this pod, just keep swimming 🐟
 		return &operations.Result{
@@ -105,9 +109,9 @@ func mutatePod(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Clu
 		patches = append(patches, operations.ReplacePatchOperation(path, replacement))
 	}
 
-	// update the image host for each ephemeral container
-	for idx, container := range pod.Spec.EphemeralContainers {
-		path := fmt.Sprintf("/spec/ephemeralContainers/%d/image", idx)
+	// update the image host for each normal container
+	for idx, container := range pod.Spec.Containers {
+		path := fmt.Sprintf("/spec/containers/%d/image", idx)
 		replacement, err := transform.ImageTransformHost(registryURL, container.Image)
 		if err != nil {
 			return nil, err
@@ -116,9 +120,55 @@ func mutatePod(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Clu
 		patches = append(patches, operations.ReplacePatchOperation(path, replacement))
 	}
 
-	// update the image host for each normal container
-	for idx, container := range pod.Spec.Containers {
-		path := fmt.Sprintf("/spec/containers/%d/image", idx)
+	// Add the "zarf-agent"="patched" label patch
+	patches = append(patches, getLabelPatch(pod.Labels))
+
+	// Add the annotations label patch
+	patches = append(patches, operations.ReplacePatchOperation("/metadata/annotations", updatedAnnotations))
+
+	return &operations.Result{
+		Allowed:  true,
+		PatchOps: patches,
+	}, nil
+}
+
+// mutatePodSubresource handles pod subresource mutation
+func mutatePodSubresource(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (*operations.Result, error) {
+	switch res := r.SubResource; res {
+	case "ephemeralcontainers":
+		return mutateEphemeralContainers(ctx, r, cluster)
+	default:
+		// this likely won't be hit as the MutatingWebhookConfiguration would need to be modified - but this can help ensure they stay synchronized
+		return nil, fmt.Errorf("attempted mutation of unsupported subresource: %s", res)
+	}
+}
+
+func mutateEphemeralContainers(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Cluster) (*operations.Result, error) {
+	l := logger.From(ctx)
+	pod, err := parsePod(r.Object.Raw)
+	if err != nil {
+		return nil, fmt.Errorf(lang.AgentErrParsePod, err)
+	}
+
+	state, err := cluster.LoadZarfState(ctx)
+	if err != nil {
+		return nil, err
+	}
+	registryURL := state.RegistryInfo.Address
+
+	// Pods do not have a metadata.name at the time of admission if from a deployment so we don't log the name
+	l.Info("using the Zarf registry URL to mutate the Pod", "registry", registryURL)
+
+	updatedAnnotations := pod.Annotations
+	if updatedAnnotations == nil {
+		updatedAnnotations = make(map[string]string)
+	}
+
+	var patches []operations.PatchOperation
+
+	// update the image host for each ephemeral container
+	for idx, container := range pod.Spec.EphemeralContainers {
+		path := fmt.Sprintf("/spec/ephemeralContainers/%d/image", idx)
 		replacement, err := transform.ImageTransformHost(registryURL, container.Image)
 		if err != nil {
 			return nil, err
@@ -127,10 +177,10 @@ func mutatePod(ctx context.Context, r *v1.AdmissionRequest, cluster *cluster.Clu
 		patches = append(patches, operations.ReplacePatchOperation(path, replacement))
 	}
 
-	patches = append(patches, getLabelPatch(pod.Labels))
-
+	// Add the annotations label patch
 	patches = append(patches, operations.ReplacePatchOperation("/metadata/annotations", updatedAnnotations))
 
+	// Return the result of the subresource mutation
 	return &operations.Result{
 		Allowed:  true,
 		PatchOps: patches,

Diff for: src/internal/agent/hooks/pods_test.go

@@ -20,7 +20,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
-func createPodAdmissionRequest(t *testing.T, op v1.Operation, pod *corev1.Pod) *v1.AdmissionRequest {
+func createPodAdmissionRequest(t *testing.T, op v1.Operation, pod *corev1.Pod, subResource string) *v1.AdmissionRequest {
 	t.Helper()
 	raw, err := json.Marshal(pod)
 	require.NoError(t, err)
@@ -29,6 +29,7 @@ func createPodAdmissionRequest(t *testing.T, op v1.Operation, pod *corev1.Pod) *
 		Object: runtime.RawExtension{
 			Raw: raw,
 		},
+		SubResource: subResource,
 	}
 }
 
@@ -52,16 +53,8 @@ func TestPodMutationWebhook(t *testing.T) {
 				Spec: corev1.PodSpec{
 					Containers:     []corev1.Container{{Name: "nginx", Image: "nginx"}},
 					InitContainers: []corev1.Container{{Name: "different", Image: "busybox"}},
-					EphemeralContainers: []corev1.EphemeralContainer{
-						{
-							EphemeralContainerCommon: corev1.EphemeralContainerCommon{
-								Name:  "alpine",
-								Image: "alpine",
-							},
-						},
-					},
 				},
-			}),
+			}, ""),
 			patch: []operations.PatchOperation{
 				operations.ReplacePatchOperation(
 					"/spec/imagePullSecrets",
@@ -71,10 +64,6 @@ func TestPodMutationWebhook(t *testing.T) {
 					"/spec/initContainers/0/image",
 					"127.0.0.1:31999/library/busybox:latest-zarf-2140033595",
 				),
-				operations.ReplacePatchOperation(
-					"/spec/ephemeralContainers/0/image",
-					"127.0.0.1:31999/library/alpine:latest-zarf-1117969859",
-				),
 				operations.ReplacePatchOperation(
 					"/spec/containers/0/image",
 					"127.0.0.1:31999/library/nginx:latest-zarf-3793515731",
@@ -90,7 +79,6 @@ func TestPodMutationWebhook(t *testing.T) {
 					"/metadata/annotations",
 					map[string]string{
 						"zarf.dev/original-image-nginx":     "nginx",
-						"zarf.dev/original-image-alpine":    "alpine",
 						"zarf.dev/original-image-different": "busybox",
 						"should-be":                         "mutated",
 					},
@@ -107,10 +95,48 @@ func TestPodMutationWebhook(t *testing.T) {
 				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{{Image: "nginx"}},
 				},
-			}),
+			}, ""),
 			patch: nil,
 			code:  http.StatusOK,
 		},
+		{
+			name: "ephermalcontainer update in pod with zarf-agent patched label should be mutated",
+			admissionReq: createPodAdmissionRequest(t, v1.Create, &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: map[string]string{"zarf-agent": "patched"},
+					Annotations: map[string]string{
+						"zarf.dev/original-image-nginx":  "nginx",
+						"zarf.dev/original-image-alpine": "alpine",
+					},
+				},
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{Name: "nginx", Image: "127.0.0.1:31999/library/nginx:latest-zarf-3793515731"}},
+					EphemeralContainers: []corev1.EphemeralContainer{
+						{
+							EphemeralContainerCommon: corev1.EphemeralContainerCommon{
+								Name:  "alpine",
+								Image: "alpine",
+							},
+						},
+					},
+					ImagePullSecrets: []corev1.LocalObjectReference{{Name: config.ZarfImagePullSecretName}},
+				},
+			}, "ephemeralcontainers"),
+			patch: []operations.PatchOperation{
+				operations.ReplacePatchOperation(
+					"/spec/ephemeralContainers/0/image",
+					"127.0.0.1:31999/library/alpine:latest-zarf-1117969859",
+				),
+				operations.ReplacePatchOperation(
+					"/metadata/annotations",
+					map[string]string{
+						"zarf.dev/original-image-nginx":  "nginx",
+						"zarf.dev/original-image-alpine": "alpine",
+					},
+				),
+			},
+			code: http.StatusOK,
+		},
 		{
 			name: "pod with no labels should not error",
 			admissionReq: createPodAdmissionRequest(t, v1.Create, &corev1.Pod{
@@ -120,7 +146,7 @@ func TestPodMutationWebhook(t *testing.T) {
 				Spec: corev1.PodSpec{
 					Containers: []corev1.Container{{Name: "nginx", Image: "nginx"}},
 				},
-			}),
+			}, ""),
 			patch: []operations.PatchOperation{
 				operations.ReplacePatchOperation(
 					"/spec/imagePullSecrets",

Diff for: src/test/e2e/38_ephemeral_container_test.go

@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: 2021-Present The Zarf Authors
+
+// Package test provides e2e tests for Zarf.
+package test
+
+import (
+	"fmt"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestEphemeralContainers(t *testing.T) {
+	t.Log("E2E: Ephemeral Containers mutation")
+
+	// cleanup - should perform cleanup in the event of pass or fail
+	t.Cleanup(func() {
+		e2e.Zarf(t, "package", "remove", "basic-pod", "--confirm") //nolint:errcheck
+	})
+
+	tmpdir := t.TempDir()
+
+	// we need to create a test package that contains the images we want to potentially use
+	// this should ideally be a single pod such that naming is static
+	stdOut, stdErr, err := e2e.Zarf(t, "package", "create", "src/test/packages/38-ephemeral-container", "-o", tmpdir, "--skip-sbom")
+	require.NoError(t, err, stdOut, stdErr)
+	packageName := fmt.Sprintf("zarf-package-basic-pod-%s-0.0.1.tar.zst", e2e.Arch)
+	path := filepath.Join(tmpdir, packageName)
+
+	// deploy the above package
+	stdOut, stdErr, err = e2e.Zarf(t, "package", "deploy", path, "--confirm")
+	require.NoError(t, err, stdOut, stdErr)
+
+	// using a pod the package deploys - run a kubectl debug command
+	stdOut, stdErr, err = e2e.Kubectl(t, "debug", "test-pod", "-n", "test", "--image=ghcr.io/zarf-dev/images/alpine:3.21.3", "--profile", "general")
+	require.NoError(t, err, stdOut, stdErr)
+
+	// wait for the ephemeralContainer to exist - as it need to traverse mutation/admission
+	stdOut, stdErr, err = e2e.Kubectl(t, "wait", "--namespace=test", "--for=jsonpath={.status.ephemeralContainerStatuses[*].image}=127.0.0.1:31337/zarf-dev/images/alpine:3.21.3-zarf-1792331847", "pod/test-pod", "--timeout=10s")
+	require.NoError(t, err, stdOut, stdErr)
+
+	podStdOut, stdErr, err := e2e.Kubectl(t, "get", "pod", "test-pod", "-n", "test", "-o", "jsonpath={.status.ephemeralContainerStatuses[*].image}")
+	require.NoError(t, err, podStdOut, stdErr)
+
+	// Ensure the image used contains the internal zarf registry (IE mutated)
+	require.Contains(t, podStdOut, "127.0.0.1:31337/zarf-dev/images/alpine:3.21.3-zarf-1792331847")
+}

Diff for: src/test/packages/38-ephemeral-container/pod.yaml

@@ -0,0 +1,16 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  creationTimestamp: null
+  labels:
+    run: test-pod
+  name: test-pod
+spec:
+  containers:
+  - image: ghcr.io/zarf-dev/images/alpine:3.21.3
+    name: test-pod
+    command: ["sleep", "3600"]
+    resources: {}
+  dnsPolicy: ClusterFirst
+  restartPolicy: Always
+status: {}

Diff for: src/test/packages/38-ephemeral-container/zarf.yaml

@@ -0,0 +1,17 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/zarf-dev/zarf/v0.49.1/zarf.schema.json
+kind: ZarfPackageConfig
+
+metadata:
+  name: basic-pod
+  version: 0.0.1
+
+components:
+  - name: alpine
+    required: true
+    manifests:
+      - name: alpine
+        namespace: test
+        files:
+          - pod.yaml
+    images:
+      - ghcr.io/zarf-dev/images/alpine:3.21.3