Skip to content

Commit

Permalink
Handle file paths base on target platform
Browse files Browse the repository at this point in the history 
This change properly handles paths on different platforms. In short, this
change checks the target platform we're building an image for and applies
normalization steps to make sure the file paths are valid. This makes buildkit
properly handle paths on both *nix systems and on Windows.

Signed-off-by: Gabriel Adrian Samfira <[email protected]>
  • Loading branch information
@gabriel-samfira
gabriel-samfira committed May 31, 2023
1 parent 80b91c5 commit ac3d451
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 89 deletions.
91 changes: 47 additions & 44 deletions client/llb/fileop.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import (
"context"
_ "crypto/sha256" // for opencontainers/go-digest
"os"
"path"
"path/filepath"
"strconv"
"strings"
"time"

"github.com/moby/buildkit/solver/pb"
"github.com/moby/buildkit/util/system"
digest "github.com/opencontainers/go-digest"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -54,7 +55,7 @@ type CopyInput interface {
}

type subAction interface {
toProtoAction(context.Context, string, pb.InputIndex) (pb.IsFileAction, error)
toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error)
}

type capAdder interface {
Expand Down Expand Up @@ -156,10 +157,14 @@ type fileActionMkdir struct {
info MkdirInfo
}

func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) {
func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) {
normalizedPath, err := system.NormalizePath(parent, a.file, platform, false)
if err != nil {
return nil, errors.Wrap(err, "normalizing path")
}
return &pb.FileAction_Mkdir{
Mkdir: &pb.FileActionMkDir{
Path: normalizePath(parent, a.file, false),
Path: normalizedPath,
Mode: int32(a.mode & 0777),
MakeParents: a.info.MakeParents,
Owner: a.info.ChownOpt.marshal(base),
Expand Down Expand Up @@ -330,10 +335,14 @@ type fileActionMkfile struct {
info MkfileInfo
}

func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) {
func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) {
normalizedPath, err := system.NormalizePath(parent, a.file, platform, false)
if err != nil {
return nil, errors.Wrap(err, "normalizing path")
}
return &pb.FileAction_Mkfile{
Mkfile: &pb.FileActionMkFile{
Path: normalizePath(parent, a.file, false),
Path: normalizedPath,
Mode: int32(a.mode & 0777),
Data: a.dt,
Owner: a.info.ChownOpt.marshal(base),
Expand Down Expand Up @@ -398,10 +407,14 @@ type fileActionRm struct {
info RmInfo
}

func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) {
func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) {
normalizedPath, err := system.NormalizePath(parent, a.file, platform, false)
if err != nil {
return nil, errors.Wrap(err, "normalizing path")
}
return &pb.FileAction_Rm{
Rm: &pb.FileActionRm{
Path: normalizePath(parent, a.file, false),
Path: normalizedPath,
AllowNotFound: a.info.AllowNotFound,
AllowWildcard: a.info.AllowWildcard,
},
Expand Down Expand Up @@ -488,14 +501,18 @@ type fileActionCopy struct {
info CopyInfo
}

func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) {
src, err := a.sourcePath(ctx)
func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) {
src, err := a.sourcePath(ctx, platform)
if err != nil {
return nil, err
}
normalizedPath, err := system.NormalizePath(parent, a.dest, platform, false)
if err != nil {
return nil, errors.Wrap(err, "normalizing path")
}
c := &pb.FileActionCopy{
Src: src,
Dest: normalizePath(parent, a.dest, true),
Dest: normalizedPath,
Owner: a.info.ChownOpt.marshal(base),
IncludePatterns: a.info.IncludePatterns,
ExcludePatterns: a.info.ExcludePatterns,
Expand All @@ -517,21 +534,22 @@ func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base
}, nil
}

func (a *fileActionCopy) sourcePath(ctx context.Context) (string, error) {
p := path.Clean(a.src)
if !path.IsAbs(p) {
func (a *fileActionCopy) sourcePath(ctx context.Context, platform string) (string, error) {
p := filepath.Clean(a.src)
if !system.IsAbs(p, platform) {
var dir string
var err error
if a.state != nil {
dir, err := a.state.GetDir(ctx)
if err != nil {
return "", err
}
p = path.Join("/", dir, p)
dir, err = a.state.GetDir(ctx)
} else if a.fas != nil {
dir, err := a.fas.state.GetDir(ctx)
if err != nil {
return "", err
}
p = path.Join("/", dir, p)
dir, err = a.fas.state.GetDir(ctx)
}
if err != nil {
return "", err
}
p, err = system.NormalizePath(dir, p, platform, false)
if err != nil {
return "", errors.Wrap(err, "normalizing source path")
}
}
return p, nil
Expand Down Expand Up @@ -749,7 +767,11 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, []
}
}

action, err := st.action.toProtoAction(ctx, parent, st.base)
var platform string
if f.constraints.Platform != nil {
platform = f.constraints.Platform.OS
}
action, err := st.action.toProtoAction(ctx, parent, st.base, platform)
if err != nil {
return "", nil, nil, nil, err
}
Expand All @@ -770,25 +792,6 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, []
return f.Load()
}

func normalizePath(parent, p string, keepSlash bool) string {
origPath := p
p = path.Clean(p)
if !path.IsAbs(p) {
p = path.Join("/", parent, p)
}
if keepSlash {
if strings.HasSuffix(origPath, "/") && !strings.HasSuffix(p, "/") {
p += "/"
} else if strings.HasSuffix(origPath, "/.") {
if p != "/" {
p += "/"
}
p += "."
}
}
return p
}

func (f *FileOp) Output() Output {
return f.output
}
Expand Down
17 changes: 11 additions & 6 deletions client/llb/meta.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import (
"context"
"fmt"
"net"
"path"

"github.com/containerd/containerd/platforms"
"github.com/google/shlex"
"github.com/moby/buildkit/solver/pb"
"github.com/moby/buildkit/util/system"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
)

type contextKeyT string
Expand Down Expand Up @@ -66,15 +67,19 @@ func dirf(value string, replace bool, v ...interface{}) StateOption {
}
return func(s State) State {
return s.withValue(keyDir, func(ctx context.Context, c *Constraints) (interface{}, error) {
if !path.IsAbs(value) {
var platform string
if c != nil && c.Platform != nil {
platform = c.Platform.OS
}
if !system.IsAbs(value, platform) {
prev, err := getDir(s)(ctx, c)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "getting dir from state")
}
if prev == "" {
prev = "/"
value, err = system.NormalizePath(prev, value, platform, false)
if err != nil {
return nil, errors.Wrap(err, "normalizing path")
}
value = path.Join(prev, value)
}
return value, nil
})
Expand Down
11 changes: 6 additions & 5 deletions client/llb/meta_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,25 @@ import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestRelativeWd(t *testing.T) {
st := Scratch().Dir("foo")
require.Equal(t, getDirHelper(t, st), "/foo")
assert.Equal(t, getDirHelper(t, st), "/foo")

st = st.Dir("bar")
require.Equal(t, getDirHelper(t, st), "/foo/bar")
assert.Equal(t, getDirHelper(t, st), "/foo/bar")

st = st.Dir("..")
require.Equal(t, getDirHelper(t, st), "/foo")
assert.Equal(t, getDirHelper(t, st), "/foo")

st = st.Dir("/baz")
require.Equal(t, getDirHelper(t, st), "/baz")
assert.Equal(t, getDirHelper(t, st), "/baz")

st = st.Dir("../../..")
require.Equal(t, getDirHelper(t, st), "/")
assert.Equal(t, getDirHelper(t, st), "/")
}

func getDirHelper(t *testing.T, s State) string {
Expand Down
57 changes: 45 additions & 12 deletions frontend/dockerfile/dockerfile2llb/convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,11 +994,16 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE
}

func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error {
d.state = d.state.Dir(c.Path)
wd := c.Path
if !path.IsAbs(c.Path) {
wd = path.Join("/", d.image.Config.WorkingDir, wd)
var platformOS string
if d != nil && d.platform != nil {
platformOS = d.platform.OS
}
wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, platformOS)
if err != nil {
return errors.Wrap(err, "normalizing workdir")
}

d.state = d.state.Dir(wd)
d.image.Config.WorkingDir = wd
if commit {
withLayer := false
Expand Down Expand Up @@ -1027,11 +1032,16 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo
}

func dispatchCopy(d *dispatchState, cfg copyConfig) error {
pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath)
var platformOS string
if d.platform != nil {
platformOS = d.platform.OS
}
pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, platformOS)
if err != nil {
return err
}
dest := path.Join("/", pp)
dest := filepath.Join("/", pp)

if cfg.params.DestPath == "." || cfg.params.DestPath == "" || cfg.params.DestPath[len(cfg.params.DestPath)-1] == filepath.Separator {
dest += string(filepath.Separator)
}
Expand Down Expand Up @@ -1135,6 +1145,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
a = a.Copy(st, f, dest, opts...)
}
} else {
src, err = system.CheckSystemDriveAndRemoveDriveLetter(src, platformOS)
if err != nil {
return errors.Wrap(err, "removing drive letter")
}

opts := append([]llb.CopyOption{&llb.CopyInfo{
Mode: mode,
FollowSymlinks: true,
Expand All @@ -1157,7 +1172,10 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
commitMessage.WriteString(" <<" + src.Path)

data := src.Data
f := src.Path
f, err := system.CheckSystemDriveAndRemoveDriveLetter(src.Path, platformOS)
if err != nil {
return errors.Wrap(err, "removing drive letter")
}
st := llb.Scratch().File(
llb.Mkfile(f, 0664, []byte(data)),
dockerui.WithInternalName("preparing inline document"),
Expand All @@ -1168,6 +1186,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
CreateDestPath: true,
}}, copyOpt...)

dest, err = system.CheckSystemDriveAndRemoveDriveLetter(dest, platformOS)
if err != nil {
return errors.Wrap(err, "removing drive letter")
}

if a == nil {
a = llb.Copy(st, f, dest, opts...)
} else {
Expand Down Expand Up @@ -1394,15 +1417,25 @@ func dispatchArg(d *dispatchState, c *instructions.ArgCommand, metaArgs []instru
return commitToHistory(&d.image, "ARG "+strings.Join(commitStrs, " "), false, nil, d.epoch)
}

func pathRelativeToWorkingDir(s llb.State, p string) (string, error) {
if path.IsAbs(p) {
return p, nil
}
func pathRelativeToWorkingDir(s llb.State, p, platform string) (string, error) {
dir, err := s.GetDir(context.TODO())
if err != nil {
return "", err
}
return path.Join(dir, p), nil

if len(p) == 0 {
return dir, nil
}

p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform)
if err != nil {
return "", errors.Wrap(err, "remving drive letter")
}

if system.IsAbs(p, platform) {
return p, nil
}
return filepath.Join(dir, p), nil
}

func addEnv(env []string, k, v string) []string {
Expand Down
7 changes: 4 additions & 3 deletions frontend/gateway/container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"syscall"

"github.com/moby/buildkit/util/bklog"
"github.com/moby/buildkit/util/system"

"github.com/moby/buildkit/cache"
"github.com/moby/buildkit/executor"
Expand Down Expand Up @@ -88,7 +89,7 @@ func NewContainer(ctx context.Context, w worker.Worker, sm *session.Manager, g s
cm = refs[m.Input].Worker.CacheManager()
}
return cm.New(ctx, ref, g)
})
}, platform.OS)
if err != nil {
for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order
p.Actives[i].Ref.Release(context.TODO())
Expand Down Expand Up @@ -138,7 +139,7 @@ type MountMutableRef struct {

type MakeMutable func(m *opspb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error)

func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manager, g session.Group, cwd string, mnts []*opspb.Mount, refs []*worker.WorkerRef, makeMutable MakeMutable) (p PreparedMounts, err error) {
func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manager, g session.Group, cwd string, mnts []*opspb.Mount, refs []*worker.WorkerRef, makeMutable MakeMutable, platform string) (p PreparedMounts, err error) {
// loop over all mounts, fill in mounts, root and outputs
for i, m := range mnts {
var (
Expand Down Expand Up @@ -261,7 +262,7 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage
} else {
mws := mountWithSession(mountable, g)
dest := m.Dest
if !filepath.IsAbs(filepath.Clean(dest)) {
if !system.IsAbs(filepath.Clean(dest), platform) {
dest = filepath.Join("/", cwd, dest)
}
mws.Dest = dest
Expand Down
Loading

0 comments on commit ac3d451

Please sign in to comment.