From afe48ba36c22e8373173a8cfaee3f4fb8aabb8de Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 29 Jun 2023 11:34:49 +0200 Subject: [PATCH] auto update: fix usage of --authfile The --authfile flag has been ignored. Fix that and add a test to make sure we won't regress another time. Requires a new --tls-verify flag to actually test the code. Also bump c/common since common/pull/1538 is required to correctly check for updates. Note that I had to use the go-mod-edit-replace trick on c/common as c/buildah would otherwise be moved back to 1.30. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2218315 Signed-off-by: Valentin Rothberg --- cmd/podman/auto-update.go | 10 ++- docs/source/markdown/options/tls-verify.md | 2 +- .../markdown/podman-auto-update.1.md.in | 1 + pkg/autoupdate/autoupdate.go | 14 +++- pkg/domain/entities/auto-update.go | 5 ++ test/system/255-auto-update.bats | 64 +++++++++++++++++++ 6 files changed, 92 insertions(+), 4 deletions(-) diff --git a/cmd/podman/auto-update.go b/cmd/podman/auto-update.go index 6a04464225..07a1a0c03e 100644 --- a/cmd/podman/auto-update.go +++ b/cmd/podman/auto-update.go @@ -8,6 +8,7 @@ import ( "github.com/containers/common/pkg/auth" "github.com/containers/common/pkg/completion" "github.com/containers/common/pkg/report" + "github.com/containers/image/v5/types" "github.com/containers/podman/v4/cmd/podman/common" "github.com/containers/podman/v4/cmd/podman/registry" "github.com/containers/podman/v4/pkg/domain/entities" @@ -17,7 +18,8 @@ import ( type cliAutoUpdateOptions struct { entities.AutoUpdateOptions - format string + format string + tlsVerify bool } var ( @@ -56,6 +58,8 @@ func init() { flags.StringVar(&autoUpdateOptions.format, "format", "", "Change the output format to JSON or a Go template") _ = autoUpdateCommand.RegisterFlagCompletionFunc("format", common.AutocompleteFormat(&autoUpdateOutput{})) + + flags.BoolVarP(&autoUpdateOptions.tlsVerify, "tls-verify", "", true, "Require HTTPS and verify certificates when contacting registries") } func autoUpdate(cmd *cobra.Command, args []string) error { @@ -64,6 +68,10 @@ func autoUpdate(cmd *cobra.Command, args []string) error { return fmt.Errorf("`%s` takes no arguments", cmd.CommandPath()) } + if cmd.Flags().Changed("tls-verify") { + autoUpdateOptions.InsecureSkipTLSVerify = types.NewOptionalBool(!autoUpdateOptions.tlsVerify) + } + allReports, failures := registry.ContainerEngine().AutoUpdate(registry.GetContext(), autoUpdateOptions.AutoUpdateOptions) if allReports == nil { return errorhandling.JoinErrors(failures) diff --git a/docs/source/markdown/options/tls-verify.md b/docs/source/markdown/options/tls-verify.md index d694bab26d..13c2a6e634 100644 --- a/docs/source/markdown/options/tls-verify.md +++ b/docs/source/markdown/options/tls-verify.md @@ -1,5 +1,5 @@ ####> This option file is used in: -####> podman build, container runlabel, create, kube play, login, manifest add, manifest create, manifest inspect, manifest push, pull, push, run, search +####> podman auto update, build, container runlabel, create, kube play, login, manifest add, manifest create, manifest inspect, manifest push, pull, push, run, search ####> If file is edited, make sure the changes ####> are applicable to all of those. #### **--tls-verify** diff --git a/docs/source/markdown/podman-auto-update.1.md.in b/docs/source/markdown/podman-auto-update.1.md.in index b44999496b..c615567ff9 100644 --- a/docs/source/markdown/podman-auto-update.1.md.in +++ b/docs/source/markdown/podman-auto-update.1.md.in @@ -79,6 +79,7 @@ Please note that detecting if a systemd unit has failed is best done by the cont For a container to send the READY message via SDNOTIFY it must be created with the `--sdnotify=container` option (see podman-run(1)). The application running inside the container can then execute `systemd-notify --ready` when ready or use the sdnotify bindings of the specific programming language (e.g., sd_notify(3)). +@@option tls-verify ## EXAMPLES Autoupdate with registry policy diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go index 4f2a4a0770..dd8c7b80c6 100644 --- a/pkg/autoupdate/autoupdate.go +++ b/pkg/autoupdate/autoupdate.go @@ -282,7 +282,10 @@ func (t *task) registryUpdateAvailable(ctx context.Context) (bool, error) { if err != nil { return false, err } - options := &libimage.HasDifferentDigestOptions{AuthFilePath: t.authfile} + options := &libimage.HasDifferentDigestOptions{ + AuthFilePath: t.authfile, + InsecureSkipTLSVerify: t.auto.options.InsecureSkipTLSVerify, + } return t.image.HasDifferentDigest(ctx, remoteRef, options) } @@ -296,6 +299,7 @@ func (t *task) registryUpdate(ctx context.Context) error { pullOptions := &libimage.PullOptions{} pullOptions.AuthFilePath = t.authfile pullOptions.Writer = os.Stderr + pullOptions.InsecureSkipTLSVerify = t.auto.options.InsecureSkipTLSVerify if _, err := t.auto.runtime.LibimageRuntime().Pull(ctx, t.rawImageName, config.PullPolicyAlways, pullOptions); err != nil { return err } @@ -416,8 +420,14 @@ func (u *updater) assembleTasks(ctx context.Context) []error { continue } + // Use user-specified auth file (CLI or env variable) unless + // the container was created with the auth-file label. + authfile := u.options.Authfile + if fromContainer, ok := labels[define.AutoUpdateAuthfileLabel]; ok { + authfile = fromContainer + } t := task{ - authfile: labels[define.AutoUpdateAuthfileLabel], + authfile: authfile, auto: u, container: ctr, policy: policy, diff --git a/pkg/domain/entities/auto-update.go b/pkg/domain/entities/auto-update.go index 5ea2cdf150..195f27f84a 100644 --- a/pkg/domain/entities/auto-update.go +++ b/pkg/domain/entities/auto-update.go @@ -1,5 +1,7 @@ package entities +import "github.com/containers/image/v5/types" + // AutoUpdateOptions are the options for running auto-update. type AutoUpdateOptions struct { // Authfile to use when contacting registries. @@ -11,6 +13,9 @@ type AutoUpdateOptions struct { // If restarting the service with the new image failed, restart it // another time with the previous image. Rollback bool + // Allow contacting registries over HTTP, or HTTPS with failed TLS + // verification. Note that this does not affect other TLS connections. + InsecureSkipTLSVerify types.OptionalBool } // AutoUpdateReport contains the results from running auto-update. diff --git a/test/system/255-auto-update.bats b/test/system/255-auto-update.bats index 7fef945299..be5b282c8d 100644 --- a/test/system/255-auto-update.bats +++ b/test/system/255-auto-update.bats @@ -4,6 +4,8 @@ # load helpers +load helpers.network +load helpers.registry load helpers.systemd SNAME_FILE=$BATS_TMPDIR/services @@ -47,6 +49,7 @@ function teardown() { # 4. Generate the service file from the container # 5. Remove the origin container # 6. Start the container from service +# 7. Use this fully-qualified image instead of 2) function generate_service() { local target_img_basename=$1 local autoupdate=$2 @@ -64,6 +67,9 @@ function generate_service() { # IMPORTANT: variable 'cname' is passed (out of scope) up to caller! cname=c_${autoupdate//\'/}_$(random_string) target_img="quay.io/libpod/$target_img_basename:latest" + if [[ -n "$7" ]]; then + target_img="$7" + fi if [[ -z "$noTag" ]]; then run_podman tag $IMAGE $target_img @@ -623,4 +629,62 @@ EOF systemctl daemon-reload } +@test "podman-auto-update --authfile" { + # Test the three supported ways of using authfiles with auto updates + # 1) Passed via --authfile CLI flag + # 2) Passed via the REGISTRY_AUTH_FILE env variable + # 3) Via a label at container creation where 1) and 2) will be ignored + + registry=localhost:${PODMAN_LOGIN_REGISTRY_PORT} + image_on_local_registry=$registry/name:tag + authfile=$PODMAN_TMPDIR/authfile.json + + # First, start the registry and populate the authfile that we can use for the test. + start_registry + run_podman login --authfile=$authfile \ + --tls-verify=false \ + --username ${PODMAN_LOGIN_USER} \ + --password ${PODMAN_LOGIN_PASS} \ + $registry + + run_podman tag $IMAGE $image_on_local_registry + run_podman push --tls-verify=false --creds "${PODMAN_LOGIN_USER}:${PODMAN_LOGIN_PASS}" $image_on_local_registry + + # Generate a systemd service with the "registry" auto-update policy running + # "top" inside the image we just pushed to the local registry. + generate_service "" registry top "" "" "" $image_on_local_registry + ctr=$cname + _wait_service_ready container-$ctr.service + + run_podman 125 auto-update + is "$output" \ + ".*Error: checking image updates for container .*: x509: .*" + + run_podman 125 auto-update --tls-verify=false + is "$output" \ + ".*Error: checking image updates for container .*: authentication required" + + # Test 1) + run_podman auto-update --authfile=$authfile --tls-verify=false --dry-run --format "{{.Unit}},{{.Image}},{{.Updated}},{{.Policy}}" + is "$output" "container-$ctr.service,$image_on_local_registry,false,registry" "auto-update works with authfile" + + # Test 2) + REGISTRY_AUTH_FILE=$authfile run_podman auto-update --tls-verify=false --dry-run --format "{{.Unit}},{{.Image}},{{.Updated}},{{.Policy}}" + is "$output" "container-$ctr.service,$image_on_local_registry,false,registry" "auto-update works with env var" + systemctl stop container-$ctr.service + run_podman rm -f -t0 --ignore $ctr + + # Create a container with the auth-file label + generate_service "" registry top "--label io.containers.autoupdate.authfile=$authfile" "" "" $image_on_local_registry + ctr=$cname + _wait_service_ready container-$ctr.service + + # Test 3) + # Also make sure that the label takes precedence over the CLI flag. + run_podman auto-update --authfile=/dev/null --tls-verify=false --dry-run --format "{{.Unit}},{{.Image}},{{.Updated}},{{.Policy}}" + is "$output" "container-$ctr.service,$image_on_local_registry,false,registry" "auto-update works with authfile container label" + run_podman rm -f -t0 --ignore $ctr + run_podman rmi $image_on_local_registry +} + # vim: filetype=sh