Merge pull request #25823 from iphands/main

cmd: Fix help text. --config specifies a dir not a regular file

Author: openshift-merge-bot[bot]

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.

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

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()