Browse code

Merge pull request #32950 from dnephin/cherry-pick-build-arg-fixes

[17.05.x] Cherry pick build arg fixes

Victor Vieux authored on 2017/05/02 14:45:51
Showing 2 changed files
... ...
@@ -408,9 +408,7 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
408 408
 	// that starts with "foo=abc" to be considered part of a build-time env var.
409 409
 	saveCmd := config.Cmd
410 410
 	if len(cmdBuildEnv) > 0 {
411
-		sort.Strings(cmdBuildEnv)
412
-		tmpEnv := append([]string{fmt.Sprintf("|%d", len(cmdBuildEnv))}, cmdBuildEnv...)
413
-		saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...))
411
+		saveCmd = prependEnvOnCmd(b.buildArgs, cmdBuildEnv, saveCmd)
414 412
 	}
415 413
 
416 414
 	b.runConfig.Cmd = saveCmd
... ...
@@ -445,26 +443,24 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
445 445
 	// properly match it.
446 446
 	b.runConfig.Env = env
447 447
 
448
-	// remove builtinAllowedBuildArgs (see: builder.go)  from the saveCmd
449
-	// these args are transparent so resulting image should be the same regardless of the value
450
-	if len(cmdBuildEnv) > 0 {
451
-		saveCmd = config.Cmd
452
-		tmpBuildEnv := make([]string, len(cmdBuildEnv))
453
-		copy(tmpBuildEnv, cmdBuildEnv)
454
-		for i, env := range tmpBuildEnv {
455
-			key := strings.SplitN(env, "=", 2)[0]
456
-			if b.buildArgs.IsUnreferencedBuiltin(key) {
457
-				tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...)
458
-			}
459
-		}
460
-		sort.Strings(tmpBuildEnv)
461
-		tmpEnv := append([]string{fmt.Sprintf("|%d", len(tmpBuildEnv))}, tmpBuildEnv...)
462
-		saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...))
463
-	}
464 448
 	b.runConfig.Cmd = saveCmd
465 449
 	return b.commit(cID, cmd, "run")
466 450
 }
467 451
 
452
+func prependEnvOnCmd(buildArgs *buildArgs, buildArgVars []string, cmd strslice.StrSlice) strslice.StrSlice {
453
+	var tmpBuildEnv []string
454
+	for _, env := range buildArgVars {
455
+		key := strings.SplitN(env, "=", 2)[0]
456
+		if !buildArgs.IsUnreferencedBuiltin(key) {
457
+			tmpBuildEnv = append(tmpBuildEnv, env)
458
+		}
459
+	}
460
+
461
+	sort.Strings(tmpBuildEnv)
462
+	tmpEnv := append([]string{fmt.Sprintf("|%d", len(tmpBuildEnv))}, tmpBuildEnv...)
463
+	return strslice.StrSlice(append(tmpEnv, cmd...))
464
+}
465
+
468 466
 // CMD foo
469 467
 //
470 468
 // Set the default command to run in the container (which may be empty).
... ...
@@ -4344,23 +4344,37 @@ func (s *DockerSuite) TestBuildTimeArgHistoryExclusions(c *check.C) {
4344 4344
 		ARG %s
4345 4345
 		ARG %s
4346 4346
 		RUN echo "Testing Build Args!"`, envKey, explicitProxyKey)
4347
-	buildImage(imgName,
4348
-		cli.WithFlags("--build-arg", fmt.Sprintf("%s=%s", envKey, envVal),
4349
-			"--build-arg", fmt.Sprintf("%s=%s", explicitProxyKey, explicitProxyVal),
4350
-			"--build-arg", proxy),
4351
-		build.WithDockerfile(dockerfile),
4352
-	).Assert(c, icmd.Success)
4353 4347
 
4354
-	out, _ := dockerCmd(c, "history", "--no-trunc", imgName)
4348
+	buildImage := func(imgName string) string {
4349
+		cli.BuildCmd(c, imgName,
4350
+			cli.WithFlags("--build-arg", "https_proxy=https://proxy.example.com",
4351
+				"--build-arg", fmt.Sprintf("%s=%s", envKey, envVal),
4352
+				"--build-arg", fmt.Sprintf("%s=%s", explicitProxyKey, explicitProxyVal),
4353
+				"--build-arg", proxy),
4354
+			build.WithDockerfile(dockerfile),
4355
+		)
4356
+		return getIDByName(c, imgName)
4357
+	}
4358
+
4359
+	origID := buildImage(imgName)
4360
+	result := cli.DockerCmd(c, "history", "--no-trunc", imgName)
4361
+	out := result.Stdout()
4362
+
4355 4363
 	if strings.Contains(out, proxy) {
4356 4364
 		c.Fatalf("failed to exclude proxy settings from history!")
4357 4365
 	}
4366
+	if strings.Contains(out, "https_proxy") {
4367
+		c.Fatalf("failed to exclude proxy settings from history!")
4368
+	}
4358 4369
 	if !strings.Contains(out, fmt.Sprintf("%s=%s", envKey, envVal)) {
4359 4370
 		c.Fatalf("explicitly defined ARG %s is not in output", explicitProxyKey)
4360 4371
 	}
4361 4372
 	if !strings.Contains(out, fmt.Sprintf("%s=%s", envKey, envVal)) {
4362 4373
 		c.Fatalf("missing build arguments from output")
4363 4374
 	}
4375
+
4376
+	cacheID := buildImage(imgName + "-two")
4377
+	c.Assert(origID, checker.Equals, cacheID)
4364 4378
 }
4365 4379
 
4366 4380
 func (s *DockerSuite) TestBuildBuildTimeArgCacheHit(c *check.C) {
... ...
@@ -6156,7 +6170,7 @@ func (s *DockerSuite) TestBuildCopyFromPreviousFromWindows(c *check.C) {
6156 6156
 func (s *DockerSuite) TestBuildCopyFromForbidWindowsSystemPaths(c *check.C) {
6157 6157
 	testRequires(c, DaemonIsWindows)
6158 6158
 	dockerfile := `
6159
-		FROM ` + testEnv.MinimalBaseImage() + `		
6159
+		FROM ` + testEnv.MinimalBaseImage() + `
6160 6160
 		FROM ` + testEnv.MinimalBaseImage() + `
6161 6161
 		COPY --from=0 %s c:\\oscopy
6162 6162
 		`
... ...
@@ -6173,7 +6187,7 @@ func (s *DockerSuite) TestBuildCopyFromForbidWindowsSystemPaths(c *check.C) {
6173 6173
 func (s *DockerSuite) TestBuildCopyFromForbidWindowsRelativePaths(c *check.C) {
6174 6174
 	testRequires(c, DaemonIsWindows)
6175 6175
 	dockerfile := `
6176
-		FROM ` + testEnv.MinimalBaseImage() + `		
6176
+		FROM ` + testEnv.MinimalBaseImage() + `
6177 6177
 		FROM ` + testEnv.MinimalBaseImage() + `
6178 6178
 		COPY --from=0 %s c:\\oscopy
6179 6179
 		`
... ...
@@ -6192,7 +6206,7 @@ func (s *DockerSuite) TestBuildCopyFromWindowsIsCaseInsensitive(c *check.C) {
6192 6192
 	testRequires(c, DaemonIsWindows)
6193 6193
 	dockerfile := `
6194 6194
 		FROM ` + testEnv.MinimalBaseImage() + `
6195
-		COPY foo /	
6195
+		COPY foo /
6196 6196
 		FROM ` + testEnv.MinimalBaseImage() + `
6197 6197
 		COPY --from=0 c:\\fOo c:\\copied
6198 6198
 		RUN type c:\\copied