fix(kubernetes-plugin): sanitize volumes configuration for helm and k…

…ubernetes type pod runners (#6251)

* fix(kubernetes-plugin): sanitize volumes configuration for helm and kubernetes type pod runners

* chore: add tests and adjust docs

* chore: remove unused import

* chore: fix test

* style: empty lines and spacing

* style: convert plain comment to JSDoc

* fix: recognize octal numbers in YAML < 1.2

* chore: remove unnecessary return value

* refactor: unwrap nested if-blocks

Use immediate returns to avoid arrow-style code.

* refactor: weaken args nullability of the function

* refactor: move pod volume sanitizer to `prepareRunPodSpec`

To call it only in one place, for all action types.

* chore: remove unnecessary arg

* chore(test): deep-clone of helm target to prevent shared data pollution

* fix: sanitize containers defined in pod spec

* docs: add warning about retained volume types to helm-pod and kubernetes-pod run

---------

Co-authored-by: Vladimir Vagaytsev <[email protected]>

core/src/plugins/kubernetes/config.ts

@@ -918,6 +918,8 @@ export const runPodResourceSchema = (kind: string) =>
         This resource will be selected from the manifests provided in this ${kind}'s \`files\` or \`manifests\` config field.
 
         The following fields from the Pod will be used (if present) when executing the ${kind}:
+
+        **Warning**: Garden will retain \`configMaps\` and \`secrets\` as volumes, but remove \`persistentVolumeClaim\` volumes from the Pod spec, as they might already be mounted.
         ${runPodSpecWhitelistDescription()}
         `
   )
@@ -932,7 +934,7 @@ export const runPodSpecSchema = (kind: string) =>
 
     You can find the full Pod spec in the [official Kubernetes documentation](https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#PodSpec)
 
-    The following Pod spec fields from the selected \`resource\` will be used (if present) when executing the ${kind}:
+    The following Pod spec fields from the \`podSpec\` will be used (if present) when executing the ${kind}:
     ${runPodSpecWhitelistDescription()}
   `
     )
@@ -949,6 +951,8 @@ export const kubernetesTaskSchema = () =>
         ${serviceResourceDescription}
 
         The following pod spec fields from the service resource will be used (if present) when executing the task:
+
+        **Warning**: Garden will retain \`configMaps\` and \`secrets\` as volumes, but remove \`persistentVolumeClaim\` volumes from the Pod spec, as they might already be mounted.
         ${runPodSpecWhitelistDescription()}`
       ),
       ...kubernetesCommonRunSchemaKeys(),
@@ -966,6 +970,8 @@ export const kubernetesTestSchema = () =>
         ${serviceResourceDescription}
 
         The following pod spec fields from the service resource will be used (if present) when executing the test suite:
+
+        **Warning**: Garden will retain \`configMaps\` and \`secrets\` as volumes, but remove \`persistentVolumeClaim\` volumes from the Pod spec, as they might already be mounted.
         ${runPodSpecWhitelistDescription()}`
       ),
       command: joi

core/src/plugins/kubernetes/helm/module-config.ts

@@ -113,6 +113,8 @@ const helmTaskSchema = () =>
         ${serviceResourceDescription}
 
         The following pod spec fields from the service resource will be used (if present) when executing the task:
+
+        **Warning**: Garden will retain \`configMaps\` and \`secrets\` as volumes, but remove \`persistentVolumeClaim\` volumes from the Pod spec, as they might already be mounted.
         ${runPodSpecWhitelistDescription}`
     ),
   })
@@ -127,6 +129,8 @@ const helmTestSchema = () =>
         ${serviceResourceDescription}
 
         The following pod spec fields from the service resource will be used (if present) when executing the test suite:
+
+        **Warning**: Garden will retain \`configMaps\` and \`secrets\` as volumes, but remove \`persistentVolumeClaim\` volumes from the Pod spec, as they might already be mounted.
         ${runPodSpecWhitelistDescription}`
     ),
   })

core/src/plugins/kubernetes/kubernetes-type/common.ts

@@ -8,7 +8,6 @@
 
 import { basename, dirname, join, resolve } from "path"
 import fsExtra from "fs-extra"
-const { pathExists, readFile } = fsExtra
 import { flatten, isEmpty, keyBy, set } from "lodash-es"
 import type { KubernetesModule } from "./module-config.js"
 import type { KubernetesResource } from "../types.js"
@@ -32,6 +31,8 @@ import pFilter from "p-filter"
 import { kubectl } from "../kubectl.js"
 import { loadAndValidateYaml } from "../../../config/base.js"
 
+const { pathExists, readFile } = fsExtra
+
 /**
  * "DeployFile": Manifest has been read from one of the files declared in Garden Deploy `spec.files`
  * "DeployInline": Manifest has been declared inline using Garden Deploy `spec.manifests`

core/src/plugins/kubernetes/run.ts

@@ -23,7 +23,7 @@ import { KubeApi, KubernetesError } from "./api.js"
 import { getPodLogs, checkPodStatus } from "./status/pod.js"
 import type { KubernetesResource, KubernetesPod, KubernetesServerResource, SupportedRuntimeAction } from "./types.js"
 import type { ContainerEnvVars, ContainerResourcesSpec, ContainerVolumeSpec } from "../container/config.js"
-import { prepareEnvVars, makePodName, renderPodEvents } from "./util.js"
+import { prepareEnvVars, makePodName, renderPodEvents, sanitizeVolumesForPodRunner } from "./util.js"
 import { dedent, deline, randomString } from "../../util/string.js"
 import type { ArtifactSpec } from "../../config/validation.js"
 import { prepareSecrets } from "./secrets.js"
@@ -319,6 +319,8 @@ export async function prepareRunPodSpec({
   // and `configmap` actions (which are only supported for `container` actions, and are currently discouraged).
   if (volumes && volumes.length && action.type === "container") {
     configureVolumes(action, preparedPodSpec, volumes)
+  } else {
+    sanitizeVolumesForPodRunner(preparedPodSpec, container)
   }
 
   if (getArtifacts) {

core/src/plugins/kubernetes/util.ts

@@ -820,3 +820,59 @@ export function renderPodEvents(events: CoreV1Event[]): string {
 export function summarize(resources: KubernetesResource[]) {
   return resources.map((r) => `${r.kind} ${r.metadata.name}`).join(", ")
 }
+
+/**
+ * Filter out all volumes and volumeMounts that are not a ConfigMaps or Secrets,
+ * since they will probably cause issues when creating a pod runner from a chart or larger manifest.
+ *
+ * This is not a pure function, i.e. it has side effects and can mutate the input arguments.
+ *
+ * This sanitization only makes sense when both `podSpec` and `containerSpec` are defined.
+ * It serves helm-pod and kubernetes-pod action types.
+ */
+export function sanitizeVolumesForPodRunner(podSpec: V1PodSpec | undefined, containerSpec: V1Container | undefined) {
+  if (!podSpec) {
+    return
+  }
+  if (!podSpec.volumes) {
+    return
+  }
+  if (!containerSpec) {
+    return
+  }
+
+  // Sanitize volumes
+  podSpec.volumes = podSpec.volumes.filter((volume) => volume.configMap || volume.secret)
+
+  // Sanitize volumeMounts
+  const retainedVolumes = new Set(podSpec?.volumes?.map((volume) => volume.name))
+  containerSpec.volumeMounts = containerSpec.volumeMounts?.filter((volumeMount) => {
+    return retainedVolumes.has(volumeMount.name)
+  })
+
+  /*
+  Here we get the `containerSpec` and the `podSpec` as separate arguments,
+  so we can't be sure that `containerSpec` object   has the same identity as one of the containers defined in `podSpec.containers`.
+
+  The names of these 2 containers can also be different
+  because we always override the container name in the caller function `prepareRunPodSpec()`.
+
+  Thus, we need to sanitize `podSpec.containers` explicitly.
+   */
+  for (const podSpecContainer of podSpec.containers) {
+    podSpecContainer.volumeMounts = podSpecContainer.volumeMounts?.filter((volumeMount) => {
+      return retainedVolumes.has(volumeMount.name)
+    })
+  }
+
+  // We also make sure the defaultMode of a configMap volume is an octal number.
+  podSpec.volumes.forEach((volume) => {
+    if (volume.configMap && volume.configMap.defaultMode && !isOctal(volume.configMap.defaultMode.toString())) {
+      volume.configMap!.defaultMode = parseInt(`0${volume.configMap?.defaultMode}`, 8)
+    }
+  })
+}
+
+export function isOctal(value: string) {
+  return /^(0o?)[0-7]+$/i.test(value)
+}