From 9544552dc2e7731663217829e76a4d7e5b58568d Mon Sep 17 00:00:00 2001 From: Anton Bronnikov Date: Sun, 11 Aug 2024 11:52:55 +0200 Subject: [PATCH] fix: deduplicate volume mounts on path, not name --- readme.md | 122 +++++++++++++++++++---------- server/k8s.go | 30 ++++--- test/cluster-role.yaml | 2 +- test/deployment-fargate.yaml | 2 +- test/deployment-node-exporter.yaml | 2 +- 5 files changed, 104 insertions(+), 54 deletions(-) diff --git a/readme.md b/readme.md index 5a709ad..73c9945 100644 --- a/readme.md +++ b/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 + ``` diff --git a/server/k8s.go b/server/k8s.go index d18f95f..1d65bbc 100644 --- a/server/k8s.go +++ b/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, diff --git a/test/cluster-role.yaml b/test/cluster-role.yaml index 418f4db..2793fd1 100644 --- a/test/cluster-role.yaml +++ b/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"] --- diff --git a/test/deployment-fargate.yaml b/test/deployment-fargate.yaml index 45b59d7..7ba2449 100644 --- a/test/deployment-fargate.yaml +++ b/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", diff --git a/test/deployment-node-exporter.yaml b/test/deployment-node-exporter.yaml index b4577a2..9fbc178 100644 --- a/test/deployment-node-exporter.yaml +++ b/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",