Browse code

Fix environ substitutions in `docker commit --change ...`

The building machinery was being handed an uninitialized container
Config. This changes it to use the target container's Config.

Resolves #30538

Signed-off-by: Anthony Sottile <asottile@umich.edu>

Anthony Sottile authored on 2017/11/23 06:31:26
Showing 5 changed files
... ...
@@ -13,7 +13,6 @@ import (
13 13
 	"github.com/docker/docker/api/server/httputils"
14 14
 	"github.com/docker/docker/api/types"
15 15
 	"github.com/docker/docker/api/types/backend"
16
-	"github.com/docker/docker/api/types/container"
17 16
 	"github.com/docker/docker/api/types/filters"
18 17
 	"github.com/docker/docker/api/types/versions"
19 18
 	"github.com/docker/docker/pkg/ioutils"
... ...
@@ -46,9 +45,6 @@ func (s *imageRouter) postCommit(ctx context.Context, w http.ResponseWriter, r *
46 46
 	if err != nil && err != io.EOF { //Do not fail if body is empty.
47 47
 		return err
48 48
 	}
49
-	if c == nil {
50
-		c = &container.Config{}
51
-	}
52 49
 
53 50
 	commitCfg := &backend.ContainerCommitConfig{
54 51
 		ContainerCommitConfig: types.ContainerCommitConfig{
... ...
@@ -396,7 +396,8 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con
396 396
 	}
397 397
 
398 398
 	dispatchRequest := newDispatchRequest(b, dockerfile.EscapeToken, nil, newBuildArgs(b.options.BuildArgs), newStagesBuildResults())
399
-	dispatchRequest.state.runConfig = config
399
+	// We make mutations to the configuration, ensure we have a copy
400
+	dispatchRequest.state.runConfig = copyRunConfig(config)
400 401
 	dispatchRequest.state.imageID = config.Image
401 402
 	for _, cmd := range commands {
402 403
 		err := dispatch(dispatchRequest, cmd)
... ...
@@ -149,6 +149,10 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str
149 149
 		defer daemon.containerUnpause(container)
150 150
 	}
151 151
 
152
+	if c.MergeConfigs && c.Config == nil {
153
+		c.Config = container.Config
154
+	}
155
+
152 156
 	newConfig, err := dockerfile.BuildFromConfig(c.Config, c.Changes)
153 157
 	if err != nil {
154 158
 		return "", err
... ...
@@ -121,11 +121,19 @@ func (s *DockerSuite) TestCommitChange(c *check.C) {
121 121
 		"test", "test-commit")
122 122
 	imageID = strings.TrimSpace(imageID)
123 123
 
124
+	// The ordering here is due to `PATH` being overridden from the container's
125
+	// ENV.  On windows, the container doesn't have a `PATH` ENV variable so
126
+	// the ordering is the same as the cli.
127
+	expectedEnv := "[PATH=/foo DEBUG=true test=1]"
128
+	if testEnv.DaemonPlatform() == "windows" {
129
+		expectedEnv = "[DEBUG=true test=1 PATH=/foo]"
130
+	}
131
+
124 132
 	prefix, slash := getPrefixAndSlashFromDaemonPlatform()
125 133
 	prefix = strings.ToUpper(prefix) // Force C: as that's how WORKDIR is normalized on Windows
126 134
 	expected := map[string]string{
127 135
 		"Config.ExposedPorts": "map[8080/tcp:{}]",
128
-		"Config.Env":          "[DEBUG=true test=1 PATH=/foo]",
136
+		"Config.Env":          expectedEnv,
129 137
 		"Config.Labels":       "map[foo:bar]",
130 138
 		"Config.Cmd":          "[/bin/sh]",
131 139
 		"Config.WorkingDir":   prefix + slash + "opt",
132 140
new file mode 100644
... ...
@@ -0,0 +1,47 @@
0
+package image
1
+
2
+import (
3
+	"context"
4
+	"testing"
5
+
6
+	"github.com/docker/docker/api/types"
7
+	"github.com/docker/docker/api/types/container"
8
+	"github.com/docker/docker/integration/util/request"
9
+	"github.com/stretchr/testify/assert"
10
+	"github.com/stretchr/testify/require"
11
+)
12
+
13
+func TestCommitInheritsEnv(t *testing.T) {
14
+	defer setupTest(t)()
15
+	client := request.NewAPIClient(t)
16
+	ctx := context.Background()
17
+
18
+	createResp1, err := client.ContainerCreate(ctx, &container.Config{Image: "busybox"}, nil, nil, "")
19
+	require.NoError(t, err)
20
+
21
+	commitResp1, err := client.ContainerCommit(ctx, createResp1.ID, types.ContainerCommitOptions{
22
+		Changes:   []string{"ENV PATH=/bin"},
23
+		Reference: "test-commit-image",
24
+	})
25
+	require.NoError(t, err)
26
+
27
+	image1, _, err := client.ImageInspectWithRaw(ctx, commitResp1.ID)
28
+	require.NoError(t, err)
29
+
30
+	expectedEnv1 := []string{"PATH=/bin"}
31
+	assert.Equal(t, expectedEnv1, image1.Config.Env)
32
+
33
+	createResp2, err := client.ContainerCreate(ctx, &container.Config{Image: image1.ID}, nil, nil, "")
34
+	require.NoError(t, err)
35
+
36
+	commitResp2, err := client.ContainerCommit(ctx, createResp2.ID, types.ContainerCommitOptions{
37
+		Changes:   []string{"ENV PATH=/usr/bin:$PATH"},
38
+		Reference: "test-commit-image",
39
+	})
40
+	require.NoError(t, err)
41
+
42
+	image2, _, err := client.ImageInspectWithRaw(ctx, commitResp2.ID)
43
+	require.NoError(t, err)
44
+	expectedEnv2 := []string{"PATH=/usr/bin:/bin"}
45
+	assert.Equal(t, expectedEnv2, image2.Config.Env)
46
+}