From 49ab77998f85a05f05814f6575b25de264bf8256 Mon Sep 17 00:00:00 2001
From: TomSweeneyRedHat <tsweeney@redhat.com>
Date: Sat, 18 Jan 2020 15:43:05 -0500
Subject: [PATCH] Fix COPY in containerfile with envvar

If a Containerfile had lines like:

```
FROM alpine
ENV VERSION=0.0.1
COPY file-${VERSION}.txt /
```

Buildah would not resolve the VERSION variable in the copy statement.
If the 'ENV' in the above Containerfile was changed to ARG, then this
would work.

A recent change to the handling of variables now only looks at variables
set by 'ARG' and not the ones set by the 'ENV' command.  This PR
adds the the variables set by the `ENV` to the list of `ARG` variables
when those variables are being resolved by the code.

This also includes added test to guard against this regression in the future.

Addresses:  https://github.com/containers/libpod/issues/4878

Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>
---
 imagebuildah/stage_executor.go       | 13 +++++++------
 tests/bud.bats                       | 12 ++++++++++++
 tests/bud/copy-envvar/Containerfile  |  3 +++
 tests/bud/copy-envvar/file-0.0.1.txt |  0
 4 files changed, 22 insertions(+), 6 deletions(-)
 create mode 100644 tests/bud/copy-envvar/Containerfile
 create mode 100644 tests/bud/copy-envvar/file-0.0.1.txt

diff --git a/imagebuildah/stage_executor.go b/imagebuildah/stage_executor.go
index 4fd630a83..60fa10888 100644
--- a/imagebuildah/stage_executor.go
+++ b/imagebuildah/stage_executor.go
@@ -253,7 +253,7 @@ func (s *StageExecutor) volumeCacheRestore() error {
 // don't care about the details of where in the filesystem the content actually
 // goes, because we're not actually going to add it here, so this is less
 // involved than Copy().
-func (s *StageExecutor) digestSpecifiedContent(node *parser.Node, argValues []string) (string, error) {
+func (s *StageExecutor) digestSpecifiedContent(node *parser.Node, argValues []string, envValues []string) (string, error) {
 	// No instruction: done.
 	if node == nil {
 		return "", nil
@@ -298,10 +298,11 @@ func (s *StageExecutor) digestSpecifiedContent(node *parser.Node, argValues []st
 		}
 	}
 
+	varValues := append(argValues, envValues...)
 	for _, src := range srcs {
 		// If src has an argument within it, resolve it to its
 		// value.  Otherwise just return the value found.
-		name, err := imagebuilder.ProcessWord(src, argValues)
+		name, err := imagebuilder.ProcessWord(src, varValues)
 		if err != nil {
 			return "", errors.Wrapf(err, "unable to resolve source %q", src)
 		}
@@ -345,7 +346,7 @@ func (s *StageExecutor) digestSpecifiedContent(node *parser.Node, argValues []st
 
 	// If destination.Value has an argument within it, resolve it to its
 	// value.  Otherwise just return the value found.
-	destValue, destErr := imagebuilder.ProcessWord(destination.Value, argValues)
+	destValue, destErr := imagebuilder.ProcessWord(destination.Value, varValues)
 	if destErr != nil {
 		return "", errors.Wrapf(destErr, "unable to resolve destination %q", destination.Value)
 	}
@@ -868,7 +869,7 @@ func (s *StageExecutor) Execute(ctx context.Context, stage imagebuilder.Stage, b
 				return "", nil, errors.Wrapf(err, "error building at STEP \"%s\"", step.Message)
 			}
 			// In case we added content, retrieve its digest.
-			addedContentDigest, err := s.digestSpecifiedContent(node, ib.Arguments())
+			addedContentDigest, err := s.digestSpecifiedContent(node, ib.Arguments(), ib.Config().Env)
 			if err != nil {
 				return "", nil, err
 			}
@@ -917,7 +918,7 @@ func (s *StageExecutor) Execute(ctx context.Context, stage imagebuilder.Stage, b
 		// cached images so far, look for one that matches what we
 		// expect to produce for this instruction.
 		if checkForLayers && !(s.executor.squash && lastInstruction && lastStage) {
-			addedContentDigest, err := s.digestSpecifiedContent(node, ib.Arguments())
+			addedContentDigest, err := s.digestSpecifiedContent(node, ib.Arguments(), ib.Config().Env)
 			if err != nil {
 				return "", nil, err
 			}
@@ -975,7 +976,7 @@ func (s *StageExecutor) Execute(ctx context.Context, stage imagebuilder.Stage, b
 				return "", nil, errors.Wrapf(err, "error building at STEP \"%s\"", step.Message)
 			}
 			// In case we added content, retrieve its digest.
-			addedContentDigest, err := s.digestSpecifiedContent(node, ib.Arguments())
+			addedContentDigest, err := s.digestSpecifiedContent(node, ib.Arguments(), ib.Config().Env)
 			if err != nil {
 				return "", nil, err
 			}
diff --git a/tests/bud.bats b/tests/bud.bats
index 022088b72..966864cae 100644
--- a/tests/bud.bats
+++ b/tests/bud.bats
@@ -1882,3 +1882,15 @@ EOM
   run_buildah 1 bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/copy/Dockerfile.url ${TESTSDIR}/bud/copy
   rm -r ${TESTSDIR}/bud/copy
 }
+
+@test "bud COPY with Env Var in Containerfile" {
+  run_buildah bud --signature-policy ${TESTSDIR}/policy.json -t testctr ${TESTSDIR}/bud/copy-envvar
+  run_buildah from testctr
+  run_buildah run testctr-working-container ls /file-0.0.1.txt
+  run_buildah rm -a
+
+  run_buildah bud --signature-policy ${TESTSDIR}/policy.json --layers -t testctr ${TESTSDIR}/bud/copy-envvar
+  run_buildah from testctr
+  run_buildah run testctr-working-container ls /file-0.0.1.txt
+  run_buildah rm -a
+}
diff --git a/tests/bud/copy-envvar/Containerfile b/tests/bud/copy-envvar/Containerfile
new file mode 100644
index 000000000..0e8c9109d
--- /dev/null
+++ b/tests/bud/copy-envvar/Containerfile
@@ -0,0 +1,3 @@
+FROM alpine 
+ENV VERSION=0.0.1
+COPY file-${VERSION}.txt /
diff --git a/tests/bud/copy-envvar/file-0.0.1.txt b/tests/bud/copy-envvar/file-0.0.1.txt
new file mode 100644
index 000000000..e69de29bb