Skip to content

Commit d1cabdf

Browse files
crazy-maxcrazy-max
committed
build: set default context builder if not specified
Signed-off-by: CrazyMax <[email protected]>
1 parent 3dfef76 commit d1cabdf

File tree

5 files changed

+154
-27
lines changed

5 files changed

+154
-27
lines changed

‎cmd/docker/aliases.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,27 @@ var allowedAliases = map[string]struct{}{
1818
keyBuilderAlias: {},
1919
}
2020

21-
func processAliases(dockerCli command.Cli, cmd *cobra.Command, args, osArgs []string) ([]string, []string, error) {
21+
func processAliases(dockerCli command.Cli, cmd *cobra.Command, args, osArgs []string) ([]string, []string, []string, error) {
2222
var err error
23+
var envs []string
2324
aliasMap := dockerCli.ConfigFile().Aliases
2425
aliases := make([][2][]string, 0, len(aliasMap))
2526

2627
for k, v := range aliasMap {
2728
if _, ok := allowedAliases[k]; !ok {
28-
return args, osArgs, errors.Errorf("not allowed to alias %q (allowed: %#v)", k, allowedAliases)
29+
return args, osArgs, envs, errors.Errorf("not allowed to alias %q (allowed: %#v)", k, allowedAliases)
2930
}
3031
if c, _, err := cmd.Find(strings.Split(v, " ")); err == nil {
3132
if !pluginmanager.IsPluginCommand(c) {
32-
return args, osArgs, errors.Errorf("not allowed to alias with builtin %q as target", v)
33+
return args, osArgs, envs, errors.Errorf("not allowed to alias with builtin %q as target", v)
3334
}
3435
}
3536
aliases = append(aliases, [2][]string{{k}, {v}})
3637
}
3738

38-
args, osArgs, err = processBuilder(dockerCli, cmd, args, os.Args)
39+
args, osArgs, envs, err = processBuilder(dockerCli, cmd, args, os.Args)
3940
if err != nil {
40-
return args, os.Args, err
41+
return args, os.Args, envs, err
4142
}
4243

4344
for _, al := range aliases {
@@ -49,5 +50,5 @@ func processAliases(dockerCli command.Cli, cmd *cobra.Command, args, osArgs []st
4950
}
5051
}
5152

52-
return args, osArgs, nil
53+
return args, osArgs, envs, nil
5354
}

‎cmd/docker/builder.go

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,16 @@ package main
22

33
import (
44
"fmt"
5+
"io"
56
"os"
67
"strconv"
8+
"strings"
79

810
pluginmanager "github.com/docker/cli/cli-plugins/manager"
911
"github.com/docker/cli/cli/command"
1012
"github.com/pkg/errors"
1113
"github.com/spf13/cobra"
14+
"github.com/spf13/pflag"
1215
)
1316

1417
const (
@@ -40,15 +43,17 @@ func newBuilderError(warn bool, err error) error {
4043
return fmt.Errorf("%s", errorMsg)
4144
}
4245

43-
func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []string) ([]string, []string, error) {
46+
//nolint:gocyclo
47+
func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []string) ([]string, []string, []string, error) {
4448
var useLegacy, useBuilder bool
49+
var envs []string
4550

4651
// check DOCKER_BUILDKIT env var is present and
4752
// if not assume we want to use the builder component
4853
if v, ok := os.LookupEnv("DOCKER_BUILDKIT"); ok {
4954
enabled, err := strconv.ParseBool(v)
5055
if err != nil {
51-
return args, osargs, errors.Wrap(err, "DOCKER_BUILDKIT environment variable expects boolean value")
56+
return args, osargs, nil, errors.Wrap(err, "DOCKER_BUILDKIT environment variable expects boolean value")
5257
}
5358
if !enabled {
5459
useLegacy = true
@@ -69,21 +74,21 @@ func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []st
6974
// is this a build that should be forwarded to the builder?
7075
fwargs, fwosargs, forwarded := forwardBuilder(builderAlias, args, osargs)
7176
if !forwarded {
72-
return args, osargs, nil
77+
return args, osargs, nil, nil
7378
}
7479

7580
// wcow build command must use the legacy builder
7681
// if not opt-in through a builder component
7782
if !useBuilder && dockerCli.ServerInfo().OSType == "windows" {
78-
return args, osargs, nil
83+
return args, osargs, nil, nil
7984
}
8085

8186
if useLegacy {
8287
// display warning if not wcow and continue
8388
if dockerCli.ServerInfo().OSType != "windows" {
8489
_, _ = fmt.Fprintln(dockerCli.Err(), newBuilderError(true, nil))
8590
}
86-
return args, osargs, nil
91+
return args, osargs, nil, nil
8792
}
8893

8994
// check plugin is available if cmd forwarded
@@ -94,14 +99,25 @@ func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []st
9499
if perr != nil {
95100
// if builder enforced with DOCKER_BUILDKIT=1, cmd must fail if plugin missing or broken
96101
if useBuilder {
97-
return args, osargs, newBuilderError(false, perr)
102+
return args, osargs, nil, newBuilderError(false, perr)
98103
}
99104
// otherwise, display warning and continue
100105
_, _ = fmt.Fprintln(dockerCli.Err(), newBuilderError(true, perr))
101-
return args, osargs, nil
106+
return args, osargs, nil, nil
102107
}
103108

104-
return fwargs, fwosargs, nil
109+
// If build subcommand is forwarded, user would expect "docker build" to
110+
// always create a local docker image (default context builder). This is
111+
// for better backward compatibility in case where a user could switch to
112+
// a docker container builder with "docker buildx --use foo" which does
113+
// not --load by default. Also makes sure that an arbitrary builder name is
114+
// not being set in the command line or in the environment before setting
115+
// the default context in env vars.
116+
if forwarded && !hasBuilderName(args, os.Environ()) {
117+
envs = append([]string{"BUILDX_BUILDER=" + dockerCli.CurrentContext()}, envs...)
118+
}
119+
120+
return fwargs, fwosargs, envs, nil
105121
}
106122

107123
func forwardBuilder(alias string, args, osargs []string) ([]string, []string, bool) {
@@ -127,3 +143,22 @@ func forwardBuilder(alias string, args, osargs []string) ([]string, []string, bo
127143
}
128144
return args, osargs, false
129145
}
146+
147+
// hasBuilderName checks if a builder name is defined in args or env vars
148+
func hasBuilderName(args []string, envs []string) bool {
149+
var builder string
150+
flagset := pflag.NewFlagSet("buildx", pflag.ContinueOnError)
151+
flagset.Usage = func() {}
152+
flagset.SetOutput(io.Discard)
153+
flagset.StringVar(&builder, "builder", "", "")
154+
_ = flagset.Parse(args)
155+
if builder != "" {
156+
return true
157+
}
158+
for _, e := range envs {
159+
if strings.HasPrefix(e, "BUILDX_BUILDER=") && e != "BUILDX_BUILDER=" {
160+
return true
161+
}
162+
}
163+
return false
164+
}

‎cmd/docker/builder_test.go

Lines changed: 94 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,33 +3,60 @@ package main
33
import (
44
"bytes"
55
"os"
6-
"runtime"
76
"testing"
87

98
"github.com/docker/cli/cli/command"
9+
"github.com/docker/cli/cli/flags"
1010
"github.com/docker/cli/internal/test/output"
1111
"gotest.tools/v3/assert"
1212
"gotest.tools/v3/fs"
1313
)
1414

1515
var pluginFilename = "docker-buildx"
1616

17-
func init() {
18-
if runtime.GOOS == "windows" {
19-
pluginFilename = pluginFilename + ".exe"
20-
}
17+
func TestBuildWithBuildx(t *testing.T) {
18+
dir := fs.NewDir(t, t.Name(),
19+
fs.WithFile(pluginFilename, `#!/bin/sh
20+
echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortDescription":"Build with BuildKit"}'`, fs.WithMode(0777)),
21+
)
22+
defer dir.Remove()
23+
24+
var b bytes.Buffer
25+
26+
t.Setenv("DOCKER_CONTEXT", "default")
27+
dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b))
28+
assert.NilError(t, err)
29+
assert.NilError(t, dockerCli.Initialize(flags.NewClientOptions()))
30+
dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()}
31+
32+
tcmd := newDockerCommand(dockerCli)
33+
tcmd.SetArgs([]string{"build", "."})
34+
35+
cmd, args, err := tcmd.HandleGlobalFlags()
36+
assert.NilError(t, err)
37+
38+
var envs []string
39+
args, os.Args, envs, err = processBuilder(dockerCli, cmd, args, os.Args)
40+
assert.NilError(t, err)
41+
assert.DeepEqual(t, []string{builderDefaultPlugin, "build", "."}, args)
42+
assert.DeepEqual(t, []string{"BUILDX_BUILDER=default"}, envs)
2143
}
2244

23-
func TestBuildWithBuilder(t *testing.T) {
45+
func TestBuildWithBuildxAndBuilder(t *testing.T) {
46+
t.Setenv("BUILDX_BUILDER", "mybuilder")
47+
2448
dir := fs.NewDir(t, t.Name(),
2549
fs.WithFile(pluginFilename, `#!/bin/sh
2650
echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortDescription":"Build with BuildKit"}'`, fs.WithMode(0777)),
2751
)
2852
defer dir.Remove()
2953

3054
var b bytes.Buffer
55+
56+
t.Setenv("DOCKER_CONTEXT", "default")
3157
dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b))
3258
assert.NilError(t, err)
59+
assert.NilError(t, dockerCli.Initialize(flags.NewClientOptions()))
3360
dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()}
3461

3562
tcmd := newDockerCommand(dockerCli)
@@ -38,9 +65,11 @@ echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortD
3865
cmd, args, err := tcmd.HandleGlobalFlags()
3966
assert.NilError(t, err)
4067

41-
args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args)
68+
var envs []string
69+
args, os.Args, envs, err = processBuilder(dockerCli, cmd, args, os.Args)
4270
assert.NilError(t, err)
4371
assert.DeepEqual(t, []string{builderDefaultPlugin, "build", "."}, args)
72+
assert.Check(t, len(envs) == 0)
4473
}
4574

4675
func TestBuildkitDisabled(t *testing.T) {
@@ -55,6 +84,7 @@ func TestBuildkitDisabled(t *testing.T) {
5584

5685
dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(b))
5786
assert.NilError(t, err)
87+
assert.NilError(t, dockerCli.Initialize(flags.NewClientOptions()))
5888
dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()}
5989

6090
tcmd := newDockerCommand(dockerCli)
@@ -63,9 +93,11 @@ func TestBuildkitDisabled(t *testing.T) {
6393
cmd, args, err := tcmd.HandleGlobalFlags()
6494
assert.NilError(t, err)
6595

66-
args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args)
96+
var envs []string
97+
args, os.Args, envs, err = processBuilder(dockerCli, cmd, args, os.Args)
6798
assert.NilError(t, err)
6899
assert.DeepEqual(t, []string{"build", "."}, args)
100+
assert.Check(t, len(envs) == 0)
69101

70102
output.Assert(t, b.String(), map[int]func(string) error{
71103
0: output.Suffix("DEPRECATED: The legacy builder is deprecated and will be removed in a future release."),
@@ -82,6 +114,7 @@ func TestBuilderBroken(t *testing.T) {
82114

83115
dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(b))
84116
assert.NilError(t, err)
117+
assert.NilError(t, dockerCli.Initialize(flags.NewClientOptions()))
85118
dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()}
86119

87120
tcmd := newDockerCommand(dockerCli)
@@ -90,9 +123,11 @@ func TestBuilderBroken(t *testing.T) {
90123
cmd, args, err := tcmd.HandleGlobalFlags()
91124
assert.NilError(t, err)
92125

93-
args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args)
126+
var envs []string
127+
args, os.Args, envs, err = processBuilder(dockerCli, cmd, args, os.Args)
94128
assert.NilError(t, err)
95129
assert.DeepEqual(t, []string{"build", "."}, args)
130+
assert.Check(t, len(envs) == 0)
96131

97132
output.Assert(t, b.String(), map[int]func(string) error{
98133
0: output.Prefix("failed to fetch metadata:"),
@@ -112,6 +147,7 @@ func TestBuilderBrokenEnforced(t *testing.T) {
112147

113148
dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(b))
114149
assert.NilError(t, err)
150+
assert.NilError(t, dockerCli.Initialize(flags.NewClientOptions()))
115151
dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()}
116152

117153
tcmd := newDockerCommand(dockerCli)
@@ -120,11 +156,59 @@ func TestBuilderBrokenEnforced(t *testing.T) {
120156
cmd, args, err := tcmd.HandleGlobalFlags()
121157
assert.NilError(t, err)
122158

123-
args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args)
159+
var envs []string
160+
args, os.Args, envs, err = processBuilder(dockerCli, cmd, args, os.Args)
124161
assert.DeepEqual(t, []string{"build", "."}, args)
162+
assert.Check(t, len(envs) == 0)
125163

126164
output.Assert(t, err.Error(), map[int]func(string) error{
127165
0: output.Prefix("failed to fetch metadata:"),
128166
2: output.Suffix("ERROR: BuildKit is enabled but the buildx component is missing or broken."),
129167
})
130168
}
169+
170+
func TestHasBuilderName(t *testing.T) {
171+
cases := []struct {
172+
name string
173+
args []string
174+
envs []string
175+
expected bool
176+
}{
177+
{
178+
name: "no args",
179+
args: []string{"docker", "build", "."},
180+
envs: []string{"FOO=bar"},
181+
expected: false,
182+
},
183+
{
184+
name: "env var",
185+
args: []string{"docker", "build", "."},
186+
envs: []string{"BUILDX_BUILDER=foo"},
187+
expected: true,
188+
},
189+
{
190+
name: "empty env var",
191+
args: []string{"docker", "build", "."},
192+
envs: []string{"BUILDX_BUILDER="},
193+
expected: false,
194+
},
195+
{
196+
name: "flag",
197+
args: []string{"docker", "build", "--builder", "foo", "."},
198+
envs: []string{"FOO=bar"},
199+
expected: true,
200+
},
201+
{
202+
name: "both",
203+
args: []string{"docker", "build", "--builder", "foo", "."},
204+
envs: []string{"BUILDX_BUILDER=foo"},
205+
expected: true,
206+
},
207+
}
208+
for _, tt := range cases {
209+
tt := tt
210+
t.Run(tt.name, func(t *testing.T) {
211+
assert.Equal(t, tt.expected, hasBuilderName(tt.args, tt.envs))
212+
})
213+
}
214+
}

‎cmd/docker/builder_windows_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
package main
2+
3+
func init() {
4+
pluginFilename = pluginFilename + ".exe"
5+
}

‎cmd/docker/docker.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,11 +178,12 @@ func setValidateArgs(dockerCli command.Cli, cmd *cobra.Command) {
178178
})
179179
}
180180

181-
func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string) error {
181+
func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string, envs []string) error {
182182
plugincmd, err := pluginmanager.PluginRunCommand(dockerCli, subcommand, cmd)
183183
if err != nil {
184184
return err
185185
}
186+
plugincmd.Env = append(envs, plugincmd.Env...)
186187

187188
go func() {
188189
// override SIGTERM handler so we let the plugin shut down first
@@ -217,7 +218,8 @@ func runDocker(dockerCli *command.DockerCli) error {
217218
return err
218219
}
219220

220-
args, os.Args, err = processAliases(dockerCli, cmd, args, os.Args)
221+
var envs []string
222+
args, os.Args, envs, err = processAliases(dockerCli, cmd, args, os.Args)
221223
if err != nil {
222224
return err
223225
}
@@ -230,7 +232,7 @@ func runDocker(dockerCli *command.DockerCli) error {
230232
if len(args) > 0 {
231233
ccmd, _, err := cmd.Find(args)
232234
if err != nil || pluginmanager.IsPluginCommand(ccmd) {
233-
err := tryPluginRun(dockerCli, cmd, args[0])
235+
err := tryPluginRun(dockerCli, cmd, args[0], envs)
234236
if !pluginmanager.IsNotFound(err) {
235237
return err
236238
}

0 commit comments

Comments
 (0)