kumahq / kuma

🐻 The multi-zone service mesh for containers, Kubernetes and VMs. Built with Envoy. CNCF Sandbox Project.
https://kuma.io/install
Apache License 2.0
3.61k stars 332 forks source link

Enable EnsurePathExistsOnAdd for ContainerPatch #4525

Closed rainest closed 2 years ago

rainest commented 2 years ago

Description

In brief Update the ContainerPatch json-patch invocation to use an ApplyWithOptions with EnsurePathExistsOnAdd.

Background

ContainerPatches may need to manipulate children of objects that don't necessarily already exist in the kuma-sidecar container. For example, for we attempted to add a ContainerPatch to mount tokens in the Helm chart, and found that we could not get the initial suggestion to work. ReplicaSets would fail to spawn Pods with

Error creating: admission webhook "namespace-kuma-injector.kuma.io" denied the request: could not apply patches ["fexample-kong-kuma-patch"]: add operation does not apply: doc is missing path: "/volumeMounts/-": missing value

Adding an empty volumeMounts array first clears this error and mounts as expected:

apiVersion: kuma.io/v1alpha1
kind: ContainerPatch
metadata:
  name: {{ template "kong.fullname" . }}-kuma-patch
  namespace: kuma-system
spec:
  sidecarPatch:
    - op: add
      path: "/volumeMounts"
      value: '[]'
    - op: add
      path: "/volumeMounts/-"
      value: '{ "mountPath": "/var/run/secrets/kubernetes.io/serviceaccount/", "name":"{{ template "kong.serviceAccountTokenName" . }}", "readOnly": true }'

However, the original suggestion working in another environment suggests that there are other Kuma options which create their own mounts. Adding the empty array as above will clear out that configuration, since a json-patch add overwrites existing configuration, and there's no add-if-absent that avoids this.

json-patch documentation suggests that the EnsurePathExistsOnAdd option can let us skip adding an empty array first, to support adding only the individual mount and not clobber other configuration. Not 100% that behaves as we expect with arrays, but the json-patch code looks like it does handle creating them when the request is for an array member.

jakubdyszkiewicz commented 2 years ago

Triage: we will change the behavior of op: add