From 11a597ab648ed4fe6ae628d1ebe3608e17f85c86 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 18 Sep 2023 09:51:19 +0100 Subject: [PATCH 1/6] chore: remove fmt.Errorf usage for windows Signed-off-by: Justin Chadwell --- cmd/buildkitd/service_windows.go | 7 +++---- util/system/atime_windows.go | 5 +++-- util/windows/util_windows.go | 5 ++--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/cmd/buildkitd/service_windows.go b/cmd/buildkitd/service_windows.go index 40221f670571..40315ec9aa1a 100644 --- a/cmd/buildkitd/service_windows.go +++ b/cmd/buildkitd/service_windows.go @@ -1,13 +1,13 @@ package main import ( - "fmt" "log" "os" "path/filepath" "time" "unsafe" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/urfave/cli" "golang.org/x/sys/windows" @@ -181,7 +181,7 @@ type handler struct { func registerUnregisterService(root string) (bool, error) { if unregisterServiceFlag { if registerServiceFlag { - return true, fmt.Errorf("--register-service and --unregister-service cannot be used together") + return true, errors.Errorf("--register-service and --unregister-service cannot be used together") } return true, unregisterService() } @@ -224,7 +224,7 @@ func registerUnregisterService(root string) (bool, error) { if logFileFlag != "" { f, err = os.OpenFile(logFileFlag, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) if err != nil { - return true, fmt.Errorf("open log file %q: %w", logFileFlag, err) + return true, errors.Wrapf(err, "open log file %q", logFileFlag) } } else { // Windows services start with NULL stdio handles, and thus os.Stderr and friends will be @@ -245,7 +245,6 @@ func registerUnregisterService(root string) (bool, error) { // dependencies. log.SetOutput(f) logrus.SetOutput(f) - } return false, nil } diff --git a/util/system/atime_windows.go b/util/system/atime_windows.go index 808408b613cf..cd5190fceb4c 100644 --- a/util/system/atime_windows.go +++ b/util/system/atime_windows.go @@ -1,16 +1,17 @@ package system import ( - "fmt" iofs "io/fs" "syscall" "time" + + "github.com/pkg/errors" ) func Atime(st iofs.FileInfo) (time.Time, error) { stSys, ok := st.Sys().(*syscall.Win32FileAttributeData) if !ok { - return time.Time{}, fmt.Errorf("expected st.Sys() to be *syscall.Win32FileAttributeData, got %T", st.Sys()) + return time.Time{}, errors.Errorf("expected st.Sys() to be *syscall.Win32FileAttributeData, got %T", st.Sys()) } // ref: https://github.com/golang/go/blob/go1.19.2/src/os/types_windows.go#L230 return time.Unix(0, stSys.LastAccessTime.Nanoseconds()), nil diff --git a/util/windows/util_windows.go b/util/windows/util_windows.go index 87b1136e0f95..135968a6fae4 100644 --- a/util/windows/util_windows.go +++ b/util/windows/util_windows.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "fmt" "strings" "syscall" @@ -76,7 +75,7 @@ func GetUserIdentFromContainer(ctx context.Context, exec executor.Executor, root var ident idtools.Identity if len(rootMounts) > 1 { - return ident, fmt.Errorf("unexpected number of root mounts: %d", len(rootMounts)) + return ident, errors.Errorf("unexpected number of root mounts: %d", len(rootMounts)) } stdout := &bytesReadWriteCloser{ @@ -118,7 +117,7 @@ type bytesReadWriteCloser struct { func (b *bytesReadWriteCloser) Write(p []byte) (int, error) { if b.bw == nil { - return 0, fmt.Errorf("invalid bytes buffer") + return 0, errors.Errorf("invalid bytes buffer") } return b.bw.Write(p) } From a9b4dd5090344391f308634a401ffc125b559b30 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 18 Sep 2023 09:52:00 +0100 Subject: [PATCH 2/6] chore: fix windows variable naming issues Signed-off-by: Justin Chadwell --- executor/runcexecutor/executor_common.go | 6 +++--- util/system/getuserinfo/userinfo_windows.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/executor/runcexecutor/executor_common.go b/executor/runcexecutor/executor_common.go index 28955f9a4596..aa67e1e3f216 100644 --- a/executor/runcexecutor/executor_common.go +++ b/executor/runcexecutor/executor_common.go @@ -14,13 +14,13 @@ import ( "golang.org/x/sync/errgroup" ) -var unsupportedConsoleError = errors.New("tty for runc is only supported on linux") +var errUnsupportedConsole = errors.New("tty for runc is only supported on linux") func updateRuncFieldsForHostOS(runtime *runc.Runc) {} func (w *runcExecutor) run(ctx context.Context, id, bundle string, process executor.ProcessInfo, started func(), keep bool) error { if process.Meta.Tty { - return unsupportedConsoleError + return errUnsupportedConsole } extraArgs := []string{} if keep { @@ -40,7 +40,7 @@ func (w *runcExecutor) run(ctx context.Context, id, bundle string, process execu func (w *runcExecutor) exec(ctx context.Context, id, bundle string, specsProcess *specs.Process, process executor.ProcessInfo, started func()) error { if process.Meta.Tty { - return unsupportedConsoleError + return errUnsupportedConsole } killer, err := newExecProcKiller(w.runc, id) diff --git a/util/system/getuserinfo/userinfo_windows.go b/util/system/getuserinfo/userinfo_windows.go index 13da39fdc9f0..e9f3f4c1c57d 100644 --- a/util/system/getuserinfo/userinfo_windows.go +++ b/util/system/getuserinfo/userinfo_windows.go @@ -35,10 +35,10 @@ func userInfoMain() { SID: sid.String(), } - asJson, err := json.Marshal(ident) + asJSON, err := json.Marshal(ident) if err != nil { fmt.Println(err) os.Exit(5) } - fmt.Fprintf(os.Stdout, "%s", string(asJson)) + fmt.Fprintf(os.Stdout, "%s", string(asJSON)) } From de91bfd57bed31d5751fb7836bf8d953f23d3d16 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 18 Sep 2023 09:52:41 +0100 Subject: [PATCH 3/6] chore: remove unused functions for windows Signed-off-by: Justin Chadwell --- client/mergediff_nolinux_test.go | 6 ------ cmd/buildkitd/util_unsupported.go | 16 ---------------- util/archutil/check_windows.go | 4 ---- 3 files changed, 26 deletions(-) delete mode 100644 cmd/buildkitd/util_unsupported.go diff --git a/client/mergediff_nolinux_test.go b/client/mergediff_nolinux_test.go index d02bf029e68d..9686df425b8c 100644 --- a/client/mergediff_nolinux_test.go +++ b/client/mergediff_nolinux_test.go @@ -10,12 +10,6 @@ import ( "github.com/pkg/errors" ) -func mknod(path string, mode os.FileMode, maj, min uint32) fstest.Applier { - return applyFn(func(string) error { - return errors.New("mknod applier not implemented yet on this platform") - }) -} - func mkfifo(path string, mode os.FileMode) fstest.Applier { return applyFn(func(string) error { return errors.New("mkfifo applier not implemented yet on this platform") diff --git a/cmd/buildkitd/util_unsupported.go b/cmd/buildkitd/util_unsupported.go deleted file mode 100644 index a7345e9e268d..000000000000 --- a/cmd/buildkitd/util_unsupported.go +++ /dev/null @@ -1,16 +0,0 @@ -//go:build !linux -// +build !linux - -package main - -import ( - "github.com/docker/docker/pkg/idtools" - "github.com/pkg/errors" -) - -func parseIdentityMapping(str string) (*idtools.IdentityMapping, error) { - if str == "" { - return nil, nil - } - return nil, errors.New("user namespaces are only supported on linux") -} diff --git a/util/archutil/check_windows.go b/util/archutil/check_windows.go index 6a656011406c..736f2216ba31 100644 --- a/util/archutil/check_windows.go +++ b/util/archutil/check_windows.go @@ -5,12 +5,8 @@ package archutil import ( "errors" - "os/exec" ) -func withChroot(cmd *exec.Cmd, dir string) { -} - func check(arch, bin string) (string, error) { return "", errors.New("binfmt is not supported on Windows") } From 4dcbc22d318a030a73f80e028b1f1bdb500542d3 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 18 Sep 2023 09:53:17 +0100 Subject: [PATCH 4/6] chore: move linux-specific oci spec to spec_linux.go These functions are unused on windows and so cause linting issues. Signed-off-by: Justin Chadwell --- executor/oci/mounts.go | 67 -------------------------------- executor/oci/spec.go | 12 ------ executor/oci/spec_linux.go | 79 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 79 deletions(-) diff --git a/executor/oci/mounts.go b/executor/oci/mounts.go index 50ed827497db..39e1da1537d1 100644 --- a/executor/oci/mounts.go +++ b/executor/oci/mounts.go @@ -24,30 +24,6 @@ func withRemovedMount(destination string) oci.SpecOpts { } } -func withROBind(src, dest string) oci.SpecOpts { - return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { - s.Mounts = append(s.Mounts, specs.Mount{ - Destination: dest, - Type: "bind", - Source: src, - Options: []string{"nosuid", "noexec", "nodev", "rbind", "ro"}, - }) - return nil - } -} - -func withCGroup() oci.SpecOpts { - return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { - s.Mounts = append(s.Mounts, specs.Mount{ - Destination: "/sys/fs/cgroup", - Type: "cgroup", - Source: "cgroup", - Options: []string{"ro", "nosuid", "noexec", "nodev"}, - }) - return nil - } -} - func hasPrefix(p, prefixDir string) bool { prefixDir = filepath.Clean(prefixDir) if filepath.Base(prefixDir) == string(filepath.Separator) { @@ -57,49 +33,6 @@ func hasPrefix(p, prefixDir string) bool { return p == prefixDir || strings.HasPrefix(p, prefixDir+string(filepath.Separator)) } -func removeMountsWithPrefix(mounts []specs.Mount, prefixDir string) []specs.Mount { - var ret []specs.Mount - for _, m := range mounts { - if !hasPrefix(m.Destination, prefixDir) { - ret = append(ret, m) - } - } - return ret -} - -func withBoundProc() oci.SpecOpts { - return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { - s.Mounts = removeMountsWithPrefix(s.Mounts, "/proc") - procMount := specs.Mount{ - Destination: "/proc", - Type: "bind", - Source: "/proc", - // NOTE: "rbind"+"ro" does not make /proc read-only recursively. - // So we keep maskedPath and readonlyPaths (although not mandatory for rootless mode) - Options: []string{"rbind"}, - } - s.Mounts = append([]specs.Mount{procMount}, s.Mounts...) - - var maskedPaths []string - for _, s := range s.Linux.MaskedPaths { - if !hasPrefix(s, "/proc") { - maskedPaths = append(maskedPaths, s) - } - } - s.Linux.MaskedPaths = maskedPaths - - var readonlyPaths []string - for _, s := range s.Linux.ReadonlyPaths { - if !hasPrefix(s, "/proc") { - readonlyPaths = append(readonlyPaths, s) - } - } - s.Linux.ReadonlyPaths = readonlyPaths - - return nil - } -} - func dedupMounts(mnts []specs.Mount) []specs.Mount { ret := make([]specs.Mount, 0, len(mnts)) visited := make(map[string]int) diff --git a/executor/oci/spec.go b/executor/oci/spec.go index 849a70b90b5c..7028b4e18f13 100644 --- a/executor/oci/spec.go +++ b/executor/oci/spec.go @@ -302,15 +302,3 @@ func sub(m mount.Mount, subPath string) (mount.Mount, error) { m.Source = src return m, nil } - -func specMapping(s []idtools.IDMap) []specs.LinuxIDMapping { - var ids []specs.LinuxIDMapping - for _, item := range s { - ids = append(ids, specs.LinuxIDMapping{ - HostID: uint32(item.HostID), - ContainerID: uint32(item.ContainerID), - Size: uint32(item.Size), - }) - } - return ids -} diff --git a/executor/oci/spec_linux.go b/executor/oci/spec_linux.go index e86834358e94..e2fa2ec3032e 100644 --- a/executor/oci/spec_linux.go +++ b/executor/oci/spec_linux.go @@ -102,6 +102,18 @@ func generateIDmapOpts(idmap *idtools.IdentityMapping) ([]oci.SpecOpts, error) { }, nil } +func specMapping(s []idtools.IDMap) []specs.LinuxIDMapping { + var ids []specs.LinuxIDMapping + for _, item := range s { + ids = append(ids, specs.LinuxIDMapping{ + HostID: uint32(item.HostID), + ContainerID: uint32(item.ContainerID), + Size: uint32(item.Size), + }) + } + return ids +} + func generateRlimitOpts(ulimits []*pb.Ulimit) ([]oci.SpecOpts, error) { if len(ulimits) == 0 { return nil, nil @@ -135,6 +147,73 @@ func withDefaultProfile() oci.SpecOpts { } } +func withROBind(src, dest string) oci.SpecOpts { + return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { + s.Mounts = append(s.Mounts, specs.Mount{ + Destination: dest, + Type: "bind", + Source: src, + Options: []string{"nosuid", "noexec", "nodev", "rbind", "ro"}, + }) + return nil + } +} + +func withCGroup() oci.SpecOpts { + return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { + s.Mounts = append(s.Mounts, specs.Mount{ + Destination: "/sys/fs/cgroup", + Type: "cgroup", + Source: "cgroup", + Options: []string{"ro", "nosuid", "noexec", "nodev"}, + }) + return nil + } +} + +func withBoundProc() oci.SpecOpts { + return func(_ context.Context, _ oci.Client, _ *containers.Container, s *specs.Spec) error { + s.Mounts = removeMountsWithPrefix(s.Mounts, "/proc") + procMount := specs.Mount{ + Destination: "/proc", + Type: "bind", + Source: "/proc", + // NOTE: "rbind"+"ro" does not make /proc read-only recursively. + // So we keep maskedPath and readonlyPaths (although not mandatory for rootless mode) + Options: []string{"rbind"}, + } + s.Mounts = append([]specs.Mount{procMount}, s.Mounts...) + + var maskedPaths []string + for _, s := range s.Linux.MaskedPaths { + if !hasPrefix(s, "/proc") { + maskedPaths = append(maskedPaths, s) + } + } + s.Linux.MaskedPaths = maskedPaths + + var readonlyPaths []string + for _, s := range s.Linux.ReadonlyPaths { + if !hasPrefix(s, "/proc") { + readonlyPaths = append(readonlyPaths, s) + } + } + s.Linux.ReadonlyPaths = readonlyPaths + + return nil + } +} + +func removeMountsWithPrefix(mounts []specs.Mount, prefixDir string) []specs.Mount { + var ret []specs.Mount + for _, m := range mounts { + if !hasPrefix(m.Destination, prefixDir) { + ret = append(ret, m) + } + } + return ret +} + func getTracingSocketMount(socket string) *specs.Mount { return &specs.Mount{ Destination: tracingSocketPath, From beaa3cae75e54f1db7fbd1e185e2d830675a5000 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 18 Sep 2023 09:54:14 +0100 Subject: [PATCH 5/6] hack: enable linting for windows Signed-off-by: Justin Chadwell --- hack/dockerfiles/lint.Dockerfile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hack/dockerfiles/lint.Dockerfile b/hack/dockerfiles/lint.Dockerfile index 577bd2b1739b..a052c58909b5 100644 --- a/hack/dockerfiles/lint.Dockerfile +++ b/hack/dockerfiles/lint.Dockerfile @@ -18,8 +18,9 @@ WORKDIR /go/src/github.com/moby/buildkit FROM base as golangci-lint ARG BUILDTAGS RUN --mount=target=/go/src/github.com/moby/buildkit --mount=target=/root/.cache,type=cache,sharing=locked \ - GOARCH=amd64 golangci-lint run --build-tags "${BUILDTAGS}" && \ - GOARCH=arm64 golangci-lint run --build-tags "${BUILDTAGS}" && \ + GOOS=linux GOARCH=amd64 golangci-lint run --build-tags "${BUILDTAGS}" && \ + GOOS=windows GOARCH=amd64 golangci-lint run --build-tags "${BUILDTAGS}" && \ + GOOS=linux GOARCH=arm64 golangci-lint run --build-tags "${BUILDTAGS}" && \ touch /golangci-lint.done FROM base as yamllint From eb32ebfc745ab3dab204987f68bb1e6ffd8d63a9 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Thu, 2 Nov 2023 12:49:49 +0000 Subject: [PATCH 6/6] hack: enable linting for freebsd Signed-off-by: Justin Chadwell --- hack/dockerfiles/lint.Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/hack/dockerfiles/lint.Dockerfile b/hack/dockerfiles/lint.Dockerfile index a052c58909b5..0a1d99908fac 100644 --- a/hack/dockerfiles/lint.Dockerfile +++ b/hack/dockerfiles/lint.Dockerfile @@ -20,6 +20,7 @@ ARG BUILDTAGS RUN --mount=target=/go/src/github.com/moby/buildkit --mount=target=/root/.cache,type=cache,sharing=locked \ GOOS=linux GOARCH=amd64 golangci-lint run --build-tags "${BUILDTAGS}" && \ GOOS=windows GOARCH=amd64 golangci-lint run --build-tags "${BUILDTAGS}" && \ + GOOS=freebsd GOARCH=amd64 golangci-lint run --build-tags "${BUILDTAGS}" && \ GOOS=linux GOARCH=arm64 golangci-lint run --build-tags "${BUILDTAGS}" && \ touch /golangci-lint.done