From 153a9758889b5e6978115a13fb8b0133a4f25315 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 17 Dec 2024 16:29:40 +0100 Subject: [PATCH 1/4] shell completion: respect CONTAINERS_REGISTRIES_CONF Found in debian testing where by default there are no unqualified search registries installed. As such the test failed as the FIXME said. Now there is no need for the test to assume anything. Instead set our own config via CONTAINERS_REGISTRIES_CONF then we can do exact matches, except that env was not read in the shell completion code so move some code around to make it read the var in the same way as podman login/logout. Signed-off-by: Paul Holzinger --- cmd/podman/common/completion.go | 5 ++++- cmd/podman/common/registries.go | 24 ++++++++++++++++++++++++ cmd/podman/login.go | 19 +------------------ cmd/podman/logout.go | 2 +- test/system/600-completion.bats | 9 +++++++-- 5 files changed, 37 insertions(+), 22 deletions(-) create mode 100644 cmd/podman/common/registries.go diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index ca886b137d..e46675733c 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -16,6 +16,7 @@ import ( "github.com/containers/common/pkg/config" "github.com/containers/common/pkg/ssh" "github.com/containers/image/v5/pkg/sysregistriesv2" + imageTypes "github.com/containers/image/v5/types" "github.com/containers/podman/v5/cmd/podman/registry" "github.com/containers/podman/v5/libpod/define" "github.com/containers/podman/v5/libpod/events" @@ -276,7 +277,9 @@ func getSecrets(cmd *cobra.Command, toComplete string, cType completeType) ([]st } func getRegistries() ([]string, cobra.ShellCompDirective) { - regs, err := sysregistriesv2.UnqualifiedSearchRegistries(nil) + sysCtx := &imageTypes.SystemContext{} + SetRegistriesConfPath(sysCtx) + regs, err := sysregistriesv2.UnqualifiedSearchRegistries(sysCtx) if err != nil { cobra.CompErrorln(err.Error()) return nil, cobra.ShellCompDirectiveNoFileComp diff --git a/cmd/podman/common/registries.go b/cmd/podman/common/registries.go new file mode 100644 index 0000000000..f444585f6c --- /dev/null +++ b/cmd/podman/common/registries.go @@ -0,0 +1,24 @@ +package common + +import ( + "os" + + "github.com/containers/image/v5/types" +) + +// SetRegistriesConfPath sets the registries.conf path for the specified context. +// NOTE: this is a verbatim copy from c/common/libimage which we're not using +// to prevent leaking c/storage into this file. Maybe this should go into c/image? +func SetRegistriesConfPath(systemContext *types.SystemContext) { + if systemContext.SystemRegistriesConfPath != "" { + return + } + if envOverride, ok := os.LookupEnv("CONTAINERS_REGISTRIES_CONF"); ok { + systemContext.SystemRegistriesConfPath = envOverride + return + } + if envOverride, ok := os.LookupEnv("REGISTRIES_CONFIG_PATH"); ok { + systemContext.SystemRegistriesConfPath = envOverride + return + } +} diff --git a/cmd/podman/login.go b/cmd/podman/login.go index 5a3edb3369..fe63a4ba3a 100644 --- a/cmd/podman/login.go +++ b/cmd/podman/login.go @@ -98,24 +98,7 @@ func login(cmd *cobra.Command, args []string) error { sysCtx := &types.SystemContext{ DockerInsecureSkipTLSVerify: skipTLS, } - setRegistriesConfPath(sysCtx) + common.SetRegistriesConfPath(sysCtx) loginOptions.GetLoginSet = cmd.Flag("get-login").Changed return auth.Login(context.Background(), sysCtx, &loginOptions.LoginOptions, args) } - -// setRegistriesConfPath sets the registries.conf path for the specified context. -// NOTE: this is a verbatim copy from c/common/libimage which we're not using -// to prevent leaking c/storage into this file. Maybe this should go into c/image? -func setRegistriesConfPath(systemContext *types.SystemContext) { - if systemContext.SystemRegistriesConfPath != "" { - return - } - if envOverride, ok := os.LookupEnv("CONTAINERS_REGISTRIES_CONF"); ok { - systemContext.SystemRegistriesConfPath = envOverride - return - } - if envOverride, ok := os.LookupEnv("REGISTRIES_CONFIG_PATH"); ok { - systemContext.SystemRegistriesConfPath = envOverride - return - } -} diff --git a/cmd/podman/logout.go b/cmd/podman/logout.go index 46a123f292..ef2dc17fe7 100644 --- a/cmd/podman/logout.go +++ b/cmd/podman/logout.go @@ -49,6 +49,6 @@ func init() { // Implementation of podman-logout. func logout(cmd *cobra.Command, args []string) error { sysCtx := &types.SystemContext{} - setRegistriesConfPath(sysCtx) + common.SetRegistriesConfPath(sysCtx) return auth.Logout(sysCtx, &logoutOptions, args) } diff --git a/test/system/600-completion.bats b/test/system/600-completion.bats index 1f300b0565..fd108e445b 100644 --- a/test/system/600-completion.bats +++ b/test/system/600-completion.bats @@ -168,10 +168,10 @@ function check_shell_completion() { *REGISTRY*) run_completion "$@" $cmd "${extra_args[@]}" "" - ### FIXME how can we get the configured registries? _check_completion_end NoFileComp - ### FIXME this fails if no registries are configured assert "${#lines[@]}" -gt 2 "$* $cmd: No REGISTRIES found in suggestions" + # We can assume quay.io as we force our own CONTAINERS_REGISTRIES_CONF below. + assert "${lines[0]}" == "quay.io" "unqualified-search-registries from registries.conf listed" match=true # resume @@ -311,6 +311,11 @@ function _check_no_suggestions() { # create secret run_podman secret create $random_secret_name $secret_file + # create our own registries.conf so we know what registry is set + local CONTAINERS_REGISTRIES_CONF="$PODMAN_TMPDIR/registries.conf" + echo 'unqualified-search-registries = ["quay.io"]' > "$CONTAINERS_REGISTRIES_CONF" + export CONTAINERS_REGISTRIES_CONF + # Called with no args -- start with 'podman --help'. check_shell_completion() will # recurse for any subcommands. check_shell_completion From 87297256841125cc7c841753ef097d09c381d696 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 17 Dec 2024 16:49:55 +0100 Subject: [PATCH 2/4] test/system: remove system dial-stdio test This test a pretty much useless, it checks that a connection attempt on the default socket fails. But this is incorrect as the socket is outside of the test control as such it might be ready to accept connections as thus the test can fail locally or as reported here in the debian tests. Given that a simple connection fails does not add any value I opted to remove it. Fixes #24803 Signed-off-by: Paul Holzinger --- test/e2e/system_dial_stdio_test.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/e2e/system_dial_stdio_test.go b/test/e2e/system_dial_stdio_test.go index 7c278f2d96..3fffb7faea 100644 --- a/test/e2e/system_dial_stdio_test.go +++ b/test/e2e/system_dial_stdio_test.go @@ -17,12 +17,6 @@ var _ = Describe("podman system dial-stdio", func() { Expect(session.OutputToString()).To(ContainSubstring("Examples: podman system dial-stdio")) }) - It("podman system dial-stdio while service is not running", func() { - if IsRemote() { - Skip("this test is only for non-remote") - } - session := podmanTest.Podman([]string{"system", "dial-stdio"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(ExitWithError(125, "Error: failed to open connection to podman")) - }) + // TODO: this should have a proper connection test where we spawn a server + // and the use dial-stdio to connect to it and send data. }) From 23d4908c8bda1e4d86d6c799c51ff9ffc3374337 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 17 Dec 2024 17:05:47 +0100 Subject: [PATCH 3/4] test/system: CopyDirectory() do not chown files If the source dir is owned by another user then the test the chown will fail assuming we run the tests rootless. This function is only used by the quadlet tests and for the purpose all we need is to read the files so the simple fix is remove the chown as this should make the tests pass on the special debian gating env. Fixes #24802 Signed-off-by: Paul Holzinger --- test/e2e/common_test.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index 939624074b..f51d1b42c8 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -20,7 +20,6 @@ import ( "strconv" "strings" "sync" - "syscall" "testing" "time" @@ -1469,11 +1468,6 @@ func CopyDirectory(srcDir, dest string) error { return err } - stat, ok := fileInfo.Sys().(*syscall.Stat_t) - if !ok { - return fmt.Errorf("failed to get raw syscall.Stat_t data for %q", sourcePath) - } - switch fileInfo.Mode() & os.ModeType { case os.ModeDir: if err := os.MkdirAll(destPath, 0755); err != nil { @@ -1492,10 +1486,6 @@ func CopyDirectory(srcDir, dest string) error { } } - if err := os.Lchown(destPath, int(stat.Uid), int(stat.Gid)); err != nil { - return err - } - fInfo, err := entry.Info() if err != nil { return err From f2f6eb88e94824a8e1e472adcae8d4327e5aac6f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 17 Dec 2024 17:20:28 +0100 Subject: [PATCH 4/4] test/system: fix "podman play --build private registry" error When running this test on a system without unqualifiedsearch registries it will fail with a different error causing the test to fail. to avoid that case define our own registries.conf that defines quay.io as registry. This should make the test pass in the debian env. Signed-off-by: Paul Holzinger --- test/system/700-play.bats | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/system/700-play.bats b/test/system/700-play.bats index a4dc40cd55..762d810362 100644 --- a/test/system/700-play.bats +++ b/test/system/700-play.bats @@ -1001,6 +1001,14 @@ _EOF # Remove the local image to make sure it will be pulled again run_podman image rm --ignore $from_image + # The error below assumes unqualified-search registries exist, however the default + # distro config may not set some and thus resulting in a different error message. + # We could try to match a third or or simply force a know static config to trigger + # the right error. + local CONTAINERS_REGISTRIES_CONF="$PODMAN_TMPDIR/registries.conf" + echo 'unqualified-search-registries = ["quay.io"]' > "$CONTAINERS_REGISTRIES_CONF" + export CONTAINERS_REGISTRIES_CONF + _write_test_yaml command=id image=$userimage run_podman 125 play kube --build --start=false $TESTYAML assert "$output" "=~" \