Skip to content

Commit 76a4fdc

Browse files
committed
cmd: Fix help text. --config specifies a dir not a regular file
This `--config` option was initially added here: 4e4c3e3 Under the hood this simply modifies env to set DOCKER_CONFIG=<passed in string> 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 <[email protected]>
1 parent 37dc5fd commit 76a4fdc

File tree

3 files changed

+36
-2
lines changed

3 files changed

+36
-2
lines changed

cmd/podman/root.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package main
33
import (
44
"errors"
55
"fmt"
6+
"io/fs"
67
"os"
78
"path/filepath"
89
"runtime"
@@ -421,6 +422,21 @@ func persistentPostRunE(cmd *cobra.Command, args []string) error {
421422

422423
func configHook() {
423424
if dockerConfig != "" {
425+
// NOTE: we dont allow pointing --config to a regular file. Code assumed config is a directory
426+
// We do allow though pointing at a nonexistent path. Some downstream code will create the folder
427+
// at runtime if it does not yet exist.
428+
statInfo, err := os.Stat(dockerConfig)
429+
if err != nil && !errors.Is(err, fs.ErrNotExist) {
430+
// Cases where the folder does not exist are allowed, BUT cases where some other Stat() error
431+
// is returned should fail
432+
fmt.Fprintf(os.Stderr, "Supplied --config folder (%s) exists but is not accessible: %s", dockerConfig, err.Error())
433+
os.Exit(1)
434+
}
435+
if err == nil && !statInfo.IsDir() {
436+
// Cases where it does exist but is a file should fail
437+
fmt.Fprintf(os.Stderr, "Supplied --config file (%s) is not a directory", dockerConfig)
438+
os.Exit(1)
439+
}
424440
if err := os.Setenv("DOCKER_CONFIG", dockerConfig); err != nil {
425441
fmt.Fprintf(os.Stderr, "cannot set DOCKER_CONFIG=%s: %s", dockerConfig, err.Error())
426442
os.Exit(1)
@@ -500,7 +516,7 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
500516
_ = lFlags.MarkHidden("host")
501517

502518
configFlagName := "config"
503-
lFlags.StringVar(&dockerConfig, "config", "", "Location of authentication config file")
519+
lFlags.StringVar(&dockerConfig, "config", "", "Path to directory containing authentication config file")
504520
_ = cmd.RegisterFlagCompletionFunc(configFlagName, completion.AutocompleteDefault)
505521

506522
// Context option added just for compatibility with DockerCLI.

pkg/domain/entities/engine.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type PodmanConfig struct {
2929
ContainersConf *config.Config
3030
ContainersConfDefaultsRO *config.Config // The read-only! defaults from containers.conf.
3131
DBBackend string // Hidden: change the database backend
32-
DockerConfig string // Location of authentication config file
32+
DockerConfig string // Path to directory containing authentication config file
3333
CgroupUsage string // rootless code determines Usage message
3434
ConmonPath string // --conmon flag will set Engine.ConmonPath
3535
CPUProfile string // Hidden: Should CPU profile be taken

test/e2e/run_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,24 @@ var _ = Describe("Podman run", func() {
8585
Expect(session.OutputToString()).To(ContainSubstring("graphRootMounted=1"))
8686
})
8787

88+
It("podman run does not fail if --config points to non-existent", func() {
89+
// Here we expect no failure. We have some existing code that will create the folder
90+
// when it detects that the folder is missing.
91+
nonExistentPath := "/tmp/def_does_not_exist"
92+
session := podmanTest.Podman([]string{"--config", nonExistentPath, "run", ALPINE, "ls"})
93+
session.WaitWithDefaultTimeout()
94+
Expect(session).Should(ExitCleanly())
95+
})
96+
97+
It("podman run fails if --config points to regular file", func() {
98+
tempFile, err := os.CreateTemp(podmanTest.TempDir, "")
99+
Expect(err).ToNot(HaveOccurred())
100+
tempFileName := tempFile.Name()
101+
session := podmanTest.Podman([]string{"--config", tempFileName, "run", ALPINE, "ls"})
102+
session.WaitWithDefaultTimeout()
103+
Expect(session).Should(ExitWithError(1, fmt.Sprintf(`Supplied --config file (%s) is not a directory`, tempFileName)))
104+
})
105+
88106
It("podman run from manifest list", func() {
89107
session := podmanTest.Podman([]string{"manifest", "create", "localhost/test:latest"})
90108
session.WaitWithDefaultTimeout()

0 commit comments

Comments
 (0)