fix: Stable environment name (#182)

* fix: stable naming of the environment

Before, the name of the internally used Environment type was different depending
on the argument passed on the command line, even though it resulted in the same
environment after all (relative vs absolute paths, etc.)

This fixes the issue by using the `jpath` package to compute baseDir and
rootDir, using the relative path from rootDir to baseDir as the name. This
results in stable names, regardless which directory representation is used.

* refactor(api): simplify

* fix(cli): remove blank line from output

* fix(cli/jsonnet): do static import analysis on environment level

All of our current commands take environments, only the import analysis works on
plain files ... this has no benefit, it actually makes the implementation
harder, as we cannot use stable paths there.

Switches to environments as the target.

Author: sh0rez

cmd/tk/main.go

@@ -5,13 +5,15 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"path/filepath"
 
 	"github.com/posener/complete"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"golang.org/x/crypto/ssh/terminal"
 
 	"github.com/grafana/tanka/pkg/cli/cmp"
+	"github.com/grafana/tanka/pkg/jsonnet/jpath"
 	"github.com/grafana/tanka/pkg/spec"
 	"github.com/grafana/tanka/pkg/spec/v1alpha1"
 )
@@ -86,7 +88,15 @@ func main() {
 }
 
 func setupConfiguration(baseDir string) *v1alpha1.Config {
-	config, err := spec.ParseDir(baseDir)
+	_, baseDir, rootDir, err := jpath.Resolve(baseDir)
+	if err != nil {
+		log.Fatalln("Resolving jpath:", err)
+	}
+
+	// name of the environment: relative path from rootDir
+	name, _ := filepath.Rel(rootDir, baseDir)
+
+	config, err := spec.ParseDir(baseDir, name)
 	if err != nil {
 		switch err.(type) {
 		// just run fine without config. Provider features won't work (apply, show, diff)

cmd/tk/tool.go

@@ -51,9 +51,12 @@ func jpathCmd() *cobra.Command {
 
 func importsCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "imports <file>",
-		Short: "list all transitive imports of a file",
+		Use:   "imports <directory>",
+		Short: "list all transitive imports of an environment",
 		Args:  cobra.ExactArgs(1),
+		Annotations: map[string]string{
+			"args": "baseDir",
+		},
 		Run: func(cmd *cobra.Command, args []string) {
 			var modFiles []string
 			if cmd.Flag("check").Changed {
@@ -64,22 +67,28 @@ func importsCmd() *cobra.Command {
 				}
 			}
 
-			f, err := filepath.Abs(args[0])
+			dir, err := filepath.Abs(args[0])
 			if err != nil {
-				log.Fatalln("Opening file:", err)
+				log.Fatalln("Loading environment:", err)
 			}
 
-			deps, err := jsonnet.TransitiveImports(f)
+			fi, err := os.Stat(dir)
 			if err != nil {
-				log.Fatalln("resolving imports:", err)
+				log.Fatalln("Loading environment:", err)
+			}
+
+			if !fi.IsDir() {
+				log.Fatalln("The argument must be an environment's directory, but this does not seem to be the case.")
 			}
 
-			// include main.jsonnet as well
-			deps = append(deps, f)
+			deps, err := jsonnet.TransitiveImports(dir)
+			if err != nil {
+				log.Fatalln("Resolving imports:", err)
+			}
 
 			root, err := gitRoot()
 			if err != nil {
-				log.Fatalln("invoking git:", err)
+				log.Fatalln("Invoking git:", err)
 			}
 			if modFiles != nil {
 				for _, m := range modFiles {

pkg/jsonnet/imports.go

@@ -3,6 +3,7 @@ package jsonnet
 import (
 	"io/ioutil"
 	"path/filepath"
+	"sort"
 
 	jsonnet "github.com/google/go-jsonnet"
 	"github.com/google/go-jsonnet/ast"
@@ -13,14 +14,21 @@ import (
 	"github.com/grafana/tanka/pkg/jsonnet/native"
 )
 
-// TransitiveImports returns all recursive imports of a file
-func TransitiveImports(filename string) ([]string, error) {
-	sonnet, err := ioutil.ReadFile(filename)
+// TransitiveImports returns all recursive imports of an environment
+func TransitiveImports(dir string) ([]string, error) {
+	dir, err := filepath.Abs(dir)
+	if err != nil {
+		return nil, err
+	}
+
+	mainFile := filepath.Join(dir, "main.jsonnet")
+
+	sonnet, err := ioutil.ReadFile(mainFile)
 	if err != nil {
 		return nil, errors.Wrap(err, "opening file")
 	}
 
-	jpath, _, _, err := jpath.Resolve(filepath.Dir(filename))
+	jpath, _, rootDir, err := jpath.Resolve(dir)
 	if err != nil {
 		return nil, errors.Wrap(err, "resolving JPATH")
 	}
@@ -37,9 +45,17 @@ func TransitiveImports(filename string) ([]string, error) {
 	}
 
 	imports := make([]string, 0)
-	err = importRecursive(&imports, vm, node, "main.jsonnet")
+	if err = importRecursive(&imports, vm, node, "main.jsonnet"); err != nil {
+		return nil, err
+	}
+
+	uniq := append(uniqueStringSlice(imports), mainFile)
+	for i := range uniq {
+		uniq[i], _ = filepath.Rel(rootDir, uniq[i])
+	}
+	sort.Strings(uniq)
 
-	return uniqueStringSlice(imports), err
+	return uniq, nil
 }
 
 // importRecursive takes a Jsonnet VM and recursively imports the AST. Every
@@ -56,7 +72,8 @@ func importRecursive(list *[]string, vm *jsonnet.VM, node ast.Node, currentPath
 			return errors.Wrap(err, "importing jsonnet")
 		}
 
-		*list = append(*list, foundAt)
+		abs, _ := filepath.Abs(foundAt)
+		*list = append(*list, abs)
 
 		if err := importRecursive(list, vm, contents, foundAt); err != nil {
 			return err
@@ -71,7 +88,8 @@ func importRecursive(list *[]string, vm *jsonnet.VM, node ast.Node, currentPath
 			return errors.Wrap(err, "importing string")
 		}
 
-		*list = append(*list, foundAt)
+		abs, _ := filepath.Abs(foundAt)
+		*list = append(*list, abs)
 
 	// neither `import` nor `importstr`, probably object or similar: try children
 	default:

pkg/jsonnet/imports_test.go

@@ -1,6 +1,7 @@
 package jsonnet
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -10,15 +11,15 @@ import (
 // TestTransitiveImports checks that TransitiveImports is able to report all
 // recursive imports of a file
 func TestTransitiveImports(t *testing.T) {
-	imports, err := TransitiveImports("testdata/main.jsonnet")
+	imports, err := TransitiveImports("testdata")
+	fmt.Println(imports)
 	require.NoError(t, err)
-	assert.ElementsMatch(t, []string{
-		"testdata/trees.jsonnet",
-
-		"testdata/trees/apple.jsonnet",
-		"testdata/trees/cherry.jsonnet",
-		"testdata/trees/peach.jsonnet",
-
-		"testdata/trees/generic.libsonnet",
+	assert.Equal(t, []string{
+		"main.jsonnet",
+		"trees.jsonnet",
+		"trees/apple.jsonnet",
+		"trees/cherry.jsonnet",
+		"trees/generic.libsonnet",
+		"trees/peach.jsonnet",
 	}, imports)
 }

pkg/jsonnet/jpath/jpath.go

@@ -32,6 +32,11 @@ func (e ErrorFileNotFound) Error() string {
 // This results in predictable imports, as it doesn't matter whether the user called
 // called the command further down tree or not. A little bit like git.
 func Resolve(workdir string) (path []string, base, root string, err error) {
+	workdir, err = filepath.Abs(workdir)
+	if err != nil {
+		return nil, "", "", err
+	}
+
 	root, err = FindParentFile("jsonnetfile.json", workdir, "/")
 	if err != nil {
 		if _, ok := err.(ErrorFileNotFound); ok {

pkg/kubernetes/kubernetes_test.go

@@ -88,7 +88,9 @@ func TestReconcile(t *testing.T) {
 
 	for _, c := range tests {
 		t.Run(c.name, func(t *testing.T) {
-			got, err := Reconcile(c.deep.(map[string]interface{}), c.spec, c.targets)
+			config := v1alpha1.New()
+			config.Spec = c.spec
+			got, err := Reconcile(c.deep.(map[string]interface{}), config.Spec, c.targets)
 
 			require.Equal(t, c.err, err)
 			assert.ElementsMatch(t, c.flat, got)
@@ -99,7 +101,7 @@ func TestReconcile(t *testing.T) {
 func TestReconcileOrder(t *testing.T) {
 	got := make([]manifest.List, 10)
 	for i := 0; i < 10; i++ {
-		r, err := Reconcile(testDataDeep().deep.(map[string]interface{}), v1alpha1.Spec{}, nil)
+		r, err := Reconcile(testDataDeep().deep.(map[string]interface{}), v1alpha1.New().Spec, nil)
 		require.NoError(t, err)
 		got[i] = r
 	}

pkg/spec/spec.go

@@ -3,7 +3,6 @@ package spec
 import (
 	"bytes"
 	"os"
-	"path/filepath"
 
 	"github.com/pkg/errors"
 	"github.com/spf13/viper"
@@ -34,7 +33,7 @@ func Parse(data []byte, name string) (*v1alpha1.Config, error) {
 
 // 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) {
+func ParseDir(baseDir, name string) (*v1alpha1.Config, error) {
 	fi, err := os.Stat(baseDir)
 	if err != nil {
 		return nil, err
@@ -51,7 +50,7 @@ func ParseDir(baseDir string) (*v1alpha1.Config, error) {
 		return nil, err
 	}
 
-	return parse(v, filepath.Base(baseDir))
+	return parse(v, name)
 }
 
 // parse accepts a viper.Viper already loaded with the actual config and
@@ -62,11 +61,7 @@ func parse(v *viper.Viper, name string) (*v1alpha1.Config, error) {
 	// 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)
-			}
+			errDepr = append(errDepr, d)
 			v.Set(d.new, v.Get(d.old))
 		}
 	}
@@ -79,6 +74,10 @@ func parse(v *viper.Viper, name string) (*v1alpha1.Config, error) {
 	// set the name field
 	config.Metadata.Name = name
 
+	if len(errDepr) == 0 {
+		return config, nil
+	}
+
 	// return depreciation notes in case any exist as well
 	return config, errDepr
 }