Repository: grafana/tanka
Pull request #92 (2019-10-08 12:51:08)

refactor(cli): spec parsing
Refactors the parsing of an environments `spec.json` into a separate package
`pkg/spec` to allow working with Tanka environments from outside of our
codebase.

A separate package was required because spec parsing also involves dealing with
deprecated ksonnet attributes and correctly setting constants and the
environments name, which would be too much logic to re-implement in every
project needing to parse our `spec.json`.


Patch:
diff --git a/cmd/tk/env.go b/cmd/tk/env.go
index 0043170e..be3ba32b 100644
--- a/cmd/tk/env.go
+++ b/cmd/tk/env.go
@@ -12,7 +12,7 @@ import (
 	"github.com/spf13/pflag"
 	"github.com/spf13/viper"
 
-	"github.com/grafana/tanka/pkg/config/v1alpha1"
+	"github.com/grafana/tanka/pkg/spec/v1alpha1"
 	"github.com/grafana/tanka/pkg/util"
 )
 
diff --git a/cmd/tk/init.go b/cmd/tk/init.go
index 269e440e..ca917e90 100644
--- a/cmd/tk/init.go
+++ b/cmd/tk/init.go
@@ -6,7 +6,7 @@ import (
 	"log"
 	"os"
 
-	"github.com/grafana/tanka/pkg/config/v1alpha1"
+	"github.com/grafana/tanka/pkg/spec/v1alpha1"
 	"github.com/spf13/cobra"
 )
 
diff --git a/cmd/tk/jsonnet.go b/cmd/tk/jsonnet.go
index 321c447a..d1bb6633 100644
--- a/cmd/tk/jsonnet.go
+++ b/cmd/tk/jsonnet.go
@@ -5,8 +5,8 @@ import (
 	"log"
 	"path/filepath"
 
-	"github.com/grafana/tanka/pkg/jpath"
 	"github.com/grafana/tanka/pkg/jsonnet"
+	"github.com/grafana/tanka/pkg/jsonnet/jpath"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
diff --git a/cmd/tk/main.go b/cmd/tk/main.go
index 37b72bab..1b6650c7 100644
--- a/cmd/tk/main.go
+++ b/cmd/tk/main.go
@@ -2,9 +2,9 @@ package main
 
 import (
 	"flag"
+	"fmt"
 	"log"
 	"os"
-	"path/filepath"
 
 	"github.com/posener/complete"
 	"github.com/spf13/cobra"
@@ -12,8 +12,9 @@ import (
 	"golang.org/x/crypto/ssh/terminal"
 
 	"github.com/grafana/tanka/pkg/cmp"
-	"github.com/grafana/tanka/pkg/config/v1alpha1"
 	"github.com/grafana/tanka/pkg/kubernetes"
+	"github.com/grafana/tanka/pkg/spec"
+	"github.com/grafana/tanka/pkg/spec/v1alpha1"
 )
 
 // Version is the current version of the tk command.
@@ -113,48 +114,22 @@ func main() {
 }
 
 func setupConfiguration(baseDir string) *v1alpha1.Config {
-	viper.SetConfigName("spec")
-
-	// if the baseDir arg is not a dir, abort
-	pwd, err := filepath.Abs(baseDir)
+	config, err := spec.ParseDir(baseDir)
 	if err != nil {
-		return nil
-	}
-	viper.AddConfigPath(pwd)
-
-	// read it
-	if err := viper.ReadInConfig(); err != nil {
+		switch err.(type) {
 		// just run fine without config. Provider features won't work (apply, show, diff)
-		if _, ok := err.(viper.ConfigFileNotFoundError); ok {
+		case viper.ConfigFileNotFoundError:
 			return nil
-		}
-
-		log.Fatalf("Reading spec.json: %s", err)
-	}
-
-	// handle deprecated ksonnet spec
-	for old, new := range deprecated {
-		if viper.IsSet(old) && !viper.IsSet(new) {
-			viper.Set(new, viper.Get(old))
+		// the config includes deprecated fields
+		case spec.ErrDeprecated:
+			if verbose {
+				fmt.Print(err)
+			}
+		// some other error
+		default:
+			log.Fatalf("Reading spec.json: %s", err)
 		}
 	}
 
-	if verbose {
-		checkDeprecated()
-	}
-
-	var config = v1alpha1.New()
-	if err := viper.Unmarshal(config); err != nil {
-		log.Fatalf("Parsing spec.json: %s", err)
-	}
-	config.Metadata.Name = filepath.Base(baseDir)
 	return config
 }
-
-func checkDeprecated() {
-	for old, use := range deprecated {
-		if viper.IsSet(old) {
-			log.Printf("Warning: `%s` is deprecated, use `%s` instead.", old, use)
-		}
-	}
-}
diff --git a/cmd/tk/other.go b/cmd/tk/other.go
index 5797739d..f160d8cb 100644
--- a/cmd/tk/other.go
+++ b/cmd/tk/other.go
@@ -5,7 +5,7 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/grafana/tanka/pkg/jpath"
+	"github.com/grafana/tanka/pkg/jsonnet/jpath"
 )
 
 // findBaseDirs searches for possible environments
diff --git a/cmd/tk/tool.go b/cmd/tk/tool.go
index 7067e5bc..b3bea658 100644
--- a/cmd/tk/tool.go
+++ b/cmd/tk/tool.go
@@ -12,8 +12,8 @@ import (
 
 	"github.com/spf13/cobra"
 
-	"github.com/grafana/tanka/pkg/jpath"
 	"github.com/grafana/tanka/pkg/jsonnet"
+	"github.com/grafana/tanka/pkg/jsonnet/jpath"
 )
 
 func toolCmd() *cobra.Command {
diff --git a/docs/jsonnet/native.md b/docs/jsonnet/native.md
index 616d61af..3d952f6d 100644
--- a/docs/jsonnet/native.md
+++ b/docs/jsonnet/native.md
@@ -1,5 +1,5 @@
 path: tree/master
-source: pkg/native/funcs.go
+source: pkg/jsonnet/native/funcs.go
 
 # Native Functions
 
diff --git a/pkg/jsonnet/eval.go b/pkg/jsonnet/eval.go
index 2f601e79..7b5ac3ce 100644
--- a/pkg/jsonnet/eval.go
+++ b/pkg/jsonnet/eval.go
@@ -7,8 +7,8 @@ import (
 	jsonnet "github.com/google/go-jsonnet"
 	"github.com/pkg/errors"
 
-	"github.com/grafana/tanka/pkg/jpath"
-	"github.com/grafana/tanka/pkg/native"
+	"github.com/grafana/tanka/pkg/jsonnet/jpath"
+	"github.com/grafana/tanka/pkg/jsonnet/native"
 )
 
 // EvaluateFile opens the file, reads it into memory and evaluates it afterwards (`Evaluate()`)
diff --git a/pkg/jsonnet/imports.go b/pkg/jsonnet/imports.go
index e20af553..de8dcd58 100644
--- a/pkg/jsonnet/imports.go
+++ b/pkg/jsonnet/imports.go
@@ -9,8 +9,8 @@ import (
 	"github.com/google/go-jsonnet/toolutils"
 	"github.com/pkg/errors"
 
-	"github.com/grafana/tanka/pkg/jpath"
-	"github.com/grafana/tanka/pkg/native"
+	"github.com/grafana/tanka/pkg/jsonnet/jpath"
+	"github.com/grafana/tanka/pkg/jsonnet/native"
 )
 
 // TransitiveImports returns all recursive imports of a file
diff --git a/pkg/jpath/jpath.go b/pkg/jsonnet/jpath/jpath.go
similarity index 100%
rename from pkg/jpath/jpath.go
rename to pkg/jsonnet/jpath/jpath.go
diff --git a/pkg/native/funcs.go b/pkg/jsonnet/native/funcs.go
similarity index 100%
rename from pkg/native/funcs.go
rename to pkg/jsonnet/native/funcs.go
diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go
index b12ef51f..3a0f9e5b 100644
--- a/pkg/kubernetes/kubernetes.go
+++ b/pkg/kubernetes/kubernetes.go
@@ -13,7 +13,7 @@ import (
 	funk "github.com/thoas/go-funk"
 	yaml "gopkg.in/yaml.v2"
 
-	"github.com/grafana/tanka/pkg/config/v1alpha1"
+	"github.com/grafana/tanka/pkg/spec/v1alpha1"
 )
 
 // Kubernetes bridges tanka to the Kubernetse orchestrator.
diff --git a/pkg/spec/depreciations.go b/pkg/spec/depreciations.go
new file mode 100644
index 00000000..32c7fa8a
--- /dev/null
+++ b/pkg/spec/depreciations.go
@@ -0,0 +1,19 @@
+package spec
+
+import "fmt"
+
+type depreciation struct {
+	old, new string
+}
+
+// ErrDeprecated is a non-fatal error that occurs when deprecated fields are
+// used in the spec.json
+type ErrDeprecated []depreciation
+
+func (e ErrDeprecated) Error() string {
+	buf := ""
+	for _, d := range e {
+		buf += fmt.Sprintf("Warning: `%s` is deprecated, use `%s` instead.\n", d.old, d.new)
+	}
+	return buf
+}
diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go
new file mode 100644
index 00000000..45b30631
--- /dev/null
+++ b/pkg/spec/spec.go
@@ -0,0 +1,81 @@
+package spec
+
+import (
+	"bytes"
+	"os"
+	"path/filepath"
+
+	"github.com/grafana/tanka/pkg/spec/v1alpha1"
+	"github.com/pkg/errors"
+	"github.com/spf13/viper"
+)
+
+// list of deprecated config keys and their alternatives
+// however, they still work and are aliased internally
+var deprecated = []depreciation{
+	{old: "namespace", new: "spec.namespace"},
+	{old: "server", new: "spec.apiServer"},
+	{old: "team", new: "metadata.labels.team"},
+}
+
+// Parse parses the json `data` into a `v1alpha1.Config` object.
+// `name` is the name of the environment
+func Parse(data []byte, name string) (*v1alpha1.Config, error) {
+	v := viper.New()
+	v.SetConfigType("json")
+	if err := v.ReadConfig(bytes.NewReader(data)); err != nil {
+		return nil, err
+	}
+	return parse(v, name)
+}
+
+// ParseDir parses the given environments `spec.json` into a `v1alpha1.Config`
+// object with the name set to the directories name
+func ParseDir(baseDir string) (*v1alpha1.Config, error) {
+	fi, err := os.Stat(baseDir)
+	if err != nil {
+		return nil, err
+	}
+	if !fi.IsDir() {
+		return nil, errors.New("baseDir is not an directory")
+	}
+
+	v := viper.New()
+	v.SetConfigName("spec")
+	v.AddConfigPath(baseDir)
+
+	if err := v.ReadInConfig(); err != nil {
+		return nil, err
+	}
+
+	return parse(v, filepath.Base(baseDir))
+}
+
+// parse accepts a viper.Viper already loaded with the actual config and
+// unmarshals it onto a v1alpha1.Config
+func parse(v *viper.Viper, name string) (*v1alpha1.Config, error) {
+	var errDepr ErrDeprecated
+
+	// handle deprecated ksonnet spec
+	for _, d := range deprecated {
+		if v.IsSet(d.old) && !v.IsSet(d.new) {
+			if errDepr == nil {
+				errDepr = ErrDeprecated{d}
+			} else {
+				errDepr = append(errDepr, d)
+			}
+			v.Set(d.new, v.Get(d.old))
+		}
+	}
+
+	config := v1alpha1.New()
+	if err := v.Unmarshal(config); err != nil {
+		return nil, errors.Wrap(err, "parsing spec.json")
+	}
+
+	// set the name field
+	config.Metadata.Name = name
+
+	// return depreciation notes in case any exist as well
+	return config, errDepr
+}
diff --git a/pkg/config/v1alpha1/config.go b/pkg/spec/v1alpha1/config.go
similarity index 85%
rename from pkg/config/v1alpha1/config.go
rename to pkg/spec/v1alpha1/config.go
index 95717394..ba527003 100644
--- a/pkg/config/v1alpha1/config.go
+++ b/pkg/spec/v1alpha1/config.go
@@ -11,6 +11,8 @@ func New() *Config {
 	// default namespace
 	c.Spec.Namespace = "default"
 
+	c.Metadata.Labels = make(map[string]string)
+
 	return &c
 }
 
@@ -25,8 +27,8 @@ type Config struct {
 
 // Metadata is meant for humans and not parsed
 type Metadata struct {
-	Name   string                 `json:"name,omitempty"`
-	Labels map[string]interface{} `json:"labels,omitempty"`
+	Name   string            `json:"name,omitempty"`
+	Labels map[string]string `json:"labels,omitempty"`
 }
 
 // Spec defines Kubernetes properties


Test patch:
diff --git a/pkg/kubernetes/kubernetes_test.go b/pkg/kubernetes/kubernetes_test.go
index 35c6e9e9..b9659746 100644
--- a/pkg/kubernetes/kubernetes_test.go
+++ b/pkg/kubernetes/kubernetes_test.go
@@ -4,7 +4,7 @@ import (
 	"regexp"
 	"testing"
 
-	"github.com/grafana/tanka/pkg/config/v1alpha1"
+	"github.com/grafana/tanka/pkg/spec/v1alpha1"
 	"github.com/stretchr/objx"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
diff --git a/pkg/spec/depreciations_test.go b/pkg/spec/depreciations_test.go
new file mode 100644
index 00000000..e934c109
--- /dev/null
+++ b/pkg/spec/depreciations_test.go
@@ -0,0 +1,37 @@
+package spec
+
+import (
+	"testing"
+
+	"github.com/grafana/tanka/pkg/spec/v1alpha1"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestDeprecated checks that deprecated fields are still respected, but can be
+// overwritten by the newer format.
+func TestDeprecated(t *testing.T) {
+	data := []byte(`
+	{
+      "spec": {
+        "namespace": "new"
+      },
+	  "server": "https://127.0.0.1",
+	  "team": "cool",
+      "namespace": "old"
+	}`)
+
+	got, err := Parse(data, "test")
+	require.Equal(t, ErrDeprecated{
+		{old: "server", new: "spec.apiServer"},
+		{old: "team", new: "metadata.labels.team"},
+	}, err)
+
+	want := v1alpha1.New()
+	want.Spec.APIServer = "https://127.0.0.1"
+	want.Spec.Namespace = "new"
+	want.Metadata.Labels["team"] = "cool"
+	want.Metadata.Name = "test"
+
+	assert.Equal(t, want, got)
+}