From 1f39eb84cfbbd9212b7b5d9bb7c49683c983b5e4 Mon Sep 17 00:00:00 2001 From: Omer Mizrahi Date: Mon, 26 Apr 2021 21:30:23 +0300 Subject: [PATCH] feat(run-mount): add support for variable expansion Signed-off-by: Omer Mizrahi --- frontend/dockerfile/dockerfile2llb/convert.go | 62 +++++---- .../dockerfile2llb/convert_runmount.go | 54 ++++---- frontend/dockerfile/dockerfile_mount_test.go | 127 ++++++++++++++++++ frontend/dockerfile/instructions/commands.go | 7 + .../instructions/commands_runmount.go | 44 ++++-- 5 files changed, 235 insertions(+), 59 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 7c749a7bbc569..a5a1c755b63c3 100644 --- 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 index 8cd928b2b6300..6369ab62211ec 100644 --- 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 index f169b44aae013..ace93acbe7fb5 100644 --- 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 index b900ad771151c..f8e7b03d4108f 100644 --- 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 index b0b0f0103b5ae..c9c80390817dc 100644 --- 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)) {