From 96f746bca32e8c9cb89a1cbcaf3c21af0cd34932 Mon Sep 17 00:00:00 2001 From: a-palchikov Date: Mon, 5 Aug 2024 23:00:29 +0200 Subject: [PATCH] Implements frontend side of #2122. 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 --- client/llb/exec.go | 53 +++++++++++++--- .../dockerfile2llb/convert_secrets.go | 19 ++++-- .../dockerfile/dockerfile_secrets_test.go | 62 +++++++++++++++++++ .../instructions/commands_runmount.go | 13 ++-- 4 files changed, 129 insertions(+), 18 deletions(-) diff --git a/client/llb/exec.go b/client/llb/exec.go index faba8bddc72b..6850f21a7459 100644 --- a/client/llb/exec.go +++ b/client/llb/exec.go @@ -6,6 +6,7 @@ import ( "fmt" "net" "sort" + "strings" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/system" @@ -290,7 +291,7 @@ func (e *ExecOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] if len(e.secrets) > 0 { addCap(&e.constraints, pb.CapExecMountSecret) for _, s := range e.secrets { - if s.IsEnv { + if s.Env != nil { addCap(&e.constraints, pb.CapExecSecretEnv) break } @@ -388,16 +389,17 @@ func (e *ExecOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] } for _, s := range e.secrets { - if s.IsEnv { + if s.Env != nil { peo.Secretenv = append(peo.Secretenv, &pb.SecretEnv{ ID: s.ID, - Name: s.Target, + Name: *s.Env, Optional: s.Optional, }) - } else { + } + if s.Target != nil { pm := &pb.Mount{ Input: pb.Empty, - Dest: s.Target, + Dest: *s.Target, MountType: pb.MountType_SECRET, SecretOpt: &pb.SecretOpt{ ID: s.ID, @@ -680,7 +682,19 @@ type SSHInfo struct { // AddSecret is a RunOption that adds a secret to the exec. func AddSecret(dest string, opts ...SecretOption) RunOption { return runOptionFunc(func(ei *ExecInfo) { - s := &SecretInfo{ID: dest, Target: dest, Mode: 0400} + s := &SecretInfo{ID: dest, Target: &dest, Mode: 0400} + for _, opt := range opts { + opt.SetSecretOption(s) + } + ei.Secrets = append(ei.Secrets, *s) + }) +} + +// AddSecretWithDest is a RunOption that adds a secret to the exec +// with an optional destination. +func AddSecretWithDest(src string, dest *string, opts ...SecretOption) RunOption { + return runOptionFunc(func(ei *ExecInfo) { + s := &SecretInfo{ID: src, Target: dest, Mode: 0400} for _, opt := range opts { opt.SetSecretOption(s) } @@ -699,13 +713,15 @@ func (fn secretOptionFunc) SetSecretOption(si *SecretInfo) { } type SecretInfo struct { - ID string - Target string + ID string + // Target optionally specifies the target for the secret mount + Target *string + // Env optionally names the environment variable for the secret + Env *string Mode int UID int GID int Optional bool - IsEnv bool } var SecretOptional = secretOptionFunc(func(si *SecretInfo) { @@ -721,7 +737,24 @@ func SecretID(id string) SecretOption { // SecretAsEnv defines if the secret should be added as an environment variable func SecretAsEnv(v bool) SecretOption { return secretOptionFunc(func(si *SecretInfo) { - si.IsEnv = v + if !v { + si.Env = nil + return + } + if si.Target == nil { + return + } + target := strings.Clone(*si.Target) + si.Env = &target + si.Target = nil + }) +} + +// SecretAsEnvName defines if the secret should be added as an environment variable +// with the specified name +func SecretAsEnvName(v string) SecretOption { + return secretOptionFunc(func(si *SecretInfo) { + si.Env = &v }) } diff --git a/frontend/dockerfile/dockerfile2llb/convert_secrets.go b/frontend/dockerfile/dockerfile2llb/convert_secrets.go index ced2bff1b070..c68373b85f94 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_secrets.go +++ b/frontend/dockerfile/dockerfile2llb/convert_secrets.go @@ -22,9 +22,17 @@ func dispatchSecret(d *dispatchState, m *instructions.Mount, loc []parser.Range) id = path.Base(m.Target) } - target := m.Target - if target == "" { - target = "/run/secrets/" + path.Base(id) + var target *string + if m.Target != "" { + target = &m.Target + } + + if m.Env == nil { + dest := m.Target + if dest == "" { + dest = "/run/secrets/" + path.Base(id) + } + target = &dest } if _, ok := d.outline.secrets[id]; !ok { @@ -39,6 +47,9 @@ func dispatchSecret(d *dispatchState, m *instructions.Mount, loc []parser.Range) if !m.Required { opts = append(opts, llb.SecretOptional) } + if m.Env != nil { + opts = append(opts, llb.SecretAsEnvName(*m.Env)) + } if m.UID != nil || m.GID != nil || m.Mode != nil { var uid, gid, mode int @@ -56,5 +67,5 @@ func dispatchSecret(d *dispatchState, m *instructions.Mount, loc []parser.Range) opts = append(opts, llb.SecretFileOpt(uid, gid, mode)) } - return llb.AddSecret(target, opts...), nil + return llb.AddSecretWithDest(id, target, opts...), nil } diff --git a/frontend/dockerfile/dockerfile_secrets_test.go b/frontend/dockerfile/dockerfile_secrets_test.go index fd72d9b053b1..3bc3e695ea02 100644 --- a/frontend/dockerfile/dockerfile_secrets_test.go +++ b/frontend/dockerfile/dockerfile_secrets_test.go @@ -16,6 +16,8 @@ import ( var secretsTests = integration.TestFuncs( testSecretFileParams, testSecretRequiredWithoutValue, + testSecretAsEnviron, + testSecretAsEnvironWithFileMount, ) func init() { @@ -80,3 +82,63 @@ RUN --mount=type=secret,required,id=mysecret foo require.Error(t, err) require.Contains(t, err.Error(), "secret mysecret: not found") } + +func testSecretAsEnviron(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + f := getFrontend(t, sb) + + dockerfile := []byte(` +FROM busybox +RUN --mount=type=secret,id=mysecret,env=SECRET_ENV [ "$SECRET_ENV" == "pw" ] && [ ! -f /run/secrets/mysecret ] || false +`) + + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + Session: []session.Attachable{secretsprovider.FromMap(map[string][]byte{ + "mysecret": []byte("pw"), + })}, + }, nil) + require.NoError(t, err) +} + +func testSecretAsEnvironWithFileMount(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + f := getFrontend(t, sb) + + dockerfile := []byte(` +FROM busybox +RUN --mount=type=secret,id=mysecret,target=/run/secrets/secret,env=SECRET_ENV [ "$SECRET_ENV" == "pw" ] && [ -f /run/secrets/secret ] || false +`) + + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + Session: []session.Attachable{secretsprovider.FromMap(map[string][]byte{ + "mysecret": []byte("pw"), + })}, + }, nil) + require.NoError(t, err) +} diff --git a/frontend/dockerfile/instructions/commands_runmount.go b/frontend/dockerfile/instructions/commands_runmount.go index cbe1a8bc0bd3..335b524e98d8 100644 --- a/frontend/dockerfile/instructions/commands_runmount.go +++ b/frontend/dockerfile/instructions/commands_runmount.go @@ -122,9 +122,12 @@ type Mount struct { CacheID string CacheSharing ShareMode Required bool - Mode *uint64 - UID *uint64 - GID *uint64 + // Env optionally specifies the name of the environment variable for a secret. + // A pointer to an empty value uses the default + Env *string + Mode *uint64 + UID *uint64 + GID *uint64 } func parseMount(val string, expander SingleWordExpander) (*Mount, error) { @@ -252,9 +255,11 @@ func parseMount(val string, expander SingleWordExpander) (*Mount, error) { return nil, errors.Errorf("invalid value %s for gid", value) } m.GID = &gid + case "env": + m.Env = &value default: allKeys := []string{ - "type", "from", "source", "target", "readonly", "id", "sharing", "required", "size", "mode", "uid", "gid", "src", "dst", "destination", "ro", "rw", "readwrite", + "type", "from", "source", "target", "readonly", "id", "sharing", "required", "size", "mode", "uid", "gid", "src", "dst", "destination", "ro", "rw", "readwrite", "env", } return nil, suggest.WrapError(errors.Errorf("unexpected key '%s' in '%s'", key, field), key, allKeys, true) }