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

[17.05.x] Cherry pick build arg fixes

builder/dockerfile/dispatchers.go

diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go
index fb064e5..ad6aff6 100644
--- a/builder/dockerfile/dispatchers.go
+++ b/builder/dockerfile/dispatchers.go
@@ -408,9 +408,7 @@
 	// that starts with "foo=abc" to be considered part of a build-time env var.
 	saveCmd := config.Cmd
 	if len(cmdBuildEnv) > 0 {
-		sort.Strings(cmdBuildEnv)
-		tmpEnv := append([]string{fmt.Sprintf("|%d", len(cmdBuildEnv))}, cmdBuildEnv...)
-		saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...))
+		saveCmd = prependEnvOnCmd(b.buildArgs, cmdBuildEnv, saveCmd)
 	}
 
 	b.runConfig.Cmd = saveCmd
@@ -445,26 +443,24 @@
 	// properly match it.
 	b.runConfig.Env = env
 
-	// remove builtinAllowedBuildArgs (see: builder.go)  from the saveCmd
-	// these args are transparent so resulting image should be the same regardless of the value
-	if len(cmdBuildEnv) > 0 {
-		saveCmd = config.Cmd
-		tmpBuildEnv := make([]string, len(cmdBuildEnv))
-		copy(tmpBuildEnv, cmdBuildEnv)
-		for i, env := range tmpBuildEnv {
-			key := strings.SplitN(env, "=", 2)[0]
-			if b.buildArgs.IsUnreferencedBuiltin(key) {
-				tmpBuildEnv = append(tmpBuildEnv[:i], tmpBuildEnv[i+1:]...)
-			}
-		}
-		sort.Strings(tmpBuildEnv)
-		tmpEnv := append([]string{fmt.Sprintf("|%d", len(tmpBuildEnv))}, tmpBuildEnv...)
-		saveCmd = strslice.StrSlice(append(tmpEnv, saveCmd...))
-	}
 	b.runConfig.Cmd = saveCmd
 	return b.commit(cID, cmd, "run")
 }
 
+func prependEnvOnCmd(buildArgs *buildArgs, buildArgVars []string, cmd strslice.StrSlice) strslice.StrSlice {
+	var tmpBuildEnv []string
+	for _, env := range buildArgVars {
+		key := strings.SplitN(env, "=", 2)[0]
+		if !buildArgs.IsUnreferencedBuiltin(key) {
+			tmpBuildEnv = append(tmpBuildEnv, env)
+		}
+	}
+
+	sort.Strings(tmpBuildEnv)
+	tmpEnv := append([]string{fmt.Sprintf("|%d", len(tmpBuildEnv))}, tmpBuildEnv...)
+	return strslice.StrSlice(append(tmpEnv, cmd...))
+}
+
 // CMD foo
 //
 // Set the default command to run in the container (which may be empty).

integration-cli/docker_cli_build_test.go

diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go
index ea25959..b046330 100644
--- a/integration-cli/docker_cli_build_test.go
+++ b/integration-cli/docker_cli_build_test.go
@@ -4344,23 +4344,37 @@
 		ARG %s
 		ARG %s
 		RUN echo "Testing Build Args!"`, envKey, explicitProxyKey)
-	buildImage(imgName,
-		cli.WithFlags("--build-arg", fmt.Sprintf("%s=%s", envKey, envVal),
-			"--build-arg", fmt.Sprintf("%s=%s", explicitProxyKey, explicitProxyVal),
-			"--build-arg", proxy),
-		build.WithDockerfile(dockerfile),
-	).Assert(c, icmd.Success)
 
-	out, _ := dockerCmd(c, "history", "--no-trunc", imgName)
+	buildImage := func(imgName string) string {
+		cli.BuildCmd(c, imgName,
+			cli.WithFlags("--build-arg", "https_proxy=https://proxy.example.com",
+				"--build-arg", fmt.Sprintf("%s=%s", envKey, envVal),
+				"--build-arg", fmt.Sprintf("%s=%s", explicitProxyKey, explicitProxyVal),
+				"--build-arg", proxy),
+			build.WithDockerfile(dockerfile),
+		)
+		return getIDByName(c, imgName)
+	}
+
+	origID := buildImage(imgName)
+	result := cli.DockerCmd(c, "history", "--no-trunc", imgName)
+	out := result.Stdout()
+
 	if strings.Contains(out, proxy) {
 		c.Fatalf("failed to exclude proxy settings from history!")
 	}
+	if strings.Contains(out, "https_proxy") {
+		c.Fatalf("failed to exclude proxy settings from history!")
+	}
 	if !strings.Contains(out, fmt.Sprintf("%s=%s", envKey, envVal)) {
 		c.Fatalf("explicitly defined ARG %s is not in output", explicitProxyKey)
 	}
 	if !strings.Contains(out, fmt.Sprintf("%s=%s", envKey, envVal)) {
 		c.Fatalf("missing build arguments from output")
 	}
+
+	cacheID := buildImage(imgName + "-two")
+	c.Assert(origID, checker.Equals, cacheID)
 }
 
 func (s *DockerSuite) TestBuildBuildTimeArgCacheHit(c *check.C) {
@@ -6156,7 +6170,7 @@
 func (s *DockerSuite) TestBuildCopyFromForbidWindowsSystemPaths(c *check.C) {
 	testRequires(c, DaemonIsWindows)
 	dockerfile := `
-		FROM ` + testEnv.MinimalBaseImage() + `		
+		FROM ` + testEnv.MinimalBaseImage() + `
 		FROM ` + testEnv.MinimalBaseImage() + `
 		COPY --from=0 %s c:\\oscopy
 		`
@@ -6173,7 +6187,7 @@
 func (s *DockerSuite) TestBuildCopyFromForbidWindowsRelativePaths(c *check.C) {
 	testRequires(c, DaemonIsWindows)
 	dockerfile := `
-		FROM ` + testEnv.MinimalBaseImage() + `		
+		FROM ` + testEnv.MinimalBaseImage() + `
 		FROM ` + testEnv.MinimalBaseImage() + `
 		COPY --from=0 %s c:\\oscopy
 		`
@@ -6192,7 +6206,7 @@
 	testRequires(c, DaemonIsWindows)
 	dockerfile := `
 		FROM ` + testEnv.MinimalBaseImage() + `
-		COPY foo /	
+		COPY foo /
 		FROM ` + testEnv.MinimalBaseImage() + `
 		COPY --from=0 c:\\fOo c:\\copied
 		RUN type c:\\copied