Merge pull request #347 from bryanl/bug-rename-env

bug: 0.1.0 apps don't rename envs in config

env/rename.go

@@ -47,46 +47,13 @@ func (r *renamer) Rename(from, to string) error {
 		return err
 	}
 
-	pathFrom := envPath(r.AppRoot, from)
-	pathTo := envPath(r.AppRoot, to)
-
-	exists, err := afero.DirExists(r.Fs, pathFrom)
-	if err != nil {
-		return err
-	}
-
-	if !exists {
-		return errors.Errorf("environment directory for %q does not exist", from)
-	}
-
 	log.Infof("Setting environment name from %q to %q", from, to)
 
-	current, err := Retrieve(r.App, from)
-	if err != nil {
-		return err
-	}
-
-	update := &app.EnvironmentSpec{
-		Destination: &app.EnvironmentDestinationSpec{
-			Namespace: current.Destination.Namespace(),
-			Server:    current.Destination.Server(),
-		},
-		KubernetesVersion: current.KubernetesVersion,
-		Targets:           current.Targets,
-		Path:              to,
-	}
-
-	k8sSpecFlag := fmt.Sprintf("version:%s", current.KubernetesVersion)
-
-	if err = r.App.AddEnvironment(from, k8sSpecFlag, update); err != nil {
-		return err
-	}
-
-	if err = moveDir(r.Fs, pathFrom, pathTo); err != nil {
+	if err := r.App.RenameEnvironment(from, to); err != nil {
 		return err
 	}
 
-	if err = cleanEmptyDirs(r.Fs, r.AppRoot); err != nil {
+	if err := cleanEmptyDirs(r.Fs, r.AppRoot); err != nil {
 		return errors.Wrap(err, "clean empty directories")
 	}
 

env/rename_test.go

@@ -3,12 +3,9 @@ package env
 import (
 	"testing"
 
-	"github.com/stretchr/testify/mock"
-
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/require"
 
-	"github.com/ksonnet/ksonnet/metadata/app"
 	"github.com/ksonnet/ksonnet/metadata/app/mocks"
 )
 
@@ -16,18 +13,7 @@ func TestRename(t *testing.T) {
 	withEnv(t, func(fs afero.Fs) {
 		appMock := &mocks.App{}
 
-		envSpec := &app.EnvironmentSpec{
-			Path:              "env1",
-			Destination:       &app.EnvironmentDestinationSpec{Namespace: "default", Server: "http://example.com"},
-			KubernetesVersion: "v1.9.2",
-		}
-		appMock.On("Environment", "env1").Return(envSpec, nil)
-
-		appMock.On(
-			"AddEnvironment",
-			"env1",
-			"version:v1.9.2",
-			mock.AnythingOfType("*app.EnvironmentSpec")).Return(nil)
+		appMock.On("RenameEnvironment", "env1", "env1-updated").Return(nil)
 
 		config := RenameConfig{
 			App:     appMock,
@@ -35,12 +21,7 @@ func TestRename(t *testing.T) {
 			Fs:      fs,
 		}
 
-		checkExists(t, fs, "/environments/env1")
-
 		err := Rename("env1", "env1-updated", config)
 		require.NoError(t, err)
-
-		checkNotExists(t, fs, "/environments/env1")
-		checkExists(t, fs, "/environments/env1-updated/main.jsonnet")
 	})
 }

metadata/app/app.go

@@ -3,6 +3,7 @@ package app
 import (
 	"os"
 	"path/filepath"
+	"sort"
 
 	"github.com/ksonnet/ksonnet/metadata/lib"
 	"github.com/pkg/errors"
@@ -40,6 +41,7 @@ type App interface {
 	LibPath(envName string) (string, error)
 	Init() error
 	Registries() RegistryRefSpecs
+	RenameEnvironment(from, to string) error
 	RemoveEnvironment(name string) error
 	Upgrade(dryRun bool) error
 }
@@ -61,7 +63,7 @@ func Load(fs afero.Fs, appRoot string) (App, error) {
 	}
 }
 
-func updateLibData(fs afero.Fs, k8sSpecFlag string, libPath string, useVersionPath bool) (string, error) {
+func updateLibData(fs afero.Fs, k8sSpecFlag, libPath string, useVersionPath bool) (string, error) {
 	lm, err := lib.NewManager(k8sSpecFlag, fs, libPath)
 	if err != nil {
 		return "", err
@@ -79,6 +81,88 @@ func app010LibPath(root string) string {
 }
 
 // StubUpdateLibData always returns no error.
-func StubUpdateLibData(fs afero.Fs, k8sSpecFlag string, libPath string, useVersionPath bool) (string, error) {
+func StubUpdateLibData(fs afero.Fs, k8sSpecFlag, libPath string, useVersionPath bool) (string, error) {
 	return "v1.8.7", nil
 }
+
+func moveEnvironment(fs afero.Fs, root, from, to string) error {
+	toPath := filepath.Join(root, EnvironmentDirName, to)
+
+	exists, err := afero.Exists(fs, filepath.Join(toPath, "main.jsonnet"))
+	if err != nil {
+		return err
+	}
+
+	if exists {
+		return errors.Errorf("unable to rename %q because %q exists", from, to)
+	}
+
+	fromPath := filepath.Join(root, EnvironmentDirName, from)
+	exists, err = afero.Exists(fs, fromPath)
+	if err != nil {
+		return err
+	}
+
+	if !exists {
+		return errors.Errorf("environment %q does not exist", from)
+	}
+
+	// create to directory
+	if err = fs.MkdirAll(toPath, DefaultFolderPermissions); err != nil {
+		return err
+	}
+
+	fis, err := afero.ReadDir(fs, fromPath)
+	if err != nil {
+		return err
+	}
+
+	for _, fi := range fis {
+		if fi.IsDir() && fi.Name() != ".metadata" {
+			continue
+		}
+
+		oldPath := filepath.Join(fromPath, fi.Name())
+		newPath := filepath.Join(toPath, fi.Name())
+		if err := fs.Rename(oldPath, newPath); err != nil {
+			return err
+		}
+	}
+
+	return cleanEnv(fs, root)
+}
+
+func cleanEnv(fs afero.Fs, root string) error {
+	var dirs []string
+
+	envDir := filepath.Join(root, EnvironmentDirName)
+	err := afero.Walk(fs, envDir, func(path string, fi os.FileInfo, err error) error {
+		if !fi.IsDir() {
+			return nil
+		}
+
+		dirs = append(dirs, path)
+		return nil
+	})
+
+	if err != nil {
+		return err
+	}
+
+	sort.Sort(sort.Reverse(sort.StringSlice(dirs)))
+
+	for _, dir := range dirs {
+		fis, err := afero.ReadDir(fs, dir)
+		if err != nil {
+			return err
+		}
+
+		if len(fis) == 0 {
+			if err := fs.RemoveAll(dir); err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}

metadata/app/app001.go

@@ -76,6 +76,11 @@ func (a *App001) AddEnvironment(name, k8sSpecFlag string, spec *EnvironmentSpec)
 	return err
 }
 
+// RenameEnvironment renames environments.
+func (a *App001) RenameEnvironment(from, to string) error {
+	return moveEnvironment(a.fs, a.root, from, to)
+}
+
 // Registries returns application registries.
 func (a *App001) Registries() RegistryRefSpecs {
 	return a.spec.Registries

metadata/app/app001_test.go

@@ -2,14 +2,80 @@ package app
 
 import (
 	"bytes"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestApp001_RenameEnvironment(t *testing.T) {
+	cases := []struct {
+		name           string
+		from           string
+		to             string
+		shouldExist    []string
+		shouldNotExist []string
+	}{
+		{
+			name: "rename",
+			from: "default",
+			to:   "renamed",
+			shouldExist: []string{
+				"/environments/renamed/.metadata",
+				"/environments/renamed/spec.json",
+			},
+			shouldNotExist: []string{
+				"/environments/default",
+			},
+		},
+		{
+			name: "rename to nested",
+			from: "default",
+			to:   "default/nested",
+			shouldExist: []string{
+				"/environments/default/nested/.metadata",
+				"/environments/default/nested/spec.json",
+			},
+			shouldNotExist: []string{
+				"/environments/default/.metadata",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		withApp001Fs(t, "app001_app.yaml", func(fs afero.Fs) {
+			t.Run(tc.name, func(t *testing.T) {
+				app, err := NewApp001(fs, "/")
+				require.NoError(t, err)
+
+				err = app.RenameEnvironment(tc.from, tc.to)
+				require.NoError(t, err)
+
+				for _, p := range tc.shouldExist {
+					checkExist(t, fs, p)
+				}
+
+				for _, p := range tc.shouldNotExist {
+					checkNotExist(t, fs, p)
+				}
+
+				app.load()
+				require.NoError(t, err)
+
+				_, err = app.Environment(tc.from)
+				assert.Error(t, err)
+
+				_, err = app.Environment(tc.to)
+				assert.NoError(t, err)
+			})
+		})
+	}
+}
+
 func TestApp001_Environments(t *testing.T) {
 	withApp001Fs(t, "app001_app.yaml", func(fs afero.Fs) {
 		app, err := NewApp001(fs, "/")
@@ -171,7 +237,12 @@ func withApp001Fs(t *testing.T, appName string, fn func(fs afero.Fs)) {
 		LibUpdater = ogLibUpdater
 	}()
 
-	fs := afero.NewMemMapFs()
+	dir, err := ioutil.TempDir("", "")
+	require.NoError(t, err)
+
+	defer os.RemoveAll(dir)
+
+	fs := afero.NewBasePathFs(afero.NewOsFs(), dir)
 
 	envDirs := []string{
 		"default",

metadata/app/app010.go

@@ -102,6 +102,24 @@ func (a *App010) AddEnvironment(name, k8sSpecFlag string, spec *EnvironmentSpec)
 	return a.save()
 }
 
+// RenameEnvironment renames environments.
+func (a *App010) RenameEnvironment(from, to string) error {
+	if err := moveEnvironment(a.fs, a.root, from, to); err != nil {
+		return err
+	}
+
+	if err := a.load(); err != nil {
+		return err
+	}
+
+	a.spec.Environments[to] = a.spec.Environments[from]
+	delete(a.spec.Environments, from)
+
+	a.spec.Environments[to].Path = to
+
+	return a.save()
+}
+
 // Registries returns application registries.
 func (a *App010) Registries() RegistryRefSpecs {
 	return a.spec.Registries

metadata/app/app010_test.go

@@ -2,13 +2,85 @@ package app
 
 import (
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestApp010_RenameEnvironment(t *testing.T) {
+	cases := []struct {
+		name           string
+		from           string
+		to             string
+		shouldExist    []string
+		shouldNotExist []string
+	}{
+		{
+			name: "rename",
+			from: "default",
+			to:   "renamed",
+			shouldExist: []string{
+				"/environments/renamed/main.jsonnet",
+			},
+			shouldNotExist: []string{
+				"/environments/default",
+			},
+		},
+		{
+			name: "rename to nested",
+			from: "default",
+			to:   "default/nested",
+			shouldExist: []string{
+				"/environments/default/nested/main.jsonnet",
+			},
+			shouldNotExist: []string{
+				"/environments/default/main.jsonnet",
+			},
+		},
+		{
+			name: "un-nest",
+			from: "us-east/test",
+			to:   "us-east",
+			shouldExist: []string{
+				"/environments/us-east/main.jsonnet",
+			},
+			shouldNotExist: []string{
+				"/environments/us-east/test",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		withApp010Fs(t, "app010_app.yaml", func(fs afero.Fs) {
+			t.Run(tc.name, func(t *testing.T) {
+				app, err := NewApp010(fs, "/")
+				require.NoError(t, err)
+
+				err = app.RenameEnvironment(tc.from, tc.to)
+				require.NoError(t, err)
+
+				for _, p := range tc.shouldExist {
+					checkExist(t, fs, p)
+				}
+
+				for _, p := range tc.shouldNotExist {
+					checkNotExist(t, fs, p)
+				}
+
+				_, err = app.Environment(tc.from)
+				assert.Error(t, err)
+
+				_, err = app.Environment(tc.to)
+				assert.NoError(t, err)
+			})
+		})
+	}
+}
+
 func TestApp0101_Environments(t *testing.T) {
 	withApp010Fs(t, "app010_app.yaml", func(fs afero.Fs) {
 		app, err := NewApp010(fs, "/")
@@ -28,21 +100,24 @@ func TestApp0101_Environments(t *testing.T) {
 					Namespace: "some-namespace",
 					Server:    "http://example.com",
 				},
-				Path: "us-east/test",
+				KubernetesVersion: "v1.7.0",
+				Path:              "us-east/test",
 			},
 			"us-west/test": &EnvironmentSpec{
 				Destination: &EnvironmentDestinationSpec{
 					Namespace: "some-namespace",
 					Server:    "http://example.com",
 				},
-				Path: "us-west/test",
+				KubernetesVersion: "v1.7.0",
+				Path:              "us-west/test",
 			},
 			"us-west/prod": &EnvironmentSpec{
 				Destination: &EnvironmentDestinationSpec{
 					Namespace: "some-namespace",
 					Server:    "http://example.com",
 				},
-				Path: "us-west/prod",
+				KubernetesVersion: "v1.7.0",
+				Path:              "us-west/prod",
 			},
 		}
 		envs, err := app.Environments()
@@ -148,22 +223,30 @@ func withApp010Fs(t *testing.T, appName string, fn func(fs afero.Fs)) {
 		LibUpdater = ogLibUpdater
 	}()
 
-	fs := afero.NewMemMapFs()
-	stageFile(t, fs, appName, "/app.yaml")
+	dir, err := ioutil.TempDir("", "")
+	require.NoError(t, err)
 
-	fn(fs)
-}
+	defer os.RemoveAll(dir)
 
-func stageFile(t *testing.T, fs afero.Fs, src, dest string) {
-	in := filepath.Join("testdata", src)
+	fs := afero.NewBasePathFs(afero.NewOsFs(), dir)
 
-	b, err := ioutil.ReadFile(in)
-	require.NoError(t, err)
+	envDirs := []string{
+		"default",
+		"us-east/test",
+		"us-west/test",
+		"us-west/prod",
+	}
 
-	dir := filepath.Dir(dest)
-	err = fs.MkdirAll(dir, 0755)
-	require.NoError(t, err)
+	for _, dir := range envDirs {
+		path := filepath.Join("/environments", dir)
+		err := fs.MkdirAll(path, DefaultFolderPermissions)
+		require.NoError(t, err)
 
-	err = afero.WriteFile(fs, dest, b, 0644)
-	require.NoError(t, err)
+		swaggerPath := filepath.Join(path, "main.jsonnet")
+		stageFile(t, fs, "main.jsonnet", swaggerPath)
+	}
+
+	stageFile(t, fs, appName, "/app.yaml")
+
+	fn(fs)
 }

metadata/app/helpers_test.go

+package app
+
+import (
+	"io/ioutil"
+	"path/filepath"
+	"testing"
+
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/require"
+)
+
+func stageFile(t *testing.T, fs afero.Fs, src, dest string) {
+	in := filepath.Join("testdata", src)
+
+	b, err := ioutil.ReadFile(in)
+	require.NoError(t, err)
+
+	dir := filepath.Dir(dest)
+	err = fs.MkdirAll(dir, 0755)
+	require.NoError(t, err)
+
+	err = afero.WriteFile(fs, dest, b, 0644)
+	require.NoError(t, err)
+}
+
+func checkExist(t *testing.T, fs afero.Fs, path string) {
+	exists, err := afero.Exists(fs, path)
+	require.NoError(t, err)
+
+	require.True(t, exists)
+}
+
+func checkNotExist(t *testing.T, fs afero.Fs, path string) {
+	exists, err := afero.Exists(fs, path)
+	require.NoError(t, err)
+
+	require.False(t, exists)
+}

metadata/app/mocks/App.go

@@ -150,6 +150,20 @@ func (_m *App) RemoveEnvironment(name string) error {
 	return r0
 }
 
+// RenameEnvironment provides a mock function with given fields: from, to
+func (_m *App) RenameEnvironment(from string, to string) error {
+	ret := _m.Called(from, to)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(string, string) error); ok {
+		r0 = rf(from, to)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // Upgrade provides a mock function with given fields: dryRun
 func (_m *App) Upgrade(dryRun bool) error {
 	ret := _m.Called(dryRun)

metadata/app/testdata/app010_app.yaml

@@ -10,19 +10,19 @@ environments:
     destination:
       namespace: some-namespace
       server: http://example.com
-    k8sVersion: ""
+    k8sVersion: v1.7.0
     path: us-east/test
   us-west/prod:
     destination:
       namespace: some-namespace
       server: http://example.com
-    k8sVersion: ""
+    k8sVersion: v1.7.0
     path: us-west/prod
   us-west/test:
     destination:
       namespace: some-namespace
       server: http://example.com
-    k8sVersion: ""
+    k8sVersion: v1.7.0
     path: us-west/test
 kind: ksonnet.io/app
 name: test-get-envs

metadata/app/testdata/main.jsonnet

+local base = import "../base.libsonnet";
+local k = import "k.libsonnet";
+
+base + {
+  // Insert user-specified overrides here. For example if a component is named "nginx-deployment", you might have something like:
+  //   "nginx-deployment"+: k.deployment.mixin.metadata.labels({foo: "bar"})
+}