Enhance webhook unit tests by checking returned JSON patch (#2615)

Enhance webhook unit tests by checking returned JSON patch

Also have labels/annotations added during injection to be added in order

Fixes #2560

Signed-off-by: Alejandro Pedraza <alejandro@buoyant.io>

controller/proxy-injector/fake/data/deployment-inject-empty.yaml

@@ -1,25 +0,0 @@
-kind: Deployment
-apiVersion: apps/v1
-metadata:
-  name: nginx
-  namespace: kube-public
-  labels:
-    app: nginx
-spec:
-  replicas: 1
-  selector:
-    matchLabels:
-      app: nginx
-  template:
-    metadata:
-      labels:
-        app: nginx
-      annotations:
-        created-by: isim
-    spec:
-      containers:
-      - name: nginx
-        image: nginx
-        ports:
-        - name: http
-          containerPort: 80

controller/proxy-injector/fake/data/deployment-inject-enabled.yaml

@@ -1,26 +0,0 @@
-kind: Deployment
-apiVersion: apps/v1
-metadata:
-  name: nginx
-  namespace: kube-public
-  labels:
-    app: nginx
-spec:
-  replicas: 1
-  selector:
-    matchLabels:
-      app: nginx
-  template:
-    metadata:
-      labels:
-        app: nginx
-      annotations:
-        linkerd.io/inject: enabled
-        created-by: isim
-    spec:
-      containers:
-      - name: nginx
-        image: nginx
-        ports:
-        - name: http
-          containerPort: 80

controller/proxy-injector/fake/data/pod-inject-empty.yaml

@@ -0,0 +1,16 @@
+kind: Pod
+apiVersion: apps/v1
+metadata:
+  name: nginx
+  namespace: kube-public
+  annotations:
+    created-by: isim
+  labels:
+    app: nginx
+spec:
+  containers:
+  - name: nginx
+    image: nginx
+    ports:
+    - name: http
+      containerPort: 80

controller/proxy-injector/fake/data/pod-inject-enabled.yaml

@@ -0,0 +1,16 @@
+kind: Pod
+apiVersion: apps/v1
+metadata:
+  name: nginx
+  namespace: kube-public
+  annotations:
+    linkerd.io/inject: enabled
+  labels:
+    app: nginx
+  spec:
+    containers:
+    - name: nginx
+      image: nginx
+      ports:
+      - name: http
+        containerPort: 80

controller/proxy-injector/fake/data/pod.patch.json

@@ -0,0 +1,150 @@
+[
+  {
+    "op": "add",
+    "path": "/metadata/annotations/linkerd.io~1proxy-version",
+    "value": ""
+  },
+  {
+    "op": "add",
+    "path": "/metadata/annotations/linkerd.io~1identity-mode",
+    "value": "disabled"
+  },
+  {
+    "op": "add",
+    "path": "/metadata/labels/linkerd.io~1control-plane-ns",
+    "value": "linkerd"
+  },
+  {
+    "op": "add",
+    "path": "/metadata/labels/linkerd.io~1proxy-deployment",
+    "value": "owner-deployment"
+  },
+  {
+    "op": "add",
+    "path": "/spec/initContainers",
+    "value": []
+  },
+  {
+    "op": "add",
+    "path": "/spec/initContainers/-",
+    "value": {
+      "name": "linkerd-init",
+      "image": "gcr.io/linkerd-io/proxy-init:",
+      "args": [
+        "--incoming-proxy-port",
+        "4143",
+        "--outgoing-proxy-port",
+        "4140",
+        "--proxy-uid",
+        "2102",
+        "--inbound-ports-to-ignore",
+        "4190,4191"
+      ],
+      "resources": {},
+      "terminationMessagePolicy": "FallbackToLogsOnError",
+      "imagePullPolicy": "IfNotPresent",
+      "securityContext": {
+        "capabilities": {
+          "add": [
+            "NET_ADMIN"
+          ]
+        },
+        "privileged": false,
+        "runAsUser": 0,
+        "runAsNonRoot": false
+      }
+    }
+  },
+  {
+    "op": "add",
+    "path": "/spec/containers/-",
+    "value": {
+      "name": "linkerd-proxy",
+      "image": "gcr.io/linkerd-io/proxy:",
+      "ports": [
+        {
+          "name": "linkerd-proxy",
+          "containerPort": 4143
+        },
+        {
+          "name": "linkerd-admin",
+          "containerPort": 4191
+        }
+      ],
+      "env": [
+        {
+          "name": "LINKERD2_PROXY_LOG",
+          "value": "warn,linkerd2_proxy=info"
+        },
+        {
+          "name": "LINKERD2_PROXY_DESTINATION_SVC_ADDR",
+          "value": "linkerd-destination.linkerd.svc.cluster.local:8086"
+        },
+        {
+          "name": "LINKERD2_PROXY_CONTROL_LISTEN_ADDR",
+          "value": "0.0.0.0:4190"
+        },
+        {
+          "name": "LINKERD2_PROXY_ADMIN_LISTEN_ADDR",
+          "value": "0.0.0.0:4191"
+        },
+        {
+          "name": "LINKERD2_PROXY_OUTBOUND_LISTEN_ADDR",
+          "value": "127.0.0.1:4140"
+        },
+        {
+          "name": "LINKERD2_PROXY_INBOUND_LISTEN_ADDR",
+          "value": "0.0.0.0:4143"
+        },
+        {
+          "name": "LINKERD2_PROXY_DESTINATION_PROFILE_SUFFIXES",
+          "value": "."
+        },
+        {
+          "name": "LINKERD2_PROXY_INBOUND_ACCEPT_KEEPALIVE",
+          "value": "10000ms"
+        },
+        {
+          "name": "LINKERD2_PROXY_OUTBOUND_CONNECT_KEEPALIVE",
+          "value": "10000ms"
+        },
+        {
+          "name": "_pod_ns",
+          "valueFrom": {
+            "fieldRef": {
+              "fieldPath": "metadata.namespace"
+            }
+          }
+        },
+        {
+          "name": "LINKERD2_PROXY_DESTINATION_CONTEXT",
+          "value": "ns:$(_pod_ns)"
+        },
+        {
+          "name": "LINKERD2_PROXY_IDENTITY_DISABLED",
+          "value": "Identity is not yet available"
+        }
+      ],
+      "resources": {},
+      "livenessProbe": {
+        "httpGet": {
+          "path": "/metrics",
+          "port": 4191
+        },
+        "initialDelaySeconds": 10
+      },
+      "readinessProbe": {
+        "httpGet": {
+          "path": "/ready",
+          "port": 4191
+        },
+        "initialDelaySeconds": 2
+      },
+      "terminationMessagePolicy": "FallbackToLogsOnError",
+      "imagePullPolicy": "IfNotPresent",
+      "securityContext": {
+        "runAsUser": 2102
+      }
+    }
+  }
+]

controller/proxy-injector/fake/factory.go

@@ -28,10 +28,10 @@ func NewFactory(rootDir string) *Factory {
 	return &Factory{rootDir: rootDir}
 }
 
-// HTTPRequestBody returns the content of the specified file as a slice of
+// FileContents returns the content of the specified file as a slice of
 // bytes. If the file doesn't exist in the 'fake/data' folder, an error will be
 // returned.
-func (f *Factory) HTTPRequestBody(filename string) ([]byte, error) {
+func (f *Factory) FileContents(filename string) ([]byte, error) {
 	return ioutil.ReadFile(filepath.Join(f.rootDir, filename))
 }
 

controller/proxy-injector/webhook_test.go

@@ -1,7 +1,9 @@
 package injector
 
 import (
+	"encoding/json"
 	"fmt"
+	"reflect"
 	"testing"
 
 	"github.com/linkerd/linkerd2/controller/gen/config"
@@ -10,10 +12,13 @@ import (
 	pkgK8s "github.com/linkerd/linkerd2/pkg/k8s"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+type unmarshalledPatch []map[string]interface{}
+
 var (
 	factory *fake.Factory
 	configs = &config.All{
@@ -68,35 +73,46 @@ func TestGetPatch(t *testing.T) {
 			expected bool
 		}{
 			{
-				filename: "deployment-inject-empty.yaml",
+				filename: "pod-inject-empty.yaml",
 				ns:       nsEnabled,
 				conf:     confNsEnabled(),
 				expected: true,
 			},
 			{
-				filename: "deployment-inject-enabled.yaml",
+				filename: "pod-inject-enabled.yaml",
 				ns:       nsEnabled,
 				conf:     confNsEnabled(),
 				expected: true,
 			},
 			{
-				filename: "deployment-inject-enabled.yaml",
+				filename: "pod-inject-enabled.yaml",
 				ns:       nsDisabled,
 				conf:     confNsDisabled(),
 				expected: true,
 			},
 		}
 
+		expectedPatchBytes, err := factory.FileContents("pod.patch.json")
+		if err != nil {
+			t.Fatalf("Unexpected error: %s", err)
+		}
+		expectedPatch, err := unmarshalPatch(expectedPatchBytes)
+		if err != nil {
+			t.Fatalf("Unexpected error: %s", err)
+		}
+
 		for id, testCase := range testCases {
 			testCase := testCase // pin
 			t.Run(fmt.Sprintf("%d", id), func(t *testing.T) {
-				deployment, err := factory.HTTPRequestBody(testCase.filename)
+				pod, err := factory.FileContents(testCase.filename)
 				if err != nil {
 					t.Fatalf("Unexpected error: %s", err)
 				}
 
-				fakeReq := getFakeReq(deployment)
-				fullConf := testCase.conf.WithKind(fakeReq.Kind.Kind)
+				fakeReq := getFakeReq(pod)
+				fullConf := testCase.conf.
+					WithKind(fakeReq.Kind.Kind).
+					WithOwnerRetriever(ownerRetriever)
 				_, err = fullConf.ParseMetaAndYAML(fakeReq.Object.Raw)
 				if err != nil {
 					t.Fatal(err)
@@ -114,15 +130,22 @@ func TestGetPatch(t *testing.T) {
 				if patchStr != "[]" && !testCase.expected {
 					t.Fatalf("Did not expect injection for file '%s'", testCase.filename)
 				}
-				if patchStr == "[]" && testCase.expected {
-					t.Fatalf("Was expecting injection for file '%s'", testCase.filename)
+
+				actualPatch, err := unmarshalPatch(patchJSON)
+				if err != nil {
+					t.Fatalf("Unexpected error: %s", err)
 				}
+				if !reflect.DeepEqual(expectedPatch, actualPatch) {
+					t.Fatalf("The actual patch didn't match what was expected.\nExpected: %s\nActual: %s",
+						expectedPatchBytes, patchJSON)
+				}
+
 			})
 		}
 	})
 
 	t.Run("by checking container spec", func(t *testing.T) {
-		deployment, err := factory.HTTPRequestBody("deployment-with-injected-proxy.yaml")
+		deployment, err := factory.FileContents("deployment-with-injected-proxy.yaml")
 		if err != nil {
 			t.Fatalf("Unexpected error: %s", err)
 		}
@@ -142,9 +165,23 @@ func TestGetPatch(t *testing.T) {
 
 func getFakeReq(b []byte) *admissionv1beta1.AdmissionRequest {
 	return &admissionv1beta1.AdmissionRequest{
-		Kind:      metav1.GroupVersionKind{Kind: "Deployment"},
+		Kind:      metav1.GroupVersionKind{Kind: "Pod"},
 		Name:      "foobar",
 		Namespace: "linkerd",
 		Object:    runtime.RawExtension{Raw: b},
 	}
 }
+
+func ownerRetriever(p *v1.Pod) (string, string) {
+	return pkgK8s.Deployment, "owner-deployment"
+}
+
+func unmarshalPatch(patchJSON []byte) (unmarshalledPatch, error) {
+	var actualPatch unmarshalledPatch
+	err := json.Unmarshal(patchJSON, &actualPatch)
+	if err != nil {
+		return nil, err
+	}
+
+	return actualPatch, nil
+}

pkg/inject/inject.go

@@ -3,6 +3,7 @@ package inject
 import (
 	"encoding/json"
 	"fmt"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -617,17 +618,17 @@ func (conf *ResourceConfig) injectObjectMeta(patch *Patch) {
 		if len(conf.pod.meta.Labels) == 0 {
 			patch.addPodLabelsRoot()
 		}
-		for k, v := range conf.pod.labels {
-			patch.addPodLabel(k, v)
+		for _, k := range sortedKeys(conf.pod.labels) {
+			patch.addPodLabel(k, conf.pod.labels[k])
 		}
 	}
 
-	for k, v := range conf.pod.annotations {
-		patch.addPodAnnotation(k, v)
+	for _, k := range sortedKeys(conf.pod.annotations) {
+		patch.addPodAnnotation(k, conf.pod.annotations[k])
 
 		// append any additional pod annotations to the pod's meta.
 		// for e.g., annotations that were converted from CLI inject options.
-		conf.pod.meta.Annotations[k] = v
+		conf.pod.meta.Annotations[k] = conf.pod.annotations[k]
 	}
 }
 
@@ -933,3 +934,14 @@ func (conf *ResourceConfig) proxyOutboundSkipPorts() string {
 	}
 	return strings.Join(ports, ",")
 }
+
+func sortedKeys(m map[string]string) []string {
+	keys := []string{}
+	for k := range m {
+		keys = append(keys, k)
+	}
+
+	sort.Strings(keys)
+
+	return keys
+}