Repository: grafana/tanka
Pull request #108 (2019-11-12 08:16:13)

Don't crash if an environment directory is missing spec.json

sh0rez: Hey, thanks for your contribution! However, the issue you are trying to fix actually lies in the `findBaseDirs` function, which should never return invalid environments. I suggest the following:

```diff
diff --git a/cmd/tk/other.go b/cmd/tk/other.go
index f160d8c..fc86aa5 100644
--- a/cmd/tk/other.go
+++ b/cmd/tk/other.go
@@ -20,7 +20,10 @@ func findBaseDirs() (dirs []string) {
 	}
 
 	if err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error {
-		if _, err := os.Stat(filepath.Join(path, "main.jsonnet")); err == nil {
+		if _, err := os.Stat(filepath.Join(path, "main.jsonnet")); err != nil {
+			return nil
+		}
+		if _, err := os.Stat(filepath.Join(path, "spec.json")); err == nil {
 			dirs = append(dirs, path)
 		}
 		return nil
```

Would you adapt your PR? Thanks!

Patch:
diff --git a/cmd/tk/env.go b/cmd/tk/env.go
index c81e05ba..f1b0acbd 100644
--- a/cmd/tk/env.go
+++ b/cmd/tk/env.go
@@ -189,7 +189,12 @@ func envListCmd() *cobra.Command {
 		}
 		for _, dir := range dirs {
 			viper.Reset()
-			envs = append(envs, *setupConfiguration(dir))
+			env := setupConfiguration(dir)
+			if env == nil {
+				log.Printf("Could not setup configuration from %q", dir)
+				continue
+			}
+			envs = append(envs, *env)
 		}
 
 		if useJSON {
diff --git a/cmd/tk/other.go b/cmd/tk/other.go
index f160d8cb..40dc694f 100644
--- a/cmd/tk/other.go
+++ b/cmd/tk/other.go
@@ -20,9 +20,14 @@ func findBaseDirs() (dirs []string) {
 	}
 
 	if err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error {
-		if _, err := os.Stat(filepath.Join(path, "main.jsonnet")); err == nil {
-			dirs = append(dirs, path)
+		requiredFiles := []string{"main.jsonnet", "spec.json"}
+		for _, name := range requiredFiles {
+			if _, err := os.Stat(filepath.Join(path, name)); err != nil {
+				// missing file, not a valid environment directory
+				return nil
+			}
 		}
+		dirs = append(dirs, path)
 		return nil
 	}); err != nil {
 		log.Fatalln(err)
diff --git a/docs/jsonnet/expected-structure.md b/docs/jsonnet/expected-structure.md
new file mode 100644
index 00000000..6723e1e5
--- /dev/null
+++ b/docs/jsonnet/expected-structure.md
@@ -0,0 +1,104 @@
+# Expected structure
+
+Tanka evaluates the `main.jsonnet` file of your [Environment](/environments) and
+filters the output (either Object or Array) for valid Kubernetes objects.  
+An object is considered valid if it has both, a `kind` and a `apiVersion` set.
+
+!!! warning
+    This behaviour is going to change in the future, `metadata.name` will
+    also become required.
+
+## Deeply nested object (Recommended)
+The most commonly used structure is a single big object that includes all of
+your configs to be applied to the cluster nested under keys.  
+How deeply encapsulated the actual object is does not matter, Tanka will
+traverse down until it finds something that has both, a `kind` and an
+`apiVersion`.  
+
+??? Example
+    ```json
+    {
+      "prometheus": {
+        "service": { // Service nested one level
+          "apiVersion": "v1",
+          "kind": "Service",
+          "metadata": {
+            "name": "promSvc"
+          }
+        },
+        "deployment": {
+          "apiVersion": "apps/v1", // apiVersion ..
+          "kind": "Deployment", // .. and kind are required
+                                // to identify an object.
+          "metadata": {
+            "name": "prom" 
+          }
+        }
+      },
+      "web": {
+        "nginx": {
+          "deployment": { // Deployment nested two levels
+            "apiVersion": "apps/v1",
+            "kind": "Deployment",
+            "metadata": {
+              "name": "nginx"
+            }
+          }
+        }
+      }
+    }
+    ```
+    
+Using this technique has the big benefit that it is self-documentary, as the
+nesting of keys can be used to logically group related manifests, for example by
+application.
+
+!!! info
+    It is also valid to use an encapsulation level of zero, which means
+    just a regular object like it could be obtained from `kubectl show -o json`:
+    ```json
+    {
+      "apiVersion": "v1",
+      "kind": "Service",
+      "metadata": {
+        "name": "foo"
+      }
+    }
+    ```
+
+
+## Array
+Using an array of objects is also fine:
+```json
+[
+  {
+    "apiVersion": "v1",
+    "kind": "Service",
+    "metadata": {
+      "name": "promSvc"
+    }
+  },
+  {
+    "apiVersion": "apps/v1",
+    "kind": "Deployment",
+    "metadata": {
+      "name": "prom"
+    }
+  }
+]
+```
+
+### `List` type
+Users of `kubectl` might have had contact with a type called `List`. It is not
+part of the official Kubernetes API but rather a pseudo-type introduced by
+`kubectl` for dealing with multiple objects at once. Thus, Tanka does not
+support it out of the box.
+
+To take full advantage of Tankas features, you can manually flatten it:
+
+```jsonnet
+local list = import "list.libsonnet";
+
+# expose the `items` array on the top level:
+list.items 
+```
diff --git a/pkg/kubernetes/reconcile.go b/pkg/kubernetes/reconcile.go
index 6bbf7545..3b99b9aa 100644
--- a/pkg/kubernetes/reconcile.go
+++ b/pkg/kubernetes/reconcile.go
@@ -20,14 +20,37 @@ func (e ErrorPrimitiveReached) Error() string {
 }
 
 // walkJSON traverses deeply nested kubernetes manifest and extracts them into a flat []dict.
-func walkJSON(deep map[string]interface{}, path string) ([]map[string]interface{}, error) {
+func walkJSON(rawDeep interface{}, path string) ([]map[string]interface{}, error) {
+	flat := make([]map[string]interface{}, 0)
+
+	// array: walkJSON for each
+	if d, ok := rawDeep.([]map[string]interface{}); ok {
+		for i, j := range d {
+			out, err := walkJSON(j, fmt.Sprintf("%s[%v]", path, i))
+			if err != nil {
+				return nil, err
+			}
+			flat = append(flat, out...)
+		}
+		return flat, nil
+	}
+
+	// assert for map[string]interface{} (also aliased objx.Map)
+	if m, ok := rawDeep.(objx.Map); ok {
+		rawDeep = map[string]interface{}(m)
+	}
+	deep, ok := rawDeep.(map[string]interface{})
+	if !ok {
+		return nil, fmt.Errorf("deep has unexpected type %T @ %s", deep, path)
+	}
+
+	// already flat?
 	r := objx.New(deep)
 	if r.Has("apiVersion") && r.Has("kind") {
 		return []map[string]interface{}{deep}, nil
 	}
 
-	flat := make([]map[string]interface{}, 0)
-
+	// walk it
 	for n, d := range deep {
 		if n == "__ksonnet" {
 			continue


Test patch:
diff --git a/pkg/kubernetes/data_test.go b/pkg/kubernetes/data_test.go
index 8aa07e47..44da9d77 100644
--- a/pkg/kubernetes/data_test.go
+++ b/pkg/kubernetes/data_test.go
@@ -213,3 +213,15 @@ func testDataDeep() testData {
 		},
 	}
 }
+
+// testDataArray is an array of (deeply nested) dicts that should be fully
+// flattened
+func testDataArray() testData {
+	return testData{
+		deep: append([]map[string]interface{}{
+			testDataDeep().deep.(map[string]interface{}),
+		}, testDataFlat().deep.(map[string]interface{})),
+
+		flat: append(testDataDeep().flat.([]map[string]interface{}), testDataFlat().flat.([]map[string]interface{})...),
+	}
+}
diff --git a/pkg/kubernetes/reconcile_test.go b/pkg/kubernetes/reconcile_test.go
index a52da6dd..8bdfb388 100644
--- a/pkg/kubernetes/reconcile_test.go
+++ b/pkg/kubernetes/reconcile_test.go
@@ -30,11 +30,15 @@ func TestWalkJSON(t *testing.T) {
 			name: "deep",
 			data: testDataDeep(),
 		},
+		{
+			name: "array",
+			data: testDataArray(),
+		},
 	}
 
 	for _, c := range tests {
 		t.Run(c.name, func(t *testing.T) {
-			got, err := walkJSON(c.data.deep.(map[string]interface{}), "")
+			got, err := walkJSON(c.data.deep, "")
 			require.Equal(t, c.err, err)
 			assert.ElementsMatch(t, c.data.flat, got)
 		})