From 49ab77998f85a05f05814f6575b25de264bf8256 Mon Sep 17 00:00:00 2001 From: TomSweeneyRedHat 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 --- 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