Fix podman build --pull-never

Currently pull policy is set incorrectly when users set --pull-never. Also pull-policy is not being translated correctly when using podman-remote. Fixes: containers#9573 Signed-off-by: Daniel J Walsh <[email protected]>
jmguzik · Apr 26, 2021 · a6068f1 · a6068f1
1 parent f398fef
commit a6068f1
Show file tree

Hide file tree

Showing 23 changed files with 142 additions and 88 deletions.
diff --git a/cmd/podman/images/build.go b/cmd/podman/images/build.go
@@ -303,6 +303,21 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
 		return nil, err
 	}
 
+	pullFlagsCount := 0
+	if c.Flag("pull").Changed {
+		pullFlagsCount++
+	}
+	if c.Flag("pull-always").Changed {
+		pullFlagsCount++
+	}
+	if c.Flag("pull-never").Changed {
+		pullFlagsCount++
+	}
+
+	if pullFlagsCount > 1 {
+		return nil, errors.Errorf("can only set one of 'pull' or 'pull-always' or 'pull-never'")
+	}
+
 	pullPolicy := define.PullIfMissing
 	if c.Flags().Changed("pull") && flags.Pull {
 		pullPolicy = define.PullAlways
@@ -312,7 +327,7 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
 	}
 
 	if flags.PullNever {
-		pullPolicy = define.PullIfMissing
+		pullPolicy = define.PullNever
 	}
 
 	args := make(map[string]string)

diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go
@@ -13,6 +13,7 @@ import (
 	"time"
 
 	"github.com/containers/buildah"
+	"github.com/containers/buildah/define"
 	"github.com/containers/buildah/imagebuildah"
 	"github.com/containers/buildah/util"
 	"github.com/containers/image/v5/types"
@@ -98,6 +99,7 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		OutputFormat           string `schema:"outputformat"`
 		Platform               string `schema:"platform"`
 		Pull                   bool   `schema:"pull"`
+		PullPolicy             string `schema:"pullpolicy"`
 		Quiet                  bool   `schema:"q"`
 		Registry               string `schema:"registry"`
 		Rm                     bool   `schema:"rm"`
@@ -275,10 +277,14 @@ func BuildImage(w http.ResponseWriter, r *http.Request) {
 		jobs = query.Jobs
 	}
 
-	pullPolicy := buildah.PullIfMissing
-	if _, found := r.URL.Query()["pull"]; found {
-		if query.Pull {
-			pullPolicy = buildah.PullAlways
+	pullPolicy := define.PullIfMissing
+	if utils.IsLibpodRequest(r) {
+		pullPolicy = define.PolicyMap[query.PullPolicy]
+	} else {
+		if _, found := r.URL.Query()["pull"]; found {
+			if query.Pull {
+				pullPolicy = define.PullAlways
+			}
 		}
 	}
 

diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go
@@ -15,7 +15,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/containers/buildah"
 	"github.com/containers/podman/v3/pkg/auth"
 	"github.com/containers/podman/v3/pkg/bindings"
 	"github.com/containers/podman/v3/pkg/domain/entities"
@@ -175,9 +174,9 @@ func Build(ctx context.Context, containerFiles []string, options entities.BuildO
 	if len(platform) > 0 {
 		params.Set("platform", platform)
 	}
-	if options.PullPolicy == buildah.PullAlways {
-		params.Set("pull", "1")
-	}
+
+	params.Set("pullpolicy", options.PullPolicy.String())
+
 	if options.Quiet {
 		params.Set("q", "1")
 	}

diff --git a/test/e2e/build/basicalpine/Containerfile b/test/e2e/build/basicalpine/Containerfile
@@ -1 +1 @@
-FROM alpine
+FROM quay.io/libpod/alpine:latest
diff --git a/test/e2e/build/basicalpine/Containerfile.path b/test/e2e/build/basicalpine/Containerfile.path
@@ -1,2 +1,2 @@
-FROM alpine
+FROM quay.io/libpod/alpine:latest
 ENV PATH=/tmp:/bin:/usr/bin:/usr/sbin
diff --git a/test/e2e/build/basicalpine/Containerfile.volume b/test/e2e/build/basicalpine/Containerfile.volume
@@ -1,2 +1,2 @@
-FROM alpine
+FROM quay.io/libpod/alpine:latest
 VOLUME "/volume0"
diff --git a/test/e2e/build/squash/Dockerfile.squash-a b/test/e2e/build/squash/Dockerfile.squash-a
@@ -1,2 +1,2 @@
-FROM busybox:latest
+FROM quay.io/libpod/busybox:latest
 ADD alpinetest.tgz /data
diff --git a/test/e2e/build/squash/Dockerfile.squash-c b/test/e2e/build/squash/Dockerfile.squash-c
@@ -1,3 +1,3 @@
-FROM busybox:latest
+FROM quay.io/libpod/busybox:latest
 ADD alpinetest.tgz /data
 RUN rm -rf /data
diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go
@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -150,7 +151,7 @@ var _ = Describe("Podman build", func() {
 		}
 
 		fakeFile := filepath.Join(os.TempDir(), "Containerfile")
-		Expect(ioutil.WriteFile(fakeFile, []byte("FROM alpine"), 0755)).To(BeNil())
+		Expect(ioutil.WriteFile(fakeFile, []byte(fmt.Sprintf("FROM %s", ALPINE)), 0755)).To(BeNil())
 
 		targetFile := filepath.Join(targetPath, "Containerfile")
 		Expect(ioutil.WriteFile(targetFile, []byte("FROM scratch"), 0755)).To(BeNil())
@@ -219,8 +220,8 @@ var _ = Describe("Podman build", func() {
 			podmanTest.StartRemoteService()
 		}
 		podmanTest.AddImageToRWStore(ALPINE)
-		dockerfile := `FROM quay.io/libpod/alpine:latest
-RUN printenv http_proxy`
+		dockerfile := fmt.Sprintf(`FROM %s
+RUN printenv http_proxy`, ALPINE)
 
 		dockerfilePath := filepath.Join(podmanTest.TempDir, "Dockerfile")
 		err := ioutil.WriteFile(dockerfilePath, []byte(dockerfile), 0755)
@@ -263,9 +264,9 @@ RUN printenv http_proxy`
 		err = ioutil.WriteFile(dummyFile, []byte("dummy"), 0644)
 		Expect(err).To(BeNil())
 
-		containerfile := `FROM quay.io/libpod/alpine:latest
+		containerfile := fmt.Sprintf(`FROM %s
 ADD . /test
-RUN find /test`
+RUN find /test`, ALPINE)
 
 		containerfilePath := filepath.Join(targetPath, "Containerfile")
 		err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644)
@@ -307,7 +308,7 @@ RUN find /test`
 		err = os.Mkdir(targetSubPath, 0755)
 		Expect(err).To(BeNil())
 
-		containerfile := `FROM quay.io/libpod/alpine:latest`
+		containerfile := fmt.Sprintf("FROM %s", ALPINE)
 
 		containerfilePath := filepath.Join(targetSubPath, "Containerfile")
 		err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644)
@@ -344,9 +345,9 @@ RUN find /test`
 		targetPath, err := CreateTempDirInTempDir()
 		Expect(err).To(BeNil())
 
-		containerfile := `FROM quay.io/libpod/alpine:latest
+		containerfile := fmt.Sprintf(`FROM %s
 ADD . /testfilter/
-RUN find /testfilter/`
+RUN find /testfilter/`, ALPINE)
 
 		containerfilePath := filepath.Join(targetPath, "Containerfile")
 		err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644)
@@ -428,10 +429,10 @@ subdir**`
 		Expect(os.Chdir(targetSubPath)).To(BeNil())
 		Expect(os.Symlink("dummy", "dummy-symlink")).To(BeNil())
 
-		containerfile := `FROM quay.io/libpod/alpine:latest
+		containerfile := fmt.Sprintf(`FROM %s
 ADD . /test
 RUN find /test
-RUN [[ -L /test/dummy-symlink ]] && echo SYMLNKOK || echo SYMLNKERR`
+RUN [[ -L /test/dummy-symlink ]] && echo SYMLNKOK || echo SYMLNKERR`, ALPINE)
 
 		containerfilePath := filepath.Join(targetSubPath, "Containerfile")
 		err = ioutil.WriteFile(containerfilePath, []byte(containerfile), 0644)
@@ -475,14 +476,14 @@ RUN grep CapEff /proc/self/status`
 
 		// When
 		session := podmanTest.Podman([]string{
-			"build", "--pull-never", "--cap-drop=all", "--cap-add=net_bind_service", "--add-host", "testhost:1.2.3.4", "--from", "alpine", targetPath,
+			"build", "--pull-never", "--cap-drop=all", "--cap-add=net_bind_service", "--add-host", "testhost:1.2.3.4", "--from", ALPINE, targetPath,
 		})
 		session.WaitWithDefaultTimeout()
 
 		// Then
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(strings.Fields(session.OutputToString())).
-			To(ContainElement("alpine"))
+			To(ContainElement(ALPINE))
 		Expect(strings.Fields(session.OutputToString())).
 			To(ContainElement("testhost"))
 		Expect(strings.Fields(session.OutputToString())).
@@ -494,23 +495,23 @@ RUN grep CapEff /proc/self/status`
 		Expect(err).To(BeNil())
 
 		containerFile := filepath.Join(targetPath, "Containerfile")
-		Expect(ioutil.WriteFile(containerFile, []byte("FROM alpine"), 0755)).To(BeNil())
+		Expect(ioutil.WriteFile(containerFile, []byte(fmt.Sprintf("FROM %s", ALPINE)), 0755)).To(BeNil())
 
 		defer func() {
 			Expect(os.RemoveAll(containerFile)).To(BeNil())
 		}()
 
 		// When
 		session := podmanTest.Podman([]string{
-			"build", "--pull-never", "--isolation", "oci", "--arch", "arm64", targetPath,
+			"build", "--isolation", "oci", "--arch", "arm64", targetPath,
 		})
 		session.WaitWithDefaultTimeout()
 		// Then
 		Expect(session.ExitCode()).To(Equal(0))
 
 		// When
 		session = podmanTest.Podman([]string{
-			"build", "--pull-never", "--isolation", "chroot", "--arch", "arm64", targetPath,
+			"build", "--isolation", "chroot", "--arch", "arm64", targetPath,
 		})
 		session.WaitWithDefaultTimeout()
 		// Then
@@ -534,8 +535,8 @@ RUN grep CapEff /proc/self/status`
 	})
 
 	It("podman build --timestamp flag", func() {
-		containerfile := `FROM quay.io/libpod/alpine:latest
-RUN echo hello`
+		containerfile := fmt.Sprintf(`FROM %s
+RUN echo hello`, ALPINE)
 
 		containerfilePath := filepath.Join(podmanTest.TempDir, "Containerfile")
 		err := ioutil.WriteFile(containerfilePath, []byte(containerfile), 0755)

diff --git a/test/e2e/containers_conf_test.go b/test/e2e/containers_conf_test.go
@@ -91,7 +91,7 @@ var _ = Describe("Podman run", func() {
 		if IsRemote() {
 			podmanTest.RestartRemoteService()
 		}
-		session := podmanTest.Podman([]string{"run", "busybox", "grep", "CapEff", "/proc/self/status"})
+		session := podmanTest.Podman([]string{"run", BB, "grep", "CapEff", "/proc/self/status"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(session.OutputToString()).ToNot(Equal(cap.OutputToString()))

diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go
@@ -475,10 +475,10 @@ var _ = Describe("Podman exec", func() {
 	})
 
 	It("podman exec preserves container groups with --user and --group-add", func() {
-		dockerfile := `FROM registry.fedoraproject.org/fedora-minimal
+		dockerfile := fmt.Sprintf(`FROM %s
 RUN groupadd -g 4000 first
 RUN groupadd -g 4001 second
-RUN useradd -u 1000 auser`
+RUN useradd -u 1000 auser`, fedoraMinimal)
 		imgName := "testimg"
 		podmanTest.BuildImage(dockerfile, imgName, "false")
 

diff --git a/test/e2e/prune_test.go b/test/e2e/prune_test.go
@@ -1,18 +1,19 @@
 package integration
 
 import (
+	"fmt"
 	"os"
 
 	. "github.com/containers/podman/v3/test/utils"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
 
-var pruneImage = `
-FROM  alpine:latest
+var pruneImage = fmt.Sprintf(`
+FROM  %s
 LABEL RUN podman --version
 RUN apk update
-RUN apk add bash`
+RUN apk add bash`, ALPINE)
 
 var _ = Describe("Podman prune", func() {
 	var (

diff --git a/test/e2e/ps_test.go b/test/e2e/ps_test.go
@@ -351,7 +351,7 @@ var _ = Describe("Podman ps", func() {
 	})
 
 	It("podman --format by size", func() {
-		session := podmanTest.Podman([]string{"create", "busybox", "ls"})
+		session := podmanTest.Podman([]string{"create", BB, "ls"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
@@ -366,7 +366,7 @@ var _ = Describe("Podman ps", func() {
 	})
 
 	It("podman --sort by size", func() {
-		session := podmanTest.Podman([]string{"create", "busybox", "ls"})
+		session := podmanTest.Podman([]string{"create", BB, "ls"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 

diff --git a/test/e2e/rmi_test.go b/test/e2e/rmi_test.go
@@ -184,19 +184,20 @@ var _ = Describe("Podman rmi", func() {
 
 	It("podman rmi with cached images", func() {
 		podmanTest.AddImageToRWStore(cirros)
-		dockerfile := `FROM quay.io/libpod/cirros:latest
+		dockerfile := fmt.Sprintf(`FROM %s
 		RUN mkdir hello
 		RUN touch test.txt
 		ENV foo=bar
-		`
+		`, cirros)
 		podmanTest.BuildImage(dockerfile, "test", "true")
 
-		dockerfile = `FROM quay.io/libpod/cirros:latest
+		dockerfile = fmt.Sprintf(`FROM %s
 		RUN mkdir hello
 		RUN touch test.txt
 		RUN mkdir blah
 		ENV foo=bar
-		`
+		`, cirros)
+
 		podmanTest.BuildImage(dockerfile, "test2", "true")
 
 		session := podmanTest.Podman([]string{"images", "-q", "-a"})
@@ -249,14 +250,15 @@ var _ = Describe("Podman rmi", func() {
 	})
 
 	It("podman rmi -a with parent|child images", func() {
-		dockerfile := `FROM quay.io/libpod/cirros:latest AS base
+		podmanTest.AddImageToRWStore(cirros)
+		dockerfile := fmt.Sprintf(`FROM %s AS base
 RUN touch /1
 ENV LOCAL=/1
 RUN find $LOCAL
 FROM base
 RUN find $LOCAL
 
-`
+`, cirros)
 		podmanTest.BuildImage(dockerfile, "test", "true")
 		session := podmanTest.Podman([]string{"rmi", "-a"})
 		session.WaitWithDefaultTimeout()
@@ -284,14 +286,15 @@ RUN find $LOCAL
 		// a race, we may not hit the condition a 100 percent of times
 		// but ocal reproducers hit it all the time.
 
+		podmanTest.AddImageToRWStore(cirros)
 		var wg sync.WaitGroup
 
 		buildAndRemove := func(i int) {
 			defer GinkgoRecover()
 			defer wg.Done()
 			imageName := fmt.Sprintf("rmtest:%d", i)
-			containerfile := `FROM quay.io/libpod/cirros:latest
-RUN ` + fmt.Sprintf("touch %s", imageName)
+			containerfile := fmt.Sprintf(`FROM %s
+RUN touch %s`, cirros, imageName)
 
 			podmanTest.BuildImage(containerfile, imageName, "false")
 			session := podmanTest.Podman([]string{"rmi", "-f", imageName})

diff --git a/test/e2e/run_passwd_test.go b/test/e2e/run_passwd_test.go
@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"fmt"
 	"os"
 
 	. "github.com/containers/podman/v3/test/utils"
@@ -60,9 +61,9 @@ var _ = Describe("Podman run passwd", func() {
 	})
 
 	It("podman can run container without /etc/passwd", func() {
-		dockerfile := `FROM alpine
+		dockerfile := fmt.Sprintf(`FROM %s
 RUN rm -f /etc/passwd /etc/shadow /etc/group
-USER 1000`
+USER 1000`, ALPINE)
 		imgName := "testimg"
 		podmanTest.BuildImage(dockerfile, imgName, "false")
 		session := podmanTest.Podman([]string{"run", "--rm", imgName, "ls", "/etc/"})
@@ -113,9 +114,9 @@ USER 1000`
 	})
 
 	It("podman run numeric group from image and no group file", func() {
-		dockerfile := `FROM alpine
+		dockerfile := fmt.Sprintf(`FROM %s
 RUN rm -f /etc/passwd /etc/shadow /etc/group
-USER 1000`
+USER 1000`, ALPINE)
 		imgName := "testimg"
 		podmanTest.BuildImage(dockerfile, imgName, "false")
 		session := podmanTest.Podman([]string{"run", "--rm", imgName, "ls", "/etc/"})