From 3ccf1b949640cadf1565df6a8437f0beefa9f6e6 Mon Sep 17 00:00:00 2001
From: TomSweeneyRedHat <tsweeney@redhat.com>
Date: Mon, 1 Jun 2020 18:41:58 -0400
Subject: [PATCH] Adjust argument slice size in builder

A recent update to the argument handling didn't increase
the argument slice to take into the account the values
from b.Args() that were now being added.  The created
unpredicatble results.

After addressing a comment from @nalind, I realized that
the value from b.Arguments should be used in the steps
as it keeps the ordering of when the ARGS and ENV were
defined in the Dockerfile.  Changed makeUserArgs
in internals.go to take two variables rather than just
a builder.

Adddresses the Buildah bug: https://github.com/containers/buildah/issues/2345

Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>
---
 builder.go     | 14 ++++++++++++--
 dispatchers.go |  4 ++--
 internals.go   |  8 ++++----
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/builder.go b/builder.go
index a67c1f80..ffc3b257 100644
--- a/builder.go
+++ b/builder.go
@@ -332,9 +332,19 @@ func ParseFile(path string) (*parser.Node, error) {
 
 // Step creates a new step from the current state.
 func (b *Builder) Step() *Step {
-	dst := make([]string, len(b.Env)+len(b.RunConfig.Env))
-	copy(dst, makeUserArgs(b))
+	argsMap := make(map[string]string)
+	for _, argsVal := range b.Arguments() {
+		val := strings.Split(argsVal, "=")
+		if len(val) > 1 {
+			argsMap[val[0]] = val[1]
+		}
+	}
+
+	userArgs := makeUserArgs(b.Env, argsMap) 
+	dst := make([]string, len(userArgs)+len(b.RunConfig.Env))
+	copy(dst, userArgs)
 	dst = append(dst, b.RunConfig.Env...)
+
 	return &Step{Env: dst}
 }
 
diff --git a/dispatchers.go b/dispatchers.go
index e6eb207b..3a350fbe 100644
--- a/dispatchers.go
+++ b/dispatchers.go
@@ -153,7 +153,7 @@ func add(b *Builder, args []string, attributes map[string]bool, flagArgs []strin
 	var chown string
 	last := len(args) - 1
 	dest := makeAbsolute(args[last], b.RunConfig.WorkingDir)
-	userArgs := makeUserArgs(b)
+	userArgs := makeUserArgs(b.Env, b.Args)
 	for _, a := range flagArgs {
 		arg, err := ProcessWord(a, userArgs)
 		if err != nil {
@@ -182,7 +182,7 @@ func dispatchCopy(b *Builder, args []string, attributes map[string]bool, flagArg
 	dest := makeAbsolute(args[last], b.RunConfig.WorkingDir)
 	var chown string
 	var from string
-	userArgs := makeUserArgs(b)
+	userArgs := makeUserArgs(b.Env, b.Args)
 	for _, a := range flagArgs {
 		arg, err := ProcessWord(a, userArgs)
 		if err != nil {
diff --git a/internals.go b/internals.go
index f1ec9435..b652dc1c 100644
--- a/internals.go
+++ b/internals.go
@@ -98,18 +98,18 @@ func parseOptInterval(f *flag.Flag) (time.Duration, error) {
 // defined by both can later be evaluated when resolving variables
 // such as ${MY_USER}.  If the variable is defined by both ARG and ENV
 // don't include the definition of the ARG variable.
-func makeUserArgs(b *Builder) (userArgs []string) {
+func makeUserArgs(bEnv []string, bArgs map[string]string) (userArgs []string) {
 
-	userArgs = b.Env
+	userArgs = bEnv
 	envMap := make(map[string]string)
-	for _, envVal := range b.Env {
+	for _, envVal := range bEnv {
 		val := strings.Split(envVal, "=")
 		if len(val) > 1 {
 			envMap[val[0]] = val[1]
 		}
 	}
 
-	for key, value := range b.Args {
+	for key, value := range bArgs {
 		if _, ok := envMap[key]; ok {
 			continue
 		}