feat(run-mount): add support for variable expansion

Signed-off-by: Omer Mizrahi <ommizrah@microsoft.com>
moby · May 1, 2021 · 1f39eb8 · 1f39eb8
1 parent df49b64
commit 1f39eb8
Show file tree

Hide file tree

Showing 5 changed files with 235 additions and 59 deletions.
diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go
@@ -433,34 +433,48 @@ func metaArgsToMap(metaArgs []instructions.KeyValuePairOptional) map[string]stri
 
 func toCommand(ic instructions.Command, allDispatchStates *dispatchStates) (command, error) {
 	cmd := command{Command: ic}
-	if c, ok := ic.(*instructions.CopyCommand); ok {
-		if c.From != "" {
-			var stn *dispatchState
-			index, err := strconv.Atoi(c.From)
-			if err != nil {
-				stn, ok = allDispatchStates.findStateByName(c.From)
-				if !ok {
-					stn = &dispatchState{
-						stage:        instructions.Stage{BaseName: c.From, Location: ic.Location()},
-						deps:         make(map[*dispatchState]struct{}),
-						unregistered: true,
-					}
-				}
-			} else {
-				stn, err = allDispatchStates.findStateByIndex(index)
-				if err != nil {
-					return command{}, err
-				}
-			}
-			cmd.sources = []*dispatchState{stn}
-		}
+	if err := buildSources(&cmd, allDispatchStates); err != nil {
+		return command{}, err
 	}
+	return cmd, nil
+}
 
-	if ok := detectRunMount(&cmd, allDispatchStates); ok {
-		return cmd, nil
+func buildSources(cmd *command, allDispatchStates *dispatchStates) error {
+	funcs := []func(*command, *dispatchStates) (bool, error){detectCopySource, detectRunMount}
+	for _, f := range funcs {
+		if ok, err := f(cmd, allDispatchStates); ok || err != nil {
+			return err
+		}
 	}
+	return nil
+}
 
-	return cmd, nil
+func detectCopySource(cmd *command, allDispatchStates *dispatchStates) (bool, error) {
+	c, ok := cmd.Command.(*instructions.CopyCommand)
+	if !ok {
+		return false, nil
+	}
+	if c.From != "" {
+		var stn *dispatchState
+		index, err := strconv.Atoi(c.From)
+		if err != nil {
+			stn, ok = allDispatchStates.findStateByName(c.From)
+			if !ok {
+				stn = &dispatchState{
+					stage:        instructions.Stage{BaseName: c.From, Location: c.Location()},
+					deps:         make(map[*dispatchState]struct{}),
+					unregistered: true,
+				}
+			}
+		} else {
+			stn, err = allDispatchStates.findStateByIndex(index)
+			if err != nil {
+				return true, err
+			}
+		}
+		cmd.sources = []*dispatchState{stn}
+	}
+	return true, nil
 }
 
 type dispatchOpt struct {

diff --git a/frontend/dockerfile/dockerfile2llb/convert_runmount.go b/frontend/dockerfile/dockerfile2llb/convert_runmount.go
@@ -15,33 +15,36 @@ import (
 	"github.com/pkg/errors"
 )
 
-func detectRunMount(cmd *command, allDispatchStates *dispatchStates) bool {
-	if c, ok := cmd.Command.(*instructions.RunCommand); ok {
-		mounts := instructions.GetMounts(c)
-		sources := make([]*dispatchState, len(mounts))
-		for i, mount := range mounts {
-			if mount.From == "" && mount.Type == instructions.MountTypeCache {
-				mount.From = emptyImageName
-			}
-			from := mount.From
-			if from == "" || mount.Type == instructions.MountTypeTmpfs {
-				continue
-			}
-			stn, ok := allDispatchStates.findStateByName(from)
-			if !ok {
-				stn = &dispatchState{
-					stage:        instructions.Stage{BaseName: from},
-					deps:         make(map[*dispatchState]struct{}),
-					unregistered: true,
-				}
+func detectRunMount(cmd *command, allDispatchStates *dispatchStates) (bool, error) {
+	c, ok := cmd.Command.(*instructions.RunCommand)
+	if !ok {
+		return false, nil
+	}
+	mounts := instructions.GetMounts(c)
+	sources := make([]*dispatchState, len(mounts))
+	for i, mount := range mounts {
+		var from string
+		if mount.From == "" {
+			// this might not be accurate because the type might not have a real source (tmpfs for instance),
+			// but since this is just for creating the sources map it should be ok (we don't want to check the value of
+			// mount.Type because it might be a variable)
+			from = emptyImageName
+		} else {
+			from = mount.From
+		}
+
+		stn, ok := allDispatchStates.findStateByName(from)
+		if !ok {
+			stn = &dispatchState{
+				stage:        instructions.Stage{BaseName: from},
+				deps:         make(map[*dispatchState]struct{}),
+				unregistered: true,
 			}
-			sources[i] = stn
 		}
-		cmd.sources = sources
-		return true
+		sources[i] = stn
 	}
-
-	return false
+	cmd.sources = sources
+	return true, nil
 }
 
 func setCacheUIDGIDFileOp(m *instructions.Mount, st llb.State) llb.State {
@@ -83,9 +86,6 @@ func dispatchRunMounts(d *dispatchState, c *instructions.RunCommand, sources []*
 	mounts := instructions.GetMounts(c)
 
 	for i, mount := range mounts {
-		if mount.From == "" && mount.Type == instructions.MountTypeCache {
-			mount.From = emptyImageName
-		}
 		st := opt.buildContext
 		if mount.From != "" {
 			st = sources[i].state

diff --git a/frontend/dockerfile/dockerfile_mount_test.go b/frontend/dockerfile/dockerfile_mount_test.go
@@ -20,6 +20,10 @@ var mountTests = []integration.Test{
 	testMountTmpfs,
 	testMountRWCache,
 	testCacheMountDefaultID,
+	testMountEnvVar,
+	testMountArg,
+	testMountEnvAcrossStages,
+	testMountMetaArg,
 }
 
 func init() {
@@ -221,3 +225,126 @@ RUN --mount=type=cache,target=/mycache [ -f /mycache/foo ]
 	}, nil)
 	require.NoError(t, err)
 }
+
+func testMountEnvVar(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM busybox
+ENV SOME_PATH=/mycache
+RUN --mount=type=cache,target=/mycache touch /mycache/foo
+RUN --mount=type=cache,target=$SOME_PATH [ -f $SOME_PATH/foo ]
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(context.TODO(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(context.TODO(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+}
+
+func testMountArg(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM busybox
+ARG MNT_TYPE=cache
+RUN --mount=type=$MNT_TYPE,target=/mycache2 touch /mycache2/foo
+RUN --mount=type=cache,target=/mycache2 [ -f /mycache2/foo ]
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(context.TODO(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(context.TODO(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+}
+
+func testMountEnvAcrossStages(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM busybox as stage1
+
+ENV MNT_ID=mycache
+ENV MNT_TYPE2=cache
+RUN --mount=type=$MNT_TYPE2,id=$MNT_ID,target=/abcabc touch /abcabc/foo
+RUN --mount=type=$MNT_TYPE2,id=$MNT_ID,target=/cbacba [ -f /cbacba/foo ]
+
+FROM stage1
+RUN --mount=type=$MNT_TYPE2,id=$MNT_ID,target=/whatever [ -f /whatever/foo ]
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(context.TODO(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(context.TODO(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+}
+
+func testMountMetaArg(t *testing.T, sb integration.Sandbox) {
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+ARG META_PATH=/tmp/meta
+
+FROM busybox
+ARG META_PATH
+RUN --mount=type=cache,id=mycache,target=/tmp/meta touch /tmp/meta/foo
+RUN --mount=type=cache,id=mycache,target=$META_PATH [ -f /tmp/meta/foo ]
+`)
+
+	dir, err := tmpdir(
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	require.NoError(t, err)
+	defer os.RemoveAll(dir)
+
+	c, err := client.New(context.TODO(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	_, err = f.Solve(context.TODO(), c, client.SolveOpt{
+		LocalDirs: map[string]string{
+			builder.DefaultLocalNameDockerfile: dir,
+			builder.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+}
diff --git a/frontend/dockerfile/instructions/commands.go b/frontend/dockerfile/instructions/commands.go
@@ -272,6 +272,13 @@ type RunCommand struct {
 	FlagsUsed []string
 }
 
+func (c *RunCommand) Expand(expander SingleWordExpander) error {
+	if err := SetMountState(c, expander); err != nil {
+		return err
+	}
+	return nil
+}
+
 // CmdCommand : CMD foo
 //
 // Set the default command to run in the container (which may be empty).

diff --git a/frontend/dockerfile/instructions/commands_runmount.go b/frontend/dockerfile/instructions/commands_runmount.go
@@ -2,6 +2,7 @@ package instructions
 
 import (
 	"encoding/csv"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -64,13 +65,25 @@ func runMountPreHook(cmd *RunCommand, req parseRequest) error {
 }
 
 func runMountPostHook(cmd *RunCommand, req parseRequest) error {
+	return setMountState(cmd, false, nil)
+}
+
+func getMountState(cmd *RunCommand) *mountState {
+	v := cmd.getExternalValue(mountsKey)
+	if v == nil {
+		return nil
+	}
+	return v.(*mountState)
+}
+
+func setMountState(cmd *RunCommand, evaluateVars bool, expander SingleWordExpander) error {
 	st := getMountState(cmd)
 	if st == nil {
 		return errors.Errorf("no mount state")
 	}
 	var mounts []*Mount
 	for _, str := range st.flag.StringValues {
-		m, err := parseMount(str)
+		m, err := parseMount(str, evaluateVars, expander)
 		if err != nil {
 			return err
 		}
@@ -80,12 +93,8 @@ func runMountPostHook(cmd *RunCommand, req parseRequest) error {
 	return nil
 }
 
-func getMountState(cmd *RunCommand) *mountState {
-	v := cmd.getExternalValue(mountsKey)
-	if v == nil {
-		return nil
-	}
-	return v.(*mountState)
+func SetMountState(cmd *RunCommand, expander SingleWordExpander) error {
+	return setMountState(cmd, true, expander)
 }
 
 func GetMounts(cmd *RunCommand) []*Mount {
@@ -111,7 +120,7 @@ type Mount struct {
 	GID          *uint64
 }
 
-func parseMount(value string) (*Mount, error) {
+func parseMount(value string, evaluateVars bool, expander SingleWordExpander) (*Mount, error) {
 	csvReader := csv.NewReader(strings.NewReader(value))
 	fields, err := csvReader.Read()
 	if err != nil {
@@ -151,6 +160,25 @@ func parseMount(value string) (*Mount, error) {
 		}
 
 		value := parts[1]
+		// possible env var, defer for later
+		matched, err := regexp.MatchString(`\$.`, value)
+		if err != nil {
+			return nil, err
+		}
+		if matched {
+			if !evaluateVars {
+				// skip validation for now
+				continue
+			}
+			if expander == nil {
+				return nil, errors.Errorf("'expander' is missing from context")
+			}
+			processed, err := expander(value)
+			if err != nil {
+				return nil, err
+			}
+			value = processed
+		}
 		switch key {
 		case "type":
 			if !isValidMountType(strings.ToLower(value)) {