From 56a1cd76cbac4b0578f5238fc56f6e6c6eb4f3d9 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Tue, 8 Sep 2015 18:28:58 -0700 Subject: [PATCH] Address changes. --- pkg/kubectl/custom_column_printer.go | 47 ++++++++++++++--------- pkg/kubectl/custom_column_printer_test.go | 19 ++++++++- pkg/kubectl/resource_printer.go | 2 +- 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/pkg/kubectl/custom_column_printer.go b/pkg/kubectl/custom_column_printer.go index 4d107867713..cc0dd5cd13e 100644 --- a/pkg/kubectl/custom_column_printer.go +++ b/pkg/kubectl/custom_column_printer.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "reflect" + "regexp" "strings" "text/tabwriter" @@ -37,6 +38,8 @@ const ( flags = 0 ) +var jsonRegexp = regexp.MustCompile("^\\{\\.?([^{}]+)\\}$|^\\.?([^{}]+)$") + // MassageJSONPath attempts to be flexible with JSONPath expressions, it accepts: // * metadata.name (no leading '.' or curly brances '{...}' // * {metadata.name} (no leading '.') @@ -44,23 +47,24 @@ const ( // * {.metadata.name} (complete expression) // And transforms them all into a valid jsonpat expression: // {.metadata.name} -func massageJSONPath(pathExpression string) string { +func massageJSONPath(pathExpression string) (string, error) { if len(pathExpression) == 0 { - return pathExpression + return pathExpression, nil } - if pathExpression[0] != '{' { - if pathExpression[0] == '.' { - return fmt.Sprintf("{%s}", pathExpression) - } else { - return fmt.Sprintf("{.%s}", pathExpression) - } + submatches := jsonRegexp.FindStringSubmatch(pathExpression) + if submatches == nil { + return "", fmt.Errorf("unexpected path string, expected a 'name1.name2' or '.name1.name2' or '{name1.name2}' or '{.name1.name2}'") + } + if len(submatches) != 3 { + return "", fmt.Errorf("unexpected submatch list: %v", submatches) + } + var fieldSpec string + if len(submatches[1]) != 0 { + fieldSpec = submatches[1] } else { - if pathExpression[1] == '.' { - return pathExpression - } else { - return fmt.Sprintf("{.%s}", pathExpression[1:len(pathExpression)-1]) - } + fieldSpec = submatches[2] } + return fmt.Sprintf("{.%s}", fieldSpec), nil } // NewCustomColumnsPrinterFromSpec creates a custom columns printer from a comma separated list of
: pairs. @@ -79,7 +83,11 @@ func NewCustomColumnsPrinterFromSpec(spec string) (*CustomColumnsPrinter, error) if len(colSpec) != 2 { return nil, fmt.Errorf("unexpected custom-columns spec: %s, expected
:", parts[ix]) } - columns[ix] = Column{Header: colSpec[0], FieldSpec: massageJSONPath(colSpec[1])} + spec, err := massageJSONPath(colSpec[1]) + if err != nil { + return nil, err + } + columns[ix] = Column{Header: colSpec[0], FieldSpec: spec} } return &CustomColumnsPrinter{Columns: columns}, nil } @@ -102,15 +110,14 @@ func splitOnWhitespace(line string) []string { func NewCustomColumnsPrinterFromTemplate(templateReader io.Reader) (*CustomColumnsPrinter, error) { scanner := bufio.NewScanner(templateReader) if !scanner.Scan() { - return nil, fmt.Errorf("invalid template, missing header line") + return nil, fmt.Errorf("invalid template, missing header line. Expected format is one line of space separated headers, one line of space separated column specs.") } headers := splitOnWhitespace(scanner.Text()) if !scanner.Scan() { - return nil, fmt.Errorf("invalid template, missing spec line") + return nil, fmt.Errorf("invalid template, missing spec line. Expected format is one line of space separated headers, one line of space separated column specs.") } specs := splitOnWhitespace(scanner.Text()) - fmt.Printf("SPECS: %v", specs) if len(headers) != len(specs) { return nil, fmt.Errorf("number of headers (%d) and field specifications (%d) don't match", len(headers), len(specs)) @@ -118,9 +125,13 @@ func NewCustomColumnsPrinterFromTemplate(templateReader io.Reader) (*CustomColum columns := make([]Column, len(headers)) for ix := range headers { + spec, err := massageJSONPath(specs[ix]) + if err != nil { + return nil, err + } columns[ix] = Column{ Header: headers[ix], - FieldSpec: massageJSONPath(specs[ix]), + FieldSpec: spec, } } return &CustomColumnsPrinter{Columns: columns}, nil diff --git a/pkg/kubectl/custom_column_printer_test.go b/pkg/kubectl/custom_column_printer_test.go index 245e1111ee8..ac9fc9699ea 100644 --- a/pkg/kubectl/custom_column_printer_test.go +++ b/pkg/kubectl/custom_column_printer_test.go @@ -29,17 +29,32 @@ func TestMassageJSONPath(t *testing.T) { tests := []struct { input string expectedOutput string + expectErr bool }{ {input: "foo.bar", expectedOutput: "{.foo.bar}"}, {input: "{foo.bar}", expectedOutput: "{.foo.bar}"}, {input: ".foo.bar", expectedOutput: "{.foo.bar}"}, {input: "{.foo.bar}", expectedOutput: "{.foo.bar}"}, {input: "", expectedOutput: ""}, + {input: "{foo.bar", expectErr: true}, + {input: "foo.bar}", expectErr: true}, + {input: "{foo.bar}}", expectErr: true}, + {input: "{{foo.bar}", expectErr: true}, } for _, test := range tests { - output := massageJSONPath(test.input) + output, err := massageJSONPath(test.input) + if err != nil && !test.expectErr { + t.Errorf("unexpected error: %v", err) + continue + } + if test.expectErr { + if err == nil { + t.Error("unexpected non-error") + } + continue + } if output != test.expectedOutput { - t.Errorf("expected: %s, saw: %s", test.expectedOutput, output) + t.Errorf("input: %s, expected: %s, saw: %s", test.input, test.expectedOutput, output) } } } diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index e7b044348c6..41836b51f92 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -99,7 +99,7 @@ func GetPrinter(format, formatArgument string) (ResourcePrinter, bool, error) { printer, err = NewJSONPathPrinter(string(data)) if err != nil { return nil, false, fmt.Errorf("error parsing template %s, %v\n", string(data), err) - } + } case "custom-columns": var err error if printer, err = NewCustomColumnsPrinterFromSpec(formatArgument); err != nil {