From a9c22e10013d5612f45d0030b9c78a46353f234a Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 19 May 2021 09:05:47 -0400 Subject: [PATCH] Don't blow up if cpp detects errors Currently the /usr/bin/cpp will blow up if a user adds a comment to a containerfile that is not a preprocessor. Since the Containerfile.in could include other Containerfile which may have comments, begining with `#` this can cause problems. If we just warn on these errors, we can successfully process all of the containerfiles. Fixes: https://github.com/containers/buildah/issues/3229 Signed-off-by: Daniel J Walsh --- docs/buildah-bud.md | 6 ++++-- imagebuildah/build.go | 20 ++++++++------------ tests/bud.bats | 12 +++++++++++- tests/bud/containerfile/Containerfile | 1 + tests/bud/containerfile/Containerfile.in | 2 ++ 5 files changed, 26 insertions(+), 15 deletions(-) create mode 100644 tests/bud/containerfile/Containerfile.in diff --git a/docs/buildah-bud.md b/docs/buildah-bud.md index b6ad8795d..8b528e089 100644 --- a/docs/buildah-bud.md +++ b/docs/buildah-bud.md @@ -20,7 +20,7 @@ The build context directory can be specified as the http(s) URL of an archive, g If no context directory is specified, then Buildah will assume the current working directory as build context, which should contain a Containerfile. -Containerfiles ending with a ".in" suffix will be preprocessed via CPP(1). This can be useful to decompose Containerfiles into several reusable parts that can be used via CPP's **#include** directive. Notice, a Containerfile.in file can still be used by other tools when manually preprocessing them via `cpp -E`. +Containerfiles ending with a ".in" suffix will be preprocessed via cpp(1). This can be useful to decompose Containerfiles into several reusable parts that can be used via CPP's **#include** directive. Notice, a Containerfile.in file can still be used by other tools when manually preprocessing them via `cpp -E`. Any comments ( Lines beginning with `#` ) in included Containerfile(s) that are not preprocess commands, will be printed as warnings during builds. When the URL is an archive, the contents of the URL is downloaded to a temporary location and extracted before execution. @@ -762,6 +762,8 @@ buildah bud --no-cache --rm=false -t imageName . buildah bud --dns-search=example.com --dns=223.5.5.5 --dns-option=use-vc . +buildah bud -f Containerfile.in -t imageName . + ### Building an multi-architecture image using a --manifest option (Requires emulation software) buildah bud --arch arm --manifest myimage /tmp/mysrc @@ -861,7 +863,7 @@ registries.conf is the configuration file which specifies which container regist Signature policy file. This defines the trust policy for container images. Controls which container registries can be used for image, and whether or not the tool should trust the images. ## SEE ALSO -buildah(1), CPP(1), buildah-login(1), docker-login(1), namespaces(7), pid\_namespaces(7), containers-policy.json(5), containers-registries.conf(5), user\_namespaces(7), crun(1), runc(8) +buildah(1), cpp(1), buildah-login(1), docker-login(1), namespaces(7), pid\_namespaces(7), containers-policy.json(5), containers-registries.conf(5), user\_namespaces(7), crun(1), runc(8) ## FOOTNOTES 1: The Buildah project is committed to inclusivity, a core value of open source. The `master` and `slave` mount propagation terminology used here is problematic and divisive, and should be changed. However, these terms are currently used within the Linux kernel and must be used as-is at this time. When the kernel maintainers rectify this usage, Buildah will follow suit immediately. diff --git a/imagebuildah/build.go b/imagebuildah/build.go index 76796de72..bfbbb085d 100644 --- a/imagebuildah/build.go +++ b/imagebuildah/build.go @@ -133,7 +133,7 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options define.B // pre-process Dockerfiles with ".in" suffix if strings.HasSuffix(dfile, ".in") { - pData, err := preprocessDockerfileContents(data, options.ContextDirectory) + pData, err := preprocessContainerfileContents(dfile, data, options.ContextDirectory) if err != nil { return "", nil, err } @@ -206,15 +206,15 @@ func warnOnUnsetBuildArgs(logger *logrus.Logger, node *parser.Node, args map[str } } -// preprocessDockerfileContents runs CPP(1) in preprocess-only mode on the input +// preprocessContainerfileContents runs CPP(1) in preprocess-only mode on the input // dockerfile content and will use ctxDir as the base include path. // // Note: we cannot use cmd.StdoutPipe() as cmd.Wait() closes it. -func preprocessDockerfileContents(r io.Reader, ctxDir string) (rdrCloser *io.ReadCloser, err error) { +func preprocessContainerfileContents(containerfile string, r io.Reader, ctxDir string) (rdrCloser *io.ReadCloser, err error) { cppPath := "/usr/bin/cpp" if _, err = os.Stat(cppPath); err != nil { if os.IsNotExist(err) { - err = errors.Errorf("error: Dockerfile.in support requires %s to be installed", cppPath) + err = errors.Errorf("error: %s support requires %s to be installed", containerfile, cppPath) } return nil, err } @@ -231,11 +231,7 @@ func preprocessDockerfileContents(r io.Reader, ctxDir string) (rdrCloser *io.Rea return nil, err } - defer func() { - if err != nil { - pipe.Close() - } - }() + defer pipe.Close() if err = cmd.Start(); err != nil { return nil, err @@ -247,10 +243,10 @@ func preprocessDockerfileContents(r io.Reader, ctxDir string) (rdrCloser *io.Rea pipe.Close() if err = cmd.Wait(); err != nil { - if stderr.Len() > 0 { - err = errors.Wrapf(err, "%v", strings.TrimSpace(stderr.String())) + if stdout.Len() == 0 { + return nil, errors.Wrapf(err, "error pre-processing Dockerfile") } - return nil, errors.Wrapf(err, "error pre-processing Dockerfile") + logrus.Warnf("Ignoring %s\n", stderr.String()) } rc := ioutil.NopCloser(bytes.NewReader(stdout.Bytes())) diff --git a/tests/bud.bats b/tests/bud.bats index dd94c8312..1a623b6ae 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -1181,7 +1181,8 @@ function _test_http() { @test "bud with preprocessor error" { target=alpine-image - run_buildah 1 bud -q --signature-policy ${TESTSDIR}/policy.json -t ${target} -f Error.in ${TESTSDIR}/bud/preprocess + run_buildah 0 bud -q --signature-policy ${TESTSDIR}/policy.json -t ${target} -f Error.in ${TESTSDIR}/bud/preprocess + expect_output --substring "Ignoring :5:2: error: #error" } @test "bud-with-rejected-name" { @@ -2047,6 +2048,15 @@ _EOF expect_output --substring "FROM alpine" } +@test "bud with Containerfile.in" { + _prefetch alpine + target=alpine-image + run_buildah bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/containerfile/Containerfile.in ${TESTSDIR}/bud/containerfile + [ "${status}" -eq 0 ] + expect_output --substring "FROM alpine" + expect_output --substring "success" +} + @test "bud with Dockerfile" { _prefetch alpine target=alpine-image diff --git a/tests/bud/containerfile/Containerfile b/tests/bud/containerfile/Containerfile index 67fd37901..0f9708ab8 100644 --- a/tests/bud/containerfile/Containerfile +++ b/tests/bud/containerfile/Containerfile @@ -1 +1,2 @@ +# This is for testing Containerfile with Buildah FROM alpine diff --git a/tests/bud/containerfile/Containerfile.in b/tests/bud/containerfile/Containerfile.in new file mode 100644 index 000000000..c387da0bd --- /dev/null +++ b/tests/bud/containerfile/Containerfile.in @@ -0,0 +1,2 @@ +# include "Containerfile" +RUN echo "success"