fix: deduplicate volume mounts on path, not name

readme.md

@@ -4,49 +4,87 @@ Initial implementation of the sidecar injector for k8s.
 
 ## TL;DR
 
-With configuration like this `kube-sidecar-injector` will make sure that any
-container that runs in EKS fargate will have prometheus node-exporter sidecar
-running next to it.
-
-```yaml
-inject:
-  - labelSelector:
-      matchExpressions:
-        - key: eks.amazonaws.com/fargate-profile
-          operator: Exists
-
-    namespaceSelector:
-      matchExpressions:
-        - key: kubernetes.io/metadata.name
-          operator: NotIn
-          values: [kube-system]
-
-    labels:
-      flashbots.net/prometheus-node-exporter: true
-
-    containers:
-      - name: node-exporter
-        image: prom/node-exporter:v1.7.0
-        args: [
-          "--log.format", "json",
-          "--web.listen-address", ":9100",
-        ]
-        ports:
-          - name: http-metrics
-            containerPort: 9100
-        resources:
-          requests:
-            cpu: 10m
-            memory: 64Mi
-
-```
+1.  With configuration like this `kube-sidecar-injector` will make sure that any
+    container that runs in EKS fargate will have prometheus node-exporter sidecar
+    running next to it:
+
+    ```yaml
+    inject:
+      - name: inject-node-exporter
+
+        labelSelector:
+          matchExpressions:
+            - key: eks.amazonaws.com/fargate-profile
+              operator: Exists
+
+        namespaceSelector:
+          matchExpressions:
+            - key: kubernetes.io/metadata.name
+              operator: NotIn
+              values: [kube-system]
+
+        labels:
+          flashbots.net/prometheus-node-exporter: true
+
+        containers:
+          - name: node-exporter
+            image: prom/node-exporter:v1.7.0
+            args: [
+              "--log.format", "json",
+              "--web.listen-address", ":9100",
+            ]
+            ports:
+              - name: node-exporter
+                containerPort: 9100
+            resources:
+              requests:
+                cpu: 10m
+                memory: 64Mi
+    ```
+
+2.  In conjunction with `trust-manager` this will allow to automatically mount
+    root CA in every pod:
+
+    ```yaml
+    inject:
+      - name: inject-internal-ca
+
+        volumes:
+          - name: internal-ca
+            configMap:
+              name: internal-ca
+
+        volumeMounts:
+          - mountPath: /usr/local/share/ca-certificates
+            name: internal-ca
+            readOnly: true
+
+          - mountPath: /etc/ssl/certs/internal-ca.crt
+            name: internal-ca
+            subPath: internal-ca.crt
+            readOnly: true
+    ```
 
 ### Caveats
 
-Single webhook configuration can me configured to apply multiple injection
-rules.  However, if these rules are supposed to interact somehow (for example
-rule A introduces changes that rule B is supposed to act upon) then they should
-be placed into _separate_ webhooks.
+- Single webhook configuration can be configured to apply multiple injection
+  rules.  However, if these rules should interact somehow (for example rule A
+  introduces changes that rule B is supposed to act upon) then these rules
+  should be placed into _separate_ webhooks.
+
+  See k8s webhook [reinvocation policy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy)
+  for the details.
+
+- It's not possible for the webhook to know at the runtime whether the patch it
+  generates is invalid.
+
+  For example, if you try to inject a container that has port name of more than
+  15 characters long k8s will not allow the modified pod to be deployed.
+
+  In situations like this, k8s will infinitely attempt the webhook admission,
+  without ever creating the pod.  In order to troubleshoot this issue it could
+  help to see actual underlying error from k8s with:
 
-See k8s webhook [reinvocation policy](https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#reinvocation-policy)
-for the details.
+  ```shell
+  kubectl get events
+  ```

server/k8s.go

@@ -143,6 +143,14 @@ func (s *Server) mutate(
 		UID:     req.UID,
 	}
 
+	if req.Kind.Kind != "Pod" || req.Kind.Group != "" {
+		l.Warn("Received admission request for a non-pod object => skipping...",
+			zap.String("group", req.Kind.Group),
+			zap.String("kind", req.Kind.Kind),
+		)
+		return res
+	}
+
 	pod := &core_v1.Pod{}
 	if err := json.Unmarshal(req.Object.Raw, pod); err != nil {
 		l.Error("Failed to decode raw object for pod",
@@ -170,6 +178,11 @@ func (s *Server) mutate(
 		zap.String("username", req.UserInfo.Username),
 	)
 
+	l.Debug("Admission request details",
+		zap.Any("admissionRequest", req),
+		zap.Any("pod", pod),
+	)
+
 	patches, err := s.mutatePod(ctx, pod, fingerprint)
 	if err != nil {
 		l.Error("Failed to mutate pod",
@@ -192,6 +205,10 @@ func (s *Server) mutate(
 		res.PatchType = &patchType
 	}
 
+	l.Debug("Admission response details",
+		zap.Any("admissionRequest", res),
+	)
+
 	return res
 }
 
@@ -264,15 +281,15 @@ func (s *Server) mutatePod(
 		for idx, c := range pod.Spec.Containers {
 			existing := make(map[string]struct{}, len(c.VolumeMounts))
 			for _, vm := range c.VolumeMounts {
-				existing[vm.Name] = struct{}{}
+				existing[vm.MountPath] = struct{}{}
 			}
 
 			volumeMounts := make([]core_v1.VolumeMount, 0, len(inject.VolumeMounts))
 			for _, vm := range inject.VolumeMounts {
-				if _, collision := existing[vm.Name]; collision {
-					l.Warn("Volume mount with the same name already exists => skipping...",
+				if _, collision := existing[vm.MountPath]; collision {
+					l.Warn("Volume mount with the same mount path already exists => skipping...",
 						zap.String("container", c.Name),
-						zap.String("volumeMount", vm.Name),
+						zap.String("mountPath", vm.MountPath),
 					)
 					continue
 				}
@@ -347,11 +364,6 @@ func (s *Server) mutatePod(
 
 	// mark pod as processed
 	if len(res) > 0 {
-		l.Debug("Created patch for pod",
-			zap.Any("pod", pod),
-			zap.Any("patch", res),
-		)
-
 		timestamp := time.Now().Format(time.RFC3339)
 		p, err := patch.UpdatePodAnnotations(pod, map[string]string{
 			annotationProcessed: timestamp,

test/cluster-role.yaml

@@ -18,7 +18,7 @@ metadata:
 rules:
   - apiGroups: ["admissionregistration.k8s.io"]
     resources: ["mutatingwebhookconfigurations"]
-    verbs: ["create", "get", "delete", "list", "patch", "update", "watch"]
+    verbs: ["create", "get", "update"]
 
 ---
 

test/deployment-fargate.yaml

@@ -35,7 +35,7 @@ spec:
       serviceAccountName: kube-sidecar-injector
       containers:
         - name: kube-sidecar-injector-fargate
-          image: kube-sidecar-injector:0.0.8-dev
+          image: kube-sidecar-injector:0.0.9-dev
           args: [
             "--log-level", "info",
             "--log-mode", "dev",

test/deployment-node-exporter.yaml

@@ -35,7 +35,7 @@ spec:
       serviceAccountName: kube-sidecar-injector
       containers:
         - name: kube-sidecar-injector-node-exporter
-          image: kube-sidecar-injector:0.0.8-dev
+          image: kube-sidecar-injector:0.0.9-dev
           args: [
             "--log-level", "info",
             "--log-mode", "dev",