From 6d689a39db5ff1d291f3e1412f3a35a2c662847b Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 25 Mar 2024 17:22:00 -0700 Subject: [PATCH 1/6] dockerfile: fix missing source mapping for COPY --link command Signed-off-by: Tonis Tiigi (cherry picked from commit eb2220752268106780b34de4ff539255e07e1f1f) --- frontend/dockerfile/dockerfile2llb/convert.go | 5 +- .../dockerfile/dockerfile_provenance_test.go | 118 ++++++++++++++++++ frontend/dockerfile/dockerfile_test.go | 1 + 3 files changed, 121 insertions(+), 3 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index b79407955542..403530a74219 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -1340,11 +1340,10 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { copyOpts := []llb.ConstraintsOpt{ llb.Platform(*d.platform), } - copy(copyOpts, fileOpt) + copyOpts = append(copyOpts, fileOpt...) copyOpts = append(copyOpts, llb.ProgressGroup(pgID, pgName, true)) - var mergeOpts []llb.ConstraintsOpt - copy(mergeOpts, fileOpt) + mergeOpts := append([]llb.ConstraintsOpt{}, fileOpt...) d.cmdIndex-- mergeOpts = append(mergeOpts, llb.ProgressGroup(pgID, pgName, false), llb.WithCustomName(prefixCommand(d, "LINK "+name, d.prefixPlatform, &platform, env))) diff --git a/frontend/dockerfile/dockerfile_provenance_test.go b/frontend/dockerfile/dockerfile_provenance_test.go index 602d38ae9602..ff74b7e2e6c0 100644 --- a/frontend/dockerfile/dockerfile_provenance_test.go +++ b/frontend/dockerfile/dockerfile_provenance_test.go @@ -1,6 +1,7 @@ package dockerfile import ( + "bytes" "context" "encoding/json" "fmt" @@ -1130,6 +1131,123 @@ func testDockerIgnoreMissingProvenance(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) } +func testCommandSourceMapping(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + ctx := sb.Context() + + c, err := client.New(ctx, sb.Address()) + require.NoError(t, err) + defer c.Close() + + dockerfile := []byte(`FROM alpine +RUN echo "hello" > foo +WORKDIR /tmp +COPY foo foo2 +COPY --link foo foo3 +ADD bar bar`) + + dir := integration.Tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + fstest.CreateFile("foo", []byte("data"), 0600), + fstest.CreateFile("bar", []byte("data2"), 0600), + ) + + registry, err := sb.NewRegistry() + if errors.Is(err, integration.ErrRequirements) { + t.Skip(err.Error()) + } + require.NoError(t, err) + + target := registry + "/buildkit/testsourcemappingprov:latest" + f := getFrontend(t, sb) + + _, err = f.Solve(sb.Context(), c, client.SolveOpt{ + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + FrontendAttrs: map[string]string{ + "attest:provenance": "mode=max", + }, + Exports: []client.ExportEntry{ + { + Type: client.ExporterImage, + Attrs: map[string]string{ + "name": target, + "push": "true", + }, + }, + }, + }, nil) + require.NoError(t, err) + + desc, provider, err := contentutil.ProviderFromRef(target) + require.NoError(t, err) + imgs, err := testutil.ReadImages(sb.Context(), provider, desc) + require.NoError(t, err) + require.Equal(t, 2, len(imgs.Images)) + + expPlatform := platforms.Format(platforms.Normalize(platforms.DefaultSpec())) + + img := imgs.Find(expPlatform) + require.NotNil(t, img) + + att := imgs.FindAttestation(expPlatform) + type stmtT struct { + Predicate provenancetypes.ProvenancePredicate `json:"predicate"` + } + var stmt stmtT + require.NoError(t, json.Unmarshal(att.LayersRaw[0], &stmt)) + pred := stmt.Predicate + + def := pred.BuildConfig.Definition + + steps := map[string]provenancetypes.BuildStep{} + for _, step := range def { + steps[step.ID] = step + } + // ensure all IDs are unique + require.Equal(t, len(steps), len(def)) + + src := pred.Metadata.BuildKitMetadata.Source + + lines := make([]bool, bytes.Count(dockerfile, []byte("\n"))+1) + + for id, loc := range src.Locations { + // - only context upload can be without source mapping + // - every step must only be in one line + // - perform bounds check for location + step, ok := steps[id] + require.True(t, ok, "definition for step %s not found", id) + + if len(loc.Locations) == 0 { + s := step.Op.GetSource() + require.NotNil(t, s, "unmapped step %s is not source", id) + require.Equal(t, "local://context", s.Identifier) + } else if len(loc.Locations) >= 1 { + require.Equal(t, 1, len(loc.Locations), "step %s has more than one location", id) + } + + for _, loc := range loc.Locations { + for _, r := range loc.Ranges { + require.Equal(t, r.Start.Line, r.End.Line, "step %s has range with multiple lines", id) + + idx := r.Start.Line - 1 + if idx < 0 || int(idx) >= len(lines) { + t.Fatalf("step %s has invalid range on line %d", id, idx) + } + lines[idx] = true + } + } + } + + // ensure all lines are covered + for i, covered := range lines { + require.True(t, covered, "line %d is not covered", i+1) + } +} + func testFrontendDeduplicateSources(t *testing.T, sb integration.Sandbox) { integration.SkipOnPlatform(t, "windows") ctx := sb.Context() diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index d8a04f99154f..9fed2a960baa 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -174,6 +174,7 @@ var allTests = integration.TestFuncs( testNilProvenance, testDuplicatePlatformProvenance, testDockerIgnoreMissingProvenance, + testCommandSourceMapping, testSBOMScannerArgs, testMultiPlatformWarnings, testNilContextInSolveGateway, From d647910aafd20745d394af5773752454168289d5 Mon Sep 17 00:00:00 2001 From: CrazyMax <1951866+crazy-max@users.noreply.github.com> Date: Thu, 25 Apr 2024 14:13:58 +0200 Subject: [PATCH 2/6] frontend: missing compat check for TestCommandSourceMapping Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com> (cherry picked from commit df5ad9c9f07d7511d73f8b6dd340cc9dc33fdcb7) --- frontend/dockerfile/dockerfile_provenance_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/dockerfile/dockerfile_provenance_test.go b/frontend/dockerfile/dockerfile_provenance_test.go index ff74b7e2e6c0..fdb0e96271f0 100644 --- a/frontend/dockerfile/dockerfile_provenance_test.go +++ b/frontend/dockerfile/dockerfile_provenance_test.go @@ -1133,6 +1133,7 @@ func testDockerIgnoreMissingProvenance(t *testing.T, sb integration.Sandbox) { func testCommandSourceMapping(t *testing.T, sb integration.Sandbox) { integration.SkipOnPlatform(t, "windows") + workers.CheckFeatureCompat(t, sb, workers.FeatureDirectPush, workers.FeatureProvenance) ctx := sb.Context() c, err := client.New(ctx, sb.Address()) From 38e6fcdedee9004ef089c5a2542c22674379974a Mon Sep 17 00:00:00 2001 From: Anthony Nandaa Date: Mon, 22 Apr 2024 15:15:09 +0300 Subject: [PATCH 3/6] fix: gc policy for windows to use percentage of disk space Initially we had the GC Policy for Windows use only 2 GB (2e9 bytes) of disk space and this was limiting for some build scenarios that need more than that, especially ServerCore images. This commit makes the policy to use percentages as it is on Linux. Also going for 20%, double that of Linux, since Windows images tend to be larger. fixes #4858 docker/buildx#2411 Also, refactors the diskSize logic to simplify it by bringing the `d.AsByte` function back from platform specific files to `gcpolicy.go`. Signed-off-by: Anthony Nandaa (cherry picked from commit a5f9e42d7cee9ba9ddf035a5f92838d310998d35) --- cmd/buildkitd/config/gcpolicy.go | 23 +++++++++++++++++++ cmd/buildkitd/config/gcpolicy_unix.go | 18 ++++----------- cmd/buildkitd/config/gcpolicy_windows.go | 29 ++++++++++++++++++++---- 3 files changed, 51 insertions(+), 19 deletions(-) diff --git a/cmd/buildkitd/config/gcpolicy.go b/cmd/buildkitd/config/gcpolicy.go index 4078cc6d5910..5e9ac6ac5c02 100644 --- a/cmd/buildkitd/config/gcpolicy.go +++ b/cmd/buildkitd/config/gcpolicy.go @@ -7,6 +7,7 @@ import ( "time" "github.com/docker/go-units" + "github.com/moby/buildkit/util/bklog" "github.com/pkg/errors" ) @@ -104,3 +105,25 @@ func stripQuotes(s string) string { } return s } + +func DetectDefaultGCCap() DiskSpace { + return DiskSpace{Percentage: DiskSpacePercentage} +} + +func (d DiskSpace) AsBytes(root string) int64 { + if d.Bytes != 0 { + return d.Bytes + } + if d.Percentage == 0 { + return 0 + } + + diskSize, err := getDiskSize(root) + if err != nil { + bklog.L.Warnf("failed to get disk size: %v", err) + return defaultCap + } + avail := diskSize * d.Percentage / 100 + rounded := (avail/(1<<30) + 1) * 1e9 // round up + return rounded +} diff --git a/cmd/buildkitd/config/gcpolicy_unix.go b/cmd/buildkitd/config/gcpolicy_unix.go index 232a9ac336cf..a66bec6d1d2f 100644 --- a/cmd/buildkitd/config/gcpolicy_unix.go +++ b/cmd/buildkitd/config/gcpolicy_unix.go @@ -7,23 +7,13 @@ import ( "syscall" ) -func DetectDefaultGCCap() DiskSpace { - return DiskSpace{Percentage: 10} -} - -func (d DiskSpace) AsBytes(root string) int64 { - if d.Bytes != 0 { - return d.Bytes - } - if d.Percentage == 0 { - return 0 - } +var DiskSpacePercentage int64 = 10 +func getDiskSize(root string) (int64, error) { var st syscall.Statfs_t if err := syscall.Statfs(root, &st); err != nil { - return defaultCap + return 0, err } diskSize := int64(st.Bsize) * int64(st.Blocks) - avail := diskSize * d.Percentage / 100 - return (avail/(1<<30) + 1) * 1e9 // round up + return diskSize, nil } diff --git a/cmd/buildkitd/config/gcpolicy_windows.go b/cmd/buildkitd/config/gcpolicy_windows.go index 55ce4dd77278..77c7099de5f8 100644 --- a/cmd/buildkitd/config/gcpolicy_windows.go +++ b/cmd/buildkitd/config/gcpolicy_windows.go @@ -3,10 +3,29 @@ package config -func DetectDefaultGCCap() DiskSpace { - return DiskSpace{Bytes: defaultCap} -} +import ( + "golang.org/x/sys/windows" +) + +// set as double that for Linux since +// Windows images are generally larger. +var DiskSpacePercentage int64 = 20 + +func getDiskSize(root string) (int64, error) { + rootUTF16, err := windows.UTF16FromString(root) + if err != nil { + return 0, err + } + var freeAvailableBytes uint64 + var totalBytes uint64 + var totalFreeBytes uint64 -func (d DiskSpace) AsBytes(root string) int64 { - return d.Bytes + if err := windows.GetDiskFreeSpaceEx( + &rootUTF16[0], + &freeAvailableBytes, + &totalBytes, + &totalFreeBytes); err != nil { + return 0, err + } + return int64(totalBytes), nil } From a0efecf1ec329554a3d2630b652dc00167f080ff Mon Sep 17 00:00:00 2001 From: Anthony Nandaa Date: Mon, 22 Apr 2024 11:52:42 +0300 Subject: [PATCH 4/6] vendor: github.com/tonistiigi/fsutil @ 497d33b Full diff: https://github.com/tonistiigi/fsutil/compare/7525a1af2bb5..497d33b Summary changes: - https://github.com/tonistiigi/fsutil/pull/195 receive: ensure callback errors are propagated - https://github.com/tonistiigi/fsutil/pull/196 Fix file transfers from windows to linux fixes #4741 - https://github.com/tonistiigi/fsutil/pull/197 recv: translate linkname to wire format Signed-off-by: Anthony Nandaa (cherry picked from commit 64ea9da1c4c948558fc4abe377c042285e6bbf7b) --- go.mod | 2 +- go.sum | 4 +- .../tonistiigi/fsutil/diff_containerd.go | 2 +- .../tonistiigi/fsutil/diskwriter.go | 20 +++++---- .../github.com/tonistiigi/fsutil/receive.go | 45 ++++++++++++++++++- vendor/github.com/tonistiigi/fsutil/send.go | 1 + vendor/modules.txt | 2 +- 7 files changed, 61 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index 5d3bef888f1d..84bd13c9efd7 100644 --- a/go.mod +++ b/go.mod @@ -67,7 +67,7 @@ require ( github.com/sirupsen/logrus v1.9.3 github.com/spdx/tools-golang v0.5.3 github.com/stretchr/testify v1.8.4 - github.com/tonistiigi/fsutil v0.0.0-20240301111122-7525a1af2bb5 + github.com/tonistiigi/fsutil v0.0.0-20240418180507-497d33b008ef github.com/tonistiigi/go-actions-cache v0.0.0-20240227172821-a0b64f338598 github.com/tonistiigi/go-archvariant v1.0.0 github.com/tonistiigi/units v0.0.0-20180711220420-6950e57a87ea diff --git a/go.sum b/go.sum index 20ef32d21f12..2285937ff6d3 100644 --- a/go.sum +++ b/go.sum @@ -405,8 +405,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -github.com/tonistiigi/fsutil v0.0.0-20240301111122-7525a1af2bb5 h1:oZS8KCqAg62sxJkEq/Ppzqrb6EooqzWtL8Oaex7bc5c= -github.com/tonistiigi/fsutil v0.0.0-20240301111122-7525a1af2bb5/go.mod h1:vbbYqJlnswsbJqWUcJN8fKtBhnEgldDrcagTgnBVKKM= +github.com/tonistiigi/fsutil v0.0.0-20240418180507-497d33b008ef h1:1rshiFn5ka7/H9oGYXvRnV1BzhtWls2WSQZDrNwVsCA= +github.com/tonistiigi/fsutil v0.0.0-20240418180507-497d33b008ef/go.mod h1:vbbYqJlnswsbJqWUcJN8fKtBhnEgldDrcagTgnBVKKM= github.com/tonistiigi/go-actions-cache v0.0.0-20240227172821-a0b64f338598 h1:DA/NDC0YbMdnfcOSUzAnbUZE6dSM54d+0hrBqG+bOfs= github.com/tonistiigi/go-actions-cache v0.0.0-20240227172821-a0b64f338598/go.mod h1:anhKd3mnC1shAbQj1Q4IJ+w6xqezxnyDYlx/yKa7IXM= github.com/tonistiigi/go-archvariant v1.0.0 h1:5LC1eDWiBNflnTF1prCiX09yfNHIxDC/aukdhCdTyb0= diff --git a/vendor/github.com/tonistiigi/fsutil/diff_containerd.go b/vendor/github.com/tonistiigi/fsutil/diff_containerd.go index 84fdc89dc5bf..86d64602737b 100644 --- a/vendor/github.com/tonistiigi/fsutil/diff_containerd.go +++ b/vendor/github.com/tonistiigi/fsutil/diff_containerd.go @@ -111,7 +111,7 @@ func doubleWalkDiff(ctx context.Context, changeFn ChangeFunc, a, b walkerFn, fil if filter != nil { filter(f2.path, &statCopy) } - f2copy = ¤tPath{path: filepath.FromSlash(f2.path), stat: &statCopy} + f2copy = ¤tPath{path: f2.path, stat: &statCopy} } k, p := pathChange(f1, f2copy) switch k { diff --git a/vendor/github.com/tonistiigi/fsutil/diskwriter.go b/vendor/github.com/tonistiigi/fsutil/diskwriter.go index 10b60851381b..a62c6b0c917b 100644 --- a/vendor/github.com/tonistiigi/fsutil/diskwriter.go +++ b/vendor/github.com/tonistiigi/fsutil/diskwriter.go @@ -37,6 +37,7 @@ type DiskWriter struct { ctx context.Context cancel func() eg *errgroup.Group + egCtx context.Context filter FilterFunc dirModTimes map[string]int64 } @@ -50,13 +51,14 @@ func NewDiskWriter(ctx context.Context, dest string, opt DiskWriterOpt) (*DiskWr } ctx, cancel := context.WithCancel(ctx) - eg, ctx := errgroup.WithContext(ctx) + eg, egCtx := errgroup.WithContext(ctx) return &DiskWriter{ opt: opt, dest: dest, eg: eg, ctx: ctx, + egCtx: egCtx, cancel: cancel, filter: opt.Filter, dirModTimes: map[string]int64{}, @@ -98,7 +100,7 @@ func (dw *DiskWriter) HandleChange(kind ChangeKind, p string, fi os.FileInfo, er } }() - destPath := filepath.Join(dw.dest, filepath.FromSlash(p)) + destPath := filepath.Join(dw.dest, p) if kind == ChangeKindDelete { if dw.filter != nil { @@ -183,12 +185,12 @@ func (dw *DiskWriter) HandleChange(kind ChangeKind, p string, fi os.FileInfo, er } default: isRegularFile = true - file, err := os.OpenFile(newPath, os.O_CREATE|os.O_WRONLY, fi.Mode()) //todo: windows + file, err := os.OpenFile(newPath, os.O_CREATE|os.O_WRONLY, fi.Mode()) if err != nil { return errors.Wrapf(err, "failed to create %s", newPath) } if dw.opt.SyncDataCb != nil { - if err := dw.processChange(ChangeKindAdd, p, fi, file); err != nil { + if err := dw.processChange(dw.ctx, ChangeKindAdd, p, fi, file); err != nil { file.Close() return err } @@ -219,7 +221,7 @@ func (dw *DiskWriter) HandleChange(kind ChangeKind, p string, fi os.FileInfo, er dw.requestAsyncFileData(p, destPath, fi, &statCopy) } } else { - return dw.processChange(kind, p, fi, nil) + return dw.processChange(dw.ctx, kind, p, fi, nil) } return nil @@ -228,7 +230,7 @@ func (dw *DiskWriter) HandleChange(kind ChangeKind, p string, fi os.FileInfo, er func (dw *DiskWriter) requestAsyncFileData(p, dest string, fi os.FileInfo, st *types.Stat) { // todo: limit worker threads dw.eg.Go(func() error { - if err := dw.processChange(ChangeKindAdd, p, fi, &lazyFileWriter{ + if err := dw.processChange(dw.egCtx, ChangeKindAdd, p, fi, &lazyFileWriter{ dest: dest, }); err != nil { return err @@ -237,7 +239,7 @@ func (dw *DiskWriter) requestAsyncFileData(p, dest string, fi os.FileInfo, st *t }) } -func (dw *DiskWriter) processChange(kind ChangeKind, p string, fi os.FileInfo, w io.WriteCloser) error { +func (dw *DiskWriter) processChange(ctx context.Context, kind ChangeKind, p string, fi os.FileInfo, w io.WriteCloser) error { origw := w var hw *hashedWriter if dw.opt.NotifyCb != nil { @@ -252,7 +254,7 @@ func (dw *DiskWriter) processChange(kind ChangeKind, p string, fi os.FileInfo, w if fn == nil && dw.opt.AsyncDataCb != nil { fn = dw.opt.AsyncDataCb } - if err := fn(dw.ctx, p, w); err != nil { + if err := fn(ctx, p, w); err != nil { return err } } else { @@ -313,7 +315,7 @@ type lazyFileWriter struct { func (lfw *lazyFileWriter) Write(dt []byte) (int, error) { if lfw.f == nil { - file, err := os.OpenFile(lfw.dest, os.O_WRONLY, 0) //todo: windows + file, err := os.OpenFile(lfw.dest, os.O_WRONLY, 0) if os.IsPermission(err) { // retry after chmod fi, er := os.Stat(lfw.dest) diff --git a/vendor/github.com/tonistiigi/fsutil/receive.go b/vendor/github.com/tonistiigi/fsutil/receive.go index 209d1d2fafa2..6a82d205b19f 100644 --- a/vendor/github.com/tonistiigi/fsutil/receive.go +++ b/vendor/github.com/tonistiigi/fsutil/receive.go @@ -1,10 +1,42 @@ +// send.go and receive.go describe the fsutil file-transfer protocol, which +// allows transferring file trees across a network connection. +// +// The protocol operates as follows: +// - The client (the receiver) connects to the server (the sender). +// - The sender walks the target tree lexicographically and sends a series of +// STAT packets that describe each file (an empty stat indicates EOF). +// - The receiver sends a REQ packet for each file it requires the contents for, +// using the ID for the file (determined as its index in the STAT sequence). +// - The sender sends a DATA packet with byte arrays for the contents of the +// file, associated with an ID (an empty array indicates EOF). +// - Once the receiver has received all files it wants, it sends a FIN packet, +// and the file transfer is complete. +// If an error is encountered on either side, an ERR packet is sent containing +// a human-readable error. +// +// All paths transferred over the protocol are normalized to unix-style paths, +// regardless of which platforms are present on either side. These path +// conversions are performed right before sending a STAT packet (for the +// sender) or right after receiving the corresponding STAT packet (for the +// receiver); this abstraction doesn't leak into the rest of fsutil, which +// operates on native platform-specific paths. +// +// Note that in the case of cross-platform file transfers, the transfer is +// best-effort. Some filenames that are valid on a unix sender would not be +// valid on a windows receiver, so these paths are rejected as they are +// received. Additionally, file metadata, like user/group owners and xattrs do +// not have an exact correspondence on windows, and so would be discarded by +// a windows receiver. + package fsutil import ( "context" "io" "os" + "path/filepath" "sync" + "syscall" "github.com/pkg/errors" "github.com/tonistiigi/fsutil/types" @@ -184,13 +216,24 @@ func (r *receiver) run(ctx context.Context) error { } break } + + // normalize unix wire-specific paths to platform-specific paths + path := filepath.FromSlash(p.Stat.Path) + if filepath.ToSlash(path) != p.Stat.Path { + // e.g. a linux path foo/bar\baz cannot be represented on windows + return errors.WithStack(&os.PathError{Path: p.Stat.Path, Err: syscall.EINVAL, Op: "unrepresentable path"}) + } + p.Stat.Path = path + p.Stat.Linkname = filepath.FromSlash(p.Stat.Linkname) + if fileCanRequestData(os.FileMode(p.Stat.Mode)) { r.mu.Lock() r.files[p.Stat.Path] = i r.mu.Unlock() } i++ - cp := ¤tPath{path: p.Stat.Path, stat: p.Stat} + + cp := ¤tPath{path: path, stat: p.Stat} if err := r.orderValidator.HandleChange(ChangeKindAdd, cp.path, &StatInfo{cp.stat}, nil); err != nil { return err } diff --git a/vendor/github.com/tonistiigi/fsutil/send.go b/vendor/github.com/tonistiigi/fsutil/send.go index ba97ef7ad0ed..43bf1717609a 100644 --- a/vendor/github.com/tonistiigi/fsutil/send.go +++ b/vendor/github.com/tonistiigi/fsutil/send.go @@ -161,6 +161,7 @@ func (s *sender) walk(ctx context.Context) error { return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"}) } stat.Path = filepath.ToSlash(stat.Path) + stat.Linkname = filepath.ToSlash(stat.Linkname) p := &types.Packet{ Type: types.PACKET_STAT, Stat: stat, diff --git a/vendor/modules.txt b/vendor/modules.txt index 5f390917fd7c..b39a813546f6 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -765,7 +765,7 @@ github.com/spdx/tools-golang/spdx/v2/v2_3 ## explicit; go 1.20 github.com/stretchr/testify/assert github.com/stretchr/testify/require -# github.com/tonistiigi/fsutil v0.0.0-20240301111122-7525a1af2bb5 +# github.com/tonistiigi/fsutil v0.0.0-20240418180507-497d33b008ef ## explicit; go 1.20 github.com/tonistiigi/fsutil github.com/tonistiigi/fsutil/copy From 4d961cad1abd60911186014c9b7d8dd9301021e8 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Tue, 23 Apr 2024 21:55:34 -0700 Subject: [PATCH 5/6] vendor: update fsutil to 91a3fc4 Brings in fix for hardlink filters. Signed-off-by: Tonis Tiigi (cherry picked from commit f67b96caa390975481b62beea0639a221b4313db) --- client/client_test.go | 52 ++++++++++++++ go.mod | 2 +- go.sum | 4 +- .../github.com/tonistiigi/fsutil/hardlinks.go | 68 +++++++++++++++++++ vendor/github.com/tonistiigi/fsutil/send.go | 2 +- vendor/modules.txt | 2 +- 6 files changed, 125 insertions(+), 5 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index deaf8336835f..4280206f673f 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -165,6 +165,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){ testFileOpInputSwap, testRelativeMountpoint, testLocalSourceDiffer, + testLocalSourceWithHardlinksFilter, testOCILayoutSource, testOCILayoutPlatformSource, testBuildExportZstd, @@ -1964,6 +1965,57 @@ func testLocalSourceWithDiffer(t *testing.T, sb integration.Sandbox, d llb.DiffT } } +// moby/buildkit#4831 +func testLocalSourceWithHardlinksFilter(t *testing.T, sb integration.Sandbox) { + requiresLinux(t) + c, err := New(context.TODO(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + dir := integration.Tmpdir( + t, + fstest.CreateFile("bar", []byte("bar"), 0600), + fstest.Link("bar", "foo1"), + fstest.Link("bar", "foo2"), + ) + + st := llb.Local("mylocal", llb.FollowPaths([]string{"foo*"})) + + def, err := st.Marshal(context.TODO()) + require.NoError(t, err) + + destDir := t.TempDir() + + _, err = c.Solve(context.TODO(), def, SolveOpt{ + Exports: []ExportEntry{ + { + Type: ExporterLocal, + OutputDir: destDir, + }, + }, + LocalMounts: map[string]fsutil.FS{ + "mylocal": dir, + }, + }, nil) + require.NoError(t, err) + + _, err = os.ReadFile(filepath.Join(destDir, "bar")) + require.Error(t, err) + require.True(t, os.IsNotExist(err)) + + dt, err := os.ReadFile(filepath.Join(destDir, "foo1")) + require.NoError(t, err) + require.Equal(t, []byte("bar"), dt) + + st1, err := os.Stat(filepath.Join(destDir, "foo1")) + require.NoError(t, err) + + st2, err := os.Stat(filepath.Join(destDir, "foo2")) + require.NoError(t, err) + + require.True(t, os.SameFile(st1, st2)) +} + func testOCILayoutSource(t *testing.T, sb integration.Sandbox) { workers.CheckFeatureCompat(t, sb, workers.FeatureOCIExporter, workers.FeatureOCILayout) requiresLinux(t) diff --git a/go.mod b/go.mod index 84bd13c9efd7..8c2216d1fb54 100644 --- a/go.mod +++ b/go.mod @@ -67,7 +67,7 @@ require ( github.com/sirupsen/logrus v1.9.3 github.com/spdx/tools-golang v0.5.3 github.com/stretchr/testify v1.8.4 - github.com/tonistiigi/fsutil v0.0.0-20240418180507-497d33b008ef + github.com/tonistiigi/fsutil v0.0.0-20240424095704-91a3fc46842c github.com/tonistiigi/go-actions-cache v0.0.0-20240227172821-a0b64f338598 github.com/tonistiigi/go-archvariant v1.0.0 github.com/tonistiigi/units v0.0.0-20180711220420-6950e57a87ea diff --git a/go.sum b/go.sum index 2285937ff6d3..6017f2e16b84 100644 --- a/go.sum +++ b/go.sum @@ -405,8 +405,8 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -github.com/tonistiigi/fsutil v0.0.0-20240418180507-497d33b008ef h1:1rshiFn5ka7/H9oGYXvRnV1BzhtWls2WSQZDrNwVsCA= -github.com/tonistiigi/fsutil v0.0.0-20240418180507-497d33b008ef/go.mod h1:vbbYqJlnswsbJqWUcJN8fKtBhnEgldDrcagTgnBVKKM= +github.com/tonistiigi/fsutil v0.0.0-20240424095704-91a3fc46842c h1:+6wg/4ORAbnSoGDzg2Q1i3CeMcT/jjhye/ZfnBHy7/M= +github.com/tonistiigi/fsutil v0.0.0-20240424095704-91a3fc46842c/go.mod h1:vbbYqJlnswsbJqWUcJN8fKtBhnEgldDrcagTgnBVKKM= github.com/tonistiigi/go-actions-cache v0.0.0-20240227172821-a0b64f338598 h1:DA/NDC0YbMdnfcOSUzAnbUZE6dSM54d+0hrBqG+bOfs= github.com/tonistiigi/go-actions-cache v0.0.0-20240227172821-a0b64f338598/go.mod h1:anhKd3mnC1shAbQj1Q4IJ+w6xqezxnyDYlx/yKa7IXM= github.com/tonistiigi/go-archvariant v1.0.0 h1:5LC1eDWiBNflnTF1prCiX09yfNHIxDC/aukdhCdTyb0= diff --git a/vendor/github.com/tonistiigi/fsutil/hardlinks.go b/vendor/github.com/tonistiigi/fsutil/hardlinks.go index ef8bbfb5daff..d9bf2fc1c0ca 100644 --- a/vendor/github.com/tonistiigi/fsutil/hardlinks.go +++ b/vendor/github.com/tonistiigi/fsutil/hardlinks.go @@ -1,6 +1,9 @@ package fsutil import ( + "context" + "io" + gofs "io/fs" "os" "syscall" @@ -46,3 +49,68 @@ func (v *Hardlinks) HandleChange(kind ChangeKind, p string, fi os.FileInfo, err return nil } + +// WithHardlinkReset returns a FS that fixes hardlinks for FS that has been filtered +// so that original hardlink sources might be missing +func WithHardlinkReset(fs FS) FS { + return &hardlinkFilter{fs: fs} +} + +type hardlinkFilter struct { + fs FS +} + +var _ FS = &hardlinkFilter{} + +func (r *hardlinkFilter) Walk(ctx context.Context, target string, fn gofs.WalkDirFunc) error { + seenFiles := make(map[string]string) + return r.fs.Walk(ctx, target, func(path string, entry gofs.DirEntry, err error) error { + if err != nil { + return err + } + + fi, err := entry.Info() + if err != nil { + return err + } + + if fi.IsDir() || fi.Mode()&os.ModeSymlink != 0 { + return fn(path, entry, nil) + } + + stat, ok := fi.Sys().(*types.Stat) + if !ok { + return errors.WithStack(&os.PathError{Path: path, Err: syscall.EBADMSG, Op: "fileinfo without stat info"}) + } + + if stat.Linkname != "" { + if v, ok := seenFiles[stat.Linkname]; !ok { + seenFiles[stat.Linkname] = stat.Path + stat.Linkname = "" + entry = &dirEntryWithStat{DirEntry: entry, stat: stat} + } else { + if v != stat.Path { + stat.Linkname = v + entry = &dirEntryWithStat{DirEntry: entry, stat: stat} + } + } + } + + seenFiles[path] = stat.Path + + return fn(path, entry, nil) + }) +} + +func (r *hardlinkFilter) Open(p string) (io.ReadCloser, error) { + return r.fs.Open(p) +} + +type dirEntryWithStat struct { + gofs.DirEntry + stat *types.Stat +} + +func (d *dirEntryWithStat) Info() (gofs.FileInfo, error) { + return &StatInfo{d.stat}, nil +} diff --git a/vendor/github.com/tonistiigi/fsutil/send.go b/vendor/github.com/tonistiigi/fsutil/send.go index 43bf1717609a..e4a315638bab 100644 --- a/vendor/github.com/tonistiigi/fsutil/send.go +++ b/vendor/github.com/tonistiigi/fsutil/send.go @@ -29,7 +29,7 @@ type Stream interface { func Send(ctx context.Context, conn Stream, fs FS, progressCb func(int, bool)) error { s := &sender{ conn: &syncStream{Stream: conn}, - fs: fs, + fs: WithHardlinkReset(fs), files: make(map[uint32]string), progressCb: progressCb, sendpipeline: make(chan *sendHandle, 128), diff --git a/vendor/modules.txt b/vendor/modules.txt index b39a813546f6..83601842aa58 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -765,7 +765,7 @@ github.com/spdx/tools-golang/spdx/v2/v2_3 ## explicit; go 1.20 github.com/stretchr/testify/assert github.com/stretchr/testify/require -# github.com/tonistiigi/fsutil v0.0.0-20240418180507-497d33b008ef +# github.com/tonistiigi/fsutil v0.0.0-20240424095704-91a3fc46842c ## explicit; go 1.20 github.com/tonistiigi/fsutil github.com/tonistiigi/fsutil/copy From 25108abda49ab2fde54c0f874e43e3dba88fb867 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 24 Apr 2024 11:50:02 -0700 Subject: [PATCH 6/6] buildkitd: allow --group for windows Signed-off-by: Tonis Tiigi (cherry picked from commit 68671bc9f9c7b625849f100482bb48cba563f80c) --- cmd/buildkitd/config/config.go | 9 +++++---- cmd/buildkitd/main.go | 35 +++++++++++++++++++++++++++------- cmd/buildkitd/main_unix.go | 6 +++++- cmd/buildkitd/main_windows.go | 26 ++++++++++++++++++++++--- 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/cmd/buildkitd/config/config.go b/cmd/buildkitd/config/config.go index 9af4156f68de..58dd2f8a047b 100644 --- a/cmd/buildkitd/config/config.go +++ b/cmd/buildkitd/config/config.go @@ -40,10 +40,11 @@ type LogConfig struct { } type GRPCConfig struct { - Address []string `toml:"address"` - DebugAddress string `toml:"debugAddress"` - UID *int `toml:"uid"` - GID *int `toml:"gid"` + Address []string `toml:"address"` + DebugAddress string `toml:"debugAddress"` + UID *int `toml:"uid"` + GID *int `toml:"gid"` + SecurityDescriptor string `toml:"securityDescriptor"` TLS TLSConfig `toml:"tls"` // MaxRecvMsgSize int `toml:"max_recv_message_size"` diff --git a/cmd/buildkitd/main.go b/cmd/buildkitd/main.go index 3a3ff16f2b1e..a62e913bd295 100644 --- a/cmd/buildkitd/main.go +++ b/cmd/buildkitd/main.go @@ -9,6 +9,7 @@ import ( "os" "os/user" "path/filepath" + "runtime" "sort" "strconv" "strings" @@ -397,9 +398,18 @@ func newGRPCListeners(cfg config.GRPCConfig) ([]net.Listener, error) { if err != nil { return nil, err } + + sd := cfg.SecurityDescriptor + if sd == "" { + sd, err = groupToSecurityDescriptor("") + if err != nil { + return nil, err + } + } + listeners := make([]net.Listener, 0, len(addrs)) for _, addr := range addrs { - l, err := getListener(addr, *cfg.UID, *cfg.GID, tlsConfig) + l, err := getListener(addr, *cfg.UID, *cfg.GID, sd, tlsConfig) if err != nil { for _, l := range listeners { l.Close() @@ -569,11 +579,19 @@ func applyMainFlags(c *cli.Context, cfg *config.Config) error { } if group := c.String("group"); group != "" { - gid, err := groupToGid(group) - if err != nil { - return err + if runtime.GOOS == "windows" { + secDescriptor, err := groupToSecurityDescriptor(group) + if err != nil { + return err + } + cfg.GRPC.SecurityDescriptor = secDescriptor + } else { + gid, err := groupToGid(group) + if err != nil { + return err + } + cfg.GRPC.GID = &gid } - cfg.GRPC.GID = &gid } if tlscert := c.String("tlscert"); tlscert != "" { @@ -628,7 +646,7 @@ func groupToGid(group string) (int, error) { return id, nil } -func getListener(addr string, uid, gid int, tlsConfig *tls.Config) (net.Listener, error) { +func getListener(addr string, uid, gid int, secDescriptor string, tlsConfig *tls.Config) (net.Listener, error) { addrSlice := strings.SplitN(addr, "://", 2) if len(addrSlice) < 2 { return nil, errors.Errorf("address %s does not contain proto, you meant unix://%s ?", @@ -641,6 +659,9 @@ func getListener(addr string, uid, gid int, tlsConfig *tls.Config) (net.Listener if tlsConfig != nil { bklog.L.Warnf("TLS is disabled for %s", addr) } + if proto == "npipe" { + return getLocalListener(listenAddr, secDescriptor) + } return sys.GetLocalListener(listenAddr, uid, gid) case "fd": return listenFD(listenAddr, tlsConfig) @@ -928,7 +949,7 @@ func parseBoolOrAuto(s string) (*bool, error) { func runTraceController(p string, exp sdktrace.SpanExporter) error { server := grpc.NewServer() tracev1.RegisterTraceServiceServer(server, &traceCollector{exporter: exp}) - l, err := getLocalListener(p) + l, err := getLocalListener(p, "") if err != nil { return errors.Wrap(err, "creating trace controller listener") } diff --git a/cmd/buildkitd/main_unix.go b/cmd/buildkitd/main_unix.go index d819d1187f59..1dc3a2ed35c9 100644 --- a/cmd/buildkitd/main_unix.go +++ b/cmd/buildkitd/main_unix.go @@ -48,7 +48,7 @@ func listenFD(addr string, tlsConfig *tls.Config) (net.Listener, error) { return nil, errors.New("not supported yet") } -func getLocalListener(listenerPath string) (net.Listener, error) { +func getLocalListener(listenerPath, _ string) (net.Listener, error) { uid := os.Getuid() l, err := sys.GetLocalListener(listenerPath, uid, uid) if err != nil { @@ -60,3 +60,7 @@ func getLocalListener(listenerPath string) (net.Listener, error) { } return l, nil } + +func groupToSecurityDescriptor(_ string) (string, error) { + return "", nil +} diff --git a/cmd/buildkitd/main_windows.go b/cmd/buildkitd/main_windows.go index 5ca20a689534..fdde4a821e5b 100644 --- a/cmd/buildkitd/main_windows.go +++ b/cmd/buildkitd/main_windows.go @@ -5,7 +5,9 @@ package main import ( "crypto/tls" + "fmt" "net" + "strings" "github.com/Microsoft/go-winio" _ "github.com/moby/buildkit/solver/llbsolver/ops" @@ -19,14 +21,18 @@ func listenFD(addr string, tlsConfig *tls.Config) (net.Listener, error) { return nil, errors.New("listening server on fd not supported on windows") } -func getLocalListener(listenerPath string) (net.Listener, error) { - pc := &winio.PipeConfig{ +func getLocalListener(listenerPath, secDescriptor string) (net.Listener, error) { + if secDescriptor == "" { // Allow generic read and generic write access to authenticated users // and system users. On Linux, this pipe seems to be given rw access to // user, group and others (666). // TODO(gabriel-samfira): should we restrict access to this pipe to just // authenticated users? Or Administrators group? - SecurityDescriptor: "D:P(A;;GRGW;;;AU)(A;;GRGW;;;SY)", + secDescriptor = "D:P(A;;GRGW;;;AU)(A;;GRGW;;;SY)" + } + + pc := &winio.PipeConfig{ + SecurityDescriptor: secDescriptor, } listener, err := winio.ListenPipe(listenerPath, pc) @@ -35,3 +41,17 @@ func getLocalListener(listenerPath string) (net.Listener, error) { } return listener, nil } + +func groupToSecurityDescriptor(group string) (string, error) { + sddl := "D:P(A;;GA;;;BA)(A;;GA;;;SY)" + if group != "" { + for _, g := range strings.Split(group, ",") { + sid, err := winio.LookupSidByName(g) + if err != nil { + return "", errors.Wrapf(err, "failed to lookup sid for group %s", g) + } + sddl += fmt.Sprintf("(A;;GRGW;;;%s)", sid) + } + } + return sddl, nil +}