From 9ee19520dd11b95dbd51d9f3ec73f1ba9ea08fa7 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 24 Apr 2023 11:14:32 +0100 Subject: [PATCH] controller: move path resolution into controller package Signed-off-by: Justin Chadwell --- commands/build.go | 159 +--------------- controller/pb/path.go | 175 ++++++++++++++++++ .../pb/path_test.go | 81 ++++---- 3 files changed, 216 insertions(+), 199 deletions(-) create mode 100644 controller/pb/path.go rename commands/build_test.go => controller/pb/path_test.go (61%) diff --git a/commands/build.go b/commands/build.go index 1a1e6a78..cf8de5d7 100644 --- a/commands/build.go +++ b/commands/build.go @@ -34,7 +34,6 @@ import ( "github.com/docker/cli/cli/command" dockeropts "github.com/docker/cli/opts" "github.com/docker/docker/api/types/versions" - "github.com/docker/docker/builder/remotecontext/urlutil" "github.com/docker/docker/pkg/ioutils" "github.com/moby/buildkit/client" "github.com/moby/buildkit/exporter/containerimage/exptypes" @@ -316,7 +315,7 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *contro // NOTE: buildx server has the current working directory different from the client // so we need to resolve paths to abosolute ones in the client. - opts, err = resolvePaths(opts) + opts, err = controllerapi.ResolveOptionPaths(opts) if err != nil { return nil, err } @@ -760,162 +759,6 @@ func dockerUlimitToControllerUlimit(u *dockeropts.UlimitOpt) *controllerapi.Ulim return &controllerapi.UlimitOpt{Values: values} } -// resolvePaths resolves all paths contained in controllerapi.BuildOptions -// and replaces them to absolute paths. -func resolvePaths(options *controllerapi.BuildOptions) (_ *controllerapi.BuildOptions, err error) { - localContext := false - if options.ContextPath != "" && options.ContextPath != "-" { - if !build.IsRemoteURL(options.ContextPath) { - localContext = true - options.ContextPath, err = filepath.Abs(options.ContextPath) - if err != nil { - return nil, err - } - } - } - if options.DockerfileName != "" && options.DockerfileName != "-" { - if localContext && !urlutil.IsURL(options.DockerfileName) { - options.DockerfileName, err = filepath.Abs(options.DockerfileName) - if err != nil { - return nil, err - } - } - } - - var contexts map[string]string - for k, v := range options.NamedContexts { - if build.IsRemoteURL(v) || strings.HasPrefix(v, "docker-image://") { - // url prefix, this is a remote path - } else if strings.HasPrefix(v, "oci-layout://") { - // oci layout prefix, this is a local path - p := strings.TrimPrefix(v, "oci-layout://") - p, err = filepath.Abs(p) - if err != nil { - return nil, err - } - v = "oci-layout://" + p - } else { - // no prefix, assume local path - v, err = filepath.Abs(v) - if err != nil { - return nil, err - } - } - - if contexts == nil { - contexts = make(map[string]string) - } - contexts[k] = v - } - options.NamedContexts = contexts - - var cacheFrom []*controllerapi.CacheOptionsEntry - for _, co := range options.CacheFrom { - switch co.Type { - case "local": - var attrs map[string]string - for k, v := range co.Attrs { - if attrs == nil { - attrs = make(map[string]string) - } - switch k { - case "src": - p := v - if p != "" { - p, err = filepath.Abs(p) - if err != nil { - return nil, err - } - } - attrs[k] = p - default: - attrs[k] = v - } - } - co.Attrs = attrs - cacheFrom = append(cacheFrom, co) - default: - cacheFrom = append(cacheFrom, co) - } - } - options.CacheFrom = cacheFrom - - var cacheTo []*controllerapi.CacheOptionsEntry - for _, co := range options.CacheTo { - switch co.Type { - case "local": - var attrs map[string]string - for k, v := range co.Attrs { - if attrs == nil { - attrs = make(map[string]string) - } - switch k { - case "dest": - p := v - if p != "" { - p, err = filepath.Abs(p) - if err != nil { - return nil, err - } - } - attrs[k] = p - default: - attrs[k] = v - } - } - co.Attrs = attrs - cacheTo = append(cacheTo, co) - default: - cacheTo = append(cacheTo, co) - } - } - options.CacheTo = cacheTo - var exports []*controllerapi.ExportEntry - for _, e := range options.Exports { - if e.Destination != "" && e.Destination != "-" { - e.Destination, err = filepath.Abs(e.Destination) - if err != nil { - return nil, err - } - } - exports = append(exports, e) - } - options.Exports = exports - - var secrets []*controllerapi.Secret - for _, s := range options.Secrets { - if s.FilePath != "" { - s.FilePath, err = filepath.Abs(s.FilePath) - if err != nil { - return nil, err - } - } - secrets = append(secrets, s) - } - options.Secrets = secrets - - var ssh []*controllerapi.SSH - for _, s := range options.SSH { - var ps []string - for _, pt := range s.Paths { - p := pt - if p != "" { - p, err = filepath.Abs(p) - if err != nil { - return nil, err - } - } - ps = append(ps, p) - - } - s.Paths = ps - ssh = append(ssh, s) - } - options.SSH = ssh - - return options, nil -} - func printWarnings(w io.Writer, warnings []client.VertexWarning, mode string) { if len(warnings) == 0 || mode == progress.PrinterModeQuiet { return diff --git a/controller/pb/path.go b/controller/pb/path.go new file mode 100644 index 00000000..b2bbe0c7 --- /dev/null +++ b/controller/pb/path.go @@ -0,0 +1,175 @@ +package pb + +import ( + "path/filepath" + "strings" + + "github.com/docker/docker/builder/remotecontext/urlutil" + "github.com/moby/buildkit/util/gitutil" +) + +// ResolveOptionPaths resolves all paths contained in BuildOptions +// and replaces them to absolute paths. +func ResolveOptionPaths(options *BuildOptions) (_ *BuildOptions, err error) { + localContext := false + if options.ContextPath != "" && options.ContextPath != "-" { + if !isRemoteURL(options.ContextPath) { + localContext = true + options.ContextPath, err = filepath.Abs(options.ContextPath) + if err != nil { + return nil, err + } + } + } + if options.DockerfileName != "" && options.DockerfileName != "-" { + if localContext && !urlutil.IsURL(options.DockerfileName) { + options.DockerfileName, err = filepath.Abs(options.DockerfileName) + if err != nil { + return nil, err + } + } + } + + var contexts map[string]string + for k, v := range options.NamedContexts { + if isRemoteURL(v) || strings.HasPrefix(v, "docker-image://") { + // url prefix, this is a remote path + } else if strings.HasPrefix(v, "oci-layout://") { + // oci layout prefix, this is a local path + p := strings.TrimPrefix(v, "oci-layout://") + p, err = filepath.Abs(p) + if err != nil { + return nil, err + } + v = "oci-layout://" + p + } else { + // no prefix, assume local path + v, err = filepath.Abs(v) + if err != nil { + return nil, err + } + } + + if contexts == nil { + contexts = make(map[string]string) + } + contexts[k] = v + } + options.NamedContexts = contexts + + var cacheFrom []*CacheOptionsEntry + for _, co := range options.CacheFrom { + switch co.Type { + case "local": + var attrs map[string]string + for k, v := range co.Attrs { + if attrs == nil { + attrs = make(map[string]string) + } + switch k { + case "src": + p := v + if p != "" { + p, err = filepath.Abs(p) + if err != nil { + return nil, err + } + } + attrs[k] = p + default: + attrs[k] = v + } + } + co.Attrs = attrs + cacheFrom = append(cacheFrom, co) + default: + cacheFrom = append(cacheFrom, co) + } + } + options.CacheFrom = cacheFrom + + var cacheTo []*CacheOptionsEntry + for _, co := range options.CacheTo { + switch co.Type { + case "local": + var attrs map[string]string + for k, v := range co.Attrs { + if attrs == nil { + attrs = make(map[string]string) + } + switch k { + case "dest": + p := v + if p != "" { + p, err = filepath.Abs(p) + if err != nil { + return nil, err + } + } + attrs[k] = p + default: + attrs[k] = v + } + } + co.Attrs = attrs + cacheTo = append(cacheTo, co) + default: + cacheTo = append(cacheTo, co) + } + } + options.CacheTo = cacheTo + var exports []*ExportEntry + for _, e := range options.Exports { + if e.Destination != "" && e.Destination != "-" { + e.Destination, err = filepath.Abs(e.Destination) + if err != nil { + return nil, err + } + } + exports = append(exports, e) + } + options.Exports = exports + + var secrets []*Secret + for _, s := range options.Secrets { + if s.FilePath != "" { + s.FilePath, err = filepath.Abs(s.FilePath) + if err != nil { + return nil, err + } + } + secrets = append(secrets, s) + } + options.Secrets = secrets + + var ssh []*SSH + for _, s := range options.SSH { + var ps []string + for _, pt := range s.Paths { + p := pt + if p != "" { + p, err = filepath.Abs(p) + if err != nil { + return nil, err + } + } + ps = append(ps, p) + + } + s.Paths = ps + ssh = append(ssh, s) + } + options.SSH = ssh + + return options, nil +} + +func isRemoteURL(c string) bool { + if urlutil.IsURL(c) { + return true + } + if _, err := gitutil.ParseGitRef(c); err == nil { + return true + } + return false +} diff --git a/commands/build_test.go b/controller/pb/path_test.go similarity index 61% rename from commands/build_test.go rename to controller/pb/path_test.go index 7fa84772..33710069 100644 --- a/commands/build_test.go +++ b/controller/pb/path_test.go @@ -1,4 +1,4 @@ -package commands +package pb import ( "os" @@ -6,7 +6,6 @@ import ( "reflect" "testing" - controllerapi "github.com/docker/buildx/controller/pb" "github.com/stretchr/testify/require" ) @@ -17,55 +16,55 @@ func TestResolvePaths(t *testing.T) { require.NoError(t, os.Chdir(tmpwd)) tests := []struct { name string - options controllerapi.BuildOptions - want controllerapi.BuildOptions + options BuildOptions + want BuildOptions }{ { name: "contextpath", - options: controllerapi.BuildOptions{ContextPath: "test"}, - want: controllerapi.BuildOptions{ContextPath: filepath.Join(tmpwd, "test")}, + options: BuildOptions{ContextPath: "test"}, + want: BuildOptions{ContextPath: filepath.Join(tmpwd, "test")}, }, { name: "contextpath-cwd", - options: controllerapi.BuildOptions{ContextPath: "."}, - want: controllerapi.BuildOptions{ContextPath: tmpwd}, + options: BuildOptions{ContextPath: "."}, + want: BuildOptions{ContextPath: tmpwd}, }, { name: "contextpath-dash", - options: controllerapi.BuildOptions{ContextPath: "-"}, - want: controllerapi.BuildOptions{ContextPath: "-"}, + options: BuildOptions{ContextPath: "-"}, + want: BuildOptions{ContextPath: "-"}, }, { name: "contextpath-ssh", - options: controllerapi.BuildOptions{ContextPath: "git@github.com:docker/buildx.git"}, - want: controllerapi.BuildOptions{ContextPath: "git@github.com:docker/buildx.git"}, + options: BuildOptions{ContextPath: "git@github.com:docker/buildx.git"}, + want: BuildOptions{ContextPath: "git@github.com:docker/buildx.git"}, }, { name: "dockerfilename", - options: controllerapi.BuildOptions{DockerfileName: "test", ContextPath: "."}, - want: controllerapi.BuildOptions{DockerfileName: filepath.Join(tmpwd, "test"), ContextPath: tmpwd}, + options: BuildOptions{DockerfileName: "test", ContextPath: "."}, + want: BuildOptions{DockerfileName: filepath.Join(tmpwd, "test"), ContextPath: tmpwd}, }, { name: "dockerfilename-dash", - options: controllerapi.BuildOptions{DockerfileName: "-", ContextPath: "."}, - want: controllerapi.BuildOptions{DockerfileName: "-", ContextPath: tmpwd}, + options: BuildOptions{DockerfileName: "-", ContextPath: "."}, + want: BuildOptions{DockerfileName: "-", ContextPath: tmpwd}, }, { name: "dockerfilename-remote", - options: controllerapi.BuildOptions{DockerfileName: "test", ContextPath: "git@github.com:docker/buildx.git"}, - want: controllerapi.BuildOptions{DockerfileName: "test", ContextPath: "git@github.com:docker/buildx.git"}, + options: BuildOptions{DockerfileName: "test", ContextPath: "git@github.com:docker/buildx.git"}, + want: BuildOptions{DockerfileName: "test", ContextPath: "git@github.com:docker/buildx.git"}, }, { name: "contexts", - options: controllerapi.BuildOptions{NamedContexts: map[string]string{"a": "test1", "b": "test2", + options: BuildOptions{NamedContexts: map[string]string{"a": "test1", "b": "test2", "alpine": "docker-image://alpine@sha256:0123456789", "project": "https://github.com/myuser/project.git"}}, - want: controllerapi.BuildOptions{NamedContexts: map[string]string{"a": filepath.Join(tmpwd, "test1"), "b": filepath.Join(tmpwd, "test2"), + want: BuildOptions{NamedContexts: map[string]string{"a": filepath.Join(tmpwd, "test1"), "b": filepath.Join(tmpwd, "test2"), "alpine": "docker-image://alpine@sha256:0123456789", "project": "https://github.com/myuser/project.git"}}, }, { name: "cache-from", - options: controllerapi.BuildOptions{ - CacheFrom: []*controllerapi.CacheOptionsEntry{ + options: BuildOptions{ + CacheFrom: []*CacheOptionsEntry{ { Type: "local", Attrs: map[string]string{"src": "test"}, @@ -76,8 +75,8 @@ func TestResolvePaths(t *testing.T) { }, }, }, - want: controllerapi.BuildOptions{ - CacheFrom: []*controllerapi.CacheOptionsEntry{ + want: BuildOptions{ + CacheFrom: []*CacheOptionsEntry{ { Type: "local", Attrs: map[string]string{"src": filepath.Join(tmpwd, "test")}, @@ -91,8 +90,8 @@ func TestResolvePaths(t *testing.T) { }, { name: "cache-to", - options: controllerapi.BuildOptions{ - CacheTo: []*controllerapi.CacheOptionsEntry{ + options: BuildOptions{ + CacheTo: []*CacheOptionsEntry{ { Type: "local", Attrs: map[string]string{"dest": "test"}, @@ -103,8 +102,8 @@ func TestResolvePaths(t *testing.T) { }, }, }, - want: controllerapi.BuildOptions{ - CacheTo: []*controllerapi.CacheOptionsEntry{ + want: BuildOptions{ + CacheTo: []*CacheOptionsEntry{ { Type: "local", Attrs: map[string]string{"dest": filepath.Join(tmpwd, "test")}, @@ -118,8 +117,8 @@ func TestResolvePaths(t *testing.T) { }, { name: "exports", - options: controllerapi.BuildOptions{ - Exports: []*controllerapi.ExportEntry{ + options: BuildOptions{ + Exports: []*ExportEntry{ { Type: "local", Destination: "-", @@ -146,8 +145,8 @@ func TestResolvePaths(t *testing.T) { }, }, }, - want: controllerapi.BuildOptions{ - Exports: []*controllerapi.ExportEntry{ + want: BuildOptions{ + Exports: []*ExportEntry{ { Type: "local", Destination: "-", @@ -177,8 +176,8 @@ func TestResolvePaths(t *testing.T) { }, { name: "secrets", - options: controllerapi.BuildOptions{ - Secrets: []*controllerapi.Secret{ + options: BuildOptions{ + Secrets: []*Secret{ { FilePath: "test1", }, @@ -192,8 +191,8 @@ func TestResolvePaths(t *testing.T) { }, }, }, - want: controllerapi.BuildOptions{ - Secrets: []*controllerapi.Secret{ + want: BuildOptions{ + Secrets: []*Secret{ { FilePath: filepath.Join(tmpwd, "test1"), }, @@ -210,8 +209,8 @@ func TestResolvePaths(t *testing.T) { }, { name: "ssh", - options: controllerapi.BuildOptions{ - SSH: []*controllerapi.SSH{ + options: BuildOptions{ + SSH: []*SSH{ { ID: "default", Paths: []string{"test1", "test2"}, @@ -222,8 +221,8 @@ func TestResolvePaths(t *testing.T) { }, }, }, - want: controllerapi.BuildOptions{ - SSH: []*controllerapi.SSH{ + want: BuildOptions{ + SSH: []*SSH{ { ID: "default", Paths: []string{filepath.Join(tmpwd, "test1"), filepath.Join(tmpwd, "test2")}, @@ -238,7 +237,7 @@ func TestResolvePaths(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := resolvePaths(&tt.options) + got, err := ResolveOptionPaths(&tt.options) require.NoError(t, err) if !reflect.DeepEqual(tt.want, *got) { t.Fatalf("expected %#v, got %#v", tt.want, *got)