Repository: grafana/tanka (go)

Issue:
injectLabels should not inject on RoleList and RoleBindingList objects
Hi,

We enabled garbagecollection via injectLabels: true on our environments (tk 0.10.0) and we hit a bug with kube-prometheus.

Prometheus generates RoleList and RoleBindingList object kinds and labels are not allowed in its metadata:

`error: error validating "STDIN": error validating data: ValidationError(RoleList.metadata): unknown field "labels" in io.k8s.apimachinery.pkg.apis.meta.v1.ListMeta; if you choose to ignore these errors, turn validation off with --validate=false`

```yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleList
metadata:
  labels:
    tanka.dev/environment: someEnv
items:
- apiVersion: rbac.authorization.k8s.io/v1
  kind: Role
  metadata:
    name: prometheus-k8s
    namespace: default
  rules:
  - apiGroups:
    - ""
    resources:
    - services
    - endpoints
    - pods
    verbs:
    - get
    - list
    - watch
...
...
```

Pull request:
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:



```go
// https://github.com/kubernetes/apimachinery/blob/61490fe38e784592212b24b9878306b09be45ab0/pkg/apis/meta/v1/unstructured/unstructured.go#L54-L61
func (obj *Unstructured) IsList() bool {
	field, ok := obj.Object["items"]
	if !ok {
		return false
	}
	_, ok = field.([]interface{})
	return ok
}
```

Also refactors SchemaError which is now used in `Unwrap` as well. The error now also displays a code snippet, so debugging becomes easier

Fixes #277 

Patch:
diff --git a/Makefile b/Makefile
index d7f545bd..90890a68 100644
--- a/Makefile
+++ b/Makefile
@@ -27,7 +27,7 @@ uninstall:
 $(GOX):
 	go get -u github.com/mitchellh/gox
 cross: $(GOX)
-	CGO_ENABLED=0 gox -output="dist/{{.Dir}}-{{.OS}}-{{.Arch}}" -ldflags=${LDFLAGS} -arch="amd64 arm64 arm" -os="linux" -osarch="darwin/amd64" ./cmd/tk
+	CGO_ENABLED=0 gox -output="dist/{{.Dir}}-{{.OS}}-{{.Arch}}" -ldflags=${LDFLAGS} -arch="amd64 arm64 arm" -os="linux" -osarch="darwin/amd64" -osarch="windows/amd64" ./cmd/tk
 
 # Docker container
 container: static
diff --git a/cmd/tk/init.go b/cmd/tk/init.go
index 2f87aa6f..b386efbb 100644
--- a/cmd/tk/init.go
+++ b/cmd/tk/init.go
@@ -60,7 +60,7 @@ func initCmd() *cli.Command {
 			}
 		}
 
-		fmt.Println("Directory structure set up! Remember to configure the API endpoint:\n`tk env set environments/default --server=127.0.0.1:6443`")
+		fmt.Println("Directory structure set up! Remember to configure the API endpoint:\n`tk env set environments/default --server=https://127.0.0.1:6443`")
 		if failed {
 			log.Println("Errors occured while initializing the project. Check the above logs for details.")
 		}
diff --git a/go.mod b/go.mod
index d52f7208..88b6b69b 100644
--- a/go.mod
+++ b/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
diff --git a/go.sum b/go.sum
index 7fcba8f1..5391d986 100644
--- a/go.sum
+++ b/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=
diff --git a/pkg/kubernetes/client/kubectl.go b/pkg/kubernetes/client/kubectl.go
index 0700ca51..bc7118eb 100644
--- a/pkg/kubernetes/client/kubectl.go
+++ b/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
diff --git a/pkg/kubernetes/manifest/errors.go b/pkg/kubernetes/manifest/errors.go
index f133eb3f..f1196b90 100644
--- a/pkg/kubernetes/manifest/errors.go
+++ b/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)
 }
diff --git a/pkg/kubernetes/manifest/manifest.go b/pkg/kubernetes/manifest/manifest.go
index 56622890..b673ecb8 100644
--- a/pkg/kubernetes/manifest/manifest.go
+++ b/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
diff --git a/pkg/process/extract.go b/pkg/process/extract.go
index e917b1a8..d6a4ba11 100644
--- a/pkg/process/extract.go
+++ b/pkg/process/extract.go
@@ -1,7 +1,9 @@
 package process
 
 import (
+	"errors"
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/stretchr/objx"
@@ -59,21 +61,28 @@ 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
 		return nil
 	}
 
-	for key, value := range obj {
-		path := append(path, key)
+	keys := make([]string, 0, len(obj))
+	for k := range obj {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
 
-		if value == nil { // result from false if condition in Jsonnet
+	for _, key := range keys {
+		path := append(path, key)
+		if obj[key] == nil { // result from false if condition in Jsonnet
 			continue
 		}
-		err := walkJSON(value, extracted, path)
+		err := walkJSON(obj[key], extracted, path)
 		if err != nil {
 			return err
 		}
diff --git a/pkg/process/process.go b/pkg/process/process.go
index 8394593d..efc0b5b8 100644
--- a/pkg/process/process.go
+++ b/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
+}
diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go
index cf2194c2..989c4895 100644
--- a/pkg/spec/spec.go
+++ b/pkg/spec/spec.go
@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
 
 	"github.com/pkg/errors"
 
@@ -55,6 +56,12 @@ func Parse(data []byte, name string) (*v1alpha1.Config, error) {
 	if err := handleDeprecated(config, data); err != nil {
 		return config, err
 	}
+
+	// default apiServer URL to https
+	if !regexp.MustCompile("^.+://").MatchString(config.Spec.APIServer) {
+		config.Spec.APIServer = "https://" + config.Spec.APIServer
+	}
+
 	return config, nil
 }
 


Test patch:
diff --git a/pkg/process/process_test.go b/pkg/process/process_test.go
index 93d75282..32395125 100644
--- a/pkg/process/process_test.go
+++ b/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)
+			}
 		})
 	}
 }
diff --git a/pkg/process/testdata/k8s.libsonnet b/pkg/process/testdata/k8s.libsonnet
index ad422db1..2f5f462a 100644
--- a/pkg/process/testdata/k8s.libsonnet
+++ b/pkg/process/testdata/k8s.libsonnet
@@ -34,4 +34,9 @@
     kind: 'Namespace',
     metadata: { name: name },
   },
+  list(items, kind=""):: {
+    apiVersion: "v1",
+    kind: "%sList" % kind,
+    items: items,
+  }
 }
diff --git a/pkg/process/testdata/tdList.jsonnet b/pkg/process/testdata/tdList.jsonnet
new file mode 100644
index 00000000..ad784b6f
--- /dev/null
+++ b/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,
+  },
+}