From 7bc36602f6c407cffdc799ca1b2fc7c00bc4f93b Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 26 Sep 2022 13:00:17 +0200 Subject: [PATCH] auto-update: validate container image Auto updates using the "registry" policy require container to be created with a fully-qualified image reference. Short names are not supported due the ambiguity of their source registry. Initially, container creation errored out for non FQN images but it seems that Podman has regressed. Fixes: #15879 Signed-off-by: Valentin Rothberg --- libpod/container_validate.go | 33 ++++++++++++++++++++ libpod/define/autoupdate.go | 9 ++++++ pkg/autoupdate/autoupdate.go | 41 ++----------------------- pkg/autoupdate/autoupdate_test.go | 50 ------------------------------- pkg/domain/infra/abi/play.go | 5 ++-- test/system/255-auto-update.bats | 20 +++++++++++++ 6 files changed, 67 insertions(+), 91 deletions(-) create mode 100644 libpod/define/autoupdate.go delete mode 100644 pkg/autoupdate/autoupdate_test.go diff --git a/libpod/container_validate.go b/libpod/container_validate.go index f4611ecce4..7224ec7dbd 100644 --- a/libpod/container_validate.go +++ b/libpod/container_validate.go @@ -3,6 +3,9 @@ package libpod import ( "fmt" + "github.com/containers/image/v5/docker" + "github.com/containers/image/v5/pkg/shortnames" + "github.com/containers/image/v5/transports/alltransports" "github.com/containers/podman/v4/libpod/define" spec "github.com/opencontainers/runtime-spec/specs-go" ) @@ -141,5 +144,35 @@ func (c *Container) validate() error { if c.config.HealthCheckOnFailureAction != define.HealthCheckOnFailureActionNone && c.config.HealthCheckConfig == nil { return fmt.Errorf("cannot set on-failure action to %s without a health check", c.config.HealthCheckOnFailureAction.String()) } + + if value, exists := c.config.Labels[define.AutoUpdateLabel]; exists { + // TODO: we cannot reference pkg/autoupdate here due to + // circular dependencies. It's worth considering moving the + // auto-update logic into the libpod package. + if value == "registry" || value == "image" { + if err := validateAutoUpdateImageReference(c.config.RawImageName); err != nil { + return err + } + } + } + + return nil +} + +// validateAutoUpdateImageReference checks if the specified imageName is a +// fully-qualified image reference to the docker transport. Such a reference +// includes a domain, name and tag (e.g., quay.io/podman/stable:latest). The +// reference may also be prefixed with "docker://" explicitly indicating that +// it's a reference to the docker transport. +func validateAutoUpdateImageReference(imageName string) error { + // Make sure the input image is a docker. + imageRef, err := alltransports.ParseImageName(imageName) + if err == nil && imageRef.Transport().Name() != docker.Transport.Name() { + return fmt.Errorf("auto updates require the docker image transport but image is of transport %q", imageRef.Transport().Name()) + } else if err != nil { + if shortnames.IsShortName(imageName) { + return fmt.Errorf("short name: auto updates require fully-qualified image reference: %q", imageName) + } + } return nil } diff --git a/libpod/define/autoupdate.go b/libpod/define/autoupdate.go new file mode 100644 index 0000000000..7c278c3c50 --- /dev/null +++ b/libpod/define/autoupdate.go @@ -0,0 +1,9 @@ +package define + +// AutoUpdateLabel denotes the container/pod label key to specify auto-update +// policies in container labels. +const AutoUpdateLabel = "io.containers.autoupdate" + +// AutoUpdateAuthfileLabel denotes the container label key to specify authfile +// in container labels. +const AutoUpdateAuthfileLabel = "io.containers.autoupdate.authfile" diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go index 9cf77d1350..a0ed8ccbac 100644 --- a/pkg/autoupdate/autoupdate.go +++ b/pkg/autoupdate/autoupdate.go @@ -9,8 +9,6 @@ import ( "github.com/containers/common/libimage" "github.com/containers/common/pkg/config" "github.com/containers/image/v5/docker" - "github.com/containers/image/v5/docker/reference" - "github.com/containers/image/v5/transports/alltransports" "github.com/containers/podman/v4/libpod" "github.com/containers/podman/v4/libpod/define" "github.com/containers/podman/v4/libpod/events" @@ -21,14 +19,6 @@ import ( "github.com/sirupsen/logrus" ) -// Label denotes the container/pod label key to specify auto-update policies in -// container labels. -const Label = "io.containers.autoupdate" - -// Label denotes the container label key to specify authfile in -// container labels. -const AuthfileLabel = "io.containers.autoupdate.authfile" - // Policy represents an auto-update policy. type Policy string @@ -102,32 +92,7 @@ func LookupPolicy(s string) (Policy, error) { return "", fmt.Errorf("invalid auto-update policy %q: valid policies are %+q", s, keys) } -// ValidateImageReference checks if the specified imageName is a fully-qualified -// image reference to the docker transport (without digest). Such a reference -// includes a domain, name and tag (e.g., quay.io/podman/stable:latest). The -// reference may also be prefixed with "docker://" explicitly indicating that -// it's a reference to the docker transport. -func ValidateImageReference(imageName string) error { - // Make sure the input image is a docker. - imageRef, err := alltransports.ParseImageName(imageName) - if err == nil && imageRef.Transport().Name() != docker.Transport.Name() { - return fmt.Errorf("auto updates require the docker image transport but image is of transport %q", imageRef.Transport().Name()) - } else if err != nil { - repo, err := reference.Parse(imageName) - if err != nil { - return fmt.Errorf("enforcing fully-qualified docker transport reference for auto updates: %w", err) - } - if _, ok := repo.(reference.NamedTagged); !ok { - return fmt.Errorf("auto updates require fully-qualified image references (no tag): %q", imageName) - } - if _, ok := repo.(reference.Digested); ok { - return fmt.Errorf("auto updates require fully-qualified image references without digest: %q", imageName) - } - } - return nil -} - -// AutoUpdate looks up containers with a specified auto-update policy and acts +/// AutoUpdate looks up containers with a specified auto-update policy and acts // accordingly. // // If the policy is set to PolicyRegistryImage, it checks if the image @@ -418,7 +383,7 @@ func (u *updater) assembleTasks(ctx context.Context) []error { // Check the container's auto-update policy which is configured // as a label. labels := ctr.Labels() - value, exists := labels[Label] + value, exists := labels[define.AutoUpdateLabel] if !exists { continue } @@ -454,7 +419,7 @@ func (u *updater) assembleTasks(ctx context.Context) []error { } t := task{ - authfile: labels[AuthfileLabel], + authfile: labels[define.AutoUpdateAuthfileLabel], auto: u, container: ctr, policy: policy, diff --git a/pkg/autoupdate/autoupdate_test.go b/pkg/autoupdate/autoupdate_test.go deleted file mode 100644 index 7a5da5bb04..0000000000 --- a/pkg/autoupdate/autoupdate_test.go +++ /dev/null @@ -1,50 +0,0 @@ -package autoupdate - -import ( - "testing" -) - -func TestValidateImageReference(t *testing.T) { - tests := []struct { - input string - valid bool - }{ - { // Fully-qualified reference - input: "quay.io/foo/bar:tag", - valid: true, - }, - { // Fully-qualified reference in transport notation - input: "docker://quay.io/foo/bar:tag", - valid: true, - }, - { // Fully-qualified reference but with digest - input: "quay.io/foo/bar@sha256:c9b1b535fdd91a9855fb7f82348177e5f019329a58c53c47272962dd60f71fc9", - valid: false, - }, - { // Reference with missing tag - input: "quay.io/foo/bar", - valid: false, - }, - { // Short name - input: "alpine", - valid: false, - }, - { // Short name with repo - input: "library/alpine", - valid: false, - }, - { // Wrong transport - input: "docker-archive:/some/path.tar", - valid: false, - }, - } - - for _, test := range tests { - err := ValidateImageReference(test.input) - if test.valid && err != nil { - t.Fatalf("parsing %q should have succeeded: %v", test.input, err) - } else if !test.valid && err == nil { - t.Fatalf("parsing %q should have failed", test.input) - } - } -} diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 847e81e698..3a5a80772b 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -19,7 +19,6 @@ import ( "github.com/containers/image/v5/types" "github.com/containers/podman/v4/libpod" "github.com/containers/podman/v4/libpod/define" - "github.com/containers/podman/v4/pkg/autoupdate" "github.com/containers/podman/v4/pkg/domain/entities" v1apps "github.com/containers/podman/v4/pkg/k8s.io/api/apps/v1" v1 "github.com/containers/podman/v4/pkg/k8s.io/api/core/v1" @@ -800,8 +799,8 @@ func (ic *ContainerEngine) getImageAndLabelInfo(ctx context.Context, cwd string, } } - setLabel(autoupdate.Label) - setLabel(autoupdate.AuthfileLabel) + setLabel(define.AutoUpdateLabel) + setLabel(define.AutoUpdateAuthfileLabel) return pulledImage, labels, nil } diff --git a/test/system/255-auto-update.bats b/test/system/255-auto-update.bats index 76f6b02e8a..6f51137795 100644 --- a/test/system/255-auto-update.bats +++ b/test/system/255-auto-update.bats @@ -133,6 +133,26 @@ function _confirm_update() { die "Timed out waiting for $cname to update; old IID=$old_iid" } +@test "podman auto-update - validate input" { + # Fully-qualified image reference is required + run_podman create --label io.containers.autoupdate=registry $IMAGE + run_podman rm -f "$output" + + # Short name does not work + shortname="shortname:latest" + run_podman image tag $IMAGE $shortname + run_podman 125 create --label io.containers.autoupdate=registry $shortname + is "$output" "Error: short name: auto updates require fully-qualified image reference: \"$shortname\"" + + # Requires docker (or no) transport + archive=$PODMAN_TMPDIR/archive.tar + run_podman save -o $archive $IMAGE + run_podman 125 create --label io.containers.autoupdate=registry docker-archive:$archive + is "$output" ".*Error: auto updates require the docker image transport but image is of transport \"docker-archive\"" + + run_podman rmi $shortname +} + # This test can fail in dev. environment because of SELinux. # quick fix: chcon -t container_runtime_exec_t ./bin/podman @test "podman auto-update - label io.containers.autoupdate=image" {