Skip to content

Commit

Permalink
Allow arbitrary data in Details map in results
Browse files Browse the repository at this point in the history
The previous `Details` structure in the Sonobuoy results format was a
`map[string]string`. As we are creating more plugins that are providing
reports in the Sonobuoy results format, this structure is limiting. To
allow more flexibility, the type is being changed to
`map[string]interface{}` to allow plugins to provide arbitrary data in
the `Details` field.

The version of the YAML library used when parsing the results in a
results tarball has been upgraded to v3. This is due to the fact that it
unmarshals data into a `map[string]interface{}` type when all the keys
are strings. In v2, the behaviour was to always marshal to
`map[interface{}]interface{}`. This is important as if the keys are not
strings then marshalling to JSON will fail when using the `--detailed`
mode of the `results` command.

Signed-off-by: Bridget McErlean <bmcerlean@vmware.com>
  • Loading branch information
zubron committed Jun 11, 2020
1 parent c4956c0 commit de8e23a
Show file tree
Hide file tree
Showing 23 changed files with 458 additions and 13 deletions.
15 changes: 14 additions & 1 deletion cmd/sonobuoy/app/results_test.go
Expand Up @@ -41,7 +41,7 @@ func ExampleNewCmdResults() {
func ExampleNewCmdResults_custom() {
cmd := NewCmdResults()
cmd.SetArgs([]string{
filepath.Join("testdata", "testResultsOutputCustomStatuses.tar.gz"),
filepath.Join("testdata", "testResultsOutput.tar.gz"),
"--plugin=custom-status",
})
cmd.Execute()
Expand Down Expand Up @@ -127,3 +127,16 @@ func ExampleNewCmdResults_skipPrefix() {
// Output:
// hello world pt2
}

func ExampleNewCmdResults_pluginDetailedArbitraryDetails() {
cmd := NewCmdResults()
cmd.SetArgs([]string{
filepath.Join("testdata", "testResultsOutput.tar.gz"),
"--plugin", "arbitrary-details",
"--mode", "detailed",
})
cmd.Execute()
// Output:
// {"name":"Item with arbitrary details","status":"complete","meta":{"path":"arbitrary-details|output-file"},"details":{"nested-details":{"key1":"value1","key2":"value2"},"string-array":["string 1","string 2","string 3"]}}
// {"name":"Another item with arbitrary details","status":"complete","meta":{"path":"arbitrary-details|output-file"},"details":{"integer-array":[1,2,3],"nested-details":{"key1":"value1","key2":"value2","key3":{"nested-key1":"nested-value1","nested-key2":"nested-value2","nested-key3":{"another-nested-key":"another-nested-value"}}}}}
}
Binary file modified cmd/sonobuoy/app/testdata/testResultsOutput.tar.gz
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions go.mod
Expand Up @@ -42,6 +42,7 @@ require (
golang.org/x/sys v0.0.0-20190712062909-fae7ac547cb7 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/yaml.v2 v2.2.2
gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c
k8s.io/api v0.0.0-20190704095032-f4ca3d3bdf1d
k8s.io/apimachinery v0.0.0-20190704094733-8f6ac2502e51
k8s.io/client-go v11.0.1-0.20190704100234-640d9f240853+incompatible
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Expand Up @@ -351,6 +351,8 @@ gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bl
gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c h1:grhR+C34yXImVGp7EzNk+DTIk+323eIUWOmEevy6bDo=
gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/results/junit.go
Expand Up @@ -251,7 +251,7 @@ func junitProcessReader(r io.Reader, name string, metadata map[string]string) (I
case JUnitSkipped(t):
status = StatusSkipped
}
testItem := Item{Name: t.Name, Status: status, Details: map[string]string{}, Metadata: map[string]string{}}
testItem := Item{Name: t.Name, Status: status, Details: map[string]interface{}{}, Metadata: map[string]string{}}

// Different JUnit implementations build the objects in slightly different ways.
// Some will only use contents, some only the message attribute. Here we just concat
Expand Down
1 change: 1 addition & 0 deletions pkg/client/results/manual.go
Expand Up @@ -58,6 +58,7 @@ func manualProcessFile(pluginDir, currentFile string) (Item, error) {

rootObj.Status = resultObj.Status
rootObj.Items = resultObj.Items
rootObj.Details = resultObj.Details

return rootObj, nil
}
Expand Down
157 changes: 157 additions & 0 deletions pkg/client/results/manual_test.go
@@ -0,0 +1,157 @@
package results

import (
"fmt"
"reflect"
"strings"
"testing"
)

func checkItem(expected Item, actual Item) error {
if expected.Name != actual.Name {
return fmt.Errorf("expected item name to be %q, got %q", expected.Name, actual.Name)
}

if expected.Status != actual.Status {
return fmt.Errorf("expected item status to be %q, got %q", expected.Status, actual.Status)
}

for k, v := range expected.Metadata {
if actual.Metadata[k] != v {
return fmt.Errorf("expected item metadata %q to be %q, got %q", k, v, actual.Metadata[k])
}
}

// Check that the unmarshalled Details object matches. This check will fail if the types don't match even if the values are the same
// e.g. (string vs interface{})
if !reflect.DeepEqual(expected.Details, actual.Details) {
return fmt.Errorf("expected item details to be %q (%v), got %q (%v)", expected.Details, reflect.TypeOf(expected.Details), actual.Details, reflect.TypeOf(actual.Details))
}

if len(expected.Items) != len(actual.Items) {
return fmt.Errorf("unexpected number of items, expected %v, got %v", len(expected.Items), len(actual.Items))
}

for i, item := range expected.Items {
err := checkItem(item, actual.Items[i])
if err != nil {
return err
}
}

return nil
}

func TestManualProcessFile(t *testing.T) {
testCases := []struct {
name string
pluginDir string
currentFile string
expectedError string
expectMetadataError bool
expectedItem Item
}{
{
name: "missing file includes error in metadata and unknown status",
pluginDir: "./testdata/mockResults/manualProcessing",
currentFile: "./testdata/mockResults/manualProcessing/missingFile.yaml",
expectedError: "opening file ./testdata/mockResults/manualProcessing/missingFile.yaml",
expectMetadataError: true,
expectedItem: Item{
Name: "missingFile.yaml",
Status: StatusUnknown,
Metadata: map[string]string{
metadataFileKey: "missingFile.yaml",
metadataTypeKey: metadataTypeFile,
},
},
},
{
name: "invalid yaml file includes error in metadata and unknown status",
pluginDir: "./testdata/mockResults/manualProcessing",
currentFile: "./testdata/mockResults/manualProcessing/invalid-results.yaml",
expectedError: "error processing manual results",
expectedItem: Item{
Name: "invalid-results.yaml",
Status: StatusUnknown,
Metadata: map[string]string{
metadataFileKey: "invalid-results.yaml",
metadataTypeKey: metadataTypeFile,
},
},
},
{
name: "status is taken from the manual results",
pluginDir: "./testdata/mockResults/manualProcessing",
currentFile: "./testdata/mockResults/manualProcessing/manual-results.yaml",
expectedItem: Item{
Name: "manual-results.yaml",
Status: "status-from-manual-results",
Metadata: map[string]string{
metadataFileKey: "manual-results.yaml",
metadataTypeKey: metadataTypeFile,
},
Items: []Item{
{
Name: "a test file",
Status: "status 1",
},
{
Name: "another test file",
Status: "status 2",
},
},
},
},
{
name: "result item with arbitrary details is valid",
pluginDir: "./testdata/mockResults/manualProcessing",
currentFile: "./testdata/mockResults/manualProcessing/manual-results-arbitrary-details.yaml",
expectedItem: Item{
Name: "manual-results-arbitrary-details.yaml",
Status: "status-from-manual-results",
Metadata: map[string]string{
metadataFileKey: "manual-results-arbitrary-details.yaml",
metadataTypeKey: metadataTypeFile,
},
Details: map[string]interface{}{
"arbitrary-data": map[interface{}]interface{}{
"key": "value",
"array-of-integers": []interface{}{
1,
2,
3,
},
},
},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
item, err := manualProcessFile(tc.pluginDir, tc.currentFile)
if tc.expectedError != "" && err == nil {
t.Fatalf("expected error %q but err was nil", tc.expectedError)
}
if err != nil {
if tc.expectedError != "" {
if _, ok := item.Metadata["error"]; !ok && tc.expectMetadataError {
t.Errorf("expected metadata error field to be set")
}

if !strings.Contains(err.Error(), tc.expectedError) {
t.Errorf("expected error %q to contain %q", err.Error(), tc.expectedError)
}
} else {
t.Errorf("unexpected error %q", err)
}
}

checkErr := checkItem(tc.expectedItem, item)
if checkErr != nil {
t.Error(checkErr)
}
})
}
}
16 changes: 8 additions & 8 deletions pkg/client/results/processing.go
Expand Up @@ -94,11 +94,11 @@ type fileSelector func(string, os.FileInfo) bool
// types can be transformed into this simple format and set at a standard
// location in our results tarball for simplified processing by any consumer.
type Item struct {
Name string `json:"name" yaml:"name"`
Status string `json:"status" yaml:"status"`
Metadata map[string]string `json:"meta,omitempty" yaml:"meta,omitempty"`
Details map[string]string `json:"details,omitempty" yaml:"details,omitempty"`
Items []Item `json:"items,omitempty" yaml:"items,omitempty"`
Name string `json:"name" yaml:"name"`
Status string `json:"status" yaml:"status"`
Metadata map[string]string `json:"meta,omitempty" yaml:"meta,omitempty"`
Details map[string]interface{} `json:"details,omitempty" yaml:"details,omitempty"`
Items []Item `json:"items,omitempty" yaml:"items,omitempty"`
}

// Empty returns true if the Item is empty.
Expand Down Expand Up @@ -366,7 +366,7 @@ func errProcessor(pluginDir string, currentFile string) (Item, error) {
Name: filepath.Base(currentFile),
Status: StatusFailed,
Metadata: map[string]string{"file": relPath},
Details: map[string]string{},
Details: map[string]interface{}{},
}

infile, err := os.Open(currentFile)
Expand All @@ -392,7 +392,7 @@ func errProcessor(pluginDir string, currentFile string) (Item, error) {
// Surface the error to be the name of the "test" to make the error mode more visible to end users.
// Seeing `error.json` wouldn't be helpful.
if resultObj.Details["error"] != "" {
resultObj.Name = resultObj.Details["error"]
resultObj.Name = fmt.Sprint(resultObj.Details["error"])
}

if isTimeoutErr(resultObj) {
Expand All @@ -405,7 +405,7 @@ func errProcessor(pluginDir string, currentFile string) (Item, error) {
// isTimeoutErr is the snippet of logic that determines whether or not a given Item represents
// a timeout error (i.e. Sonobuoy timed out waiting for results).
func isTimeoutErr(i Item) bool {
return strings.Contains(i.Details["error"], "timeout")
return strings.Contains(fmt.Sprint(i.Details["error"]), "timeout")
}

// processDir will walk the files in a given directory, using the fileSelector function to
Expand Down
8 changes: 8 additions & 0 deletions pkg/client/results/processing_test.go
Expand Up @@ -194,6 +194,14 @@ func TestPostProcessPlugin(t *testing.T) {
desc: "DS Manual results with no file specified and default sonobuoy_results, default file processed",
key: "ds-manual-04",
plugin: getPlugin("ds-manual-04", "daemonset", "manual", []string{}),
}, {
desc: "Job Manual results with arbitrary details",
key: "job-manual-arbitrary-details",
plugin: getPlugin("job-manual-arbitrary-details", "job", "manual", []string{"manual-results-arbitrary-details.yaml"}),
}, {
desc: "DS Manual results with arbitrary details",
key: "ds-manual-arbitrary-details",
plugin: getPlugin("ds-manual-arbitrary-details", "daemonset", "manual", []string{"manual-results-arbitrary-details.yaml"}),
},
}
for _, tc := range testCases {
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/results/reader.go
Expand Up @@ -23,7 +23,7 @@ import (
"encoding/json"
"encoding/xml"
"fmt"
"gopkg.in/yaml.v2"
"gopkg.in/yaml.v3"
"io"
"os"
"path"
Expand Down
@@ -0,0 +1,9 @@
name: invalid-results
status: status-from-manual-results
meta:
type: summary
items:
- name: a test file
status: status 1
meta:
- invalid: data
@@ -0,0 +1,11 @@
name: manual-results
status: status-from-manual-results
meta:
type: summary
details:
arbitrary-data:
key: value
array-of-integers:
- 1
- 2
- 3
@@ -0,0 +1,15 @@
name: manual-results
status: status-from-manual-results
meta:
type: summary
items:
- name: a test file
status: status 1
meta:
file: results/global/junit_01.xml
type: file
- name: another test file
status: status 2
meta:
file: results/global/junit_01.xml
type: file

0 comments on commit de8e23a

Please sign in to comment.