From fbf4511d7974f156690e262dbe789cbd820b461a Mon Sep 17 00:00:00 2001 From: Nalin Dahyabhai Date: Thu, 11 Apr 2024 15:48:57 -0400 Subject: [PATCH] Don't expand RUN heredocs ourselves, let the shell do it When handling RUN instructions that use heredoc syntax, don't bother interpolating environment variables and argument values, and let the command that's running handle it. Signed-off-by: Nalin Dahyabhai --- docker/types.go | 7 +- go.mod | 4 +- go.sum | 8 +- tests/conformance/conformance_test.go | 7 + .../testdata/Dockerfile.heredoc-quoting | 215 ++++++++++++++++++ .../fsouza/go-dockerclient/container.go | 8 +- .../openshift/imagebuilder/.travis.yml | 4 + .../openshift/imagebuilder/dispatchers.go | 18 +- .../imagebuilder/dockerclient/client.go | 2 +- .../openshift/imagebuilder/imagebuilder.spec | 4 +- vendor/modules.txt | 6 +- 11 files changed, 260 insertions(+), 23 deletions(-) create mode 100644 tests/conformance/testdata/Dockerfile.heredoc-quoting diff --git a/docker/types.go b/docker/types.go index b0ed2e4c021..275951d039f 100644 --- a/docker/types.go +++ b/docker/types.go @@ -60,9 +60,10 @@ type HealthConfig struct { Test []string `json:",omitempty"` // Zero means to inherit. Durations are expressed as integer nanoseconds. - Interval time.Duration `json:",omitempty"` // Interval is the time to wait between checks. - Timeout time.Duration `json:",omitempty"` // Timeout is the time to wait before considering the check to have hung. - StartPeriod time.Duration `json:",omitempty"` // Time to wait after the container starts before running the first check. + Interval time.Duration `json:",omitempty"` // Interval is the time to wait between checks. + Timeout time.Duration `json:",omitempty"` // Timeout is the time to wait before considering the check to have hung. + StartPeriod time.Duration `json:",omitempty"` // Time to wait after the container starts before running the first check. + StartInterval time.Duration `json:",omitempty"` // Time to wait between checks during the StartPeriod. // Retries is the number of consecutive failures needed to consider a container as unhealthy. // Zero means inherit. diff --git a/go.mod b/go.mod index 1903330b4ac..8cc74f81703 100644 --- a/go.mod +++ b/go.mod @@ -29,7 +29,7 @@ require ( github.com/docker/distribution v2.8.3+incompatible github.com/docker/docker v26.1.2+incompatible github.com/docker/go-units v0.5.0 - github.com/fsouza/go-dockerclient v1.10.1 + github.com/fsouza/go-dockerclient v1.11.0 github.com/hashicorp/go-multierror v1.1.1 github.com/mattn/go-shellwords v1.0.12 github.com/moby/buildkit v0.12.5 @@ -41,7 +41,7 @@ require ( github.com/opencontainers/runtime-spec v1.2.0 github.com/opencontainers/runtime-tools v0.9.1-0.20230914150019-408c51e934dc github.com/opencontainers/selinux v1.11.0 - github.com/openshift/imagebuilder v1.2.6 + github.com/openshift/imagebuilder v1.2.9 github.com/seccomp/libseccomp-golang v0.10.0 github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.8.0 diff --git a/go.sum b/go.sum index 9beb654a7d4..9ab52f9c322 100644 --- a/go.sum +++ b/go.sum @@ -116,8 +116,8 @@ github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMo github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= -github.com/fsouza/go-dockerclient v1.10.1 h1:bSU5Wu2ARdub+iv9VtoDsN8yBUI0vgflmshbeQLKhvc= -github.com/fsouza/go-dockerclient v1.10.1/go.mod h1:dyzGriw6v3pK4O4O1u/X+vXxDDsrnLLkCqYkcLsDq2k= +github.com/fsouza/go-dockerclient v1.11.0 h1:4ZAk6W7rPAtPXm7198EFqA5S68rwnNQORxlOA5OurCA= +github.com/fsouza/go-dockerclient v1.11.0/go.mod h1:0I3TQCRseuPTzqlY4Y3ajfsg2VAdMQoazrkxJTiJg8s= github.com/go-jose/go-jose/v3 v3.0.3 h1:fFKWeig/irsp7XD2zBxvnmA/XaRWp5V3CBsZXJF7G7k= github.com/go-jose/go-jose/v3 v3.0.3/go.mod h1:5b+7YgP7ZICgJDBdfjZaIt+H/9L9T/YQrVfLAMboGkQ= github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas= @@ -294,8 +294,8 @@ github.com/opencontainers/runtime-tools v0.9.1-0.20230914150019-408c51e934dc h1: github.com/opencontainers/runtime-tools v0.9.1-0.20230914150019-408c51e934dc/go.mod h1:8tx1helyqhUC65McMm3x7HmOex8lO2/v9zPuxmKHurs= github.com/opencontainers/selinux v1.11.0 h1:+5Zbo97w3Lbmb3PeqQtpmTkMwsW5nRI3YaLpt7tQ7oU= github.com/opencontainers/selinux v1.11.0/go.mod h1:E5dMC3VPuVvVHDYmi78qvhJp8+M586T4DlDRYpFkyec= -github.com/openshift/imagebuilder v1.2.6 h1:ge+HILDVaB3c65KhH0nrM/Z1f9EdN8NUqxigd4qGqqo= -github.com/openshift/imagebuilder v1.2.6/go.mod h1:6VbTJ5CK7+OOTWcQlc/Cp86ML7pKlxOwCJNESQPbtgw= +github.com/openshift/imagebuilder v1.2.9 h1:830/kg5FWtpLsQ6JcCQ23qOeb/KfzMK66pai544rAUI= +github.com/openshift/imagebuilder v1.2.9/go.mod h1:KkkXOyRjJlZEXWQtHNBNzVHqh4vf/0xX5cDIQ2gr+5I= github.com/ostreedev/ostree-go v0.0.0-20210805093236-719684c64e4f h1:/UDgs8FGMqwnHagNDPGOlts35QkhAZ8by3DR7nMih7M= github.com/ostreedev/ostree-go v0.0.0-20210805093236-719684c64e4f/go.mod h1:J6OG6YJVEWopen4avK3VNQSnALmmjvniMmni/YFYAwc= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= diff --git a/tests/conformance/conformance_test.go b/tests/conformance/conformance_test.go index 53f6f8233eb..5e61b6af4a5 100644 --- a/tests/conformance/conformance_test.go +++ b/tests/conformance/conformance_test.go @@ -3102,6 +3102,13 @@ var internalTestCases = []testCase{ contextDir: "multistage/copyback", dockerUseBuildKit: true, }, + + { + name: "heredoc-quoting", + dockerfile: "Dockerfile.heredoc-quoting", + dockerUseBuildKit: true, + fsSkip: []string{"(dir):etc:(dir):hostname"}, // buildkit does not create a phantom /etc/hostname + }, } func TestCommit(t *testing.T) { diff --git a/tests/conformance/testdata/Dockerfile.heredoc-quoting b/tests/conformance/testdata/Dockerfile.heredoc-quoting new file mode 100644 index 00000000000..cf83b364396 --- /dev/null +++ b/tests/conformance/testdata/Dockerfile.heredoc-quoting @@ -0,0 +1,215 @@ +FROM busybox +ARG argA=argvA +ENV varA=valueA + +# An argument, an environment variable, and one set in the heredoc +RUN <, --chown=, and --checksum= flags") } } - files, err := processHereDocs(original, heredocs, userArgs) + files, err := processHereDocs(buildkitcommand.Add, original, heredocs, userArgs) if err != nil { return err } @@ -256,7 +257,7 @@ func dispatchCopy(b *Builder, args []string, attributes map[string]bool, flagArg return fmt.Errorf("COPY only supports the --chmod= --chown= and the --from= flags") } } - files, err := processHereDocs(original, heredocs, userArgs) + files, err := processHereDocs(buildkitcommand.Copy, original, heredocs, userArgs) if err != nil { return err } @@ -422,7 +423,7 @@ func run(b *Builder, args []string, attributes map[string]bool, flagArgs []strin } } - files, err := processHereDocs(original, heredocs, userArgs) + files, err := processHereDocs(buildkitcommand.Run, original, heredocs, userArgs) if err != nil { return err } @@ -606,6 +607,7 @@ func healthcheck(b *Builder, args []string, attributes map[string]bool, flagArgs flags := flag.NewFlagSet("", flag.ContinueOnError) flags.String("start-period", "", "") + flags.String("start-interval", "", "") flags.String("interval", "", "") flags.String("timeout", "", "") flRetries := flags.String("retries", "", "") @@ -642,6 +644,12 @@ func healthcheck(b *Builder, args []string, attributes map[string]bool, flagArgs } healthcheck.Interval = interval + startInterval, err := parseOptInterval(flags.Lookup("start-interval")) + if err != nil { + return err + } + healthcheck.StartInterval = startInterval + timeout, err := parseOptInterval(flags.Lookup("timeout")) if err != nil { return err diff --git a/vendor/github.com/openshift/imagebuilder/dockerclient/client.go b/vendor/github.com/openshift/imagebuilder/dockerclient/client.go index 1e8c83f8697..7e9f35681a4 100644 --- a/vendor/github.com/openshift/imagebuilder/dockerclient/client.go +++ b/vendor/github.com/openshift/imagebuilder/dockerclient/client.go @@ -1058,7 +1058,7 @@ func (e *ClientExecutor) CopyContainer(container *docker.Container, excludes []s } chmod = func(h *tar.Header, r io.Reader) (data []byte, update bool, skip bool, err error) { mode := h.Mode &^ 0o777 - mode |= parsed & 0o777 + mode |= parsed & 0o7777 h.Mode = mode return nil, false, false, nil } diff --git a/vendor/github.com/openshift/imagebuilder/imagebuilder.spec b/vendor/github.com/openshift/imagebuilder/imagebuilder.spec index 5d4f2b70133..7819bdc8ba8 100644 --- a/vendor/github.com/openshift/imagebuilder/imagebuilder.spec +++ b/vendor/github.com/openshift/imagebuilder/imagebuilder.spec @@ -11,8 +11,8 @@ # Customize from here. # -%global golang_version 1.8.1 -%{!?version: %global version 1.2.6} +%global golang_version 1.19 +%{!?version: %global version 1.2.9} %{!?release: %global release 1} %global package_name imagebuilder %global product_name Container Image Builder diff --git a/vendor/modules.txt b/vendor/modules.txt index a1c53ce0e9e..69442fec1b1 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -377,8 +377,8 @@ github.com/felixge/httpsnoop # github.com/fsnotify/fsnotify v1.7.0 ## explicit; go 1.17 github.com/fsnotify/fsnotify -# github.com/fsouza/go-dockerclient v1.10.1 -## explicit; go 1.20 +# github.com/fsouza/go-dockerclient v1.11.0 +## explicit; go 1.21 github.com/fsouza/go-dockerclient # github.com/go-jose/go-jose/v3 v3.0.3 ## explicit; go 1.12 @@ -644,7 +644,7 @@ github.com/opencontainers/selinux/go-selinux github.com/opencontainers/selinux/go-selinux/label github.com/opencontainers/selinux/pkg/pwalk github.com/opencontainers/selinux/pkg/pwalkdir -# github.com/openshift/imagebuilder v1.2.6 +# github.com/openshift/imagebuilder v1.2.9 ## explicit; go 1.19 github.com/openshift/imagebuilder github.com/openshift/imagebuilder/dockerclient