Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
May 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions frontend/dockerfile/dockerfile2llb/convert_runmount.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ func detectRunMount(cmd *command, allDispatchStates *dispatchStates) bool {
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
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 {
Expand Down
160 changes: 160 additions & 0 deletions frontend/dockerfile/dockerfile_mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ var mountTests = []integration.Test{
testMountTmpfs,
testMountRWCache,
testCacheMountDefaultID,
testMountEnvVar,
testMountArg,
testMountEnvAcrossStages,
testMountMetaArg,
testMountFromError,
}

func init() {
Expand Down Expand Up @@ -221,3 +226,158 @@ 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=cache,id=mycache,target=/abcabc touch /abcabc/foo
RUN --mount=type=$MNT_TYPE2,id=$MNT_ID,target=/cbacba [ -f /cbacba/foo ]
omermizr marked this conversation as resolved.
Show resolved Hide resolved

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)
}

func testMountFromError(t *testing.T, sb integration.Sandbox) {
f := getFrontend(t, sb)

dockerfile := []byte(`
FROM busybox as test
RUN touch /tmp/test

FROM busybox
ENV ttt=test
RUN --mount=from=$ttt,type=cache,target=/tmp ls
`)

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.Error(t, err)
require.Contains(t, err.Error(), "'from' doesn't support variable expansion, define alias stage instead")
}
7 changes: 7 additions & 0 deletions frontend/dockerfile/instructions/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
26 changes: 24 additions & 2 deletions frontend/dockerfile/instructions/commands_runmount.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package instructions

import (
"encoding/csv"
"regexp"
"strconv"
"strings"

Expand Down Expand Up @@ -64,13 +65,17 @@ func runMountPreHook(cmd *RunCommand, req parseRequest) error {
}

func runMountPostHook(cmd *RunCommand, req parseRequest) error {
return setMountState(cmd, nil)
}

func setMountState(cmd *RunCommand, 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, expander)
if err != nil {
return err
}
Expand Down Expand Up @@ -111,7 +116,7 @@ type Mount struct {
GID *uint64
}

func parseMount(value string) (*Mount, error) {
func parseMount(value string, expander SingleWordExpander) (*Mount, error) {
csvReader := csv.NewReader(strings.NewReader(value))
fields, err := csvReader.Read()
if err != nil {
Expand Down Expand Up @@ -151,6 +156,23 @@ func parseMount(value string) (*Mount, error) {
}

value := parts[1]
// check for potential variable
if expander != nil {
omermizr marked this conversation as resolved.
Show resolved Hide resolved
processed, err := expander(value)
if err != nil {
return nil, err
}
value = processed
} else if key == "from" {
if matched, err := regexp.MatchString(`\$.`, value); err != nil { //nolint
return nil, err
} else if matched {
return nil, errors.Errorf("'%s' doesn't support variable expansion, define alias stage instead", key)
}
} else {
// if we don't have an expander, defer evaluation to later
continue
}
switch key {
case "type":
if !isValidMountType(strings.ToLower(value)) {
Expand Down