make param diff work across all component types

Signed-off-by: bryanl <bryanliles@gmail.com>

Gopkg.lock

@@ -695,6 +695,6 @@
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "94ae14fc6f06fe920766e3a94e71635c3071d4afa4a9013b81fc32d23b24d3b5"
+  inputs-digest = "4b474470769b133e1e691682dc68f8ca81036b0b9c85b993ed4d02743752cd58"
   solver-name = "gps-cdcl"
   solver-version = 1

actions/actions.go

@@ -41,6 +41,10 @@ const (
 	OptionDryRun = "dry-run"
 	// OptionEnvName is envName option.
 	OptionEnvName = "env-name"
+	// OptionEnvName1 is envName1. Used for param diff.
+	OptionEnvName1 = "env-name-1"
+	// OptionEnvName2 is envName1. Used for param diff.
+	OptionEnvName2 = "env-name-2"
 	// OptionFormat is format option.
 	OptionFormat = "format"
 	// OptionFs is fs option.

actions/param_diff.go

+// Copyright 2018 The ksonnet 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 actions
+
+import (
+	"io"
+	"os"
+
+	"github.com/ksonnet/ksonnet/component"
+	"github.com/ksonnet/ksonnet/metadata/app"
+	"github.com/ksonnet/ksonnet/pkg/util/table"
+)
+
+// RunParamDiff runs `param diff`.
+func RunParamDiff(m map[string]interface{}) error {
+	pd, err := NewParamDiff(m)
+	if err != nil {
+		return err
+	}
+
+	return pd.Run()
+}
+
+// ParamDiff shows difference between params in two environments.
+type ParamDiff struct {
+	app           app.App
+	envName1      string
+	envName2      string
+	componentName string
+
+	modulesFromEnvFn func(app.App, string) ([]component.Module, error)
+	out              io.Writer
+}
+
+// NewParamDiff creates an instance of ParamDiff.
+func NewParamDiff(m map[string]interface{}) (*ParamDiff, error) {
+	ol := newOptionLoader(m)
+
+	pd := &ParamDiff{
+		app:           ol.LoadApp(),
+		envName1:      ol.LoadString(OptionEnvName1),
+		envName2:      ol.LoadString(OptionEnvName2),
+		componentName: ol.LoadOptionalString(OptionComponentName),
+
+		modulesFromEnvFn: component.ModulesFromEnv,
+		out:              os.Stdout,
+	}
+
+	if ol.err != nil {
+		return nil, ol.err
+	}
+
+	return pd, nil
+}
+
+// Run runs the action.
+func (pd *ParamDiff) Run() error {
+	env1Params, err := pd.moduleParams(pd.envName1)
+	if err != nil {
+		return err
+	}
+
+	env2Params, err := pd.moduleParams(pd.envName2)
+	if err != nil {
+		return err
+	}
+
+	var rows [][]string
+
+	rows = append(rows, pd.checkDiff(env1Params, env2Params)...)
+	rows = append(rows, pd.checkMissing(env1Params, env2Params)...)
+
+	return pd.print(rows)
+}
+
+func (pd *ParamDiff) checkDiff(env1, env2 []component.ModuleParameter) [][]string {
+	var rows [][]string
+
+	for _, mp1 := range env1 {
+		if pd.componentName != "" && pd.componentName != mp1.Component {
+			continue
+		}
+		for _, mp2 := range env2 {
+			if mp1.IsSameType(mp2) && mp1.Value != mp2.Value {
+				rows = append(rows, []string{mp1.Component, mp1.Index, mp1.Key, mp1.Value, mp2.Value})
+			}
+		}
+	}
+
+	return rows
+}
+
+// nolint: gocyclo
+func (pd *ParamDiff) checkMissing(env1, env2 []component.ModuleParameter) [][]string {
+	var rows [][]string
+
+	for _, mp1 := range env1 {
+		if pd.componentName != "" && pd.componentName != mp1.Component {
+			continue
+		}
+		found := false
+		for _, mp2 := range env2 {
+			if mp1.IsSameType(mp2) {
+				found = true
+			}
+		}
+
+		if !found {
+			rows = append(rows, []string{mp1.Component, mp1.Index, mp1.Key, mp1.Value})
+		}
+	}
+
+	for _, mp1 := range env2 {
+		if pd.componentName != "" && pd.componentName != mp1.Component {
+			continue
+		}
+		found := false
+		for _, mp2 := range env1 {
+			if mp1.IsSameType(mp2) {
+				found = true
+			}
+		}
+
+		if !found {
+			rows = append(rows, []string{mp1.Component, mp1.Index, mp1.Key, "", mp1.Value})
+		}
+	}
+
+	return rows
+}
+
+func (pd *ParamDiff) moduleParams(envName string) ([]component.ModuleParameter, error) {
+	modules, err := pd.modulesFromEnvFn(pd.app, envName)
+	if err != nil {
+		return nil, err
+	}
+
+	var moduleParams []component.ModuleParameter
+	for _, module := range modules {
+		p, err := module.Params(envName)
+		if err != nil {
+			return nil, err
+		}
+
+		moduleParams = append(moduleParams, p...)
+	}
+
+	return moduleParams, nil
+}
+
+func (pd *ParamDiff) print(rows [][]string) error {
+	table := table.New(pd.out)
+
+	table.SetHeader([]string{"COMPONENT", "INDEX", "PARAM", "ENV1", "ENV2"})
+	table.AppendBulk(rows)
+
+	return table.Render()
+}

actions/param_diff_test.go

+// Copyright 2018 The ksonnet 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 actions
+
+import (
+	"bytes"
+	"path/filepath"
+	"testing"
+
+	"github.com/ksonnet/ksonnet/component"
+	"github.com/ksonnet/ksonnet/component/mocks"
+	"github.com/ksonnet/ksonnet/metadata/app"
+	amocks "github.com/ksonnet/ksonnet/metadata/app/mocks"
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParamDiff(t *testing.T) {
+	withApp(t, func(appMock *amocks.App) {
+		env1 := "env1"
+		env2 := "env2"
+
+		moduleEnv1 := &mocks.Module{}
+		env1Params := []component.ModuleParameter{
+			{Component: "a", Key: "a", Value: "a"},
+			{Component: "a", Key: "b", Value: "b1"},
+			{Component: "c", Key: "c", Value: "c"},
+		}
+		moduleEnv1.On("Params", "env1").Return(env1Params, nil)
+
+		moduleEnv2 := &mocks.Module{}
+		env2Params := []component.ModuleParameter{
+			{Component: "a", Key: "a", Value: "a"},
+			{Component: "a", Key: "b", Value: "b2"},
+			{Component: "d", Key: "d", Value: "d"},
+		}
+		moduleEnv2.On("Params", "env2").Return(env2Params, nil)
+
+		in := map[string]interface{}{
+			OptionApp:      appMock,
+			OptionEnvName1: env1,
+			OptionEnvName2: env2,
+		}
+
+		a, err := NewParamDiff(in)
+		require.NoError(t, err)
+
+		a.modulesFromEnvFn = func(_ app.App, envName string) ([]component.Module, error) {
+			switch envName {
+			case env1:
+				return []component.Module{moduleEnv1}, nil
+			case env2:
+				return []component.Module{moduleEnv2}, nil
+			default:
+				return nil, errors.Errorf("unknown env %s", envName)
+			}
+		}
+
+		var buf bytes.Buffer
+		a.out = &buf
+
+		err = a.Run()
+		require.NoError(t, err)
+
+		assertOutput(t, filepath.Join("param", "diff", "output.txt"), buf.String())
+	})
+}

actions/testdata/param/diff/output.txt

+COMPONENT INDEX PARAM ENV1 ENV2
+========= ===== ===== ==== ====
+a               b     b1   b2
+c               c     c
+d               d          d

cmd/actions.go

@@ -69,18 +69,18 @@ var (
 		actionComponentRm:   actions.RunComponentRm,
 		actionDelete:        actions.RunDelete,
 		// actionDiff
-		actionEnvAdd:       actions.RunEnvAdd,
-		actionEnvCurrent:   actions.RunEnvCurrent,
-		actionEnvDescribe:  actions.RunEnvDescribe,
-		actionEnvList:      actions.RunEnvList,
-		actionEnvRm:        actions.RunEnvRm,
-		actionEnvSet:       actions.RunEnvSet,
-		actionEnvTargets:   actions.RunEnvTargets,
-		actionImport:       actions.RunImport,
-		actionInit:         actions.RunInit,
-		actionModuleCreate: actions.RunModuleCreate,
-		actionModuleList:   actions.RunModuleList,
-		// actionParamDiff
+		actionEnvAdd:            actions.RunEnvAdd,
+		actionEnvCurrent:        actions.RunEnvCurrent,
+		actionEnvDescribe:       actions.RunEnvDescribe,
+		actionEnvList:           actions.RunEnvList,
+		actionEnvRm:             actions.RunEnvRm,
+		actionEnvSet:            actions.RunEnvSet,
+		actionEnvTargets:        actions.RunEnvTargets,
+		actionImport:            actions.RunImport,
+		actionInit:              actions.RunInit,
+		actionModuleCreate:      actions.RunModuleCreate,
+		actionModuleList:        actions.RunModuleList,
+		actionParamDiff:         actions.RunParamDiff,
 		actionParamDelete:       actions.RunParamDelete,
 		actionParamUnset:        actions.RunParamDelete,
 		actionParamList:         actions.RunParamList,

cmd/param_diff.go

@@ -17,38 +17,32 @@ package cmd
 
 import (
 	"fmt"
-	"os"
 
-	"github.com/ksonnet/ksonnet/pkg/kubecfg"
+	"github.com/ksonnet/ksonnet/actions"
 	"github.com/spf13/cobra"
+	"github.com/spf13/viper"
+)
+
+const (
+	vParamDiffComponent = "param-diff-component"
 )
 
 var paramDiffCmd = &cobra.Command{
 	Use:   "diff <env1> <env2> [--component <component-name>]",
 	Short: paramShortDesc["diff"],
 	RunE: func(cmd *cobra.Command, args []string) error {
-		flags := cmd.Flags()
 		if len(args) != 2 {
 			return fmt.Errorf("'param diff' takes exactly two arguments: the respective names of the environments being diffed")
 		}
 
-		cwd, err := os.Getwd()
-		if err != nil {
-			return err
+		m := map[string]interface{}{
+			actions.OptionApp:           ka,
+			actions.OptionEnvName1:      args[0],
+			actions.OptionEnvName2:      args[1],
+			actions.OptionComponentName: viper.GetString(vParamDiffComponent),
 		}
 
-		env1 := args[0]
-		env2 := args[1]
-
-		component, err := flags.GetString(flagComponent)
-		if err != nil {
-			return err
-		}
-
-		c := kubecfg.NewParamDiffCmd(appFs, cwd, env1, env2, component)
-
-		// TODO: convert param diff to action
-		return c.Run(cmd.OutOrStdout())
+		return runAction(actionParamDiff, m)
 	},
 	Long: `
 The ` + "`diff`" + ` command pretty prints differences between the component parameters
@@ -78,5 +72,7 @@ func init() {
 
 	paramListCmd.PersistentFlags().String(flagEnv, "", "Specify environment to list parameters for")
 	paramListCmd.Flags().String(flagModule, "", "Specify module to list parameters for")
-	paramDiffCmd.PersistentFlags().String(flagComponent, "", "Specify the component to diff against")
+
+	paramDiffCmd.Flags().String(flagComponent, "", "Specify the component to diff against")
+	viper.BindPFlag(vParamDiffComponent, paramDiffCmd.Flags().Lookup(flagComponent))
 }

cmd/param_diff_test.go

+// Copyright 2018 The ksonnet 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 cmd
+
+import (
+	"testing"
+
+	"github.com/ksonnet/ksonnet/actions"
+)
+
+func Test_paramDiffCmd(t *testing.T) {
+	cases := []cmdTestCase{
+		{
+			name:   "with a component",
+			args:   []string{"param", "diff", "env1", "env2", "--component", "component-name"},
+			action: actionParamDiff,
+			expected: map[string]interface{}{
+				actions.OptionApp:           ka,
+				actions.OptionComponentName: "component-name",
+				actions.OptionEnvName1:      "env1",
+				actions.OptionEnvName2:      "env2",
+			},
+		},
+		{
+			name:   "without a component",
+			args:   []string{"param", "diff", "env1", "env2"},
+			action: actionParamDiff,
+			expected: map[string]interface{}{
+				actions.OptionApp:           ka,
+				actions.OptionComponentName: "component-name",
+				actions.OptionEnvName1:      "env1",
+				actions.OptionEnvName2:      "env2",
+			},
+		},
+	}
+
+	runTestCmd(t, cases)
+}

component/module.go

@@ -160,6 +160,14 @@ type ModuleParameter struct {
 	Value     string
 }
 
+// IsSameType returns true if the other ModuleParams is the same type. The types
+// are the same if the component, index, and key match.
+func (mp *ModuleParameter) IsSameType(other ModuleParameter) bool {
+	return mp.Component == other.Component &&
+		mp.Index == other.Index &&
+		mp.Key == other.Key
+}
+
 // ResolvedParams resolves paramaters for a module. It returns a JSON encoded
 // string of component parameters.
 func (m *FilesystemModule) ResolvedParams() (string, error) {

e2e/param_test.go

@@ -96,6 +96,55 @@ var _ = Describe("ks param", func() {
 		})
 	})
 
+	Describe("diff", func() {
+		var extraOptions []string
+		var diffOutput *output
+
+		BeforeEach(func() {
+			a.generateDeployedService()
+
+			o := a.runKs("env", "add", "env1")
+			assertExitStatus(o, 0)
+
+			o = a.runKs("param", "set", "guestbook-ui", "replicas", "4", "--env", "env1")
+			assertExitStatus(o, 0)
+
+			o = a.runKs("env", "add", "env2")
+			assertExitStatus(o, 0)
+
+			o = a.runKs("param", "set", "guestbook-ui", "replicas", "3", "--env", "env2")
+			assertExitStatus(o, 0)
+		})
+
+		JustBeforeEach(func() {
+			options := append([]string{"param", "diff", "env1", "env2"}, extraOptions...)
+			diffOutput = a.runKs(options...)
+		})
+
+		It("runs successfully", func() {
+			assertExitStatus(diffOutput, 0)
+		})
+
+		It("lists the differences", func() {
+			assertOutput(filepath.Join("param", "diff", "output.txt"), diffOutput.stdout)
+		})
+
+		Context("with a component", func() {
+			BeforeEach(func() {
+				extraOptions = []string{"--component", "guestbook-ui"}
+			})
+
+			It("runs successfully", func() {
+				assertExitStatus(diffOutput, 0)
+			})
+
+			It("lists the differences", func() {
+				assertOutput(filepath.Join("param", "diff", "output.txt"), diffOutput.stdout)
+			})
+
+		})
+	})
+
 	Describe("list", func() {
 		var (
 			listOutput *output

e2e/testdata/output/param/diff/output.txt

+COMPONENT    INDEX PARAM    ENV1 ENV2
+=========    ===== =====    ==== ====
+guestbook-ui 0     replicas 4    3

metadata/params/params.go

@@ -18,7 +18,6 @@ package params
 import (
 	"bytes"
 	"fmt"
-	"runtime/debug"
 	"sort"
 	"strconv"
 	"strings"
@@ -59,7 +58,6 @@ func findComponentsObj(node ast.Node) (*ast.Object, error) {
 			if *f.Id == componentsID {
 				c, isObj := f.Expr2.(*ast.Object)
 				if !isObj {
-					debug.PrintStack()
 					return nil, errors.Errorf("expected components node type to be object, it was a a %T", f.Expr2)
 				}
 				return c, nil

pkg/kubecfg/param_diff.go

-// Copyright 2018 The ksonnet 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 kubecfg
-
-import (
-	"fmt"
-	"io"
-	"reflect"
-	"sort"
-
-	"github.com/fatih/color"
-	"github.com/ksonnet/ksonnet/component"
-	param "github.com/ksonnet/ksonnet/metadata/params"
-	str "github.com/ksonnet/ksonnet/strings"
-	log "github.com/sirupsen/logrus"
-	"github.com/spf13/afero"
-)
-
-// ParamDiffCmd stores the information necessary to diff between environment
-// parameters.
-type ParamDiffCmd struct {
-	fs   afero.Fs
-	root string
-	env1 string
-	env2 string
-
-	component string
-}
-
-// NewParamDiffCmd acts as a constructor for ParamDiffCmd.
-func NewParamDiffCmd(fs afero.Fs, root, env1, env2, componentName string) *ParamDiffCmd {
-	return &ParamDiffCmd{
-		fs:        fs,
-		root:      root,
-		env1:      env1,
-		env2:      env2,
-		component: componentName,
-	}
-}
-
-type paramDiffRecord struct {
-	component string
-	param     string
-	value1    string
-	value2    string
-}
-
-// Run executes the diffing of environment params.
-func (c *ParamDiffCmd) Run(out io.Writer) error {
-	const (
-		componentHeader = "COMPONENT"
-		paramHeader     = "PARAM"
-	)
-
-	manager, err := manager()
-	if err != nil {
-		return err
-	}
-
-	ksApp, err := manager.App()
-	if err != nil {
-		return err
-	}
-
-	ns, componentName := component.ExtractModuleComponent(ksApp, c.component)
-
-	params1, err := manager.GetEnvironmentParams(c.env1, ns.Name())
-	if err != nil {
-		return err
-	}
-
-	params2, err := manager.GetEnvironmentParams(c.env2, ns.Name())
-	if err != nil {
-		return err
-	}
-
-	if len(c.component) != 0 {
-		params1 = map[string]param.Params{componentName: params1[componentName]}
-		params2 = map[string]param.Params{componentName: params2[componentName]}
-	}
-
-	if reflect.DeepEqual(params1, params2) {
-		log.Info("No differences found.")
-		return nil
-	}
-
-	componentNames := collectComponents(params1, params2)
-