From bd70199fbb404a93b39a1eea1e9ab311c7c9f87b Mon Sep 17 00:00:00 2001
From: Ammar Bandukwala <ammar@ammar.io>
Date: Tue, 28 Mar 2023 01:27:01 +0000
Subject: [PATCH] fix(clibase): allow empty values to unset defaults

---
 cli/clibase/cmd.go      | 10 +++++++++-
 cli/clibase/cmd_test.go | 32 ++++++++++++++++++++++++++++++++
 cli/clibase/option.go   |  3 +++
 cli/clibase/values.go   |  4 ++++
 4 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/cli/clibase/cmd.go b/cli/clibase/cmd.go
index 313a2afc9454a..49a3cae718a18 100644
--- a/cli/clibase/cmd.go
+++ b/cli/clibase/cmd.go
@@ -293,11 +293,19 @@ func (i *Invocation) run(state *runState) error {
 				return
 			}
 
+			// If flag was changed, we need to remove the default values.
 			sv, ok := f.Value.(pflag.SliceValue)
 			if !ok {
 				panic("defaulted array option is not a slice value")
 			}
-			err := sv.Replace(sv.GetSlice()[i:])
+			ss := sv.GetSlice()
+			if len(ss) == 0 {
+				// Slice likely zeroed by a flag.
+				// E.g. "--fruit" may default to "apples,oranges" but the user
+				// provided "--fruit=""".
+				return
+			}
+			err := sv.Replace(ss[i:])
 			if err != nil {
 				panic(err)
 			}
diff --git a/cli/clibase/cmd_test.go b/cli/clibase/cmd_test.go
index cc6ff5858c3e8..cf835327cf822 100644
--- a/cli/clibase/cmd_test.go
+++ b/cli/clibase/cmd_test.go
@@ -503,3 +503,35 @@ func TestCommand_SliceFlags(t *testing.T) {
 	err = cmd("bad", "bad", "bad").Invoke().Run()
 	require.NoError(t, err)
 }
+
+func TestCommand_EmptySlice(t *testing.T) {
+	t.Parallel()
+
+	cmd := func(want ...string) *clibase.Cmd {
+		var got []string
+		return &clibase.Cmd{
+			Use: "root",
+			Options: clibase.OptionSet{
+				{
+					Name:    "arr",
+					Flag:    "arr",
+					Default: "bad,bad,bad",
+					Env:     "ARR",
+					Value:   clibase.StringArrayOf(&got),
+				},
+			},
+			Handler: (func(i *clibase.Invocation) error {
+				require.Equal(t, want, got)
+				return nil
+			}),
+		}
+	}
+
+	// Base-case
+	err := cmd("bad", "bad", "bad").Invoke().Run()
+	require.NoError(t, err)
+
+	inv := cmd().Invoke("--arr", "")
+	err = inv.Run()
+	require.NoError(t, err)
+}
diff --git a/cli/clibase/option.go b/cli/clibase/option.go
index 836517979db6c..05b444c24803b 100644
--- a/cli/clibase/option.go
+++ b/cli/clibase/option.go
@@ -126,6 +126,9 @@ func (s *OptionSet) ParseEnv(vs []EnvVar) error {
 		// way for a user to change a Default value to an empty string from
 		// the environment. Unfortunately, we have old configuration files
 		// that rely on the faulty behavior.
+		//
+		// TODO: We should remove this hack in May 2023, when deployments
+		// have had months to migrate to the new behavior.
 		if !ok || envVal == "" {
 			continue
 		}
diff --git a/cli/clibase/values.go b/cli/clibase/values.go
index acb4cab5d50f7..0ddce1ba8f1a5 100644
--- a/cli/clibase/values.go
+++ b/cli/clibase/values.go
@@ -146,6 +146,10 @@ func writeAsCSV(vals []string) string {
 }
 
 func (s *StringArray) Set(v string) error {
+	if v == "" {
+		*s = nil
+		return nil
+	}
 	ss, err := readAsCSV(v)
 	if err != nil {
 		return err