frontend: allow mounting secret environment variables by a-palchikov · Pull Request #5215 · moby/buildkit

a-palchikov · 2024-08-05T21:04:18Z

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 · 2024-08-06T09:16:31Z

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 · 2024-08-06T19:49:22Z

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 · 2024-08-06T22:41:19Z

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 · 2024-08-07T10:03:39Z

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 · 2024-08-07T10:52:56Z

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 · 2024-08-07T15:27:26Z

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 · 2024-08-08T12:03:11Z

+	})
+}
+
+// SecretAsEnvName defines if the secret should be added as an environment variable


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

tonistiigi · 2024-08-15T08:40:07Z

-			}
-			peo.Mounts = append(peo.Mounts, pm)
 		}
+		pm := &pb.Mount{


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 · 2024-08-15T08:43:35Z

+
+	dockerfile := []byte(`
+FROM busybox
+RUN --mount=type=secret,id=mysecret,env [ "$mysecret" == "pw" ] && [ -f /run/secrets/mysecret ] || false


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

a-palchikov · 2024-08-19T13:19:35Z

@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 · 2024-08-19T13:58:19Z

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.

This adds the syntax to Dockerfile frontend. I purposedly 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=true ``` will mount the secret with id MYSECRET as a new environment variable with the same name. Using 'target', it's possible to create a different environment variable: ``` RUN --mount=type=secret,id=mysecret,target=MY_SECRET,env ``` will mount 'mysecret' secret as MY_SECRET environment variable. Any suggestions on making it more ergonomic are welcome. Signed-off-by: a-palchikov <[email protected]>

dco added

github-actions Bot added area/dockerfile area/testing area/frontend labels Aug 5, 2024

crazy-max previously requested changes Aug 6, 2024

View reviewed changes

crazy-max added this to the v0.16.0 milestone Aug 6, 2024

a-palchikov force-pushed the dockerfile/secretasenv branch from 20c94fb to 85c9af4 Compare August 6, 2024 10:52

a-palchikov requested a review from crazy-max August 6, 2024 11:19

crazy-max modified the milestones: v0.16.0, v0.17.0 Aug 7, 2024

a-palchikov force-pushed the dockerfile/secretasenv branch from 85c9af4 to d66f9c9 Compare August 8, 2024 11:58

github-actions Bot added area/llb area/client labels Aug 8, 2024

a-palchikov commented Aug 8, 2024

View reviewed changes

a-palchikov force-pushed the dockerfile/secretasenv branch 4 times, most recently from 2b083b3 to 1e498a4 Compare August 13, 2024 17:04

tonistiigi reviewed Aug 15, 2024

View reviewed changes

thompson-shaun modified the milestones: v0.17.0, v0.16.0 Aug 15, 2024

thompson-shaun assigned crazy-max Aug 15, 2024

a-palchikov force-pushed the dockerfile/secretasenv branch from 1e498a4 to 0fc5f16 Compare August 19, 2024 13:14

tonistiigi added the needs/docs-follow-up label Aug 19, 2024

tonistiigi approved these changes Aug 19, 2024

View reviewed changes

a-palchikov force-pushed the dockerfile/secretasenv branch from 0fc5f16 to 96f746b Compare August 19, 2024 15:55

tonistiigi approved these changes Aug 21, 2024

View reviewed changes

tonistiigi merged commit 53d77f7 into moby:master Aug 21, 2024

This was referenced Sep 4, 2024

docs: run mount secret env dockerfile example #5296

Merged

build: run mount secrets as env docker/docs#20790

Merged

petski mentioned this pull request Sep 17, 2024

Allow new field option 'env' when mounting Secrets inside RUN commands hadolint/hadolint#1039

Closed

3 tasks

thaJeztah mentioned this pull request Oct 20, 2024

Mounting secret as environment variable does not behave as expected. #5439

Closed

dvdksn removed the needs/docs-follow-up label Nov 18, 2024

qryxip mentioned this pull request May 2, 2025

build!: DockerではPyinstallerビルドのエンジンを用いる VOICEVOX/voicevox_engine#1669

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

frontend: allow mounting secret environment variables#5215

frontend: allow mounting secret environment variables#5215
tonistiigi merged 1 commit intomoby:masterfrom
a-palchikov:dockerfile/secretasenv

a-palchikov commented Aug 5, 2024 •

edited

Loading

Uh oh!

crazy-max commented Aug 6, 2024 •

edited

Loading

Uh oh!

crazy-max left a comment

Uh oh!

a-palchikov commented Aug 6, 2024

Uh oh!

thaJeztah commented Aug 6, 2024

Uh oh!

a-palchikov commented Aug 7, 2024

Uh oh!

thaJeztah commented Aug 7, 2024

Uh oh!

crazy-max commented Aug 7, 2024

Uh oh!

a-palchikov Aug 8, 2024

Uh oh!

tonistiigi Aug 15, 2024

Uh oh!

tonistiigi Aug 15, 2024

Uh oh!

a-palchikov commented Aug 19, 2024

Uh oh!

tonistiigi commented Aug 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

a-palchikov commented Aug 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crazy-max commented Aug 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crazy-max left a comment

Choose a reason for hiding this comment

Uh oh!

a-palchikov commented Aug 6, 2024

Uh oh!

thaJeztah commented Aug 6, 2024

Uh oh!

a-palchikov commented Aug 7, 2024

Uh oh!

thaJeztah commented Aug 7, 2024

Uh oh!

crazy-max commented Aug 7, 2024

Uh oh!

a-palchikov Aug 8, 2024

Choose a reason for hiding this comment

Uh oh!

tonistiigi Aug 15, 2024

Choose a reason for hiding this comment

Uh oh!

tonistiigi Aug 15, 2024

Choose a reason for hiding this comment

Uh oh!

a-palchikov commented Aug 19, 2024

Uh oh!

tonistiigi commented Aug 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

a-palchikov commented Aug 5, 2024 •

edited

Loading

crazy-max commented Aug 6, 2024 •

edited

Loading