Skip to content

Commit

Permalink
auto-update: validate container image
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
vrothberg committed Sep 26, 2022
1 parent 17f3756 commit 7bc3660
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 91 deletions.
33 changes: 33 additions & 0 deletions libpod/container_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
9 changes: 9 additions & 0 deletions libpod/define/autoupdate.go
Original file line number Diff line number Diff line change
@@ -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"
41 changes: 3 additions & 38 deletions pkg/autoupdate/autoupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down
50 changes: 0 additions & 50 deletions pkg/autoupdate/autoupdate_test.go

This file was deleted.

5 changes: 2 additions & 3 deletions pkg/domain/infra/abi/play.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down
20 changes: 20 additions & 0 deletions test/system/255-auto-update.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down

0 comments on commit 7bc3660

Please sign in to comment.