review comments, and small fix for writing config entry that doesn't previously exist

Author: nkubala

cmd/skaffold/app/cmd/config/config.go

@@ -22,6 +22,6 @@ type Config struct {
 }
 
 type ContextConfig struct {
-	Kubectx     string `yaml:"kubectx,omitempty"`
+	Kubecontext string `yaml:"kube-context,omitempty"`
 	DefaultRepo string `yaml:"default-repo"`
 }

cmd/skaffold/app/cmd/config/flags.go

@@ -20,12 +20,12 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var configFile, kubectx string
+var configFile, kubecontext string
 var showAll, global bool
 
 func AddConfigFlags(cmd *cobra.Command) {
 	cmd.Flags().StringVarP(&configFile, "config", "c", "", "path to skaffold config")
-	cmd.Flags().StringVarP(&kubectx, "kubectx", "k", "", "kubectl context to set values against")
+	cmd.Flags().StringVarP(&kubecontext, "kube-context", "k", "", "kubectl context to set values against")
 }
 
 func AddListFlags(cmd *cobra.Command) {

cmd/skaffold/app/cmd/config/list.go

@@ -52,11 +52,11 @@ func runList(out io.Writer) error {
 			return errors.Wrap(err, "marshaling config")
 		}
 	} else {
-		configs, err := getConfigForKubectx()
+		config, err := getConfigForKubectx()
 		if err != nil {
 			return err
 		}
-		configYaml, err = yaml.Marshal(&configs)
+		configYaml, err = yaml.Marshal(&config)
 		if err != nil {
 			return errors.Wrap(err, "marshaling config")
 		}

cmd/skaffold/app/cmd/config/set.go

@@ -44,7 +44,7 @@ func NewCmdSet(out io.Writer) *cobra.Command {
 }
 
 func setConfigValue(name string, value interface{}) error {
-	cfg, err := getConfigForKubectx()
+	cfg, err := getOrCreateConfigForKubectx()
 	if err != nil {
 		return err
 	}
@@ -77,20 +77,24 @@ func setConfigValue(name string, value interface{}) error {
 }
 
 func writeConfig(cfg *ContextConfig) error {
-	configs, err := readConfig()
+	fullConfig, err := readConfig()
 	if err != nil {
 		return err
 	}
 	if global {
-		configs.Global = cfg
+		fullConfig.Global = cfg
 	} else {
-		for i, contextCfg := range configs.ContextConfigs {
-			if contextCfg.Kubectx == kubectx {
-				configs.ContextConfigs[i] = cfg
+		for i, contextCfg := range fullConfig.ContextConfigs {
+			if contextCfg.Kubecontext == kubecontext {
+				fullConfig.ContextConfigs[i] = cfg
 			}
 		}
 	}
-	contents, err := yaml.Marshal(configs)
+	return writeFullConfig(fullConfig)
+}
+
+func writeFullConfig(cfg *Config) error {
+	contents, err := yaml.Marshal(cfg)
 	if err != nil {
 		return errors.Wrap(err, "marshaling config")
 	}

cmd/skaffold/app/cmd/config/util.go

@@ -33,20 +33,20 @@ import (
 const defaultConfigLocation = ".skaffold/config"
 
 func resolveKubectlContext() {
-	if kubectx != "" {
+	if kubecontext != "" {
 		return
 	}
 
 	context, err := context.CurrentContext()
 	if err != nil {
 		logrus.Warn(errors.Wrap(err, "retrieving current kubectl context"))
-		kubectx = "default"
+		kubecontext = "default"
 	}
 	if context == "" {
 		logrus.Infof("no context currently set, falling back to default")
-		kubectx = "default"
+		kubecontext = "default"
 	}
-	kubectx = context
+	kubecontext = context
 }
 
 func resolveConfigFile() error {
@@ -95,7 +95,7 @@ func readConfig() (*Config, error) {
 	return ReadConfigForFile(configFile)
 }
 
-// return the specific config to be modified based on the provided kubectx.
+// return the specific config to be modified based on the provided kube context.
 // either returns the config corresponding to the provided or current context,
 // or the global config if that is specified (or if no current context is set).
 func getConfigForKubectx() (*ContextConfig, error) {
@@ -107,9 +107,34 @@ func getConfigForKubectx() (*ContextConfig, error) {
 		return cfg.Global, nil
 	}
 	for _, contextCfg := range cfg.ContextConfigs {
-		if contextCfg.Kubectx == kubectx {
+		if contextCfg.Kubecontext == kubecontext {
 			return contextCfg, nil
 		}
 	}
-	return nil, fmt.Errorf("no config entry found for kubectx %s", kubectx)
+	return nil, fmt.Errorf("no config entry found for kube-context %s", kubecontext)
+}
+
+func getOrCreateConfigForKubectx() (*ContextConfig, error) {
+	cfg, err := readConfig()
+	if err != nil {
+		return nil, err
+	}
+	if global {
+		return cfg.Global, nil
+	}
+	for _, contextCfg := range cfg.ContextConfigs {
+		if contextCfg.Kubecontext == kubecontext {
+			return contextCfg, nil
+		}
+	}
+	newCfg := &ContextConfig{
+		Kubecontext: kubecontext,
+	}
+	cfg.ContextConfigs = append(cfg.ContextConfigs, newCfg)
+
+	if err := writeFullConfig(cfg); err != nil {
+		return nil, err
+	}
+
+	return newCfg, nil
 }

cmd/skaffold/app/cmd/config_test.go

@@ -31,7 +31,7 @@ var baseConfig = &config.Config{
 	},
 	ContextConfigs: []*config.ContextConfig{
 		{
-			Kubectx:     "test-context",
+			Kubecontext: "test-context",
 			DefaultRepo: "context-local-repository",
 		},
 	},
@@ -64,31 +64,3 @@ func TestReadConfig(t *testing.T) {
 		testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedCfg, cfg)
 	}
 }
-
-// func TestSetConfig(t *testing.T) {
-// 	c, _ := yaml.Marshal(*baseConfig)
-// 	cfg, teardown := testutil.TempFile(t, "config", c)
-// 	defer teardown()
-
-// 	var tests = []struct {
-
-// 		expectedCfg *config.Config
-// 		shouldErr   bool
-// 	}{
-// 		{
-// 			filename:  "",
-// 			shouldErr: true,
-// 		},
-// 		{
-// 			filename:    cfg,
-// 			expectedCfg: baseConfig,
-// 			shouldErr:   false,
-// 		},
-// 	}
-
-// 	for _, test := range tests {
-// 		cfg, err := config.ReadConfigForFile(test.filename)
-
-// 		testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedCfg, cfg)
-// 	}
-// }