Skip to content

Commit 686a84b

Browse files
gabriel-samfiragabriel-samfira
committed
Handle file paths base on target platform
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]>
1 parent cdf28d6 commit 686a84b

File tree

7 files changed

+154
-89
lines changed

7 files changed

+154
-89
lines changed

‎client/llb/fileop.go

Lines changed: 47 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ import (
44
"context"
55
_ "crypto/sha256" // for opencontainers/go-digest
66
"os"
7-
"path"
7+
"path/filepath"
88
"strconv"
99
"strings"
1010
"time"
1111

1212
"github.com/moby/buildkit/solver/pb"
13+
"github.com/moby/buildkit/util/system"
1314
digest "github.com/opencontainers/go-digest"
1415
"github.com/pkg/errors"
1516
)
@@ -54,7 +55,7 @@ type CopyInput interface {
5455
}
5556

5657
type subAction interface {
57-
toProtoAction(context.Context, string, pb.InputIndex) (pb.IsFileAction, error)
58+
toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error)
5859
}
5960

6061
type capAdder interface {
@@ -160,10 +161,14 @@ type fileActionMkdir struct {
160161
info MkdirInfo
161162
}
162163

163-
func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) {
164+
func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) {
165+
normalizedPath, err := system.NormalizePath(parent, a.file, platform, false)
166+
if err != nil {
167+
return nil, errors.Wrap(err, "normalizing path")
168+
}
164169
return &pb.FileAction_Mkdir{
165170
Mkdir: &pb.FileActionMkDir{
166-
Path: normalizePath(parent, a.file, false),
171+
Path: normalizedPath,
167172
Mode: int32(a.mode & 0777),
168173
MakeParents: a.info.MakeParents,
169174
Owner: a.info.ChownOpt.marshal(base),
@@ -334,10 +339,14 @@ type fileActionMkfile struct {
334339
info MkfileInfo
335340
}
336341

337-
func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) {
342+
func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) {
343+
normalizedPath, err := system.NormalizePath(parent, a.file, platform, false)
344+
if err != nil {
345+
return nil, errors.Wrap(err, "normalizing path")
346+
}
338347
return &pb.FileAction_Mkfile{
339348
Mkfile: &pb.FileActionMkFile{
340-
Path: normalizePath(parent, a.file, false),
349+
Path: normalizedPath,
341350
Mode: int32(a.mode & 0777),
342351
Data: a.dt,
343352
Owner: a.info.ChownOpt.marshal(base),
@@ -402,10 +411,14 @@ type fileActionRm struct {
402411
info RmInfo
403412
}
404413

405-
func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) {
414+
func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) {
415+
normalizedPath, err := system.NormalizePath(parent, a.file, platform, false)
416+
if err != nil {
417+
return nil, errors.Wrap(err, "normalizing path")
418+
}
406419
return &pb.FileAction_Rm{
407420
Rm: &pb.FileActionRm{
408-
Path: normalizePath(parent, a.file, false),
421+
Path: normalizedPath,
409422
AllowNotFound: a.info.AllowNotFound,
410423
AllowWildcard: a.info.AllowWildcard,
411424
},
@@ -492,14 +505,18 @@ type fileActionCopy struct {
492505
info CopyInfo
493506
}
494507

495-
func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) {
496-
src, err := a.sourcePath(ctx)
508+
func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) {
509+
src, err := a.sourcePath(ctx, platform)
497510
if err != nil {
498511
return nil, err
499512
}
513+
normalizedPath, err := system.NormalizePath(parent, a.dest, platform, false)
514+
if err != nil {
515+
return nil, errors.Wrap(err, "normalizing path")
516+
}
500517
c := &pb.FileActionCopy{
501518
Src: src,
502-
Dest: normalizePath(parent, a.dest, true),
519+
Dest: normalizedPath,
503520
Owner: a.info.ChownOpt.marshal(base),
504521
IncludePatterns: a.info.IncludePatterns,
505522
ExcludePatterns: a.info.ExcludePatterns,
@@ -521,21 +538,22 @@ func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base
521538
}, nil
522539
}
523540

524-
func (a *fileActionCopy) sourcePath(ctx context.Context) (string, error) {
525-
p := path.Clean(a.src)
526-
if !path.IsAbs(p) {
541+
func (a *fileActionCopy) sourcePath(ctx context.Context, platform string) (string, error) {
542+
p := filepath.Clean(a.src)
543+
if !system.IsAbs(p, platform) {
544+
var dir string
545+
var err error
527546
if a.state != nil {
528-
dir, err := a.state.GetDir(ctx)
529-
if err != nil {
530-
return "", err
531-
}
532-
p = path.Join("/", dir, p)
547+
dir, err = a.state.GetDir(ctx)
533548
} else if a.fas != nil {
534-
dir, err := a.fas.state.GetDir(ctx)
535-
if err != nil {
536-
return "", err
537-
}
538-
p = path.Join("/", dir, p)
549+
dir, err = a.fas.state.GetDir(ctx)
550+
}
551+
if err != nil {
552+
return "", err
553+
}
554+
p, err = system.NormalizePath(dir, p, platform, false)
555+
if err != nil {
556+
return "", errors.Wrap(err, "normalizing source path")
539557
}
540558
}
541559
return p, nil
@@ -753,7 +771,11 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, []
753771
}
754772
}
755773

756-
action, err := st.action.toProtoAction(ctx, parent, st.base)
774+
var platform string
775+
if f.constraints.Platform != nil {
776+
platform = f.constraints.Platform.OS
777+
}
778+
action, err := st.action.toProtoAction(ctx, parent, st.base, platform)
757779
if err != nil {
758780
return "", nil, nil, nil, err
759781
}
@@ -774,25 +796,6 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, []
774796
return f.Load()
775797
}
776798

777-
func normalizePath(parent, p string, keepSlash bool) string {
778-
origPath := p
779-
p = path.Clean(p)
780-
if !path.IsAbs(p) {
781-
p = path.Join("/", parent, p)
782-
}
783-
if keepSlash {
784-
if strings.HasSuffix(origPath, "/") && !strings.HasSuffix(p, "/") {
785-
p += "/"
786-
} else if strings.HasSuffix(origPath, "/.") {
787-
if p != "/" {
788-
p += "/"
789-
}
790-
p += "."
791-
}
792-
}
793-
return p
794-
}
795-
796799
func (f *FileOp) Output() Output {
797800
return f.output
798801
}

‎client/llb/meta.go

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ import (
44
"context"
55
"fmt"
66
"net"
7-
"path"
87

98
"github.com/containerd/containerd/platforms"
109
"github.com/google/shlex"
1110
"github.com/moby/buildkit/solver/pb"
11+
"github.com/moby/buildkit/util/system"
1212
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
13+
"github.com/pkg/errors"
1314
)
1415

1516
type contextKeyT string
@@ -75,15 +76,19 @@ func dirf(value string, replace bool, v ...interface{}) StateOption {
7576
}
7677
return func(s State) State {
7778
return s.withValue(keyDir, func(ctx context.Context, c *Constraints) (interface{}, error) {
78-
if !path.IsAbs(value) {
79+
var platform string
80+
if c != nil && c.Platform != nil {
81+
platform = c.Platform.OS
82+
}
83+
if !system.IsAbs(value, platform) {
7984
prev, err := getDir(s)(ctx, c)
8085
if err != nil {
81-
return nil, err
86+
return nil, errors.Wrap(err, "getting dir from state")
8287
}
83-
if prev == "" {
84-
prev = "/"
88+
value, err = system.NormalizePath(prev, value, platform, false)
89+
if err != nil {
90+
return nil, errors.Wrap(err, "normalizing path")
8591
}
86-
value = path.Join(prev, value)
8792
}
8893
return value, nil
8994
})

‎client/llb/meta_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,25 @@ import (
44
"context"
55
"testing"
66

7+
"github.com/stretchr/testify/assert"
78
"github.com/stretchr/testify/require"
89
)
910

1011
func TestRelativeWd(t *testing.T) {
1112
st := Scratch().Dir("foo")
12-
require.Equal(t, getDirHelper(t, st), "/foo")
13+
assert.Equal(t, getDirHelper(t, st), "/foo")
1314

1415
st = st.Dir("bar")
15-
require.Equal(t, getDirHelper(t, st), "/foo/bar")
16+
assert.Equal(t, getDirHelper(t, st), "/foo/bar")
1617

1718
st = st.Dir("..")
18-
require.Equal(t, getDirHelper(t, st), "/foo")
19+
assert.Equal(t, getDirHelper(t, st), "/foo")
1920

2021
st = st.Dir("/baz")
21-
require.Equal(t, getDirHelper(t, st), "/baz")
22+
assert.Equal(t, getDirHelper(t, st), "/baz")
2223

2324
st = st.Dir("../../..")
24-
require.Equal(t, getDirHelper(t, st), "/")
25+
assert.Equal(t, getDirHelper(t, st), "/")
2526
}
2627

2728
func getDirHelper(t *testing.T, s State) string {

‎frontend/dockerfile/dockerfile2llb/convert.go

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -994,11 +994,16 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE
994994
}
995995

996996
func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error {
997-
d.state = d.state.Dir(c.Path)
998-
wd := c.Path
999-
if !path.IsAbs(c.Path) {
1000-
wd = path.Join("/", d.image.Config.WorkingDir, wd)
997+
var platformOS string
998+
if d != nil && d.platform != nil {
999+
platformOS = d.platform.OS
10011000
}
1001+
wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, platformOS)
1002+
if err != nil {
1003+
return errors.Wrap(err, "normalizing workdir")
1004+
}
1005+
1006+
d.state = d.state.Dir(wd)
10021007
d.image.Config.WorkingDir = wd
10031008
if commit {
10041009
withLayer := false
@@ -1027,11 +1032,16 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo
10271032
}
10281033

10291034
func dispatchCopy(d *dispatchState, cfg copyConfig) error {
1030-
pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath)
1035+
var platformOS string
1036+
if d.platform != nil {
1037+
platformOS = d.platform.OS
1038+
}
1039+
pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, platformOS)
10311040
if err != nil {
10321041
return err
10331042
}
1034-
dest := path.Join("/", pp)
1043+
dest := filepath.Join("/", pp)
1044+
10351045
if cfg.params.DestPath == "." || cfg.params.DestPath == "" || cfg.params.DestPath[len(cfg.params.DestPath)-1] == filepath.Separator {
10361046
dest += string(filepath.Separator)
10371047
}
@@ -1135,6 +1145,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
11351145
a = a.Copy(st, f, dest, opts...)
11361146
}
11371147
} else {
1148+
src, err = system.CheckSystemDriveAndRemoveDriveLetter(src, platformOS)
1149+
if err != nil {
1150+
return errors.Wrap(err, "removing drive letter")
1151+
}
1152+
11381153
opts := append([]llb.CopyOption{&llb.CopyInfo{
11391154
Mode: mode,
11401155
FollowSymlinks: true,
@@ -1157,7 +1172,10 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
11571172
commitMessage.WriteString(" <<" + src.Path)
11581173

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

1189+
dest, err = system.CheckSystemDriveAndRemoveDriveLetter(dest, platformOS)
1190+
if err != nil {
1191+
return errors.Wrap(err, "removing drive letter")
1192+
}
1193+
11711194
if a == nil {
11721195
a = llb.Copy(st, f, dest, opts...)
11731196
} else {
@@ -1394,15 +1417,25 @@ func dispatchArg(d *dispatchState, c *instructions.ArgCommand, metaArgs []instru
13941417
return commitToHistory(&d.image, "ARG "+strings.Join(commitStrs, " "), false, nil, d.epoch)
13951418
}
13961419

1397-
func pathRelativeToWorkingDir(s llb.State, p string) (string, error) {
1398-
if path.IsAbs(p) {
1399-
return p, nil
1400-
}
1420+
func pathRelativeToWorkingDir(s llb.State, p, platform string) (string, error) {
14011421
dir, err := s.GetDir(context.TODO())
14021422
if err != nil {
14031423
return "", err
14041424
}
1405-
return path.Join(dir, p), nil
1425+
1426+
if len(p) == 0 {
1427+
return dir, nil
1428+
}
1429+
1430+
p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform)
1431+
if err != nil {
1432+
return "", errors.Wrap(err, "remving drive letter")
1433+
}
1434+
1435+
if system.IsAbs(p, platform) {
1436+
return p, nil
1437+
}
1438+
return filepath.Join(dir, p), nil
14061439
}
14071440

14081441
func addEnv(env []string, k, v string) []string {

‎frontend/gateway/container/container.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/moby/buildkit/session/secrets"
1414
"github.com/moby/buildkit/util/bklog"
15+
"github.com/moby/buildkit/util/system"
1516

1617
"github.com/moby/buildkit/cache"
1718
"github.com/moby/buildkit/executor"
@@ -92,7 +93,7 @@ func NewContainer(ctx context.Context, w worker.Worker, sm *session.Manager, g s
9293
cm = refs[m.Input].Worker.CacheManager()
9394
}
9495
return cm.New(ctx, ref, g)
95-
})
96+
}, platform.OS)
9697
if err != nil {
9798
for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order
9899
p.Actives[i].Ref.Release(context.TODO())
@@ -142,7 +143,7 @@ type MountMutableRef struct {
142143

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

145-
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) {
146+
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) {
146147
// loop over all mounts, fill in mounts, root and outputs
147148
for i, m := range mnts {
148149
var (
@@ -265,7 +266,7 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage
265266
} else {
266267
mws := MountWithSession(mountable, g)
267268
dest := m.Dest
268-
if !filepath.IsAbs(filepath.Clean(dest)) {
269+
if !system.IsAbs(filepath.Clean(dest), platform) {
269270
dest = filepath.Join("/", cwd, dest)
270271
}
271272
mws.Dest = dest

0 commit comments

Comments
 (0)