From 90634d5ee2524b532769959bba9f728b520ed1ac Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 7 Sep 2022 14:08:52 +0200 Subject: podman volume ls: use report.Formatter over Template Currently the podman command --format output code uses a mix of report.Formatter and report.Template. I patched report.Formatter to correctly handle newlines[1]. Since we cannot fix this with report.Template we have to migrate all users to report.Formatter. This ensures consistent behavior for all commands. This change does not change the output, we can add a new test for the newline bug when the common PR is vendored in. Also fixa bug since the table format is expected to print headers as well. [1] https://github.com/containers/common/pull/1146 Signed-off-by: Paul Holzinger --- test/e2e/volume_ls_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/test/e2e/volume_ls_test.go b/test/e2e/volume_ls_test.go index dcfb13f4e..24ea50b26 100644 --- a/test/e2e/volume_ls_test.go +++ b/test/e2e/volume_ls_test.go @@ -75,7 +75,10 @@ var _ = Describe("Podman volume ls", func() { session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - Expect(session.OutputToStringArray()).To(HaveLen(1), session.OutputToString()) + arr := session.OutputToStringArray() + Expect(arr).To(HaveLen(2)) + Expect(arr[0]).To(ContainSubstring("NAME")) + Expect(arr[1]).To(ContainSubstring("myvol")) }) It("podman ls volume with --filter flag", func() { -- cgit v1.2.3-54-g00ecf From 1acebdda371ae84c4354de0b6eff22e9e70f31fe Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Wed, 7 Sep 2022 12:35:43 -0600 Subject: tests for 15673 Signed-off-by: Ed Santiago --- test/system/610-format.bats | 152 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 test/system/610-format.bats (limited to 'test') diff --git a/test/system/610-format.bats b/test/system/610-format.bats new file mode 100644 index 000000000..77921b9a2 --- /dev/null +++ b/test/system/610-format.bats @@ -0,0 +1,152 @@ +#!/usr/bin/env bats -*- bats -*- +# +# PR #15673: For all commands that accept --format '{{.GoTemplate}}', +# invoke with --format '{{"\n"}}' and make sure they don't choke. +# + +load helpers + +function teardown() { + # In case test fails: standard teardown does not wipe machines + run_podman '?' machine rm -f mymachine + + basic_teardown +} + +# Most commands can't just be run with --format; they need an argument or +# option. This table defines what those are. +# +# FIXME: once you've finished fixing them all, remove the SKIPs (just +# remove the entire lines, except for pod-inspect, just remove the SKIP +# but leave "mypod") +extra_args_table=" +info | SKIP +system info | SKIP +machine info | SKIP +version | SKIP + +history | $IMAGE +image history | $IMAGE +image inspect | $IMAGE +container inspect | no-such-container +machine inspect | mymachine + +volume inspect | -a +secret inspect | -a +network inspect | podman +ps | -a + +image search | sdfsdf +search | sdfsdf + +pod inspect | mypod SKIP + +container stats | --no-stream +pod stats | --no-stream +stats | --no-stream +events | --stream=false --events-backend=file +" + +# Main test loop. Recursively runs 'podman [subcommand] help', looks for: +# > '[command]', which indicates, recurse; or +# > '--format', in which case we +# > check autocompletion, look for Go templates, in which case we +# > run the command with --format '{{"\n"}}' and make sure it passes +function check_subcommand() { + for cmd in $(_podman_commands "$@"); do + # Human-readable podman command string, with multiple spaces collapsed + command_string="podman $* $cmd" + command_string=${command_string// / } # 'podman x' -> 'podman x' + + # Run --help, decide if this is a subcommand with subcommands + run_podman "$@" $cmd --help + local full_help="$output" + + # The line immediately after 'Usage:' gives us a 1-line synopsis + usage=$(echo "$full_help" | grep -A1 '^Usage:' | tail -1) + assert "$usage" != "" "podman $cmd: no Usage message found" + + # Strip off the leading command string; we no longer need it + usage=$(sed -e "s/^ $command_string \?//" <<<"$usage") + + # If usage ends in '[command]', recurse into subcommands + if expr "$usage" : '\[command\]' >/dev/null; then + # (except for 'podman help', which is a special case) + if [[ $cmd != "help" ]]; then + check_subcommand "$@" $cmd + fi + continue + fi + + # Not a subcommand-subcommand. Look for --format option + if [[ ! "$output" =~ "--format" ]]; then + continue + fi + + # Have --format. Make sure it's a Go-template option, not like --push + run_podman __completeNoDesc "$@" "$cmd" --format '{{.' + if [[ ! "$output" =~ \{\{\.[A-Z] ]]; then + continue + fi + + # Got one. + dprint "$command_string has --format" + + # Whatever is needed to make a runnable command + local extra=${extra_args[$command_string]} + if [[ -n "$extra" ]]; then + # Cross off our list + unset extra_args["$command_string"] + fi + + # FIXME: you can remove this once you're finished with #15673 + if [[ "$extra" =~ SKIP ]]; then + continue + fi + + # This is what does the work. We should never see the unterminated err + run_podman '?' "$@" "$cmd" $extra --format '{{"\n"}}' + assert "$output" !~ "unterminated quoted string" \ + "$command_string --format " + + # This will (probably) only trigger if we get a new podman subcommand. + # It means someone needs to figure out the right magic args to use + # when invoking the subcommand. + if [[ $status -ne 0 ]]; then + if [[ -z "$extra" ]]; then + die "'$command_string' barfed with '$output'. You probably need to special-case this command in extra_args_table in this script." + fi + fi + done +} + +# Test entry point +@test "check Go template formatting" { + skip_if_remote + + # Convert the table at top to an associative array, keyed on subcommand + declare -A extra_args + while read subcommand extra; do + extra_args["podman $subcommand"]=$extra + done < <(parse_table "$extra_args_table") + + # Setup: 'pod ps' needs an actual pod; 'machine inspect' needs a machine + run_podman pod create mypod + run_podman machine init --image-path=/dev/null mymachine + + # Run the test + check_subcommand + + # Clean up + run_podman pod rm mypod + run_podman rmi $(pause_image) + run_podman machine rm -f mymachine + + # Make sure there are no leftover commands in our table - this would + # indicate a typo in the table, or a flaw in our logic such that + # we're not actually recursing. + local leftovers="${!extra_args[@]}" + assert "$leftovers" = "" "Did not find (or test) subcommands:" +} + +# vim: filetype=sh -- cgit v1.2.3-54-g00ecf From e5389e98f78294f404ba611fc0d6a6396e8ca6f0 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 8 Sep 2022 12:48:28 +0200 Subject: podman info: use report.Formatter over Template Currently the podman command --format output code uses a mix of report.Formatter and report.Template. I patched report.Formatter to correctly handle newlines[1]. Since we cannot fix this with report.Template we have to migrate all users to report.Formatter. This ensures consistent behavior for all commands. This change does not change the output. [1] https://github.com/containers/common/pull/1146 Signed-off-by: Paul Holzinger --- cmd/podman/system/info.go | 13 +++++++------ test/system/610-format.bats | 2 -- 2 files changed, 7 insertions(+), 8 deletions(-) (limited to 'test') diff --git a/cmd/podman/system/info.go b/cmd/podman/system/info.go index 296fa4def..9613d5e56 100644 --- a/cmd/podman/system/info.go +++ b/cmd/podman/system/info.go @@ -3,7 +3,6 @@ package system import ( "fmt" "os" - "text/template" "github.com/containers/common/pkg/completion" "github.com/containers/common/pkg/report" @@ -86,14 +85,16 @@ func info(cmd *cobra.Command, args []string) error { } fmt.Println(string(b)) case cmd.Flags().Changed("format"): - // Cannot use report.New() as it enforces {{range .}} for OriginUser templates - tmpl := template.New(cmd.Name()).Funcs(template.FuncMap(report.DefaultFuncs)) - inFormat = report.NormalizeFormat(inFormat) - tmpl, err := tmpl.Parse(inFormat) + rpt := report.New(os.Stdout, cmd.Name()) + defer rpt.Flush() + + // Use OriginUnknown so it does not add an extra range since it + // will only be called for a single element and not a slice. + rpt, err = rpt.Parse(report.OriginUnknown, inFormat) if err != nil { return err } - return tmpl.Execute(os.Stdout, info) + return rpt.Execute(info) default: b, err := yaml.Marshal(info) if err != nil { diff --git a/test/system/610-format.bats b/test/system/610-format.bats index 77921b9a2..3781bc910 100644 --- a/test/system/610-format.bats +++ b/test/system/610-format.bats @@ -20,8 +20,6 @@ function teardown() { # remove the entire lines, except for pod-inspect, just remove the SKIP # but leave "mypod") extra_args_table=" -info | SKIP -system info | SKIP machine info | SKIP version | SKIP -- cgit v1.2.3-54-g00ecf From 0c21dcf70c0a69f4614ca5d28c461f8ea50665dd Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 8 Sep 2022 13:48:37 +0200 Subject: podman machine info: use report.Formatter over Template Currently the podman command --format output code uses a mix of report.Formatter and report.Template. I patched report.Formatter to correctly handle newlines[1]. Since we cannot fix this with report.Template we have to migrate all users to report.Formatter. This ensures consistent behavior for all commands. This change does not change the output. [1] https://github.com/containers/common/pull/1146 Signed-off-by: Paul Holzinger --- cmd/podman/machine/info.go | 12 +++++++----- test/system/610-format.bats | 1 - 2 files changed, 7 insertions(+), 6 deletions(-) (limited to 'test') diff --git a/cmd/podman/machine/info.go b/cmd/podman/machine/info.go index 1151f9e86..9c7454379 100644 --- a/cmd/podman/machine/info.go +++ b/cmd/podman/machine/info.go @@ -5,7 +5,6 @@ package machine import ( "fmt" - "html/template" "os" "runtime" @@ -75,13 +74,16 @@ func info(cmd *cobra.Command, args []string) error { } fmt.Println(string(b)) case cmd.Flags().Changed("format"): - tmpl := template.New(cmd.Name()).Funcs(template.FuncMap(report.DefaultFuncs)) - inFormat = report.NormalizeFormat(inFormat) - tmpl, err := tmpl.Parse(inFormat) + rpt := report.New(os.Stdout, cmd.Name()) + defer rpt.Flush() + + // Use OriginUnknown so it does not add an extra range since it + // will only be called for a single element and not a slice. + rpt, err = rpt.Parse(report.OriginUnknown, inFormat) if err != nil { return err } - return tmpl.Execute(os.Stdout, info) + return rpt.Execute(info) default: b, err := yaml.Marshal(info) if err != nil { diff --git a/test/system/610-format.bats b/test/system/610-format.bats index 3781bc910..b0dbf2f48 100644 --- a/test/system/610-format.bats +++ b/test/system/610-format.bats @@ -20,7 +20,6 @@ function teardown() { # remove the entire lines, except for pod-inspect, just remove the SKIP # but leave "mypod") extra_args_table=" -machine info | SKIP version | SKIP history | $IMAGE -- cgit v1.2.3-54-g00ecf From 7f8e99ded41fe08564546277ce7f922434c354e8 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 8 Sep 2022 14:02:40 +0200 Subject: podman version: use report.Formatter over Template Currently the podman command --format output code uses a mix of report.Formatter and report.Template. I patched report.Formatter to correctly handle newlines[1]. Since we cannot fix this with report.Template we have to migrate all users to report.Formatter. This ensures consistent behavior for all commands. This change does not change the output. [1] https://github.com/containers/common/pull/1146 Signed-off-by: Paul Holzinger --- cmd/podman/system/version.go | 19 +++++++++++-------- test/system/610-format.bats | 2 -- 2 files changed, 11 insertions(+), 10 deletions(-) (limited to 'test') diff --git a/cmd/podman/system/version.go b/cmd/podman/system/version.go index 7202b2c08..33ab0f757 100644 --- a/cmd/podman/system/version.go +++ b/cmd/podman/system/version.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "strings" - "text/template" "github.com/containers/common/pkg/completion" "github.com/containers/common/pkg/report" @@ -12,6 +11,7 @@ import ( "github.com/containers/podman/v4/cmd/podman/registry" "github.com/containers/podman/v4/cmd/podman/validate" "github.com/containers/podman/v4/pkg/domain/entities" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) @@ -53,22 +53,25 @@ func version(cmd *cobra.Command, args []string) error { } if cmd.Flag("format").Changed { - // Cannot use report.New() as it enforces {{range .}} for OriginUser templates - tmpl := template.New(cmd.Name()).Funcs(template.FuncMap(report.DefaultFuncs)) + rpt := report.New(os.Stdout, cmd.Name()) + defer rpt.Flush() - versionFormat = report.NormalizeFormat(versionFormat) - tmpl, err := tmpl.Parse(versionFormat) + // Use OriginUnknown so it does not add an extra range since it + // will only be called for a single element and not a slice. + rpt, err = rpt.Parse(report.OriginUnknown, versionFormat) if err != nil { return err } - if err := tmpl.Execute(os.Stdout, versions); err != nil { + if err := rpt.Execute(versions); err != nil { + // only log at debug since we fall back to the client only template + logrus.Debugf("Failed to execute template: %v", err) // On Failure, assume user is using older version of podman version --format and check client versionFormat = strings.ReplaceAll(versionFormat, ".Server.", ".") - tmpl, err := tmpl.Parse(versionFormat) + rpt, err := rpt.Parse(report.OriginUnknown, versionFormat) if err != nil { return err } - if err := tmpl.Execute(os.Stdout, versions.Client); err != nil { + if err := rpt.Execute(versions.Client); err != nil { return err } } diff --git a/test/system/610-format.bats b/test/system/610-format.bats index b0dbf2f48..770f91cf5 100644 --- a/test/system/610-format.bats +++ b/test/system/610-format.bats @@ -20,8 +20,6 @@ function teardown() { # remove the entire lines, except for pod-inspect, just remove the SKIP # but leave "mypod") extra_args_table=" -version | SKIP - history | $IMAGE image history | $IMAGE image inspect | $IMAGE -- cgit v1.2.3-54-g00ecf From 908458d0639c349a37275dff09153adfcb65b7eb Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 8 Sep 2022 17:43:07 +0200 Subject: enable podman pod inspect --format test Now that commit d10e77e1bcd2 is merged, it will reuse the same template logic as inspect and therefore should just work. Also remove the FIXME from eds test. Signed-off-by: Paul Holzinger --- test/system/610-format.bats | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'test') diff --git a/test/system/610-format.bats b/test/system/610-format.bats index 770f91cf5..3d545f29f 100644 --- a/test/system/610-format.bats +++ b/test/system/610-format.bats @@ -34,7 +34,7 @@ ps | -a image search | sdfsdf search | sdfsdf -pod inspect | mypod SKIP +pod inspect | mypod container stats | --no-stream pod stats | --no-stream @@ -94,11 +94,6 @@ function check_subcommand() { unset extra_args["$command_string"] fi - # FIXME: you can remove this once you're finished with #15673 - if [[ "$extra" =~ SKIP ]]; then - continue - fi - # This is what does the work. We should never see the unterminated err run_podman '?' "$@" "$cmd" $extra --format '{{"\n"}}' assert "$output" !~ "unterminated quoted string" \ -- cgit v1.2.3-54-g00ecf From 9d41b95d72ef29870e6c325557c89c2db818a371 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 12 Sep 2022 11:39:55 +0200 Subject: rework --format system test This version does a much better job of error reporting and also catches more commands. Changes from edsantiago. Signed-off-by: Paul Holzinger --- test/system/610-format.bats | 63 ++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 18 deletions(-) (limited to 'test') diff --git a/test/system/610-format.bats b/test/system/610-format.bats index 3d545f29f..096d0228b 100644 --- a/test/system/610-format.bats +++ b/test/system/610-format.bats @@ -7,8 +7,9 @@ load helpers function teardown() { - # In case test fails: standard teardown does not wipe machines + # In case test fails: standard teardown does not wipe machines or secrets run_podman '?' machine rm -f mymachine + run_podman '?' secret rm mysecret basic_teardown } @@ -23,11 +24,11 @@ extra_args_table=" history | $IMAGE image history | $IMAGE image inspect | $IMAGE -container inspect | no-such-container +container inspect | mycontainer machine inspect | mymachine volume inspect | -a -secret inspect | -a +secret inspect | mysecret network inspect | podman ps | -a @@ -49,6 +50,14 @@ events | --stream=false --events-backend=file # > run the command with --format '{{"\n"}}' and make sure it passes function check_subcommand() { for cmd in $(_podman_commands "$@"); do + # Special case: 'podman machine' can't be run as root. No override. + if [[ "$cmd" = "machine" ]]; then + if ! is_rootless; then + unset extra_args["podman machine inspect"] + continue + fi + fi + # Human-readable podman command string, with multiple spaces collapsed command_string="podman $* $cmd" command_string=${command_string// / } # 'podman x' -> 'podman x' @@ -94,25 +103,35 @@ function check_subcommand() { unset extra_args["$command_string"] fi - # This is what does the work. We should never see the unterminated err + # This is what does the work. We run with '?' so we can offer + # better error messages than just "exited with error status". run_podman '?' "$@" "$cmd" $extra --format '{{"\n"}}' - assert "$output" !~ "unterminated quoted string" \ - "$command_string --format " - - # This will (probably) only trigger if we get a new podman subcommand. - # It means someone needs to figure out the right magic args to use - # when invoking the subcommand. - if [[ $status -ne 0 ]]; then - if [[ -z "$extra" ]]; then - die "'$command_string' barfed with '$output'. You probably need to special-case this command in extra_args_table in this script." - fi - fi + + # Output must always be empty. + # + # - If you see "unterminated quoted string" here, there's a + # regression, and you need to fix --format (see PR #15673) + # + # - If you see any other error, it probably means that someone + # added a new podman subcommand that supports --format but + # needs some sort of option or argument to actually run. + # See 'extra_args_table' at the top of this script. + # + assert "$output" = "" "$command_string --format '{{\"\n\"}}'" + + # *Now* check exit status. This should never, ever, ever trigger! + # If it does, it means the podman command failed without an err msg! + assert "$status" = "0" \ + "$command_string --format '{{\"\n\"}}' failed with no output!" done } # Test entry point @test "check Go template formatting" { skip_if_remote + if is_ubuntu; then + skip 'ubuntu VMs do not have qemu (exec: "qemu-system-x86_64": executable file not found in $PATH)' + fi # Convert the table at top to an associative array, keyed on subcommand declare -A extra_args @@ -120,9 +139,13 @@ function check_subcommand() { extra_args["podman $subcommand"]=$extra done < <(parse_table "$extra_args_table") - # Setup: 'pod ps' needs an actual pod; 'machine inspect' needs a machine + # Setup: some commands need a container, pod, machine, or secret + run_podman run -d --name mycontainer $IMAGE top run_podman pod create mypod - run_podman machine init --image-path=/dev/null mymachine + run_podman secret create mysecret /etc/hosts + if is_rootless; then + run_podman machine init --image-path=/dev/null mymachine + fi # Run the test check_subcommand @@ -130,7 +153,11 @@ function check_subcommand() { # Clean up run_podman pod rm mypod run_podman rmi $(pause_image) - run_podman machine rm -f mymachine + run_podman rm -f -t0 mycontainer + run_podman secret rm mysecret + if is_rootless; then + run_podman machine rm -f mymachine + fi # Make sure there are no leftover commands in our table - this would # indicate a typo in the table, or a flaw in our logic such that -- cgit v1.2.3-54-g00ecf