Merge pull request #9592 from rhatdan/timestamp

Numerous buildah fixes found by Ed's testing of buildah tests against podman.

cmd/podman/images/build.go

@@ -2,6 +2,7 @@ package images
 
 import (
 	"io"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -19,7 +20,6 @@ import (
 	"github.com/containers/podman/v3/cmd/podman/registry"
 	"github.com/containers/podman/v3/cmd/podman/utils"
 	"github.com/containers/podman/v3/pkg/domain/entities"
-	"github.com/docker/go-units"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
@@ -298,6 +298,11 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
 		}
 	}
 
+	commonOpts, err := parse.CommonBuildOptions(c)
+	if err != nil {
+		return nil, err
+	}
+
 	pullPolicy := imagebuildah.PullIfMissing
 	if c.Flags().Changed("pull") && flags.Pull {
 		pullPolicy = imagebuildah.PullAlways
@@ -317,7 +322,12 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
 			if len(av) > 1 {
 				args[av[0]] = av[1]
 			} else {
-				delete(args, av[0])
+				// check if the env is set in the local environment and use that value if it is
+				if val, present := os.LookupEnv(av[0]); present {
+					args[av[0]] = val
+				} else {
+					delete(args, av[0])
+				}
 			}
 		}
 	}
@@ -356,22 +366,6 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
 		reporter = logfile
 	}
 
-	var memoryLimit, memorySwap int64
-	var err error
-	if c.Flags().Changed("memory") {
-		memoryLimit, err = units.RAMInBytes(flags.Memory)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	if c.Flags().Changed("memory-swap") {
-		memorySwap, err = units.RAMInBytes(flags.MemorySwap)
-		if err != nil {
-			return nil, err
-		}
-	}
-
 	nsValues, networkPolicy, err := parse.NamespaceOptions(c)
 	if err != nil {
 		return nil, err
@@ -449,29 +443,15 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
 	}
 
 	opts := imagebuildah.BuildOptions{
-		AddCapabilities: flags.CapAdd,
+		AddCapabilities:         flags.CapAdd,
-		AdditionalTags:  tags,
+		AdditionalTags:          tags,
-		Annotations:     flags.Annotation,
+		Annotations:             flags.Annotation,
-		Architecture:    arch,
+		Architecture:            arch,
-		Args:            args,
+		Args:                    args,
-		BlobDirectory:   flags.BlobCache,
+		BlobDirectory:           flags.BlobCache,
-		CNIConfigDir:    flags.CNIConfigDir,
+		CNIConfigDir:            flags.CNIConfigDir,
-		CNIPluginPath:   flags.CNIPlugInPath,
+		CNIPluginPath:           flags.CNIPlugInPath,
-		CommonBuildOpts: &buildah.CommonBuildOptions{
+		CommonBuildOpts:         commonOpts,
-			AddHost:      flags.AddHost,
-			CPUPeriod:    flags.CPUPeriod,
-			CPUQuota:     flags.CPUQuota,
-			CPUSetCPUs:   flags.CPUSetCPUs,
-			CPUSetMems:   flags.CPUSetMems,
-			CPUShares:    flags.CPUShares,
-			CgroupParent: flags.CgroupParent,
-			HTTPProxy:    flags.HTTPProxy,
-			Memory:       memoryLimit,
-			MemorySwap:   memorySwap,
-			ShmSize:      flags.ShmSize,
-			Ulimit:       flags.Ulimit,
-			Volumes:      flags.Volumes,
-		},
 		Compression:             compression,
 		ConfigureNetwork:        networkPolicy,
 		ContextDirectory:        contextDir,
@@ -512,6 +492,14 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
 		TransientMounts:         flags.Volumes,
 	}
 
+	if flags.IgnoreFile != "" {
+		excludes, err := parseDockerignore(flags.IgnoreFile)
+		if err != nil {
+			return nil, errors.Wrapf(err, "unable to obtain decrypt config")
+		}
+		opts.Excludes = excludes
+	}
+
 	if c.Flag("timestamp").Changed {
 		timestamp := time.Unix(flags.Timestamp, 0).UTC()
 		opts.Timestamp = &timestamp
@@ -534,3 +522,18 @@ func getDecryptConfig(decryptionKeys []string) (*encconfig.DecryptConfig, error)
 
 	return decConfig, nil
 }
+
+func parseDockerignore(ignoreFile string) ([]string, error) {
+	excludes := []string{}
+	ignore, err := ioutil.ReadFile(ignoreFile)
+	if err != nil {
+		return excludes, err
+	}
+	for _, e := range strings.Split(string(ignore), "\n") {
+		if len(e) == 0 || e[0] == '#' {
+			continue
+		}
+		excludes = append(excludes, e)
+	}
+	return excludes, nil
+}

docs/source/markdown/podman-build.1.md

@@ -259,7 +259,7 @@ solely for scripting compatibility.
 
 #### **--dns**=*dns*
 
-Set custom DNS servers
+Set custom DNS servers to be used during the build.
 
 This option can be used to override the DNS configuration passed to the
 container. Typically this is necessary when the host DNS configuration is
@@ -272,11 +272,11 @@ image will be used without changes.
 
 #### **--dns-option**=*option*
 
-Set custom DNS options
+Set custom DNS options to be used during the build.
 
 #### **--dns-search**=*domain*
 
-Set custom DNS search domains
+Set custom DNS search domains to be used during the build.
 
 #### **--file**, **-f**=*Containerfile*
 

pkg/api/handlers/compat/images_build.go

@@ -77,6 +77,9 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		Devices                string `schema:"devices"`
 		Dockerfile             string `schema:"dockerfile"`
 		DropCapabilities       string `schema:"dropcaps"`
+		DNSServers             string `schema:"dnsservers"`
+		DNSOptions             string `schema:"dnsoptions"`
+		DNSSearch              string `schema:"dnssearch"`
 		Excludes               string `schema:"excludes"`
 		ForceRm                bool   `schema:"forcerm"`
 		From                   string `schema:"from"`
@@ -160,6 +163,36 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		devices = m
 	}
 
+	var dnsservers = []string{}
+	if _, found := r.URL.Query()["dnsservers"]; found {
+		var m = []string{}
+		if err := json.Unmarshal([]byte(query.DNSServers), &m); err != nil {
+			utils.BadRequest(w, "dnsservers", query.DNSServers, err)
+			return
+		}
+		dnsservers = m
+	}
+
+	var dnsoptions = []string{}
+	if _, found := r.URL.Query()["dnsoptions"]; found {
+		var m = []string{}
+		if err := json.Unmarshal([]byte(query.DNSOptions), &m); err != nil {
+			utils.BadRequest(w, "dnsoptions", query.DNSOptions, err)
+			return
+		}
+		dnsoptions = m
+	}
+
+	var dnssearch = []string{}
+	if _, found := r.URL.Query()["dnssearch"]; found {
+		var m = []string{}
+		if err := json.Unmarshal([]byte(query.DNSSearch), &m); err != nil {
+			utils.BadRequest(w, "dnssearches", query.DNSSearch, err)
+			return
+		}
+		dnssearch = m
+	}
+
 	var output string
 	if len(query.Tag) > 0 {
 		output = query.Tag[0]
@@ -285,6 +318,9 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 			CPUQuota:   query.CpuQuota,
 			CPUShares:  query.CpuShares,
 			CPUSetCPUs: query.CpuSetCpus,
+			DNSServers: dnsservers,
+			DNSOptions: dnsoptions,
+			DNSSearch:  dnssearch,
 			HTTPProxy:  query.HTTPProxy,
 			Memory:     query.Memory,
 			MemorySwap: query.MemSwap,

pkg/bindings/images/build.go

@@ -87,6 +87,28 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 		params.Add("devices", d)
 	}
 
+	if dnsservers := options.CommonBuildOpts.DNSServers; len(dnsservers) > 0 {
+		c, err := jsoniter.MarshalToString(dnsservers)
+		if err != nil {
+			return nil, err
+		}
+		params.Add("dnsservers", c)
+	}
+	if dnsoptions := options.CommonBuildOpts.DNSOptions; len(dnsoptions) > 0 {
+		c, err := jsoniter.MarshalToString(dnsoptions)
+		if err != nil {
+			return nil, err
+		}
+		params.Add("dnsoptions", c)
+	}
+	if dnssearch := options.CommonBuildOpts.DNSSearch; len(dnssearch) > 0 {
+		c, err := jsoniter.MarshalToString(dnssearch)
+		if err != nil {
+			return nil, err
+		}
+		params.Add("dnssearch", c)
+	}
+
 	if caps := options.DropCapabilities; len(caps) > 0 {
 		c, err := jsoniter.MarshalToString(caps)
 		if err != nil {

test/system/070-build.bats

@@ -168,6 +168,9 @@ EOF
         CAT_SECRET="cat /run/secrets/$secret_filename"
     fi
 
+    # For --dns-search: a domain that is unlikely to exist
+    local nosuchdomain=nx$(random_string 10).net
+
     # Command to run on container startup with no args
     cat >$tmpdir/mycmd <<EOF
 #!/bin/sh
@@ -188,11 +191,17 @@ EOF
 https_proxy=https-proxy-in-env-file
 EOF
 
+    # Build args: one explicit (foo=bar), one implicit (foo)
+    local arg_implicit_value=implicit_$(random_string 15)
+    local arg_explicit_value=explicit_$(random_string 15)
+
     # NOTE: it's important to not create the workdir.
     # Podman will make sure to create a missing workdir
     # if needed. See #9040.
     cat >$tmpdir/Containerfile <<EOF
 FROM $IMAGE
+ARG arg_explicit
+ARG arg_implicit
 LABEL $label_name=$label_value
 WORKDIR $workdir
 
@@ -217,18 +226,47 @@ RUN chown 2:3 /bin/mydefaultcmd
 
 RUN $CAT_SECRET
 
+RUN echo explicit-build-arg=\$arg_explicit
+RUN echo implicit-build-arg=\$arg_implicit
+
 CMD ["/bin/mydefaultcmd","$s_echo"]
+RUN cat /etc/resolv.conf
 EOF
 
+    # The goal is to test that a missing value will be inherited from
+    # environment - but that can't work with remote, so for simplicity
+    # just make it explicit in that case too.
+    local build_arg_implicit="--build-arg arg_implicit"
+    if is_remote; then
+        build_arg_implicit+="=$arg_implicit_value"
+    fi
+
     # cd to the dir, so we test relative paths (important for podman-remote)
     cd $PODMAN_TMPDIR
+    export arg_explicit="THIS SHOULD BE OVERRIDDEN BY COMMAND LINE!"
+    export arg_implicit=${arg_implicit_value}
     run_podman ${MOUNTS_CONF} build \
+               --build-arg arg_explicit=${arg_explicit_value} \
+               $build_arg_implicit \
+               --dns-search $nosuchdomain \
                -t build_test -f build-test/Containerfile build-test
     local iid="${lines[-1]}"
 
+    if [[ $output =~ missing.*build.argument ]]; then
+        die "podman did not see the given --build-arg(s)"
+    fi
+
     # Make sure 'podman build' had the secret mounted
     is "$output" ".*$secret_contents.*" "podman build has /run/secrets mounted"
 
+    # --build-arg should be set, both via 'foo=bar' and via just 'foo' ($foo)
+    is "$output" ".*explicit-build-arg=${arg_explicit_value}" \
+       "--build-arg arg_explicit=explicit-value works"
+    is "$output" ".*implicit-build-arg=${arg_implicit_value}" \
+       "--build-arg arg_implicit works (inheriting from environment)"
+    is "$output" ".*search $nosuchdomain" \
+       "--dns-search added to /etc/resolv.conf"
+
     if is_remote; then
         ENVHOST=""
     else
@@ -362,6 +400,82 @@ Labels.$label_name | $label_value
     run_podman rmi -f build_test
 }
 
+@test "podman build - COPY with ignore" {
+    local tmpdir=$PODMAN_TMPDIR/build-test-$(random_string 10)
+    mkdir -p $tmpdir/subdir
+
+    # Create a bunch of files. Declare this as an array to avoid duplication
+    # because we iterate over that list below, checking for each file.
+    # A leading "-" indicates that the file SHOULD NOT exist in the built image
+    local -a files=(
+        -test1 -test1.txt
+         test2  test2.txt
+         subdir/sub1  subdir/sub1.txt
+         -subdir/sub2 -subdir/sub2.txt
+         this-file-does-not-match-anything-in-ignore-file
+         comment
+    )
+    for f in ${files[@]}; do
+        # The magic '##-' strips off the '-' prefix
+        echo "$f" > $tmpdir/${f##-}
+    done
+
+    # Directory that doesn't exist in the image; COPY should create it
+    local newdir=/newdir-$(random_string 12)
+    cat >$tmpdir/Containerfile <<EOF
+FROM $IMAGE
+COPY ./ $newdir/
+EOF
+
+    # Run twice: first with a custom --ignorefile, then with a default one.
+    # This ordering is deliberate: if we were to run with .dockerignore
+    # first, and forget to rm it, and then run with --ignorefile, _and_
+    # there was a bug in podman where --ignorefile was a NOP (eg #9570),
+    # the test might pass because of the existence of .dockerfile.
+    for ignorefile in ignoreme-$(random_string 5) .dockerignore; do
+        # Patterns to ignore. Mostly copied from buildah/tests/bud/dockerignore
+        cat >$tmpdir/$ignorefile <<EOF
+# comment
+test*
+!test2*
+subdir
+!*/sub1*
+EOF
+
+        # Build an image. For .dockerignore
+        local -a ignoreflag
+	unset ignoreflag
+        if [[ $ignorefile != ".dockerignore" ]]; then
+            ignoreflag="--ignorefile $tmpdir/$ignorefile"
+        fi
+        run_podman build -t build_test ${ignoreflag} $tmpdir
+
+        # Delete the ignore file! Otherwise, in the next iteration of the loop,
+        # we could end up with an existing .dockerignore that invisibly
+        # takes precedence over --ignorefile
+        rm -f $tmpdir/$ignorefile
+
+        # It would be much more readable, and probably safer, to iterate
+        # over each file, running 'podman run ... ls -l $f'. But each podman run
+        # takes a second or so, and we are mindful of each second.
+        run_podman run --rm build_test find $newdir -type f
+        for f in ${files[@]}; do
+            if [[ $f =~ ^- ]]; then
+                f=${f##-}
+                if [[ $output =~ $f ]]; then
+                    die "File '$f' found in image; it should have been ignored via $ignorefile"
+                fi
+            else
+                is "$output" ".*$newdir/$f" \
+                   "File '$f' should exist in container (no match in $ignorefile)"
+            fi
+        done
+
+        # Clean up
+        run_podman rmi -f build_test
+    done
+}
+
 @test "podman build - stdin test" {
     # Random workdir, and random string to verify build output
     workdir=/$(random_string 10)