feat(container): first-class BuildKit secrets support (#6294)

* feat(container): first-class BuildKit secrets support

First-class BuildKit secrets support for BuildKit in-cluster building,
Garden Cloud Builder and building locally.

Kaniko is not supported due to lack of support (See also
GoogleContainerTools/kaniko#3028)

* Update core/src/plugins/container/config.ts

* improvement: protect leaking secrets in logs

* improvement: update docs

* test: extensive test coverage for secret values and maybeSecret

Co-authored-by: Vova <vova@garden.io>

* fix: fixes after manual testing

Co-authored-by: Vova <vova@garden.io>

* fix: format

* improvement: undo unnecessary change

* fix: framework test

* improvement: remove unnecessary maybeSecret

* improvement: clearer doc strings and minor changes

---------

Co-authored-by: Vova <vova@garden.io>

cli/src/build-pkg.ts

@@ -329,7 +329,7 @@ async function buildBinaries(args: string[]) {
     cwd: repoRoot,
     stdio: "inherit",
     // We have to pass the garden version explicitly to rollup due to an issue with the json() plugin loading the wrong package.json files
-    env: { GARDEN_CORE_VERSION: versionInBinary },
+    environment: { GARDEN_CORE_VERSION: versionInBinary },
   })
 
   await zipAndHash({

core/src/commands/util/mutagen.ts

@@ -58,7 +58,7 @@ export class MutagenCommand extends Command<{}, {}> {
     const result = await exec(mutagenPath, args["$all"]?.slice(2) || [], {
       cwd: mutagenDir,
       stdio: "inherit",
-      env: {
+      environment: {
         MUTAGEN_DATA_DIRECTORY: mutagenDir,
       },
       reject: false,

core/src/plugins/container/build.ts

@@ -7,11 +7,11 @@
  */
 
 import { containerHelpers } from "./helpers.js"
-import { ConfigurationError, toGardenError } from "../../exceptions.js"
+import { ConfigurationError, InternalError, toGardenError } from "../../exceptions.js"
 import type { PrimitiveMap } from "../../config/common.js"
 import split2 from "split2"
 import type { BuildActionHandler } from "../../plugin/action-types.js"
-import type { ContainerBuildAction, ContainerBuildOutputs } from "./config.js"
+import type { ContainerBuildAction, ContainerBuildActionSpec, ContainerBuildOutputs } from "./config.js"
 import { defaultDockerfileName } from "./config.js"
 import { joinWithPosix } from "../../util/fs.js"
 import type { Resolved } from "../../actions/types.js"
@@ -29,6 +29,7 @@ import { cloudBuilder } from "./cloudbuilder.js"
 import { styles } from "../../logger/styles.js"
 import type { CloudBuilderAvailableV2 } from "../../cloud/api.js"
 import type { SpawnOutput } from "../../util/util.js"
+import { isSecret, type Secret } from "../../util/secrets.js"
 
 export const validateContainerBuild: BuildActionHandler<"validate", ContainerBuildAction> = async ({ action }) => {
   // configure concurrency limit for build status task nodes.
@@ -153,6 +154,9 @@ async function buildContainerLocally({
 
   const dockerFlags = [...getDockerBuildFlags(action, ctx.provider.config), ...extraDockerOpts]
 
+  const { secretArgs, secretEnvVars } = getDockerSecrets(action.getSpec())
+  dockerFlags.push(...secretArgs)
+
   // If there already is a --tag flag, another plugin like the Kubernetes plugin already decided how to tag the image.
   // In this case, we don't want to add another local tag.
   // TODO: it would be nice to find a better way to become aware of the parent plugin's concerns in the container plugin.
@@ -165,7 +169,7 @@ async function buildContainerLocally({
     }
   }
 
-  const cmdOpts = ["build", ...dockerFlags, "--file", dockerfilePath]
+  const cmdOpts = ["buildx", "build", ...dockerFlags, "--file", dockerfilePath]
   try {
     return await containerHelpers.dockerCli({
       cwd: buildPath,
@@ -175,6 +179,7 @@ async function buildContainerLocally({
       stderr: outputStream,
       timeout,
       ctx,
+      env: secretEnvVars,
     })
   } catch (e) {
     const error = toGardenError(e)
@@ -262,6 +267,43 @@ export function getContainerBuildActionOutputs(action: Resolved<ContainerBuildAc
   return containerHelpers.getBuildActionOutputs(action, undefined)
 }
 
+export function getDockerSecrets(actionSpec: ContainerBuildActionSpec): {
+  secretArgs: string[]
+  secretEnvVars: Record<string, Secret>
+} {
+  const args: string[] = []
+  const env: Record<string, Secret> = {}
+
+  for (const [secretKey, secretValue] of Object.entries(actionSpec.secrets || {})) {
+    if (!secretKey.match(/^[a-zA-Z0-9\._-]+$/)) {
+      throw new ConfigurationError({
+        message: `Invalid secret ID '${secretKey}'. Only alphanumeric characters (a-z, A-Z, 0-9), underscores (_), dashes (-) and dots (.) are allowed.`,
+      })
+    }
+    if (!isSecret(secretValue)) {
+      throw new InternalError({
+        message: "joi schema did not call makeSecret for every secret value.",
+      })
+    }
+
+    // determine env var names. There can be name collisions due to the fact that we replace special characters with underscores.
+    let envVarname: string
+    let i = 1
+    do {
+      envVarname = `GARDEN_BUILD_SECRET_${secretKey.toUpperCase().replaceAll(/[-\.]/g, "_")}${i > 1 ? `_${i}` : ""}`
+      i += 1
+    } while (env[envVarname])
+
+    env[envVarname] = secretValue
+    args.push("--secret", `id=${secretKey},env=${envVarname}`)
+  }
+
+  return {
+    secretArgs: args,
+    secretEnvVars: env,
+  }
+}
+
 export function getDockerBuildFlags(
   action: Resolved<ContainerBuildAction>,
   containerProviderConfig: ContainerProviderConfig

core/src/plugins/container/config.ts

@@ -32,6 +32,7 @@ import type Joi from "@hapi/joi"
 import type { OctalPermissionMask } from "../kubernetes/types.js"
 import { templateStringLiteral } from "../../docs/common.js"
 import { syncGuideLink } from "../kubernetes/constants.js"
+import { makeSecret, type Secret } from "../../util/secrets.js"
 
 export const defaultDockerfileName = "Dockerfile"
 
@@ -1013,6 +1014,7 @@ export interface ContainerBuildActionSpec {
   buildArgs: PrimitiveMap
   dockerfile: string
   extraFlags: string[]
+  secrets?: Record<string, Secret>
   localId?: string
   publishId?: string
   targetStage?: string
@@ -1061,9 +1063,30 @@ export const containerCommonBuildSpecKeys = memoize(() => ({
     Specify extra flags to use when building the container image.
     Note that arguments may not be portable across implementations.`),
   platforms: joi.sparseArray().items(joi.string()).description(dedent`
-    Specify the platforms to build the image for. This is useful when building multi-platform images.
-    The format is \`os/arch\`, e.g. \`linux/amd64\`, \`linux/arm64\`, etc.
-  `),
+      Specify the platforms to build the image for. This is useful when building multi-platform images.
+      The format is \`os/arch\`, e.g. \`linux/amd64\`, \`linux/arm64\`, etc.
+    `),
+  secrets: joi
+    .object()
+    .pattern(/.+/, joi.string().custom(makeSecret))
+    .description(
+      dedent`
+      Secret values that can be mounted in the Dockerfile, but do not become part of the image filesystem or image manifest. This is useful e.g. for private registry auth tokens.
+
+      Build arguments and environment variables are inappropriate for secrets, as they persist in the final image.
+
+      The secret can later be consumed in the Dockerfile like so:
+      \`\`\`
+        RUN --mount=type=secret,id=mytoken \
+            TOKEN=$(cat /run/secrets/mytoken) ...
+      \`\`\`
+
+      See also https://docs.docker.com/build/building/secrets/
+    `
+    )
+    .example({
+      mytoken: "supersecret",
+    }),
 }))
 
 export const containerBuildSpecSchema = createSchema({

core/src/plugins/container/helpers.ts

@@ -33,6 +33,7 @@ import type { Resolved } from "../../actions/types.js"
 import pMemoize from "../../lib/p-memoize.js"
 import { styles } from "../../logger/styles.js"
 import type { ContainerProviderConfig } from "./container.js"
+import { type MaybeSecret } from "../../util/secrets.js"
 
 const { readFile, pathExists, lstat } = fsExtra
 
@@ -382,6 +383,7 @@ const helpers = {
     stdout,
     stderr,
     timeout,
+    env,
   }: {
     cwd: string
     args: string[]
@@ -391,14 +393,15 @@ const helpers = {
     stdout?: Writable
     stderr?: Writable
     timeout?: number
+    env?: { [key: string]: MaybeSecret }
   }) {
     const docker = ctx.tools["container.docker"]
 
     try {
       const res = await docker.spawnAndWait({
         args,
         cwd,
-        env: { ...process.env, DOCKER_CLI_EXPERIMENTAL: "enabled" },
+        env,
         ignoreError,
         log,
         stdout,

core/src/plugins/exec/common.ts

@@ -75,7 +75,7 @@ export async function execRunCommand({
     ...opts,
     shell,
     cwd: action.getBuildPath(),
-    env: envVars,
+    environment: envVars,
     stdout: outputStream,
     stderr: outputStream,
   })

core/src/plugins/kubernetes/api.ts

@@ -71,6 +71,7 @@ import type { RequestOptions } from "http"
 import https from "node:https"
 import http from "node:http"
 import { ProxyAgent } from "proxy-agent"
+import { type MaybeSecret, toClearText } from "../../util/secrets.js"
 
 interface ApiGroupMap {
   [groupVersion: string]: V1APIGroup
@@ -876,7 +877,7 @@ export class KubeApi {
     namespace: string
     podName: string
     containerName: string
-    command: string[]
+    command: MaybeSecret[]
     stdout?: Writable
     stderr?: Writable
     stdin?: Readable
@@ -956,7 +957,7 @@ export class KubeApi {
               namespace,
               podName,
               containerName,
-              command,
+              command.map(toClearText),
               _stdout,
               _stderr,
               stdin || null,

core/src/plugins/kubernetes/container/build/buildkit.ts

@@ -36,7 +36,7 @@ import {
 import { getNamespaceStatus } from "../../namespace.js"
 import { sleep } from "../../../../util/util.js"
 import type { ContainerBuildAction, ContainerModuleOutputs } from "../../../container/moduleConfig.js"
-import { getDockerBuildArgs } from "../../../container/build.js"
+import { getDockerBuildArgs, getDockerSecrets } from "../../../container/build.js"
 import type { Resolved } from "../../../../actions/types.js"
 import { PodRunner } from "../../run.js"
 import { prepareSecrets } from "../../secrets.js"
@@ -47,6 +47,7 @@ import { stringifyResources } from "../util.js"
 import { styles } from "../../../../logger/styles.js"
 import type { ResolvedBuildAction } from "../../../../actions/build.js"
 import { commandListToShellScript } from "../../../../util/escape.js"
+import { type MaybeSecret, maybeSecret } from "../../../../util/secrets.js"
 
 const AWS_ECR_REGEX = /^([^\.]+\.)?dkr\.ecr\.([^\.]+\.)amazonaws\.com\//i // AWS Elastic Container Registry
 
@@ -279,7 +280,9 @@ export function makeBuildkitBuildCommand({
   action: ResolvedBuildAction
   contextPath: string
   dockerfile: string
-}): string[] {
+}): MaybeSecret[] {
+  const { secretArgs, secretEnvVars } = getDockerSecrets(action.getSpec())
+
   const buildctlCommand = [
     "buildctl",
     "build",
@@ -290,6 +293,7 @@ export function makeBuildkitBuildCommand({
     "dockerfile=" + contextPath,
     "--opt",
     "filename=" + dockerfile,
+    ...secretArgs,
     ...getBuildkitImageFlags(
       provider.config.clusterBuildkit!.cache,
       outputs,
@@ -298,7 +302,11 @@ export function makeBuildkitBuildCommand({
     ...getBuildkitFlags(action),
   ]
 
-  return ["sh", "-c", `cd ${contextPath} && ${commandListToShellScript(buildctlCommand)}`]
+  return [
+    "sh",
+    "-c",
+    maybeSecret`cd ${contextPath} && ${commandListToShellScript({ command: buildctlCommand, env: secretEnvVars })}`,
+  ]
 }
 
 export function getBuildkitFlags(action: Resolved<ContainerBuildAction>) {

core/src/plugins/kubernetes/container/build/kaniko.ts

@@ -91,6 +91,18 @@ export const kanikoBuild: BuildHandler = async (params) => {
   const projectNamespace = (await getNamespaceStatus({ log, ctx: k8sCtx, provider })).namespaceName
 
   const spec = action.getSpec()
+
+  if (spec.secrets) {
+    throw new ConfigurationError({
+      message: dedent`
+        Unfortunately Kaniko does not support secret build arguments.
+        Garden Cloud Builder and the Kubernetes BuildKit in-cluster builder both support secrets.
+
+        See also https://github.com/GoogleContainerTools/kaniko/issues/3028
+      `,
+    })
+  }
+
   const outputs = k8sGetContainerBuildActionOutputs({ provider, action })
 
   const localId = outputs.localImageId
@@ -318,7 +330,7 @@ export function getKanikoBuilderPodManifest({
             n=0
             until [ "$n" -ge 30 ]
             do
-              rsync ${commandListToShellScript(syncArgs)} && break
+              rsync ${commandListToShellScript({ command: syncArgs })} && break
               n=$((n+1))
               sleep 1
             done
@@ -352,9 +364,9 @@ export function getKanikoBuilderPodManifest({
           "/bin/sh",
           "-c",
           dedent`
-            ${commandListToShellScript(kanikoCommand)};
+            ${commandListToShellScript({ command: kanikoCommand })};
             export exitcode=$?;
-            ${commandListToShellScript(["touch", `${sharedMountPath}/done`])};
+            ${commandListToShellScript({ command: ["touch", `${sharedMountPath}/done`] })};
             exit $exitcode;
           `,
         ],

core/src/plugins/kubernetes/jib-container.ts

@@ -74,6 +74,7 @@ async function buildAndPushViaRemote(params: BuildActionParams<"build", Containe
 
   // Build the tarball with the base handler
   const spec: any = action.getSpec()
+
   spec.tarOnly = true
   spec.tarFormat = "oci"
 

core/src/plugins/kubernetes/run.ts

@@ -45,6 +45,7 @@ import { LogLevel } from "../../logger/logger.js"
 import { getResourceEvents } from "./status/events.js"
 import stringify from "json-stringify-safe"
 import { commandListToShellScript } from "../../util/escape.js"
+import { maybeSecret, type MaybeSecret, transformSecret } from "../../util/secrets.js"
 
 // ref: https://kubernetes.io/docs/reference/labels-annotations-taints/#kubectl-kubernetes-io-default-container
 export const K8_POD_DEFAULT_CONTAINER_ANNOTATION_KEY = "kubectl.kubernetes.io/default-container"
@@ -448,14 +449,14 @@ async function runWithoutArtifacts({
  * See https://stackoverflow.com/a/20564208
  * @param cmd the command to wrap
  */
-function getCommandExecutionScript(cmd: string[]) {
-  return `
+function getCommandExecutionScript(cmd: MaybeSecret[]) {
+  return maybeSecret`
 exec 1<&-
 exec 2<&-
 exec 1<>/tmp/output
 exec 2>&1
 
-${commandListToShellScript(cmd)}
+${commandListToShellScript({ command: cmd })}
 `
 }
 
@@ -469,25 +470,27 @@ ${commandListToShellScript(cmd)}
  */
 function getArtifactsTarScript(artifacts: ArtifactSpec[]) {
   const directoriesToCreate = artifacts.map((a) => a.target).filter((target) => !!target && target !== ".")
-  const tmpPath = commandListToShellScript(["/tmp/.garden-artifacts-" + randomString(8)])
+  const tmpPath = commandListToShellScript({ command: ["/tmp/.garden-artifacts-" + randomString(8)] })
 
   const createDirectoriesCommands = directoriesToCreate.map((target) =>
-    commandListToShellScript(["mkdir", "-p", target])
+    commandListToShellScript({ command: ["mkdir", "-p", target] })
   )
 
   const copyArtifactsCommands = artifacts.map(({ source, target }) => {
-    const escapedTarget = commandListToShellScript([target || "."])
+    const escapedTarget = commandListToShellScript({ command: [target || "."] })
 
     // Allow globs (*) in the source path
     // Note: This works because `commandListToShellScript` wraps every parameter in single quotes, escaping contained single quotes.
     // The string `bin/*` will be transformed to `'bin/*'` by `commandListToShellScript`. The shell would treat `*` as literal and not expand it.
     // `replaceAll` transforms that string then to `'bin/'*''`, which allows the shell to expand the glob, everything else is treated as literal.
-    const escapedSource = commandListToShellScript([source]).replaceAll("*", "'*'")
+    const escapedSource = transformSecret(commandListToShellScript({ command: [source] }), (s) =>
+      s.replaceAll("*", "'*'")
+    )
 
-    return `cp -r ${escapedSource} ${escapedTarget} >/dev/null || true`
+    return maybeSecret`cp -r ${escapedSource} ${escapedTarget} >/dev/null || true`
   })
 
-  return `
+  return maybeSecret`
 rm -rf ${tmpPath} >/dev/null || true
 mkdir -p ${tmpPath}
 cd ${tmpPath}
@@ -710,7 +713,7 @@ interface StartParams {
 }
 
 export type PodRunnerExecParams = StartParams & {
-  command: string[]
+  command: MaybeSecret[]
   containerName?: string
   stdout?: Writable
   stderr?: Writable