Merge pull request #7199 from jwhonce/jira/run-898

Restore "table" --format from V1

cmd/podman/containers/diff.go

@@ -1,6 +1,7 @@
 package containers
 
 import (
+	"github.com/containers/podman/v2/cmd/podman/parse"
 	"github.com/containers/podman/v2/cmd/podman/registry"
 	"github.com/containers/podman/v2/cmd/podman/report"
 	"github.com/containers/podman/v2/cmd/podman/validate"
@@ -52,11 +53,11 @@ func diff(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	switch diffOpts.Format {
+	switch {
-	case "":
+	case parse.MatchesJSONFormat(diffOpts.Format):
-		return report.ChangesToTable(results)
-	case "json":
 		return report.ChangesToJSON(results)
+	case diffOpts.Format == "":
+		return report.ChangesToTable(results)
 	default:
 		return errors.New("only supported value for '--format' is 'json'")
 	}

cmd/podman/images/list.go

@@ -11,7 +11,9 @@ import (
 	"unicode"
 
 	"github.com/containers/image/v5/docker/reference"
+	"github.com/containers/podman/v2/cmd/podman/parse"
 	"github.com/containers/podman/v2/cmd/podman/registry"
+	"github.com/containers/podman/v2/cmd/podman/report"
 	"github.com/containers/podman/v2/pkg/domain/entities"
 	"github.com/docker/go-units"
 	"github.com/pkg/errors"
@@ -106,9 +108,12 @@ func images(cmd *cobra.Command, args []string) error {
 	switch {
 	case listFlag.quiet:
 		return writeID(imgs)
-	case cmd.Flag("format").Changed && listFlag.format == "json":
+	case parse.MatchesJSONFormat(listFlag.format):
 		return writeJSON(imgs)
 	default:
+		if cmd.Flag("format").Changed {
+			listFlag.noHeading = true // V1 compatibility
+		}
 		return writeTemplate(imgs)
 	}
 }
@@ -156,25 +161,29 @@ func writeJSON(images []imageReporter) error {
 }
 
 func writeTemplate(imgs []imageReporter) error {
-	var (
+	hdrs := report.Headers(imageReporter{}, map[string]string{
-		hdr, row string
+		"ID":       "IMAGE ID",
-	)
+		"ReadOnly": "R/O",
-	if len(listFlag.format) < 1 {
+	})
-		hdr, row = imageListFormat(listFlag)
+
+	var row string
+	if listFlag.format == "" {
+		row = lsFormatFromFlags(listFlag)
 	} else {
-		row = listFlag.format
+		row = report.NormalizeFormat(listFlag.format)
-		if !strings.HasSuffix(row, "\n") {
-			row += "\n"
-		}
 	}
-	format := hdr + "{{range . }}" + row + "{{end}}"
+
-	tmpl, err := template.New("list").Parse(format)
+	format := "{{range . }}" + row + "{{end}}"
-	if err != nil {
+	tmpl := template.Must(template.New("list").Parse(format))
-		return err
-	}
-	tmpl = template.Must(tmpl, nil)
 	w := tabwriter.NewWriter(os.Stdout, 8, 2, 2, ' ', 0)
 	defer w.Flush()
+
+	if !listFlag.noHeading {
+		if err := tmpl.Execute(w, hdrs); err != nil {
+			return err
+		}
+	}
+
 	return tmpl.Execute(w, imgs)
 }
 
@@ -276,40 +285,27 @@ func sortFunc(key string, data []imageReporter) func(i, j int) bool {
 	}
 }
 
-func imageListFormat(flags listFlagType) (string, string) {
+func lsFormatFromFlags(flags listFlagType) string {
-	// Defaults
+	row := []string{
-	hdr := "REPOSITORY\tTAG"
+		"{{if .Repository}}{{.Repository}}{{else}}<none>{{end}}",
-	row := "{{.Repository}}\t{{if .Tag}}{{.Tag}}{{else}}<none>{{end}}"
+		"{{if .Tag}}{{.Tag}}{{else}}<none>{{end}}",
-
-	if flags.digests {
-		hdr += "\tDIGEST"
-		row += "\t{{.Digest}}"
 	}
 
-	hdr += "\tIMAGE ID"
+	if flags.digests {
-	row += "\t{{.ID}}"
+		row = append(row, "{{.Digest}}")
+	}
 
-	hdr += "\tCREATED\tSIZE"
+	row = append(row, "{{.ID}}", "{{.Created}}", "{{.Size}}")
-	row += "\t{{.Created}}\t{{.Size}}"
 
 	if flags.history {
-		hdr += "\tHISTORY"
+		row = append(row, "{{if .History}}{{.History}}{{else}}<none>{{end}}")
-		row += "\t{{if .History}}{{.History}}{{else}}<none>{{end}}"
 	}
 
 	if flags.readOnly {
-		hdr += "\tReadOnly"
+		row = append(row, "{{.ReadOnly}}")
-		row += "\t{{.ReadOnly}}"
 	}
 
-	if flags.noHeading {
+	return strings.Join(row, "\t") + "\n"
-		hdr = ""
-	} else {
-		hdr += "\n"
-	}
-
-	row += "\n"
-	return hdr, row
 }
 
 type imageReporter struct {

cmd/podman/parse/json.go

@@ -2,8 +2,9 @@ package parse
 
 import "regexp"
 
-var jsonFormatRegex = regexp.MustCompile(`^(\s*json\s*|\s*{{\s*json\s*\.\s*}}\s*)$`)
+var jsonFormatRegex = regexp.MustCompile(`^\s*(json|{{\s*json\s*( \.)?\s*}})\s*$`)
 
+// MatchesJSONFormat test CLI --format string to be a JSON request
 func MatchesJSONFormat(s string) bool {
 	return jsonFormatRegex.Match([]byte(s))
 }

cmd/podman/parse/json_test.go

@@ -1,6 +1,8 @@
 package parse
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -13,18 +15,31 @@ func TestMatchesJSONFormat(t *testing.T) {
 	}{
 		{"json", true},
 		{" json", true},
-		{"json ", true},
+		{" json ", true},
 		{"  json   ", true},
+		{"{{json}}", true},
+		{"{{json }}", true},
 		{"{{json .}}", true},
 		{"{{ json .}}", true},
-		{"{{json .   }}", true},
+		{"{{ json . }}", true},
-		{"  {{  json .    }}   ", true},
+		{"  {{   json   .  }}  ", true},
-		{"{{json }}", false},
+		{"{{ json .", false},
-		{"{{json .", false},
 		{"json . }}", false},
+		{"{{.ID }} json .", false},
+		{"json .", false},
+		{"{{json.}}", false},
 	}
 
 	for _, tt := range tests {
 		assert.Equal(t, tt.expected, MatchesJSONFormat(tt.input))
 	}
+
+	for _, tc := range tests {
+		tc := tc
+		label := "MatchesJSONFormat/" + strings.ReplaceAll(tc.input, " ", "_")
+		t.Run(label, func(t *testing.T) {
+			t.Parallel()
+			assert.Equal(t, tc.expected, MatchesJSONFormat(tc.input), fmt.Sprintf("Scanning %q failed", tc.input))
+		})
+	}
 }

cmd/podman/report/format.go

@@ -0,0 +1,68 @@
+package report
+
+import (
+	"reflect"
+	"strings"
+)
+
+// tableReplacer will remove 'table ' prefix and clean up tabs
+var tableReplacer = strings.NewReplacer(
+	"table ", "",
+	`\t`, "\t",
+	`\n`, "\n",
+	" ", "\t",
+)
+
+// escapedReplacer will clean up escaped characters from CLI
+var escapedReplacer = strings.NewReplacer(
+	`\t`, "\t",
+	`\n`, "\n",
+)
+
+// NormalizeFormat reads given go template format provided by CLI and munges it into what we need
+func NormalizeFormat(format string) string {
+	f := format
+	// two replacers used so we only remove the prefix keyword `table`
+	if strings.HasPrefix(f, "table ") {
+		f = tableReplacer.Replace(f)
+	} else {
+		f = escapedReplacer.Replace(format)
+	}
+
+	if !strings.HasSuffix(f, "\n") {
+		f += "\n"
+	}
+
+	return f
+}
+
+// Headers queries the interface for field names
+func Headers(object interface{}, overrides map[string]string) []map[string]string {
+	value := reflect.ValueOf(object)
+	if value.Kind() == reflect.Ptr {
+		value = value.Elem()
+	}
+
+	// Column header will be field name upper-cased.
+	headers := make(map[string]string, value.NumField())
+	for i := 0; i < value.Type().NumField(); i++ {
+		field := value.Type().Field(i)
+		// Recurse to find field names from promoted structs
+		if field.Type.Kind() == reflect.Struct && field.Anonymous {
+			h := Headers(reflect.New(field.Type).Interface(), nil)
+			for k, v := range h[0] {
+				headers[k] = v
+			}
+			continue
+		}
+		headers[field.Name] = strings.ToUpper(field.Name)
+	}
+
+	if len(overrides) > 0 {
+		// Override column header as provided
+		for k, v := range overrides {
+			headers[k] = strings.ToUpper(v)
+		}
+	}
+	return []map[string]string{headers}
+}

cmd/podman/report/format_test.go

@@ -0,0 +1,35 @@
+package report
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestNormalizeFormat(t *testing.T) {
+	cases := []struct {
+		format   string
+		expected string
+	}{
+		{"table {{.ID}}", "{{.ID}}\n"},
+		{"table {{.ID}} {{.C}}", "{{.ID}}\t{{.C}}\n"},
+		{"{{.ID}}", "{{.ID}}\n"},
+		{"{{.ID}}\n", "{{.ID}}\n"},
+		{"{{.ID}} {{.C}}", "{{.ID}} {{.C}}\n"},
+		{"\t{{.ID}}", "\t{{.ID}}\n"},
+		{`\t` + "{{.ID}}", "\t{{.ID}}\n"},
+		{"table {{.ID}}\t{{.C}}", "{{.ID}}\t{{.C}}\n"},
+		{"{{.ID}} table {{.C}}", "{{.ID}} table {{.C}}\n"},
+	}
+	for _, tc := range cases {
+		tc := tc
+
+		label := strings.ReplaceAll(tc.format, " ", "<sp>")
+		t.Run("NormalizeFormat/"+label, func(t *testing.T) {
+			t.Parallel()
+			actual := NormalizeFormat(tc.format)
+			if actual != tc.expected {
+				t.Errorf("Expected %q, actual %q", tc.expected, actual)
+			}
+		})
+	}
+}

test/e2e/info_test.go

@@ -50,15 +50,17 @@ var _ = Describe("Podman Info", func() {
 			{"{{ json .}}", true, 0},
 			{"{{json .   }}", true, 0},
 			{"  {{  json .    }}   ", true, 0},
-			{"{{json }}", false, 125},
+			{"{{json }}", true, 0},
 			{"{{json .", false, 125},
-			{"json . }}", false, 0}, // Note: this does NOT fail but produces garbage
+			{"json . }}", false, 0}, // without opening {{ template seen as string literal
 		}
 		for _, tt := range tests {
 			session := podmanTest.Podman([]string{"info", "--format", tt.input})
 			session.WaitWithDefaultTimeout()
-			Expect(session).Should(Exit(tt.exitCode))
+
-			Expect(session.IsJSONOutputValid()).To(Equal(tt.success))
+			desc := fmt.Sprintf("JSON test(%q)", tt.input)
+			Expect(session).Should(Exit(tt.exitCode), desc)
+			Expect(session.IsJSONOutputValid()).To(Equal(tt.success), desc)
 		}
 	})
 

test/e2e/version_test.go

@@ -1,6 +1,7 @@
 package integration
 
 import (
+	"fmt"
 	"os"
 
 	. "github.com/containers/podman/v2/test/utils"
@@ -68,15 +69,17 @@ var _ = Describe("Podman version", func() {
 			{"{{ json .}}", true, 0},
 			{"{{json .   }}", true, 0},
 			{"  {{  json .    }}   ", true, 0},
-			{"{{json }}", false, 125},
+			{"{{json }}", true, 0},
 			{"{{json .", false, 125},
-			{"json . }}", false, 0}, // Note: this does NOT fail but produces garbage
+			{"json . }}", false, 0}, // without opening {{ template seen as string literal
 		}
 		for _, tt := range tests {
 			session := podmanTest.Podman([]string{"version", "--format", tt.input})
 			session.WaitWithDefaultTimeout()
-			Expect(session).Should(Exit(tt.exitCode))
+
-			Expect(session.IsJSONOutputValid()).To(Equal(tt.success))
+			desc := fmt.Sprintf("JSON test(%q)", tt.input)
+			Expect(session).Should(Exit(tt.exitCode), desc)
+			Expect(session.IsJSONOutputValid()).To(Equal(tt.success), desc)
 		}
 	})