From dbb22beb4df5c3c972c6dd0b37f186833d1ca96d Mon Sep 17 00:00:00 2001 From: Matej Vasek Date: Fri, 11 Aug 2023 17:41:31 +0200 Subject: [PATCH] fix: pull parma parsing for the /build compat ep The standard Docker client passes "1" for pull query parameter. This means our compat endpoint has to work with this. The endpoint was recently modified to accept pull-policy however this is not how Docker actually work. Docker actually treats `pull` as boolean. For sake of compatibility I decide to preserve pull-policy parsing too. We can consider this podman's extension. I should not affect standard clients. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2232127 Signed-off-by: Matej Vasek Signed-off-by: Paul Holzinger --- pkg/api/handlers/compat/images_build.go | 4 +- test/python/docker/compat/test_containers.py | 46 ++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 984660c32c..0dc55cf0b9 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -581,9 +581,9 @@ func BuildImage(w http.ResponseWriter, r *http.Request) { } else { if _, found := r.URL.Query()["pull"]; found { switch strings.ToLower(query.Pull) { - case "false": + case "0", "f", "false": pullPolicy = buildahDefine.PullIfMissing - case "true": + case "on", "1", "t", "true": pullPolicy = buildahDefine.PullAlways default: policyFromMap, foundPolicy := buildahDefine.PolicyMap[query.Pull] diff --git a/test/python/docker/compat/test_containers.py b/test/python/docker/compat/test_containers.py index fab8013443..47601a69be 100644 --- a/test/python/docker/compat/test_containers.py +++ b/test/python/docker/compat/test_containers.py @@ -8,6 +8,7 @@ import time from typing import IO, List, Optional from docker import errors +from docker.errors import BuildError from docker.models.containers import Container from docker.models.images import Image from docker.models.volumes import Volume @@ -282,3 +283,48 @@ class TestContainers(common.DockerTestCase): finally: ctr.stop() ctr.remove(force=True) + + def test_build_pull_true(self): + dockerfile = ( + b"FROM quay.io/libpod/alpine:latest\n" + ) + img: Image + img, logs = self.docker.images.build(fileobj=io.BytesIO(dockerfile), quiet=False, pull=True) + has_tried_pull = False + for e in logs: + if "stream" in e and "trying to pull" in e["stream"].lower(): + has_tried_pull = True + self.assertTrue(has_tried_pull, "the build process has not tried to pull the base image") + + def test_build_pull_one(self): + dockerfile = ( + b"FROM quay.io/libpod/alpine:latest\n" + ) + img: Image + img, logs = self.docker.images.build(fileobj=io.BytesIO(dockerfile), quiet=False, pull=1) + has_tried_pull = False + for e in logs: + if "stream" in e and "trying to pull" in e["stream"].lower(): + has_tried_pull = True + self.assertTrue(has_tried_pull, "the build process has not tried to pull the base image") + + def test_build_pull_false(self): + dockerfile = ( + b"FROM quay.io/libpod/alpine:latest\n" + ) + img, logs = self.docker.images.build(fileobj=io.BytesIO(dockerfile), quiet=False, pull=False) + has_tried_pull = False + for e in logs: + if "stream" in e and "trying to pull" in e["stream"].lower(): + has_tried_pull = True + self.assertFalse(has_tried_pull, "the build process has tried tried to pull the base image") + + def test_build_pull_never(self): + try: + dockerfile = ( + b"FROM quay.io/libpod/does-not-exist:latest\n" + ) + _, _ = self.docker.images.build(fileobj=io.BytesIO(dockerfile), quiet=False, pull="never") + self.fail("this line should not have been reached") + except BuildError as e: + self.assertTrue("image not known" in e.msg, "the exception should have been caused by missing base image")