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 <dwalsh@redhat.com>

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
 <a name="Footnote1">1</a>: 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.

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() {
+	defer pipe.Close()
-		if err != nil {
-			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 {
+		if stdout.Len() == 0 {
-			err = errors.Wrapf(err, "%v", strings.TrimSpace(stderr.String()))
+			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()))

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 <stdin>: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

tests/bud/containerfile/Containerfile

@@ -1 +1,2 @@
+# This is for testing Containerfile with Buildah
 FROM alpine

tests/bud/containerfile/Containerfile.in

@@ -0,0 +1,2 @@
+# include "Containerfile"
+RUN echo "success"