Merge pull request #855 from shomron/qualify-libraries-and-refactor-app

Qualify library names in app.yaml to avoid cross-registry conflicts

Also:

Refactor schema to support explicit migrations

    * Dropped support for 0.0.1 apps
    * Versioning has been pushed up into the Schema types instead of App.
    * Added migrations framework for migrating schema versions, one hop at time

Refactor override handling

    * baseApp.load() / baseApp.save() are override-aware
    * app.read() / app.write() (schema.go) are not - they only serialize/deserialize app.yaml
    * baseApp.load() / baseApp.save() now call app.read() / app.write() instead of duplicating serialization logic
    * Removed isOverride flag from EnvironmentConfig, RegistryConfig
    * Removed override logic from app.Load() - this is handled in baseApp.load() now
    * env set command now respects the --override flag to indicate where to write changes

Closes #830
Closes #849
Closes #617
Signed-off-by: Oren Shomron <shomron@gmail.com>

pkg/actions/actions.go

@@ -16,6 +16,7 @@
 package actions
 
 import (
+	"bytes"
 	"crypto/tls"
 	"fmt"
 	"net"
@@ -24,6 +25,8 @@ import (
 
 	"github.com/ksonnet/ksonnet/pkg/app"
 	"github.com/ksonnet/ksonnet/pkg/client"
+	"github.com/ksonnet/ksonnet/pkg/registry"
+	"github.com/ksonnet/ksonnet/pkg/upgrade"
 	"github.com/pkg/errors"
 	"github.com/spf13/afero"
 )
@@ -378,7 +381,8 @@ func (o *optionLoader) LoadApp() app.App {
 	}
 
 	if !o.LoadOptionalBool(OptionSkipCheckUpgrade) {
-		if _, err := a.CheckUpgrade(); err != nil {
+		pm := registry.NewPackageManager(a)
+		if _, err := upgrade.CheckUpgrade(a, new(bytes.Buffer), pm, false); err != nil {
 			o.err = errors.Wrap(err, "checking for app upgrades")
 			return nil
 		}

pkg/actions/actions_test.go

@@ -278,12 +278,11 @@ func mockNsWithName(name string) *cmocks.Module {
 	return m
 }
 
-func mockRegistry(name string, isOverride bool) *rmocks.Registry {
+func mockRegistry(name string) *rmocks.Registry {
 	m := &rmocks.Registry{}
 	m.On("Name").Return(name)
 	m.On("Protocol").Return(registry.ProtocolGitHub)
 	m.On("URI").Return("github.com/ksonnet/parts/tree/master/incubator")
-	m.On("IsOverride").Return(isOverride)
 
 	return m
 }

pkg/actions/env_list.go

@@ -38,9 +38,10 @@ func RunEnvList(m map[string]interface{}) error {
 // EnvList lists available namespaces. To initialize EnvList,
 // use the `NewEnvList` constructor.
 type EnvList struct {
-	envListFn  func() (app.EnvironmentConfigs, error)
-	outputType string
-	out        io.Writer
+	envListFn       func() (app.EnvironmentConfigs, error)
+	envIsOverrideFn func(name string) bool
+	outputType      string
+	out             io.Writer
 }
 
 // NewEnvList creates an instance of EnvList
@@ -55,9 +56,10 @@ func NewEnvList(m map[string]interface{}) (*EnvList, error) {
 	}
 
 	el := &EnvList{
-		outputType: outputType,
-		envListFn:  a.Environments,
-		out:        os.Stdout,
+		outputType:      outputType,
+		envListFn:       a.Environments,
+		envIsOverrideFn: a.IsEnvOverride,
+		out:             os.Stdout,
 	}
 
 	return el, nil
@@ -83,7 +85,7 @@ func (el *EnvList) Run() error {
 
 	for name, env := range environments {
 		override := ""
-		if env.IsOverride() {
+		if el.envIsOverrideFn(name) {
 			override = "*"
 		}
 

pkg/actions/env_list_test.go

@@ -24,6 +24,7 @@ import (
 	amocks "github.com/ksonnet/ksonnet/pkg/app/mocks"
 	"github.com/ksonnet/ksonnet/pkg/util/test"
 	"github.com/pkg/errors"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
@@ -52,10 +53,12 @@ func TestEnvList(t *testing.T) {
 		}
 
 		appMock.On("Environments").Return(envs, nil)
+		appMock.On("IsEnvOverride", mock.Anything).Return(false)
 	}
 
 	envListFail := func(appMock *amocks.App) {
 		appMock.On("Environments").Return(nil, errors.New("failed"))
+		appMock.On("IsEnvOverride", mock.Anything).Return(false)
 	}
 
 	cases := []struct {

pkg/actions/env_set.go

@@ -61,6 +61,7 @@ type EnvSet struct {
 	newNsName  string
 	newServer  string
 	newAPISpec string
+	isOverride bool
 
 	envRenameFn envRenameFn
 	saveFn      saveFn
@@ -77,6 +78,7 @@ func NewEnvSet(m map[string]interface{}) (*EnvSet, error) {
 		newNsName:  ol.LoadOptionalString(OptionNamespace),
 		newServer:  ol.LoadOptionalString(OptionServer),
 		newAPISpec: ol.LoadOptionalString(OptionSpecFlag),
+		isOverride: ol.LoadOptionalBool(OptionOverride),
 
 		envRenameFn: env.Rename,
 		saveFn:      save,
@@ -96,11 +98,11 @@ func (es *EnvSet) Run() error {
 		return err
 	}
 
-	if err := es.updateName(env.IsOverride()); err != nil {
+	if err := es.updateName(es.isOverride); err != nil {
 		return err
 	}
 
-	if err := es.updateEnvConfig(*env, es.newNsName, es.newServer, es.newAPISpec, env.IsOverride()); err != nil {
+	if err := es.updateEnvConfig(*env, es.newNsName, es.newServer, es.newAPISpec, es.isOverride); err != nil {
 		return err
 	}
 

pkg/actions/env_set_test.go

@@ -155,6 +155,7 @@ func TestEnvSet(t *testing.T) {
 					}
 				},
 			},
+			// TODO add tests for overrides here
 		}
 
 		for _, tc := range cases {

pkg/actions/pkg_remove.go

@@ -71,12 +71,7 @@ func RunPkgRemove(m map[string]interface{}) error {
 
 // Run removes packages
 func (pr *PkgRemove) Run() error {
-	desc, err := pkg.Parse(pr.pkgName)
-	if err != nil {
-		return err
-	}
-
-	oldCfg, err := pr.libUpdateFn(desc.Name, pr.envName, nil)
+	oldCfg, err := pr.libUpdateFn(pr.pkgName, pr.envName, nil)
 	if err != nil {
 		return err
 	}

pkg/actions/registry_list.go

@@ -41,8 +41,9 @@ type RegistryList struct {
 	app        app.App
 	outputType string
 
-	registryListFn func(ksApp app.App) ([]registry.Registry, error)
-	out            io.Writer
+	registryListFn       func(ksApp app.App) ([]registry.Registry, error)
+	registryIsOverrideFn func(name string) bool
+	out                  io.Writer
 }
 
 // NewRegistryList creates an instance of RegistryList
@@ -50,14 +51,20 @@ func NewRegistryList(m map[string]interface{}) (*RegistryList, error) {
 	ol := newOptionLoader(m)
 
 	httpClient := ol.LoadHTTPClient()
+	a := ol.LoadApp()
+	if ol.err != nil {
+		return nil, ol.err
+	}
+
 	rl := &RegistryList{
-		app:        ol.LoadApp(),
+		app:        a,
 		outputType: ol.LoadOptionalString(OptionOutput),
 
 		registryListFn: func(ksApp app.App) ([]registry.Registry, error) {
 			return registry.List(ksApp, httpClient)
 		},
-		out: os.Stdout,
+		registryIsOverrideFn: a.IsRegistryOverride,
+		out:                  os.Stdout,
 	}
 
 	if ol.err != nil {
@@ -87,7 +94,7 @@ func (rl *RegistryList) Run() error {
 
 	for _, r := range registries {
 		override := ""
-		if r.IsOverride() {
+		if rl.registryIsOverrideFn(r.Name()) {
 			override = "*"
 		}
 

pkg/actions/registry_list_test.go

@@ -66,11 +66,17 @@ func TestRegistryList(t *testing.T) {
 
 				a.registryListFn = func(app.App) ([]registry.Registry, error) {
 					registries := []registry.Registry{
-						mockRegistry("override", true),
-						mockRegistry("incubator", false),
+						mockRegistry("override"),
+						mockRegistry("incubator"),
 					}
 					return registries, nil
 				}
+				a.registryIsOverrideFn = func(name string) bool {
+					if name == "override" {
+						return true
+					}
+					return false
+				}
 
 				err = a.Run()
 				if tc.isErr {

pkg/app/app.go

@@ -22,7 +22,6 @@ import (
 	"sort"
 
 	"github.com/pkg/errors"
-	log "github.com/sirupsen/logrus"
 	"github.com/spf13/afero"
 )
 
@@ -69,8 +68,10 @@ type App interface {
 	Fs() afero.Fs
 	// HTTPClient is the app's http client
 	HTTPClient() *http.Client
-	// CheckUpgrade checks whether an app should be upgraded.
-	CheckUpgrade() (bool, error)
+	// IsEnvOverride returns whether the specified environment has overriding configuration
+	IsEnvOverride(name string) bool
+	// IsRegistryOverride returns whether the specified registry has overriding configuration
+	IsRegistryOverride(name string) bool
 	// LibPath returns the path of the lib for an environment.
 	LibPath(envName string) (string, error)
 	// Libraries returns all environments.
@@ -95,42 +96,33 @@ type App interface {
 	UpdateLib(name string, env string, spec *LibraryConfig) (*LibraryConfig, error)
 	// UpdateRegistry updates a registry.
 	UpdateRegistry(spec *RegistryConfig) error
-	// Upgrade upgrades an application to the current version.
-	Upgrade(dryRun bool) error
-
+	// Upgrade upgrades an application (app.yaml) to the current version.
+	Upgrade(bool) error
 	// VendorPath returns the root of the vendor path.
 	VendorPath() string
 }
 
 // Load loads the application configuration.
 func Load(fs afero.Fs, httpClient *http.Client, appRoot string) (App, error) {
-	log := log.WithField("action", "app.Load")
+	if fs == nil {
+		return nil, errors.New("nil fs interface")
+	}
 
-	spec, err := read(fs, appRoot)
+	_, err := fs.Stat(specPath(appRoot))
 	if os.IsNotExist(err) {
 		// During `ks init`, app.yaml will not yet exist - generate a new one.
-		return NewApp010(fs, appRoot, httpClient), nil
+		return NewBaseApp(fs, appRoot, httpClient), nil
 	}
 	if err != nil {
-		return nil, errors.Wrap(err, "reading app configuration")
+		return nil, errors.Wrap(err, "checking existence of app configuration")
 	}
 
-	switch spec.APIVersion {
-	default:
-		return nil, errors.Errorf("unknown apiVersion %q in %s", spec.APIVersion, appYamlName)
-	case "0.0.1":
-		return NewApp001(fs, appRoot, httpClient), nil
-	case "0.1.0", "0.2.0":
-		// TODO TODO
-		// 0.1.0 will auto-upgraded to 0.2.0. 0.1.0 is read-compatible with
-		// 0.2.0, but will be persisted back as 0.2.0. This behavior will be
-		// subsequently changed with new upgrade framework.
-		a := NewApp010(fs, appRoot, httpClient)
-		log.Debugf("Interpreting app version as latest (0.2.0)", a.baseApp)
-		a.config.APIVersion = "0.2.0"
-		a.baseApp.config.APIVersion = "0.2.0"
-		return a, nil
+	a := NewBaseApp(fs, appRoot, httpClient)
+	if err := a.doLoad(); err != nil {
+		return nil, errors.Wrap(err, "reading app configuration")
 	}
+
+	return a, nil
 }
 
 func app010LibPath(root string) string {

pkg/app/app001.go

-// Copyright 2018 The kubecfg authors
-//
-//
-//    Licensed under the Apache License, Version 2.0 (the "License");
-//    you may not use this file except in compliance with the License.
-//    You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-//    Unless required by applicable law or agreed to in writing, software
-//    distributed under the License is distributed on an "AS IS" BASIS,
-//    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-//    See the License for the specific language governing permissions and
-//    limitations under the License.
-
-package app
-
-import (
-	"encoding/json"
-	"fmt"
-	"io"
-	"net/http"
-	"os"
-	"path/filepath"
-	"strings"
-
-	"github.com/ghodss/yaml"
-	"github.com/pkg/errors"
-	log "github.com/sirupsen/logrus"
-	"github.com/spf13/afero"
-)
-
-const (
-	// app001specJSON is the name for environment schema
-	app001specJSON = "spec.json"
-)
-
-// App001 is a ksonnet 0.0.1 application.
-type App001 struct {
-	out io.Writer
-	*baseApp
-}
-
-var _ App = (*App001)(nil)
-
-// NewApp001 creates an App001 instance.
-func NewApp001(fs afero.Fs, root string, httpClient *http.Client) *App001 {
-	ba := newBaseApp(fs, root, httpClient)
-
-	return &App001{
-		out:     os.Stdout,
-		baseApp: ba,
-	}
-}
-
-// AddEnvironment adds an environment spec to the app spec. If the spec already exists,
-// it is overwritten.
-func (a *App001) AddEnvironment(e *EnvironmentConfig, k8sSpecFlag string, isOverride bool) error {
-	if e == nil {
-		return errors.Errorf("nil environment configuraion")
-	}
-	if e.Name == "" {
-		return errors.Errorf("invalid environment name")
-	}
-
-	// if it is an override, write the destination to override file. If not, do the normal thing.
-
-	envPath := filepath.Join(a.root, EnvironmentDirName, e.Name)
-	if err := a.fs.MkdirAll(envPath, DefaultFolderPermissions); err != nil {
-		return err
-	}
-
-	specPath := filepath.Join(envPath, app001specJSON)
-
-	b, err := json.Marshal(e.Destination)
-	if err != nil {
-		return err
-	}
-
-	if err = afero.WriteFile(a.fs, specPath, b, DefaultFilePermissions); err != nil {
-		return err
-	}
-
-	_, err = a.libUpdater.UpdateKSLib(k8sSpecFlag, a.appLibPath(e.Name))
-	return err
-}
-
-func (a *App001) overrideDestintation(name string, envSpec *EnvironmentConfig) error {
-	return nil
-}
-
-// Environment returns the spec for an environment. In 0.1.0, the file lives in
-// /environments/name/spec.json.
-func (a *App001) Environment(name string) (*EnvironmentConfig, error) {
-	path := filepath.Join(a.root, EnvironmentDirName, name, app001specJSON)
-	return read001EnvSpec(a.fs, name, path)
-}
-
-// Environments returns specs for all environments. In 0.1.0, the environment spec
-// lives in spec.json files.
-func (a *App001) Environments() (EnvironmentConfigs, error) {
-	specs := EnvironmentConfigs{}
-
-	root := filepath.Join(a.root, EnvironmentDirName)
-
-	err := afero.Walk(a.fs, root, func(path string, fi os.FileInfo, err error) error {
-		if fi.IsDir() {
-			return nil
-		}
-
-		if fi.Name() == app001specJSON {
-			dir := filepath.Dir(path)
-			envName := strings.TrimPrefix(dir, root+"/")
-			spec, err := read001EnvSpec(a.fs, envName, path)
-			if err != nil {
-				return err
-			}
-
-			specs[envName] = spec
-		}
-
-		return nil
-	})
-	if err != nil {
-		return nil, err
-	}
-
-	return specs, nil
-}
-
-// CheckUpgrade initializes the App.
-func (a *App001) CheckUpgrade() (bool, error) {
-	msg := "Your application's apiVersion is below 0.1.0. In order to use all ks features, you " +
-		"can upgrade your application using `ks upgrade`."
-	log.Warn(msg)
-
-	return true, nil
-}
-
-// LibPath returns the lib path for an env environment.
-func (a *App001) LibPath(envName string) (string, error) {
-	return filepath.Join(a.envDir(envName), ".metadata"), nil
-}
-
-// Libraries returns application libraries.
-func (a *App001) Libraries() (LibraryConfigs, error) {
-	if err := a.load(); err != nil {
-		return nil, errors.Wrap(err, "load configuration")
-	}
-
-	return a.config.Libraries, nil
-}
-
-// Registries returns application registries.
-func (a *App001) Registries() (RegistryConfigs, error) {
-	if err := a.load(); err != nil {
-		return nil, errors.Wrap(err, "load configuration")
-	}
-
-	registries := RegistryConfigs{}
-	for k, v := range a.config.Registries {
-		registries[k] = v
-	}
-
-	for k, v := range a.overrides.Registries {
-		registries[k] = v
-	}
-
-	return registries, nil
-}
-
-// RemoveEnvironment removes an environment.
-func (a *App001) RemoveEnvironment(envName string, override bool) error {
-	a.Fs().RemoveAll(a.envDir(envName))
-	return nil
-}
-
-// RenameEnvironment renames environments.
-func (a *App001) RenameEnvironment(from, to string, override bool) error {
-	return moveEnvironment(a.fs, a.root, from, to)
-}
-
-// UpdateTargets returns an error since 0.0.1 based applications don't have support
-// for targets.
-func (a *App001) UpdateTargets(envName string, targets []string) error {
-	return errors.New("ks apps with version 0.0.1 do not have support for targets")
-}
-
-// Upgrade upgrades the app to the latest apiVersion.
-func (a *App001) Upgrade(dryRun bool) error {
-	if err := a.load(); err != nil {
-		return errors.Wrap(err, "load configuration")
-	}
-
-	if dryRun {
-		fmt.Fprintf(a.out, "\n[dry run] Upgrading application settings from version 0.0.1 to to 0.1.0.\n")
-	}
-
-	envs, err := a.Environments()
-	if err != nil {
-		return err
-	}
-
-	if dryRun {
-		fmt.Fprintf(a.out, "[dry run] Converting 0.0.1 environments to 0.1.0a:\n")
-	}
-	for _, env := range envs {
-		a.convertEnvironment(env.Path, dryRun)
-	}
-
-	a.config.APIVersion = "0.1.0"
-
-	if dryRun {
-		data, err := yaml.Marshal(a.config)
-		if err != nil {
-			return err
-		}
-
-		fmt.Fprintf(a.out, "\n[dry run] Upgraded app.yaml:\n%s\n", string(data))
-		fmt.Fprintf(a.out, "[dry run] You can perform the migration by running `ks upgrade`.\n")
-		return nil
-	}
-
-	return a.save()
-}
-
-type k8sSchema struct {
-	Info struct {
-		Version string `json:"version,omitempty"`
-	} `json:"info,omitempty"`
-}
-
-func read001EnvSpec(fs afero.Fs, name, path string) (*EnvironmentConfig, error) {
-	b, err := afero.ReadFile(fs, path)
-	if err != nil {
-		return nil, err
-	}
-
-	var s EnvironmentDestinationSpec
-	if err = json.Unmarshal(b, &s); err != nil {
-		return nil, err
-	}
-
-	if s.Namespace == "" {
-		s.Namespace = "default"
-	}
-
-	envPath := filepath.Dir(path)
-	swaggerPath := filepath.Join(envPath, ".metadata", "swagger.json")
-
-	b, err = afero.ReadFile(fs, swaggerPath)
-	if err != nil {
-		return nil, err
-	}
-
-	var ks k8sSchema
-	if err = json.Unmarshal(b, &ks); err != nil {
-		return nil, err
-	}
-
-	if ks.Info.Version == "" {
-		return nil, errors.New("unable to determine environment Kubernetes version")
-	}
-
-	spec := EnvironmentConfig{
-		Path:              name,
-		Destination:       &s,
-		KubernetesVersion: ks.Info.Version,
-	}
-
-	return &spec, nil
-}
-
-func (a *App001) convertEnvironment(envName string, dryRun bool) error {
-	if err := a.load(); err != nil {
-		return errors.Wrap(err, "load configuration")
-	}
-
-	path := filepath.Join(a.root, EnvironmentDirName, envName, "spec.json")
-	env, err := read001EnvSpec(a.fs, envName, path)
-	if err != nil {
-		return err
-	}
-
-	a.config.Environments[envName] = env
-
-	if dryRun {
-		fmt.Fprintf(a.out, "[dry run]\t* adding the environment description in environment `%s to `app.yaml`.\n",
-			envName)
-		return nil
-	}
-
-	if err = a.fs.Remove(path); err != nil {
-		return err
-	}
-
-	k8sSpecFlag := fmt.Sprintf("version:%s", env.KubernetesVersion)
-	_, err = a.libUpdater.UpdateKSLib(k8sSpecFlag, app010LibPath(a.root))
-	if err != nil {
-		return err
-	}
-
-	return a.save()
-}
-
-func (a *App001) appLibPath(envName string) string {
-	return filepath.Join(a.root, EnvironmentDirName, envName, ".metadata")
-}
-
-func (a *App001) envDir(envName string) string {
-	envParts := strings.Split(envName, "/")
-	envRoot := filepath.Join(a.Root(), EnvironmentDirName)
-	return filepath.Join(append([]string{envRoot}, envParts...)...)
-}

pkg/app/app001_test.go

-// Copyright 2018 The kubecfg authors
-//
-//
-//    Licensed under the Apache License, Version 2.0 (the "License");
-//    you may not use this file except in compliance with the License.