Merge pull request #22086 from vespa-engine/mpolden/config-unset

Add config unset command

6 files changed, 107 insertions, 15 deletions

diff --git a/client/go/cmd/config.go b/client/go/cmd/config.go
index 7b198ff82f8..b3a0c8133c1 100644
--- a/client/go/cmd/config.go
+++ b/client/go/cmd/config.go
@@ -153,6 +153,42 @@ $ vespa config set --local wait 600
 	return cmd
 }
 
+func newConfigUnsetCmd(cli *CLI) *cobra.Command {
+	var localArg bool
+	cmd := &cobra.Command{
+		Use:   "unset option-name",
+		Short: "Unset a configuration option.",
+		Long: `Unset a configuration option.
+
+Unsetting a configuration option will reset it to its default value, which may be empty.
+`,
+		Example: `# Reset target to its default value
+$ vespa config unset target
+
+# Stop overriding application option in local config
+$ vespa config unset --local application
+`,
+		DisableAutoGenTag: true,
+		SilenceUsage:      true,
+		Args:              cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			config := cli.config
+			if localArg {
+				if _, err := cli.applicationPackageFrom(nil, false); err != nil {
+					return fmt.Errorf("failed to write local configuration: %w", err)
+				}
+				config = cli.config.local
+			}
+			if err := config.unset(args[0]); err != nil {
+				return err
+			}
+			return config.write()
+		},
+	}
+	cmd.Flags().BoolVarP(&localArg, "local", "l", false, "Unset option in local configuration, i.e. for the current application")
+	return cmd
+}
+
 func newConfigGetCmd(cli *CLI) *cobra.Command {
 	var localArg bool
 	cmd := &cobra.Command{
@@ -185,7 +221,7 @@ $ vespa config get --local
 					config.printOption(option)
 				}
 			} else {
-				config.printOption(args[0])
+				return config.printOption(args[0])
 			}
 			return nil
 		},
@@ -532,11 +568,35 @@ func (c *Config) set(option, value string) error {
 			c.config.Set(option, value)
 			return nil
 		}
+	case zoneFlag:
+		if _, err := vespa.ZoneFromString(value); err != nil {
+			return err
+		}
+		c.config.Set(option, value)
+		return nil
 	}
 	return fmt.Errorf("invalid option or value: %s = %s", option, value)
 }
 
-func (c *Config) printOption(option string) {
+func (c *Config) unset(option string) error {
+	if err := c.checkOption(option); err != nil {
+		return err
+	}
+	c.config.Del(option)
+	return nil
+}
+
+func (c *Config) checkOption(option string) error {
+	if _, ok := c.flags[option]; !ok {
+		return fmt.Errorf("invalid option: %s", option)
+	}
+	return nil
+}
+
+func (c *Config) printOption(option string) error {
+	if err := c.checkOption(option); err != nil {
+		return err
+	}
 	value, ok := c.get(option)
 	if !ok {
 		faintColor := color.New(color.FgWhite, color.Faint)
@@ -545,6 +605,7 @@ func (c *Config) printOption(option string) {
 		value = color.CyanString(value)
 	}
 	log.Printf("%s = %s", option, value)
+	return nil
 }
 
 func vespaCliHome(env map[string]string) (string, error) {
diff --git a/client/go/cmd/config_test.go b/client/go/cmd/config_test.go
index 673d90506f8..3a8766aae13 100644
--- a/client/go/cmd/config_test.go
+++ b/client/go/cmd/config_test.go
@@ -15,8 +15,10 @@ import (
 func TestConfig(t *testing.T) {
 	configHome := t.TempDir()
 	assertConfigCommandErr(t, configHome, "Error: invalid option or value: foo = bar\n", "config", "set", "foo", "bar")
-	assertConfigCommand(t, configHome, "foo = <unset>\n", "config", "get", "foo")
-	assertConfigCommand(t, configHome, "target = local\n", "config", "get", "target")
+	assertConfigCommandErr(t, configHome, "Error: invalid option: foo\n", "config", "get", "foo")
+
+	// target
+	assertConfigCommand(t, configHome, "target = local\n", "config", "get", "target") // default value
 	assertConfigCommand(t, configHome, "", "config", "set", "target", "hosted")
 	assertConfigCommand(t, configHome, "target = hosted\n", "config", "get", "target")
 	assertConfigCommand(t, configHome, "", "config", "set", "target", "cloud")
@@ -25,30 +27,48 @@ func TestConfig(t *testing.T) {
 	assertConfigCommand(t, configHome, "", "config", "set", "target", "https://127.0.0.1")
 	assertConfigCommand(t, configHome, "target = https://127.0.0.1\n", "config", "get", "target")
 
+	// application
 	assertConfigCommandErr(t, configHome, "Error: invalid application: \"foo\"\n", "config", "set", "application", "foo")
 	assertConfigCommand(t, configHome, "application = <unset>\n", "config", "get", "application")
 	assertConfigCommand(t, configHome, "", "config", "set", "application", "t1.a1.i1")
 	assertConfigCommand(t, configHome, "application = t1.a1.i1\n", "config", "get", "application")
+	assertConfigCommand(t, configHome, "", "config", "set", "application", "t1.a1")
+	assertConfigCommand(t, configHome, "application = t1.a1.default\n", "config", "get", "application")
 
-	assertConfigCommand(t, configHome, "", "config", "set", "wait", "60")
+	// instance
+	assertConfigCommand(t, configHome, "instance = <unset>\n", "config", "get", "instance")
+	assertConfigCommand(t, configHome, "", "config", "set", "instance", "i2")
+	assertConfigCommand(t, configHome, "instance = i2\n", "config", "get", "instance")
+
+	// wait
 	assertConfigCommandErr(t, configHome, "Error: wait option must be an integer >= 0, got \"foo\"\n", "config", "set", "wait", "foo")
+	assertConfigCommand(t, configHome, "", "config", "set", "wait", "60")
 	assertConfigCommand(t, configHome, "wait = 60\n", "config", "get", "wait")
 	assertConfigCommand(t, configHome, "wait = 30\n", "config", "get", "--wait", "30", "wait") // flag overrides global config
 
+	// color
+	assertConfigCommandErr(t, configHome, "Error: invalid option or value: color = foo\n", "config", "set", "color", "foo")
+	assertConfigCommand(t, configHome, "", "config", "set", "color", "never")
+	assertConfigCommand(t, configHome, "color = never\n", "config", "get", "color")
+	assertConfigCommand(t, configHome, "", "config", "unset", "color")
+	assertConfigCommand(t, configHome, "color = auto\n", "config", "get", "color")
+
+	// quiet
 	assertConfigCommand(t, configHome, "", "config", "set", "quiet", "true")
 	assertConfigCommand(t, configHome, "", "config", "set", "quiet", "false")
 
-	assertConfigCommand(t, configHome, "", "config", "set", "instance", "i2")
-	assertConfigCommand(t, configHome, "instance = i2\n", "config", "get", "instance")
-
-	assertConfigCommand(t, configHome, "", "config", "set", "application", "t1.a1")
-	assertConfigCommand(t, configHome, "application = t1.a1.default\n", "config", "get", "application")
+	// zone
+	assertConfigCommand(t, configHome, "", "config", "set", "zone", "dev.us-east-1")
+	assertConfigCommand(t, configHome, "zone = dev.us-east-1\n", "config", "get", "zone")
 
-	// Write empty value, which should be ignored. This is for compatibility with older config formats
+	// Write empty value to YAML config, which should be ignored. This is for compatibility with older config formats
 	configFile := filepath.Join(configHome, "config.yaml")
+	assertConfigCommand(t, configHome, "", "config", "unset", "zone")
 	data, err := os.ReadFile(configFile)
 	require.Nil(t, err)
-	config := string(data) + "zone: \"\"\n"
+	yamlConfig := string(data)
+	assert.NotContains(t, yamlConfig, "zone:")
+	config := yamlConfig + "zone: \"\"\n"
 	require.Nil(t, os.WriteFile(configFile, []byte(config), 0600))
 	assertConfigCommand(t, configHome, "zone = <unset>\n", "config", "get", "zone")
 }
diff --git a/client/go/cmd/root.go b/client/go/cmd/root.go
index f5e4a3d6016..e88398a7fde 100644
--- a/client/go/cmd/root.go
+++ b/client/go/cmd/root.go
@@ -229,6 +229,7 @@ func (c *CLI) configureCommands() {
 	rootCmd.AddCommand(newCloneCmd(c))              // clone
 	configCmd.AddCommand(newConfigGetCmd(c))        // config get
 	configCmd.AddCommand(newConfigSetCmd(c))        // config set
+	configCmd.AddCommand(newConfigUnsetCmd(c))      // config unset
 	rootCmd.AddCommand(configCmd)                   // config
 	rootCmd.AddCommand(newCurlCmd(c))               // curl
 	rootCmd.AddCommand(newDeployCmd(c))             // deploy
diff --git a/client/go/cmd/testutil_test.go b/client/go/cmd/testutil_test.go
index 26d0369a215..09c0478a45a 100644
--- a/client/go/cmd/testutil_test.go
+++ b/client/go/cmd/testutil_test.go
@@ -10,6 +10,7 @@ import (
 )
 
 func newTestCLI(t *testing.T, envVars ...string) (*CLI, *bytes.Buffer, *bytes.Buffer) {
+	t.Helper()
 	homeDir := filepath.Join(t.TempDir(), ".vespa")
 	cacheDir := filepath.Join(t.TempDir(), ".cache", "vespa")
 	env := []string{"VESPA_CLI_HOME=" + homeDir, "VESPA_CLI_CACHE_DIR=" + cacheDir}
diff --git a/client/go/config/config.go b/client/go/config/config.go
index 7e859d9908a..beffff6e257 100644
--- a/client/go/config/config.go
+++ b/client/go/config/config.go
@@ -46,6 +46,13 @@ func (c *Config) Set(key, value string) {
 	c.values[key] = value
 }
 
+// Del removes the value associated with key.
+func (c *Config) Del(key string) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	delete(c.values, key)
+}
+
 // Write writes config in YAML format to writer w.
 func (c *Config) Write(w io.Writer) error {
 	c.mu.RLock()
diff --git a/client/go/config/config_test.go b/client/go/config/config_test.go
index 8bc3835efae..1458771a5f5 100644
--- a/client/go/config/config_test.go
+++ b/client/go/config/config_test.go
@@ -15,12 +15,14 @@ func TestConfig(t *testing.T) {
 	config := New()
 	config.Set("key1", "value1")
 	config.Set("key2", "value2")
-	assert.Equal(t, []string{"key1", "key2"}, config.Keys())
+	config.Set("key3", "value3")
+	assert.Equal(t, []string{"key1", "key2", "key3"}, config.Keys())
 
-	v, ok := config.Get("key1")
+	v, ok := config.Get("key3")
 	assert.True(t, ok)
-	assert.Equal(t, "value1", v)
+	assert.Equal(t, "value3", v)
 
+	config.Del("key3")
 	_, ok = config.Get("key3")
 	assert.False(t, ok)