feat(process): Unwrap List types (#306)

* feat(process): Unwrap List types

This PR adds native support to Tanka for unwrapping List objects, those
being k8s pseudo objects used to bundle multiple resources into one
object.

To detect whether something is a List or not, the existence of `items`
is used, the same criteria the offical `k8s.io` packages use:

https://github.com/kubernetes/apimachinery/blob/61490fe38e784592212b24b9878306b09be45ab0/pkg/apis/meta/v1/unstructured/unstructured.go#L54

* feat(process): improve schema errors

Now always gives name, reason and a code sample of the violating object

go.mod

@@ -8,6 +8,7 @@ require (
 	github.com/fatih/structs v1.1.0
 	github.com/go-clix/cli v0.1.1
 	github.com/gobwas/glob v0.2.3
+	github.com/google/go-cmp v0.3.0
 	github.com/google/go-jsonnet v0.15.1-0.20200331184325-4f4aa80dd785
 	github.com/karrick/godirwalk v1.15.5
 	github.com/pkg/errors v0.8.1

go.sum

@@ -31,6 +31,7 @@ github.com/golang/groupcache v0.0.0-20160516000752-02826c3e7903/go.mod h1:cIg4er
 github.com/golang/protobuf v0.0.0-20161109072736-4bd1920723d7/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
+github.com/google/go-cmp v0.3.0 h1:crn/baboCvb5fXaQ0IJ1SGTsTVrWpDsCWC8EGETZijY=
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-jsonnet v0.15.1-0.20200331184325-4f4aa80dd785 h1:+dlQ7fPoeAqO0U9V+94golo/rW1/V2Pn+v8aPp3ljRM=
 github.com/google/go-jsonnet v0.15.1-0.20200331184325-4f4aa80dd785/go.mod h1:sOcuej3UW1vpPTZOr8L7RQimqai1a57bt5j22LzGZCw=

pkg/kubernetes/client/kubectl.go

@@ -3,7 +3,6 @@ package client
 import (
 	"bytes"
 	"encoding/json"
-	"fmt"
 	"os"
 	"regexp"
 
@@ -76,18 +75,13 @@ func (k Kubectl) Namespaces() (map[string]bool, error) {
 		return nil, err
 	}
 
-	items, ok := list["items"].([]interface{})
-	if !ok {
-		return nil, fmt.Errorf("listing namespaces: expected items to be an object, but got %T instead", list["items"])
+	items, err := list.Items()
+	if err != nil {
+		return nil, err
 	}
 
 	namespaces := make(map[string]bool)
-	for _, i := range items {
-		m, err := manifest.New(i.(map[string]interface{}))
-		if err != nil {
-			return nil, err
-		}
-
+	for _, m := range items {
 		namespaces[m.Metadata().Name()] = true
 	}
 	return namespaces, nil

pkg/kubernetes/manifest/errors.go

@@ -7,37 +7,53 @@ import (
 
 // SchemaError means that some expected fields were missing
 type SchemaError struct {
-	fields map[string]bool
-	name   string
+	Fields   map[string]bool
+	Name     string
+	Manifest Manifest
 }
 
 // Error returns the fields the manifest at the path is missing
 func (s *SchemaError) Error() string {
-	e := ""
-	for k, missing := range s.fields {
+	fields := make([]string, 0, len(s.Fields))
+	for k, missing := range s.Fields {
 		if !missing {
 			continue
 		}
-		e += ", " + k
+		fields = append(fields, k)
 	}
-	e = strings.TrimPrefix(e, ", ")
-	return fmt.Sprintf("%s missing or invalid fields: %s", s.name, e)
-}
 
-func (s *SchemaError) add(field string) {
-	if s.fields == nil {
-		s.fields = make(map[string]bool)
+	if s.Name == "" {
+		s.Name = "Resource"
+	}
+
+	msg := fmt.Sprintf("%s has missing or invalid fields: %s", s.Name, strings.Join(fields, ", "))
+
+	if s.Manifest != nil {
+		msg += fmt.Sprintf(":\n\n%s\n\nPlease check above object.", SampleString(s.Manifest.String()).Indent(2))
 	}
-	s.fields[field] = true
+
+	return msg
 }
 
-// Missing returns whether a field is missing
-func (s *SchemaError) Missing(field string) bool {
-	return s.fields[field]
+// SampleString is used for displaying code samples for error messages. It
+// truncates the output to 10 lines
+type SampleString string
+
+func (s SampleString) String() string {
+	lines := strings.Split(strings.TrimSpace(string(s)), "\n")
+	truncate := len(lines) >= 10
+	if truncate {
+		lines = lines[0:10]
+	}
+	out := strings.Join(lines, "\n")
+	if truncate {
+		out += "\n..."
+	}
+	return out
 }
 
-// WithName inserts a path into the error message
-func (s *SchemaError) WithName(name string) *SchemaError {
-	s.name = name
-	return s
+func (s SampleString) Indent(n int) string {
+	indent := strings.Repeat(" ", n)
+	lines := strings.Split(s.String(), "\n")
+	return indent + strings.Join(lines, "\n"+indent)
 }

pkg/kubernetes/manifest/manifest.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/stretchr/objx"
@@ -42,30 +41,67 @@ func (m Manifest) String() string {
 // Verify checks whether the manifest is correctly structured
 func (m Manifest) Verify() error {
 	o := m2o(m)
-	var err SchemaError
+	fields := make(map[string]bool)
 
 	if !o.Get("kind").IsStr() {
-		err.add("kind")
+		fields["kind"] = true
 	}
 	if !o.Get("apiVersion").IsStr() {
-		err.add("apiVersion")
+		fields["apiVersion"] = true
 	}
 
 	// Lists don't have `metadata`
-	if !strings.HasSuffix(m.Kind(), "List") {
+	if !m.IsList() {
 		if !o.Get("metadata").IsMSI() {
-			err.add("metadata")
+			fields["metadata"] = true
 		}
 		if !o.Get("metadata.name").IsStr() {
-			err.add("metadata.name")
+			fields["metadata.name"] = true
 		}
 	}
 
-	if len(err.fields) == 0 {
+	if len(fields) == 0 {
 		return nil
 	}
 
-	return &err
+	return &SchemaError{
+		Fields:   fields,
+		Manifest: m,
+	}
+}
+
+// IsList returns whether the manifest is a List type, containing other
+// manifests as children. Code based on
+// https://github.com/kubernetes/apimachinery/blob/61490fe38e784592212b24b9878306b09be45ab0/pkg/apis/meta/v1/unstructured/unstructured.go#L54
+func (m Manifest) IsList() bool {
+	items, ok := m["items"]
+	if !ok {
+		return false
+	}
+	_, ok = items.([]interface{})
+	return ok
+}
+
+// Items returns list items if the manifest is of List type
+func (m Manifest) Items() (List, error) {
+	if !m.IsList() {
+		return nil, fmt.Errorf("Attempt to unwrap non-list object '%s' of kind '%s'", m.Metadata().Name(), m.Kind())
+	}
+
+	// This is safe, IsList() asserts this
+	items := m["items"].([]interface{})
+	list := make(List, 0, len(items))
+	for _, i := range items {
+		child, ok := i.(map[string]interface{})
+		if !ok {
+			return nil, fmt.Errorf("Unwrapped list item is not an object, but '%T'", child)
+		}
+
+		m := Manifest(child)
+		list = append(list, m)
+	}
+
+	return list, nil
 }
 
 // Kind returns the kind of the API object

pkg/process/extract.go

@@ -1,6 +1,7 @@
 package process
 
 import (
+	"errors"
 	"fmt"
 	"sort"
 	"strings"
@@ -60,8 +61,10 @@ func walkObj(obj objx.Map, extracted map[string]manifest.Manifest, path trace) e
 	// This looks like a kubernetes manifest, so make one and return it
 	if isKubernetesManifest(obj) {
 		m, err := manifest.NewFromObj(obj)
-		if err != nil {
-			return err.(*manifest.SchemaError).WithName(path.Full())
+		var e *manifest.SchemaError
+		if errors.As(err, &e) {
+			e.Name = path.Full()
+			return e
 		}
 
 		extracted[path.Full()] = m

pkg/process/process.go

@@ -1,6 +1,9 @@
 package process
 
 import (
+	"errors"
+	"fmt"
+
 	"github.com/grafana/tanka/pkg/kubernetes/manifest"
 	"github.com/grafana/tanka/pkg/spec/v1alpha1"
 )
@@ -22,6 +25,11 @@ func Process(raw map[string]interface{}, cfg v1alpha1.Config, exprs Matchers) (m
 		return nil, err
 	}
 
+	// Unwrap *List types
+	if err := Unwrap(extracted); err != nil {
+		return nil, err
+	}
+
 	out := make(manifest.List, 0, len(extracted))
 	for _, m := range extracted {
 		out = append(out, m)
@@ -53,3 +61,34 @@ func Label(list manifest.List, cfg v1alpha1.Config) manifest.List {
 
 	return list
 }
+
+// Unwrap returns all Kubernetes objects in the manifest. If m is not a List
+// type, a one item List is returned
+func Unwrap(manifests map[string]manifest.Manifest) error {
+	for path, m := range manifests {
+		if !m.IsList() {
+			continue
+		}
+
+		items, err := m.Items()
+		if err != nil {
+			return err
+		}
+
+		for index, i := range items {
+			name := fmt.Sprintf("%s.items[%v]", path, index)
+
+			var e *manifest.SchemaError
+			if errors.As(i.Verify(), &e) {
+				e.Name = name
+				return e
+			}
+
+			manifests[name] = i
+		}
+
+		delete(manifests, path)
+	}
+
+	return nil
+}

pkg/process/process_test.go

@@ -3,9 +3,9 @@ package process
 import (
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/grafana/tanka/pkg/kubernetes/manifest"
 	"github.com/grafana/tanka/pkg/spec/v1alpha1"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -64,6 +64,14 @@ func TestProcess(t *testing.T) {
 				`DePlOyMeNt/GrAfAnA`,
 			),
 		},
+		{
+			name: "unwrap-list",
+			deep: loadFixture("list").Deep,
+			flat: manifest.List{
+				loadFixture("list").Flat["foo.items[0]"],
+				loadFixture("list").Flat["foo.items[1]"],
+			},
+		},
 	}
 
 	for _, c := range tests {
@@ -82,7 +90,10 @@ func TestProcess(t *testing.T) {
 			got, err := Process(c.deep.(map[string]interface{}), *config, c.targets)
 			require.Equal(t, c.err, err)
 
-			assert.ElementsMatch(t, c.flat, got)
+			Sort(c.flat)
+			if s := cmp.Diff(c.flat, got); s != "" {
+				t.Error(s)
+			}
 		})
 	}
 }

pkg/process/testdata/k8s.libsonnet

@@ -34,4 +34,9 @@
     kind: 'Namespace',
     metadata: { name: name },
   },
+  list(items, kind=""):: {
+    apiVersion: "v1",
+    kind: "%sList" % kind,
+    items: items,
+  }
 }

pkg/process/testdata/tdList.jsonnet

@@ -0,0 +1,15 @@
+local k = (import "./k8s.libsonnet");
+
+local deployment = (import './resources.jsonnet').deployment;
+local service = (import './resources.jsonnet').service;
+
+// NOTE: This testdata also needs Unwrap() in addition to Process()
+{
+  deep: {
+    foo: k.list([deployment, service]),
+  },
+  flat: {
+    "foo.items[0]": deployment,
+    "foo.items[1]": service,
+  },
+}