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/system') 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/system') 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/system') 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/system') 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/system') 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/system') 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