containers
/
automation-tests
mirror of https://github.com/containers/automation-tests.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #20945 from Luap99/string-array 

cli: use StringArray over StringSlice Part 1
This commit is contained in:
openshift-merge-bot[bot] 2023-12-09 21:11:39 +00:00 committed by GitHub
parent d513749085 4590b663a7
commit c87311b6d5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 85 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
cmd/podman/README.md
View File

@ -109,3 +109,11 @@ The complete set can be found in the `validate` package, here are some examples:
created := validate.ChoiceValue(&opts.Sort, "command", "created", "id", "image", "names", "runningfor", "size", "status")
flags.Var(created, "sort", "Sort output by: "+created.Choices())
```
## Adding CLI flags
When adding adding a new cli option that accepts a string array, there are two options to choose from: `StringSlice()` and `StringArray()`.
They differ slightly in their behavior: `StringSlice()` allows the values to be comma separated so `--opt v1,v2 --opt v3` results in
`[]string{"v1", "v2", "v3"}`, while `StringArray()` would result in `[]string{"v1,v2", "v3"}`. Thus it is impossible to use values with comma in `StringSlice()`, which makes it unsuitable for flags that accept arbitrary values such as file paths as example. Also, because `StringSlice()` uses the csv lib to parse the values, it has special escaping rules for things like quotes, see https://github.com/containers/podman/issues/20064 for an example of how complicated things can get because of this.
Thus use `StringSlice()` only when the option accepts predefined values that do not contain special characters, for example `--cap-add` and `--cap-drop` are a good example for this. Using `--cap-add NET_ADMIN,NET_RAW` is equal to `--cap-add NET_ADMIN --cap-add NET_RAW` so it is better suited to save some typing for users.
When in doubt always choose `StringArray()` over `StringSlice()`.

24
cmd/podman/common/create.go
View File

@ -33,7 +33,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
if mode == entities.CreateMode { // regular create flags
annotationFlagName := "annotation"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.Annotation,
annotationFlagName, []string{},
"Add annotations to container (key=value)",
@ -147,7 +147,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
}
envFileFlagName := "env-file"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.EnvFile,
envFileFlagName, []string{},
"Read in a file of environment variables",
@ -282,7 +282,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
_ = cmd.RegisterFlagCompletionFunc(logDriverFlagName, AutocompleteLogDriver)
logOptFlagName := "log-opt"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.LogOptions,
logOptFlagName, []string{},
"Logging driver options",
@ -595,7 +595,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
_ = cmd.RegisterFlagCompletionFunc(pidFileFlagName, completion.AutocompleteDefault)
chrootDirsFlagName := "chrootdirs"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.ChrootDirs,
chrootDirsFlagName, []string{},
"Chroot directories inside the container",
@ -611,7 +611,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
_ = cmd.RegisterFlagCompletionFunc(groupEntryName, completion.AutocompleteNone)
decryptionKeysFlagName := "decryption-key"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.DecryptionKeys,
decryptionKeysFlagName, []string{},
"Key needed to decrypt the image (e.g. /path/to/key.pem)",
@ -772,7 +772,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
_ = cmd.RegisterFlagCompletionFunc(labelFlagName, completion.AutocompleteNone)
labelFileFlagName := "label-file"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.LabelFile,
labelFileFlagName, []string{},
"Read in a line delimited file of labels",
@ -814,7 +814,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
_ = cmd.RegisterFlagCompletionFunc(volumeFlagName, AutocompleteVolumeFlag)
deviceFlagName := "device"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.Devices,
deviceFlagName, devices(),
"Add a host device to the container",
@ -898,7 +898,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
}
if mode == entities.CreateMode || mode == entities.UpdateMode {
deviceReadIopsFlagName := "device-read-iops"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.DeviceReadIOPs,
deviceReadIopsFlagName, []string{},
"Limit read rate (IO per second) from a device (e.g. --device-read-iops=/dev/sda:1000)",
@ -906,7 +906,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
_ = cmd.RegisterFlagCompletionFunc(deviceReadIopsFlagName, completion.AutocompleteDefault)
deviceWriteIopsFlagName := "device-write-iops"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.DeviceWriteIOPs,
deviceWriteIopsFlagName, []string{},
"Limit write rate (IO per second) to a device (e.g. --device-write-iops=/dev/sda:1000)",
@ -970,7 +970,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
_ = cmd.RegisterFlagCompletionFunc(memorySwapFlagName, completion.AutocompleteNone)
deviceReadBpsFlagName := "device-read-bps"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.DeviceReadBPs,
deviceReadBpsFlagName, []string{},
"Limit read rate (bytes per second) from a device (e.g. --device-read-bps=/dev/sda:1mb)",
@ -978,7 +978,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
_ = cmd.RegisterFlagCompletionFunc(deviceReadBpsFlagName, completion.AutocompleteDefault)
deviceWriteBpsFlagName := "device-write-bps"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.DeviceWriteBPs,
deviceWriteBpsFlagName, []string{},
"Limit write rate (bytes per second) to a device (e.g. --device-write-bps=/dev/sda:1mb)",
@ -994,7 +994,7 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
_ = cmd.RegisterFlagCompletionFunc(blkioWeightFlagName, completion.AutocompleteNone)
blkioWeightDeviceFlagName := "blkio-weight-device"
createFlags.StringSliceVar(
createFlags.StringArrayVar(
&cf.BlkIOWeightDevice,
blkioWeightDeviceFlagName, []string{},
"Block IO weight (relative device weight, format: `DEVICE_NAME:WEIGHT`)",

2
cmd/podman/containers/exec.go
View File

@ -68,7 +68,7 @@ func execFlags(cmd *cobra.Command) {
_ = cmd.RegisterFlagCompletionFunc(envFlagName, completion.AutocompleteNone)
envFileFlagName := "env-file"
flags.StringSliceVar(&envFile, envFileFlagName, []string{}, "Read in a file of environment variables")
flags.StringArrayVar(&envFile, envFileFlagName, []string{}, "Read in a file of environment variables")
_ = cmd.RegisterFlagCompletionFunc(envFileFlagName, completion.AutocompleteDefault)
flags.BoolVarP(&execOpts.Interactive, "interactive", "i", false, "Keep STDIN open even if not attached")

2
cmd/podman/registry/config.go
View File

@ -72,7 +72,7 @@ func containersConfModules() ([]string, error) {
fs.ParseErrorsWhitelist.UnknownFlags = true
fs.Usage = func() {}
fs.SetInterspersed(false)
fs.StringSliceVar(&modules, "module", nil, "")
fs.StringArrayVar(&modules, "module", nil, "")
fs.BoolP("help", "h", false, "") // Need a fake help flag to avoid the `pflag: help requested` error
return modules, fs.Parse(os.Args[index:])
}

4
cmd/podman/root.go
View File

@ -507,7 +507,7 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
// as a flag here to a) make sure that rootflags are aware of
// this flag and b) to have shell completions.
moduleFlagName := "module"
lFlags.StringSlice(moduleFlagName, nil, "Load the containers.conf(5) module")
lFlags.StringArray(moduleFlagName, nil, "Load the containers.conf(5) module")
_ = cmd.RegisterFlagCompletionFunc(moduleFlagName, common.AutocompleteContainersConfModules)
// A *hidden* flag to change the database backend.
@ -541,7 +541,7 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
_ = cmd.RegisterFlagCompletionFunc(eventsBackendFlagName, common.AutocompleteEventBackend)
hooksDirFlagName := "hooks-dir"
pFlags.StringSliceVar(&podmanConfig.HooksDir, hooksDirFlagName, podmanConfig.ContainersConfDefaultsRO.Engine.HooksDir.Get(), "Set the OCI hooks directory path (may be set multiple times)")
pFlags.StringArrayVar(&podmanConfig.HooksDir, hooksDirFlagName, podmanConfig.ContainersConfDefaultsRO.Engine.HooksDir.Get(), "Set the OCI hooks directory path (may be set multiple times)")
_ = cmd.RegisterFlagCompletionFunc(hooksDirFlagName, completion.AutocompleteDefault)
pFlags.IntVar(&podmanConfig.MaxWorks, "max-workers", (runtime.NumCPU()*3)+1, "The maximum number of workers for parallel operations")

8
test/e2e/create_test.go
View File

@ -101,7 +101,7 @@ var _ = Describe("Podman create", func() {
})
It("podman create adds annotation", func() {
session := podmanTest.Podman([]string{"create", "--annotation", "HELLO=WORLD", "--name", "annotate_test", ALPINE, "ls"})
session := podmanTest.Podman([]string{"create", "--annotation", "HELLO=WORLD,WithComma", "--name", "annotate_test", ALPINE, "ls"})
session.WaitWithDefaultTimeout()
Expect(session).Should(ExitCleanly())
Expect(podmanTest.NumberOfContainers()).To(Equal(1))
@ -109,7 +109,7 @@ var _ = Describe("Podman create", func() {
check := podmanTest.Podman([]string{"inspect", "annotate_test"})
check.WaitWithDefaultTimeout()
data := check.InspectContainerToJSON()
Expect(data[0].Config.Annotations).To(HaveKeyWithValue("HELLO", "WORLD"))
Expect(data[0].Config.Annotations).To(HaveKeyWithValue("HELLO", "WORLD,WithComma"))
})
It("podman create --entrypoint command", func() {
@ -717,7 +717,7 @@ var _ = Describe("Podman create", func() {
})
It("podman create --chrootdirs functionality test", func() {
session := podmanTest.Podman([]string{"create", "-t", "--chrootdirs", "/var/local/qwerty", ALPINE, "/bin/cat"})
session := podmanTest.Podman([]string{"create", "-t", "--chrootdirs", "/var/local/qwerty,withcomma", ALPINE, "/bin/cat"})
session.WaitWithDefaultTimeout()
Expect(session).Should(ExitCleanly())
ctrID := session.OutputToString()
@ -726,7 +726,7 @@ var _ = Describe("Podman create", func() {
setup.WaitWithDefaultTimeout()
Expect(setup).Should(ExitCleanly())
setup = podmanTest.Podman([]string{"exec", ctrID, "cmp", "/etc/resolv.conf", "/var/local/qwerty/etc/resolv.conf"})
setup = podmanTest.Podman([]string{"exec", ctrID, "cmp", "/etc/resolv.conf", "/var/local/qwerty,withcomma/etc/resolv.conf"})
setup.WaitWithDefaultTimeout()
Expect(setup).Should(ExitCleanly())
})

4
test/e2e/logs_test.go
View File

@ -536,7 +536,7 @@ var _ = Describe("Podman logs", func() {
It("using journald for container with container tag", func() {
SkipIfJournaldUnavailable()
logc := podmanTest.Podman([]string{"run", "--log-driver", "journald", "--log-opt=tag={{.ImageName}}", "-d", ALPINE, "sh", "-c", "echo podman; sleep 0.1; echo podman; sleep 0.1; echo podman"})
logc := podmanTest.Podman([]string{"run", "--log-driver", "journald", "--log-opt=tag={{.ImageName}},withcomma", "-d", ALPINE, "sh", "-c", "echo podman; sleep 0.1; echo podman; sleep 0.1; echo podman"})
logc.WaitWithDefaultTimeout()
Expect(logc).To(ExitCleanly())
cid := logc.OutputToString()
@ -549,7 +549,7 @@ var _ = Describe("Podman logs", func() {
cmd := exec.Command("journalctl", "--no-pager", "-o", "json", "--output-fields=CONTAINER_TAG", fmt.Sprintf("CONTAINER_ID_FULL=%s", cid))
out, err := cmd.CombinedOutput()
g.Expect(err).ToNot(HaveOccurred())
g.Expect(string(out)).To(ContainSubstring("alpine"))
g.Expect(string(out)).To(ContainSubstring(ALPINE + ",withcomma"))
}).Should(Succeed())
})

2
test/e2e/run_test.go
View File

@ -916,7 +916,7 @@ USER bin`, BB)
It("podman test hooks", func() {
SkipIfRemote("--hooks-dir does not work with remote")
hooksDir := tempdir + "/hooks"
hooksDir := tempdir + "/hooks,withcomma"
err := os.Mkdir(hooksDir, 0755)
Expect(err).ToNot(HaveOccurred())
hookJSONPath := filepath.Join(hooksDir, "checkhooks.json")

55
test/system/300-cli-parsing.bats
View File

@ -100,7 +100,7 @@ function _check_env {
}
@test "podman run --env-file" {
@test "podman run/exec --env-file" {
declare -A expect=(
[simple]="abc"
[special]="bcd#e!f|g hij=lmnop"
@ -116,7 +116,7 @@ function _check_env {
# Write two files, so we confirm that podman can accept multiple values
# and that the second will override the first
local envfile1="$PODMAN_TMPDIR/envfile-in-1"
local envfile1="$PODMAN_TMPDIR/envfile-in-1,withcomma"
local envfile2="$PODMAN_TMPDIR/envfile-in-2"
cat >$envfile1 <<EOF
infile2=this is set in env-file-1 and should be overridden in env-file-2
@ -158,6 +158,19 @@ EOF
expect[weird*na#me!]=$weirdname
_check_env $resultsfile
# Now check the same with podman exec
run_podman run -d --name testctr \
-v "$resultsfile:/envresults:Z" \
$IMAGE top
run_podman exec --env-file $envfile1 \
--env-file $envfile2 testctr \
sh -c 'env -0 >/envresults'
_check_env $resultsfile
run_podman rm -f -t0 testctr
}
# Obscure feature: '--env FOO*' will pass all env starting with FOO
@ -196,7 +209,7 @@ EOF
fi
# Same, with --env-file
local envfile="$PODMAN_TMPDIR/envfile-in-1"
local envfile="$PODMAN_TMPDIR/envfile-in-1,withcomma"
cat >$envfile <<EOF
$prefix*
NOT*DEFINED
@ -213,4 +226,40 @@ EOF
}
@test "podman create --label-file" {
declare -A expect=(
[simple]="abc"
[special]="bcd#e!f|g hij=lmnop"
[withquotes]='"withquotes"'
[withsinglequotes]="'withsingle'"
)
# Write two files, so we confirm that podman can accept multiple values
# and that the second will override the first
local labelfile1="$PODMAN_TMPDIR/label-file1,withcomma"
local labelfile2="$PODMAN_TMPDIR/label-file2"
cat >$labelfile1 <<EOF
simple=value1
# Comments ignored
EOF
for v in "${!expect[@]}"; do
echo "$v=${expect[$v]}" >>$labelfile2
done
run_podman create --rm --name testctr --label-file $labelfile1 \
--label-file $labelfile2 $IMAGE
for v in "${!expect[@]}"; do
run_podman inspect testctr --format "{{index .Config.Labels \"$v\"}}"
assert "$output" == "${expect[$v]}" "label $v"
done
run_podman rm testctr
}
# vim: filetype=sh

4
test/system/800-config.bats
View File

@ -104,8 +104,8 @@ See 'podman create --help'" "--module must be specified before the command"
run_podman rm -f $cid
# Nonexistent module path
nonesuch=${PODMAN_TMPDIR}/nonexistent
# Nonexistent module path with comma
nonesuch=${PODMAN_TMPDIR}/nonexistent,withcomma
run_podman 1 --module=$nonesuch sdfsdfdsf
is "$output" "Failed to obtain podman configuration: could not resolve module \"$nonesuch\": stat $nonesuch: no such file or directory" \
"--module=ENOENT"