✨ Fix up the webhook to only touch what it actually cares about.

This specifically means it won't try to erase fields it thinks shouldn't be there.

Author: coderanger

.github/workflows/main.yml

@@ -38,7 +38,7 @@ jobs:
         run: |
           os=$(go env GOOS)
           arch=$(go env GOARCH)
-          version=1.24.2
+          version=1.29.3
           curl -L https://storage.googleapis.com/kubebuilder-tools/kubebuilder-tools-${version}-${os}-${arch}.tar.gz | tar -xz -C /tmp/
           sudo mv /tmp/kubebuilder /usr/local/kubebuilder
       - run: make test

webhook/webhook.go

@@ -18,14 +18,13 @@ package webhook
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"os"
 
 	"github.com/pkg/errors"
+	jsonpatch "gomodules.xyz/jsonpatch/v2"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/resource"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -90,6 +89,23 @@ func (hook *initInjector) handleInner(ctx context.Context, req admission.Request
 		}
 	}
 
+	if len(migrators) == 0 {
+		// Nothing to do.
+		resp := admission.Allowed("no migrators")
+		return &resp, nil
+	}
+
+	patches := []jsonpatch.JsonPatchOperation{}
+	// Check that initContainers exists at all.
+	if len(pod.Spec.InitContainers) == 0 {
+		patch := jsonpatch.JsonPatchOperation{
+			Operation: "add",
+			Path:      "/spec/initContainers",
+			Value:     []interface{}{},
+		}
+		patches = append(patches, patch)
+	}
+
 	// For each migrator, inject an initContainer.
 	for _, m := range migrators {
 		// Look for the container named in the migrator and pull the image from that. If no container
@@ -100,27 +116,26 @@ func (hook *initInjector) handleInner(ctx context.Context, req admission.Request
 				podIdx = i
 			}
 		}
-		initContainer := corev1.Container{
-			Name:    fmt.Sprintf("migrate-wait-%s", m.Name),
-			Image:   os.Getenv("WAITER_IMAGE"),
-			Command: []string{"/waiter", pod.Spec.Containers[podIdx].Image, m.Namespace, m.Name, os.Getenv("API_HOSTNAME")},
-			Resources: corev1.ResourceRequirements{
-				Requests: corev1.ResourceList{
-					corev1.ResourceMemory: resource.MustParse("16M"),
-					corev1.ResourceCPU:    resource.MustParse("10m"),
+		patch := jsonpatch.JsonPatchOperation{
+			Operation: "add",
+			Path:      "/spec/initContainers/-",
+			Value: map[string]interface{}{
+				"name":    fmt.Sprintf("migrate-wait-%s", m.Name),
+				"image":   os.Getenv("WAITER_IMAGE"),
+				"command": []string{"/waiter", pod.Spec.Containers[podIdx].Image, m.Namespace, m.Name, os.Getenv("API_HOSTNAME")},
+				"resources": map[string]interface{}{
+					"requests": map[string]string{
+						"memory": "16M",
+						"cpu":    "10m",
+					},
 				},
 			},
 		}
 		log.Info("Injecting init container", "pod", fmt.Sprintf("%s/%s", req.Namespace, req.Name), "migrator", fmt.Sprintf("%s/%s", m.Namespace, m.Name))
-		pod.Spec.InitContainers = append(pod.Spec.InitContainers, initContainer)
-	}
-
-	marshaledPod, err := json.Marshal(pod)
-	if err != nil {
-		return nil, errors.Wrap(err, "error encoding response object")
+		patches = append(patches, patch)
 	}
 
-	resp := admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod)
+	resp := admission.Patched("injecting init containers", patches...)
 	return &resp, nil
 }
 

webhook/webhook_test.go

@@ -25,6 +25,8 @@ import (
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	migrationsv1beta1 "github.com/coderanger/migrations-operator/api/v1beta1"
 )
@@ -63,6 +65,32 @@ var _ = Describe("InitInjector", func() {
 		Expect(pod.Spec.InitContainers).To(BeEmpty())
 	})
 
+	It("doesn't touch existing init containers with no migrators", func() {
+		c := helper.TestClient
+
+		pod := &corev1.Pod{
+			ObjectMeta: metav1.ObjectMeta{Name: "testing", Labels: map[string]string{"app": "testing"}},
+			Spec: corev1.PodSpec{
+				Containers: []corev1.Container{
+					{
+						Name:  "main",
+						Image: "fake",
+					},
+				},
+				InitContainers: []corev1.Container{
+					{
+						Name:  "init",
+						Image: "fake",
+					},
+				},
+			},
+		}
+		c.Create(pod)
+
+		c.EventuallyGetName("testing", pod)
+		Expect(pod.Spec.InitContainers[0].Name).To(Equal("init"))
+	})
+
 	It("injects with a matching migrator", func() {
 		c := helper.TestClient
 
@@ -252,4 +280,35 @@ var _ = Describe("InitInjector", func() {
 		err := c.Create(context.Background(), pod)
 		Expect(err).To(MatchError(ContainSubstring("no migrators found matching pod")))
 	})
+
+	It("doesn't remove restartPolicy from init containers", func() {
+		c := helper.TestClient
+
+		pod := &unstructured.Unstructured{}
+		pod.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"})
+		pod.SetName("testing")
+		pod.UnstructuredContent()["spec"] = map[string][]map[string]string{
+			"containers": {
+				{
+					"name":  "main",
+					"image": "fake",
+				},
+			},
+			"initContainers": {
+				{
+					"name":          "sidecar",
+					"image":         "fake",
+					"restartPolicy": "Always",
+				},
+			},
+		}
+		c.Create(pod)
+
+		c.EventuallyGetName("testing", pod)
+		spec := pod.UnstructuredContent()["spec"].(map[string]interface{})
+		println(spec)
+		initContainers := spec["initContainers"].([]interface{})
+		sidecar := initContainers[0].(map[string]interface{})
+		Expect(sidecar["restartPolicy"]).To(Equal("Always"))
+	})
 })