frontend: allow mounting secret environment variables

[a-palchikov]

Implements frontend side of #2122.

This adds the syntax to Dockerfile frontend.
I purposefully chose to use a simple format for this as it's likely going to be debated. As implemented, the following format is supported:

RUN --mount=type=secret,id=MYSECRET,env

or, more explicitly:

RUN --mount=type=secret,id=mysecret,env=MYSECRET

will mount the secret with id mysecret as a new environment variable with the name MYSECRET).

Either way, the secret will be mounted as a file (existing behavior).
Note, that this already implements a suggestion from the PR comments.

Any suggestions on making it even more ergonomic or straightforward are welcome.

[crazy-max]

Using target, it's possible to create a different environment variable:

RUN --mount=type=secret,id=mysecret,target=MY_SECRET,env

Instead of target we could just use env value like:

RUN --mount=type=secret,id=mysecret,env=MY_SECRET

Might be confusing though

[crazy-max]

Missing DCO https://github.com/moby/buildkit/pull/5215/checks?check_run_id=28374175851

[a-palchikov]

Instead of target we could just use env value like:

RUN --mount=type=secret,id=mysecret,env=MY_SECRET

Might be confusing though

Yes, it might be confusing given that regular secret mounts use the target. Though using env=NAME would be the preferred way as far as clarity is concerned.

Ultimately, both target and env could be supported as described. And the env attribute could act both as a toggle (env or env=true) or as the clearer target replacement (env=NAME) while still having target as a fallback.

[thaJeztah]

Ultimately, both target and env could be supported as described.

Wondering; would there be use-cases where both a mount (/run/secrets/foo) and an env-var would be useful?

In that case, target could continue to be used to specify the name of the mounted secret, and env to optionally export as an env-var. Without value, the name of the env-var would match the default, and env=name a custom name (separate from the name of the target).

That would likely mean that env=true would not be supported; or at least; not as boolean (env=true would export to an env-var named true)

[a-palchikov]

Ultimately, both target and env could be supported as described.

Wondering; would there be use-cases where both a mount (/run/secrets/foo) and an env-var would be useful?

Hmm, that's a very good point. I don't think it's possible in LLB as currently implemented:

buildkit/client/llb/exec.go

Lines 391 to 410 in 979542e

	if s.IsEnv {
	peo.Secretenv = append(peo.Secretenv, &pb.SecretEnv{
	ID: s.ID,
	Name: s.Target,
	Optional: s.Optional,
	})
	} else {
	pm := &pb.Mount{
	Dest: s.Target,
	MountType: pb.MountType_SECRET,
	SecretOpt: &pb.SecretOpt{
	ID: s.ID,
	Uid: uint32(s.UID),
	Gid: uint32(s.GID),
	Optional: s.Optional,
	Mode: uint32(s.Mode),
	},
	}
	peo.Mounts = append(peo.Mounts, pm)
	}

but I can take a stab at it.

[thaJeztah]

Hmm, that's a very good point. I don't think it's possible in LLB as currently implemented:

Oh! Yes, I'm not super familiar with the BuildKit internals.

For clarity; I'm not a maintainer for BuildKit, so mostly commenting as a user interested in this feature, so happy to hear from BuildKit maintainers on my suggestions as well 😅

[crazy-max]

Ultimately, both target and env could be supported as described.

Wondering; would there be use-cases where both a mount (/run/secrets/foo) and an env-var would be useful?

In that case, target could continue to be used to specify the name of the mounted secret, and env to optionally export as an env-var. Without value, the name of the env-var would match the default, and env=name a custom name (separate from the name of the target).

That would likely mean that env=true would not be supported; or at least; not as boolean (env=true would export to an env-var named true)

Yes I think that would be fine to have both but would need LLB changes as stated by @a-palchikov.

Not for this PR but we should also consider having a build check that warns if a secret env overrides ones from ARG or ENV imo. (cc @daghack @colinhemmings)

[a-palchikov]

I kept the original functional option (SecretAsEnv) intact to keep backwards compatibility as I think it might already be used.

[tonistiigi]

This seems to be a behavior change of creating a secret mount when SecretAsEnv was set.

If there is a need for mounting the same secret both as file and env (can't think of one) then it should be optional.

[tonistiigi]

I'm not sure if this is needed in Dockerfile syntax. Would be more clearer if env always has a string value.

[a-palchikov]

@tonistiigi Updated with the following semantics:

A secret is defined with a tuple (id, target) which can be aliased as (source, destination) using the following format:

RUN --mount=type=secret,id=FOO,target=/path/to/secret ...

Existing secret rules:

• if source (id) is empty, it is assumed to equal to path.Base(target)
• if target (destination) is empty, it is assumed to equal to
  "/run/secrets/" + path.Base(id)

Adding env as an additional attribute to the mount command to specify that the secret should be mounted as an environment variable (specifies the name of the variable):

RUN --mount=type=secret,id=FOO,env=FOO

with the following semantics:

• it allows the old behavior of mounting the secret as path (using the target/destination attribute)
• it is optional (e.g. with no env specified, the secret is mounted as a file following the existing rules)
• if specified, the file mount is optional (uses empty target/destination as indicator)
• it is possible to mount a secret as both - an environment variable and as a file (this is still debatable - I'm following the advice from above but given enough reasons not to, this can be dropped)

[tonistiigi]

I think you can squash the commits as don't think they are useful on their own. If you want, you can leave the client-llb and dockerfile changes separate.

dco added