From 76a4fdc358f154bb3556892a27c2702d7cd9dc47 Mon Sep 17 00:00:00 2001 From: Ian Page Hands Date: Mon, 7 Apr 2025 16:41:52 -0700 Subject: [PATCH] cmd: Fix help text. --config specifies a dir not a regular file This `--config` option was initially added here: https://github.com/containers/podman/commit/4e4c3e3dbffd6c8fc2c1a8674a581aed61f2d629 Under the hood this simply modifies env to set DOCKER_CONFIG= The DOCKER_CONFIG env var is used as a directory that contains multiple config files... of which podman and container libs probably only use `$DIR/config.json`. See: https://docs.docker.com/reference/cli/docker/#environment-variables The old CMD and help text was misleading... if we point the at a regular file we can see errors like: ``` $ touch /tmp/foo/tmpcr9zrx71 $ /bin/podman --config /tmp/foo/tmpcr9zrx71 build -t foobar:latest Error: creating build container: initializing source docker://quay.io/centos/centos:stream9: getting username and password: reading JSON file "/tmp/foo/tmpcr9zrx71/config.json": open /tmp/foo/tmpcr9zrx71/config.json: not a directory ``` ^^ In this case we had created `/tmp/foo/tmpcr9zrx71` as a regular file. Signed-off-by: Ian Page Hands --- cmd/podman/root.go | 18 +++++++++++++++++- pkg/domain/entities/engine.go | 2 +- test/e2e/run_test.go | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/cmd/podman/root.go b/cmd/podman/root.go index fc7ebd69b8..cc35c6c09d 100644 --- a/cmd/podman/root.go +++ b/cmd/podman/root.go @@ -3,6 +3,7 @@ package main import ( "errors" "fmt" + "io/fs" "os" "path/filepath" "runtime" @@ -421,6 +422,21 @@ func persistentPostRunE(cmd *cobra.Command, args []string) error { func configHook() { if dockerConfig != "" { + // NOTE: we dont allow pointing --config to a regular file. Code assumed config is a directory + // We do allow though pointing at a nonexistent path. Some downstream code will create the folder + // at runtime if it does not yet exist. + statInfo, err := os.Stat(dockerConfig) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + // Cases where the folder does not exist are allowed, BUT cases where some other Stat() error + // is returned should fail + fmt.Fprintf(os.Stderr, "Supplied --config folder (%s) exists but is not accessible: %s", dockerConfig, err.Error()) + os.Exit(1) + } + if err == nil && !statInfo.IsDir() { + // Cases where it does exist but is a file should fail + fmt.Fprintf(os.Stderr, "Supplied --config file (%s) is not a directory", dockerConfig) + os.Exit(1) + } if err := os.Setenv("DOCKER_CONFIG", dockerConfig); err != nil { fmt.Fprintf(os.Stderr, "cannot set DOCKER_CONFIG=%s: %s", dockerConfig, err.Error()) os.Exit(1) @@ -500,7 +516,7 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) { _ = lFlags.MarkHidden("host") configFlagName := "config" - lFlags.StringVar(&dockerConfig, "config", "", "Location of authentication config file") + lFlags.StringVar(&dockerConfig, "config", "", "Path to directory containing authentication config file") _ = cmd.RegisterFlagCompletionFunc(configFlagName, completion.AutocompleteDefault) // Context option added just for compatibility with DockerCLI. diff --git a/pkg/domain/entities/engine.go b/pkg/domain/entities/engine.go index c5e15de1d9..92695d9476 100644 --- a/pkg/domain/entities/engine.go +++ b/pkg/domain/entities/engine.go @@ -29,7 +29,7 @@ type PodmanConfig struct { ContainersConf *config.Config ContainersConfDefaultsRO *config.Config // The read-only! defaults from containers.conf. DBBackend string // Hidden: change the database backend - DockerConfig string // Location of authentication config file + DockerConfig string // Path to directory containing authentication config file CgroupUsage string // rootless code determines Usage message ConmonPath string // --conmon flag will set Engine.ConmonPath CPUProfile string // Hidden: Should CPU profile be taken diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 9b0eea74fa..a7c9f99e51 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -85,6 +85,24 @@ var _ = Describe("Podman run", func() { Expect(session.OutputToString()).To(ContainSubstring("graphRootMounted=1")) }) + It("podman run does not fail if --config points to non-existent", func() { + // Here we expect no failure. We have some existing code that will create the folder + // when it detects that the folder is missing. + nonExistentPath := "/tmp/def_does_not_exist" + session := podmanTest.Podman([]string{"--config", nonExistentPath, "run", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + }) + + It("podman run fails if --config points to regular file", func() { + tempFile, err := os.CreateTemp(podmanTest.TempDir, "") + Expect(err).ToNot(HaveOccurred()) + tempFileName := tempFile.Name() + session := podmanTest.Podman([]string{"--config", tempFileName, "run", ALPINE, "ls"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError(1, fmt.Sprintf(`Supplied --config file (%s) is not a directory`, tempFileName))) + }) + It("podman run from manifest list", func() { session := podmanTest.Podman([]string{"manifest", "create", "localhost/test:latest"}) session.WaitWithDefaultTimeout()