Add values.yaml option for a custom injection template (#29731)

* Add values.yaml option for a custom injection template

Part of
https://docs.google.com/document/d/1Rmp9B3DDypgMCau-YAqidx_r3qvKLZj6jUX-t22zj84/edit

* Expand test

manifests/charts/istio-control/istio-discovery/files/gen-istio.yaml

@@ -183,7 +183,8 @@ data:
           "autoInject": true,
           "enabled": true
         },
-        "rewriteAppHTTPProbe": true
+        "rewriteAppHTTPProbe": true,
+        "templates": {}
       }
     }
 

manifests/charts/istio-control/istio-discovery/templates/istiod-injector-configmap.yaml

@@ -33,7 +33,12 @@ data:
       "{{ $key }}": "{{ $val }}"
       {{- end }}
     templates:
+{{- if not (hasKey .Values.sidecarInjectorWebhook.templates "sidecar") }}
       sidecar: |
 {{ .Files.Get "files/injection-template.yaml" | trim | indent 8 }}
+{{- end }}
+{{- with .Values.sidecarInjectorWebhook.templates }}
+{{ toYaml . | trim | indent 6 }}
+{{- end }}
 
 {{- end }}

manifests/charts/istio-control/istio-discovery/values.yaml

@@ -97,6 +97,19 @@ sidecarInjectorWebhook:
     autoInject: true
 
   rewriteAppHTTPProbe: true
+
+  # Templates defines a set of custom injection templates that can be used. For example, defining:
+  #
+  # templates:
+  #   hello:
+  #    metadata
+  #      labels:
+  #        hello: world
+  #
+  # Then starting a pod with the `inject.istio.io/templates: hello` annotation, will result in the pod
+  # being injected with the hello=world labels.
+  # This is intended for advanced configuration only; most users should use the built in template
+  templates: {}
 istiodRemote:
   # Sidecar injector mutating webhook configuration url
   # For example: https://$remotePilotAddress:15017/inject

manifests/charts/istiod-remote/files/gen-istiod-remote.yaml

@@ -163,7 +163,8 @@ data:
           "autoInject": true,
           "enabled": true
         },
-        "rewriteAppHTTPProbe": true
+        "rewriteAppHTTPProbe": true,
+        "templates": {}
       }
     }
 

manifests/charts/istiod-remote/templates/istiod-injector-configmap.yaml

@@ -33,7 +33,12 @@ data:
       "{{ $key }}": "{{ $val }}"
       {{- end }}
     templates:
+{{- if not (hasKey .Values.sidecarInjectorWebhook.templates "sidecar") }}
       sidecar: |
 {{ .Files.Get "files/injection-template.yaml" | trim | indent 8 }}
+{{- end }}
+{{- with .Values.sidecarInjectorWebhook.templates }}
+{{ toYaml . | trim | indent 6 }}
+{{- end }}
 
 {{- end }}

manifests/charts/istiod-remote/values.yaml

@@ -77,6 +77,18 @@ sidecarInjectorWebhook:
     enabled: true
     autoInject: true
   rewriteAppHTTPProbe: true
+  # Templates defines a set of custom injection templates that can be used. For example, defining:
+  #
+  # templates:
+  #   hello:
+  #    metadata
+  #      labels:
+  #        hello: world
+  #
+  # Then starting a pod with the `inject.istio.io/templates: hello` annotation, will result in the pod
+  # being injected with the hello=world labels.
+  # This is intended for advanced configuration only; most users should use the built in template
+  templates: {}
 istiodRemote:
   # Sidecar injector mutating webhook configuration url
   # For example: https://$remotePilotAddress:15017/inject

operator/pkg/apis/istio/v1alpha1/values_types.proto

@@ -983,6 +983,19 @@ message SidecarInjectorConfig {
 
   // Configure the injection url for sidecar injector webhook
   string injectionURL = 22;
+
+  // Templates defines a set of custom injection templates that can be used. For example, defining:
+  //
+  // templates:
+  //   hello:
+  //    metadata
+  //      labels:
+  //        hello: world
+  //
+  // Then starting a pod with the `inject.istio.io/templates: hello` annotation, will result in the pod
+  // being injected with the hello=world labels.
+  // This is intended for advanced configuration only; most users should use the built in template
+  TypeMapStringInterface templates = 23;
 }
 
 // Configuration for each of the supported tracers.

pkg/kube/inject/inject.go

@@ -328,7 +328,10 @@ func RunTemplate(params InjectionParameters) (mergedPod *corev1.Pod, templatePod
 		return nil, nil, multierror.Prefix(err, "could not parse configuration values:")
 	}
 
-	strippedPod := stripPod(params)
+	strippedPod, err := reinsertOverrides(stripPod(params))
+	if err != nil {
+		return nil, nil, err
+	}
 
 	data := SidecarTemplateData{
 		TypeMeta:       params.typeMeta,

pkg/kube/inject/inject_test.go

@@ -266,6 +266,11 @@ func TestInjection(t *testing.T) {
 			in:   "proxy-override-args.yaml",
 			want: "proxy-override-args.yaml.injected",
 		},
+		{
+			in:         "custom-template.yaml",
+			want:       "custom-template.yaml.injected",
+			inFilePath: "custom-template.iop.yaml",
+		},
 	}
 	// Keep track of tests we add options above
 	// We will search for all test files and skip these ones

pkg/kube/inject/testdata/inject/custom-template.iop.yaml

@@ -0,0 +1,37 @@
+apiVersion: install.istio.io/v1alpha1
+kind: IstioOperator
+metadata:
+  namespace: istio-system
+  name: example-istiocontrolplane
+spec:
+  values:
+    sidecarInjectorWebhook:
+      templates:
+        custom: |
+          metadata:
+            annotations:
+              # Disable the built-in transformations. In the future we may want a template-level API
+              prometheus.istio.io/merge-metrics: "false"
+              sidecar.istio.io/rewriteAppHTTPProbers: "false"
+              foo: bar
+          spec:
+            containers:
+            {{- range $index, $container := .Spec.Containers }}
+            - name: {{ $container.Name }}
+              env:
+              - name: SOME_ENV
+                value: "true"
+              - name: SOME_FILE
+                value: /var/lib/data/foo.json
+              volumeMounts:
+              - mountPath: /var/lib/data/foo.json
+                subPath: foo.json
+                name: some-injected-file
+            {{- end}}
+            volumes:
+            - name: some-injected-file
+              downwardAPI:
+                items:
+                - path: foo.json
+                  fieldRef:
+                    fieldPath: "metadata.annotations['foo']"

pkg/kube/inject/testdata/inject/custom-template.yaml

@@ -0,0 +1,21 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: hello
+spec:
+  selector:
+    matchLabels:
+      app: hello
+  template:
+    metadata:
+      annotations:
+        inject.istio.io/templates: "custom"
+      labels:
+        app: hello
+    spec:
+      containers:
+      - name: hello
+        image: "fake.docker.io/google-samples/hello-go-gke:1.0"
+        readinessProbe:
+          httpGet:
+            port: 80

pkg/kube/inject/testdata/inject/custom-template.yaml.injected

@@ -0,0 +1,48 @@
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  creationTimestamp: null
+  name: hello
+spec:
+  selector:
+    matchLabels:
+      app: hello
+  strategy: {}
+  template:
+    metadata:
+      annotations:
+        foo: bar
+        inject.istio.io/templates: custom
+        prometheus.istio.io/merge-metrics: "false"
+        proxy.istio.io/overrides: '{"containers":[{"name":"hello","image":"fake.docker.io/google-samples/hello-go-gke:1.0","resources":{},"readinessProbe":{"httpGet":{"port":80}}}]}'
+        sidecar.istio.io/rewriteAppHTTPProbers: "false"
+        sidecar.istio.io/status: '{"initContainers":null,"containers":["hello"],"volumes":["some-injected-file"],"imagePullSecrets":null}'
+      creationTimestamp: null
+      labels:
+        app: hello
+    spec:
+      containers:
+      - env:
+        - name: SOME_ENV
+          value: "true"
+        - name: SOME_FILE
+          value: /var/lib/data/foo.json
+        image: fake.docker.io/google-samples/hello-go-gke:1.0
+        name: hello
+        readinessProbe:
+          httpGet:
+            port: 80
+        resources: {}
+        volumeMounts:
+        - mountPath: /var/lib/data/foo.json
+          name: some-injected-file
+          subPath: foo.json
+      volumes:
+      - downwardAPI:
+          items:
+          - fieldRef:
+              fieldPath: metadata.annotations['foo']
+            path: foo.json
+        name: some-injected-file
+status: {}
+---

pkg/kube/inject/webhook.go

@@ -511,6 +511,44 @@ func reapplyOverwrittenContainers(finalPod *corev1.Pod, originalPod *corev1.Pod,
 	return finalPod, nil
 }
 
+// reinsertOverrides applies the containers listed in OverrideAnnotation to a pod. This is to achieve
+// idempotency by handling an edge case where an injection template is modifying a container already
+// present in the pod spec. In these cases, the logic to strip injected containers would remove the
+// original injected parts as well, leading to the templating logic being different (for example,
+// reading the .Spec.Containers field would be empty).
+func reinsertOverrides(pod *corev1.Pod) (*corev1.Pod, error) {
+	type podOverrides struct {
+		Containers     []corev1.Container `json:"containers,omitempty"`
+		InitContainers []corev1.Container `json:"initContainers,omitempty"`
+	}
+
+	existingOverrides := podOverrides{}
+	if annotationOverrides, f := pod.Annotations[OverrideAnnotation]; f {
+		if err := json.Unmarshal([]byte(annotationOverrides), &existingOverrides); err != nil {
+			return nil, err
+		}
+	}
+
+	pod = pod.DeepCopy()
+	for _, c := range existingOverrides.Containers {
+		match := FindContainer(c.Name, pod.Spec.Containers)
+		if match != nil {
+			continue
+		}
+		pod.Spec.Containers = append(pod.Spec.Containers, c)
+	}
+
+	for _, c := range existingOverrides.InitContainers {
+		match := FindContainer(c.Name, pod.Spec.InitContainers)
+		if match != nil {
+			continue
+		}
+		pod.Spec.InitContainers = append(pod.Spec.InitContainers, c)
+	}
+
+	return pod, nil
+}
+
 func createPatch(pod *corev1.Pod, original []byte) ([]byte, error) {
 	reinjected, err := json.Marshal(pod)
 	if err != nil {