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