Revisit secret mounting as environment variable:

* mount the secret as a file (existing behavior) * additionally, mount the secret as an environment variable 'env' flag is used both as a boolean toggle (without value) to inject the secret as an environment variable using the value of 'id' as a default. If provided a value ('env=MY_SECRET'), the secret will be injected into the environment variable with the specified name. In either case, the secret is always mounted as a file using prior behavior. Signed-off-by: a-palchikov <[email protected]>
moby · Aug 8, 2024 · d66f9c9 · d66f9c9
1 parent b0717bb
commit d66f9c9
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 36 deletions.
diff --git a/client/llb/exec.go b/client/llb/exec.go
@@ -290,7 +290,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,26 +388,25 @@ 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 {
-			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)
 		}
+		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)
 	}
 
 	for _, s := range e.ssh {
@@ -697,13 +696,14 @@ func (fn secretOptionFunc) SetSecretOption(si *SecretInfo) {
 }
 
 type SecretInfo struct {
-	ID       string
-	Target   string
+	ID     string
+	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) {
@@ -719,7 +719,16 @@ 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
+		envDefault := ""
+		si.Env = &envDefault
+	})
+}
+
+// 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
@@ -39,11 +39,12 @@ func dispatchSecret(d *dispatchState, m *instructions.Mount, loc []parser.Range)
 	if !m.Required {
 		opts = append(opts, llb.SecretOptional)
 	}
-	if m.SecretAsEnv {
-		if m.Target == "" {
-			target = path.Base(id)
+	if m.Env != nil {
+		env := *m.Env
+		if env == "" {
+			env = path.Base(id)
 		}
-		opts = append(opts, llb.SecretAsEnv(true))
+		opts = append(opts, llb.SecretAsEnvName(env))
 	}
 
 	if m.UID != nil || m.GID != nil || m.Mode != nil {

diff --git a/frontend/dockerfile/dockerfile_secrets_test.go b/frontend/dockerfile/dockerfile_secrets_test.go
@@ -89,7 +89,7 @@ func testSecretAsEnviron(t *testing.T, sb integration.Sandbox) {
 
 	dockerfile := []byte(`
 FROM busybox
-RUN --mount=type=secret,id=mysecret,env=true [ "$mysecret" == "pw" ] || false
+RUN --mount=type=secret,id=mysecret,env [ "$mysecret" == "pw" ] && [ -f /run/secrets/mysecret ] || false
 `)
 
 	dir := integration.Tmpdir(
@@ -119,7 +119,7 @@ func testSecretAsEnvironWithOverride(t *testing.T, sb integration.Sandbox) {
 
 	dockerfile := []byte(`
 FROM busybox
-RUN --mount=type=secret,id=mysecret,target=MY_SECRET,env [ "$MY_SECRET" == "pw" ] || false
+RUN --mount=type=secret,id=mysecret,env=MY_SECRET [ "$MY_SECRET" == "pw" ] || false
 `)
 
 	dir := integration.Tmpdir(

diff --git a/frontend/dockerfile/instructions/commands_runmount.go b/frontend/dockerfile/instructions/commands_runmount.go
@@ -122,10 +122,12 @@ type Mount struct {
 	CacheID      string
 	CacheSharing ShareMode
 	Required     bool
-	SecretAsEnv  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) {
@@ -137,6 +139,7 @@ func parseMount(val string, expander SingleWordExpander) (*Mount, error) {
 	m := &Mount{Type: MountTypeBind}
 
 	roAuto := true
+	envDefault := ""
 
 	for _, field := range fields {
 		key, value, ok := strings.Cut(field, "=")
@@ -156,7 +159,7 @@ func parseMount(val string, expander SingleWordExpander) (*Mount, error) {
 				roAuto = false
 				continue
 			case "env":
-				m.SecretAsEnv = true
+				m.Env = &envDefault
 				continue
 			case "required":
 				if m.Type == MountTypeSecret || m.Type == MountTypeSSH {
@@ -257,11 +260,7 @@ func parseMount(val string, expander SingleWordExpander) (*Mount, error) {
 			}
 			m.GID = &gid
 		case "env":
-			env, err := strconv.ParseBool(value)
-			if err != nil {
-				return nil, errors.Errorf("invalid value for %q: %q", key, value)
-			}
-			m.SecretAsEnv = env
+			m.Env = &value
 		default:
 			allKeys := []string{
 				"type", "from", "source", "target", "readonly", "id", "sharing", "required", "mode", "uid", "gid", "src", "dst", "ro", "rw", "readwrite", "env",