fix: pipeline stringArray flag usage

ENG-2735 Signed-off-by: Russell Centanni <[email protected]>
devspace-sh · Feb 16, 2024 · 9163728 · 9163728
1 parent 088997e
commit 9163728
Show file tree

Hide file tree

Showing 7 changed files with 231 additions and 12 deletions.
diff --git a/docs/pages/configuration/pipelines/README.mdx b/docs/pages/configuration/pipelines/README.mdx
@@ -326,8 +326,8 @@ pipelines:
       short: e
       type: stringArray
     run: |-
-      extraEnv=$(get_flag "env")    # Retrieve the value of the `env` flag and store it in a variable
-      echo ${extraEnv[1]}
+      extraEnv=($(get_flag "env"))    # Retrieve the value of the `env` flag and store it in an array variable
+      echo ${extraEnv[0]}             # Arrays are zero indexed
 
       TERMINAL_ENABLED=true
       if [ $(get_flag "logs") == "true" ]; then     # Test if --logs/-l flag is used or not

diff --git a/e2e/tests/pipelines/framework.go b/e2e/tests/pipelines/framework.go
@@ -4,5 +4,5 @@ import "github.com/onsi/ginkgo/v2"
 
 // DevSpaceDescribe annotates the test with the label.
 func DevSpaceDescribe(text string, body func()) bool {
-	return ginkgo.Describe("[pipelines] "+text, body)
+	return ginkgo.FDescribe("[pipelines] "+text, body)
 }
diff --git a/e2e/tests/pipelines/pipelines.go b/e2e/tests/pipelines/pipelines.go
@@ -75,6 +75,8 @@ var _ = DevSpaceDescribe("pipelines", func() {
 		framework.ExpectLocalFileContentsImmediately("other.txt", "test\n")
 		framework.ExpectLocalFileContentsImmediately("other2.txt", "false\n")
 		framework.ExpectLocalFileContentsImmediately("other3.txt", "true\n")
+		framework.ExpectLocalFileContentsImmediately("other4-0.txt", "one\n")
+		framework.ExpectLocalFileContentsImmediately("other4-1.txt", "two\n")
 		framework.ExpectLocalFileContentsImmediately("other-profile.txt", "profile1\n")
 		framework.ExpectLocalFileContentsImmediately("dep1-test.txt", "test\n")
 		framework.ExpectLocalFileContentsImmediately("dep1-test2.txt", "true\n")
@@ -85,6 +87,143 @@ var _ = DevSpaceDescribe("pipelines", func() {
 		framework.ExpectLocalFileContentsImmediately("dep1-other-profile.txt", "profile1\n")
 	})
 
+	ginkgo.It("should resolve pipeline override array flags", func() {
+		tempDir, err := framework.CopyToTempDir("tests/pipelines/testdata/flags")
+		framework.ExpectNoError(err)
+		defer framework.CleanupTempDir(initialDir, tempDir)
+
+		ns, err := kubeClient.CreateNamespace("pipelines")
+		framework.ExpectNoError(err)
+		defer framework.ExpectDeleteNamespace(kubeClient, ns)
+
+		rootCmd := cmd.NewRootCmd(f)
+		persistentFlags := rootCmd.PersistentFlags()
+		globalFlags := flags.SetGlobalFlags(persistentFlags)
+		globalFlags.NoWarn = true
+		globalFlags.Namespace = ns
+		globalFlags.Profiles = []string{"profile1"}
+
+		cmdCtx := values.WithCommandFlags(context.Background(), globalFlags.Flags)
+		cmdCtx = values.WithFlagsMap(cmdCtx, map[string]string{
+			"other":  "test",
+			"other2": "false",
+			"other3": "true",
+			"other4": "three four",
+		})
+
+		devCmd := &cmd.RunPipelineCmd{
+			GlobalFlags: globalFlags,
+			Pipeline:    "other",
+			Ctx:         cmdCtx,
+		}
+		err = devCmd.RunDefault(f)
+		framework.ExpectNoError(err)
+
+		framework.ExpectLocalFileContentsImmediately("other.txt", "test\n")
+		framework.ExpectLocalFileContentsImmediately("other2.txt", "false\n")
+		framework.ExpectLocalFileContentsImmediately("other3.txt", "true\n")
+		framework.ExpectLocalFileContentsImmediately("other-profile.txt", "profile1\n")
+		framework.ExpectLocalFileContentsImmediately("other4-0.txt", "three\n")
+		framework.ExpectLocalFileContentsImmediately("other4-1.txt", "four\n")
+	})
+
+	ginkgo.It("should resolve pipeline override with --set-flags", func() {
+		tempDir, err := framework.CopyToTempDir("tests/pipelines/testdata/flags")
+		framework.ExpectNoError(err)
+		defer framework.CleanupTempDir(initialDir, tempDir)
+
+		ns, err := kubeClient.CreateNamespace("pipelines")
+		framework.ExpectNoError(err)
+		defer framework.ExpectDeleteNamespace(kubeClient, ns)
+
+		rootCmd := cmd.NewRootCmd(f)
+		persistentFlags := rootCmd.PersistentFlags()
+		globalFlags := flags.SetGlobalFlags(persistentFlags)
+		globalFlags.NoWarn = true
+		globalFlags.Namespace = ns
+		globalFlags.Profiles = []string{"profile1"}
+
+		cmdCtx := values.WithCommandFlags(context.Background(), globalFlags.Flags)
+		cmdCtx = values.WithFlagsMap(cmdCtx, map[string]string{})
+
+		devCmd := &cmd.RunPipelineCmd{
+			GlobalFlags: globalFlags,
+			Pipeline:    "other-override",
+			Ctx:         cmdCtx,
+		}
+		err = devCmd.RunDefault(f)
+		framework.ExpectNoError(err)
+
+		framework.ExpectLocalFileContentsImmediately("other.txt", "test\n")
+		framework.ExpectLocalFileContentsImmediately("other2.txt", "true\n")
+		framework.ExpectLocalFileContentsImmediately("other3.txt", "true\n")
+		framework.ExpectLocalFileContentsImmediately("other-profile.txt", "profile1\n")
+		framework.ExpectLocalFileContentsImmediately("other4-0.txt", "five\n")
+		framework.ExpectLocalFileContentsImmediately("other4-1.txt", "six\n")
+	})
+
+	ginkgo.It("should resolve dependency pipeline flag defaults", func() {
+		tempDir, err := framework.CopyToTempDir("tests/pipelines/testdata/flags")
+		framework.ExpectNoError(err)
+		defer framework.CleanupTempDir(initialDir, tempDir)
+
+		ns, err := kubeClient.CreateNamespace("pipelines")
+		framework.ExpectNoError(err)
+		defer framework.ExpectDeleteNamespace(kubeClient, ns)
+
+		rootCmd := cmd.NewRootCmd(f)
+		persistentFlags := rootCmd.PersistentFlags()
+		globalFlags := flags.SetGlobalFlags(persistentFlags)
+		globalFlags.NoWarn = true
+		globalFlags.Namespace = ns
+		globalFlags.Profiles = []string{"profile1"}
+
+		cmdCtx := values.WithCommandFlags(context.Background(), globalFlags.Flags)
+		cmdCtx = values.WithFlagsMap(cmdCtx, map[string]string{})
+
+		devCmd := &cmd.RunPipelineCmd{
+			GlobalFlags: globalFlags,
+			Pipeline:    "arr-dep1",
+			Ctx:         cmdCtx,
+		}
+		err = devCmd.RunDefault(f)
+		framework.ExpectNoError(err)
+
+		framework.ExpectLocalFileContentsImmediately("arr-0.txt", "one")
+		framework.ExpectLocalFileContentsImmediately("arr-1.txt", "two")
+	})
+
+	ginkgo.It("should resolve dependency pipeline flag defaults", func() {
+		tempDir, err := framework.CopyToTempDir("tests/pipelines/testdata/flags")
+		framework.ExpectNoError(err)
+		defer framework.CleanupTempDir(initialDir, tempDir)
+
+		ns, err := kubeClient.CreateNamespace("pipelines")
+		framework.ExpectNoError(err)
+		defer framework.ExpectDeleteNamespace(kubeClient, ns)
+
+		rootCmd := cmd.NewRootCmd(f)
+		persistentFlags := rootCmd.PersistentFlags()
+		globalFlags := flags.SetGlobalFlags(persistentFlags)
+		globalFlags.NoWarn = true
+		globalFlags.Namespace = ns
+		globalFlags.Profiles = []string{"profile1"}
+
+		cmdCtx := values.WithCommandFlags(context.Background(), globalFlags.Flags)
+		cmdCtx = values.WithFlagsMap(cmdCtx, map[string]string{})
+
+		devCmd := &cmd.RunPipelineCmd{
+			GlobalFlags: globalFlags,
+			Pipeline:    "arr-dep1-override",
+			Ctx:         cmdCtx,
+		}
+		err = devCmd.RunDefault(f)
+		framework.ExpectNoError(err)
+
+		framework.ExpectLocalFileContentsImmediately("arr-0.txt", "three")
+		framework.ExpectLocalFileContentsImmediately("arr-1.txt", "")
+	})
+
 	ginkgo.It("should exec container", func() {
 		tempDir, err := framework.CopyToTempDir("tests/pipelines/testdata/exec_container")
 		framework.ExpectNoError(err)

diff --git a/e2e/tests/pipelines/testdata/flags/dep1.yaml b/e2e/tests/pipelines/testdata/flags/dep1.yaml
@@ -35,3 +35,15 @@ pipelines:
       echo $(get_flag test3) > dep1-test2.txt
       echo $(get_flag profile) > dep1-dev-profile.txt
       run_pipelines other --set-flag other2=false
+
+  array:
+    flags:
+      - name: arr
+        type: stringArray
+        default:
+          - one
+          - two
+    run: |-
+      arr=($(get_flag arr))
+      echo -n ${arr[0]} > arr-0.txt
+      echo -n ${arr[1]} > arr-1.txt
diff --git a/e2e/tests/pipelines/testdata/flags/devspace.yaml b/e2e/tests/pipelines/testdata/flags/devspace.yaml
@@ -15,6 +15,11 @@ pipelines:
         default: true
       - name: other3
         default: true
+      - name: other4
+        type: stringArray
+        default:
+          - one
+          - two
     run: |-
       if get_flag test; then
         exit 1
@@ -24,6 +29,22 @@ pipelines:
       echo $(get_flag other2) > other2.txt
       echo $(get_flag other3) > other3.txt
       echo $(get_flag profile) > other-profile.txt
+      
+      other4=($(get_flag other4))
+      echo ${other4[0]} > other4-0.txt
+      echo ${other4[1]} > other4-1.txt
+
+  other-override:
+    run: |-
+      run_pipelines other --set-flag other4=five --set-flag other4=six
+
+  arr-dep1:
+    run: |-
+      run_dependency_pipelines dep1 --pipeline array
+
+  arr-dep1-override:
+    run: |-
+      run_dependency_pipelines dep1 --pipeline array --set-flag arr=three
 
   dev:
     flags:

diff --git a/pkg/devspace/pipeline/flags.go b/pkg/devspace/pipeline/flags.go
@@ -2,6 +2,7 @@ package pipeline
 
 import (
 	"fmt"
+	"strconv"
 
 	"github.com/loft-sh/devspace/pkg/devspace/config/versions/latest"
 )
@@ -31,18 +32,42 @@ func GetDefaultValue(pipelineFlag latest.PipelineFlag) (interface{}, error) {
 	case latest.PipelineFlagTypeInteger:
 		val := 0
 		if pipelineFlag.Default != nil {
-			val, ok = pipelineFlag.Default.(int)
-			if !ok {
-				return nil, fmt.Errorf("default is not an integer")
+			switch pipelineFlag.Default.(type) {
+			case float64:
+				floatVal, ok := pipelineFlag.Default.(float64)
+				if !ok {
+					return nil, fmt.Errorf("default is not an integer")
+				}
+				return int(floatVal), nil
+			case int:
+				intVal, ok := pipelineFlag.Default.(int)
+				if !ok {
+					return nil, fmt.Errorf("default is not an integer")
+				}
+				return intVal, nil
+			case string:
+				strVal, ok := pipelineFlag.Default.(string)
+				if !ok {
+					return nil, fmt.Errorf("default is not an integer")
+				}
+				intVal, err := strconv.ParseInt(strVal, 10, 0)
+				if err != nil {
+					return nil, err
+				}
+				return int(intVal), nil
 			}
+			return nil, fmt.Errorf("default is not an integer")
 		}
 		return val, nil
 	case latest.PipelineFlagTypeStringArray:
 		val := []string{}
 		if pipelineFlag.Default != nil {
-			val, ok = pipelineFlag.Default.([]string)
-			if !ok {
-				return nil, fmt.Errorf("default is not a string array")
+			for _, anyVal := range pipelineFlag.Default.([]interface{}) {
+				strVal, ok := anyVal.(string)
+				if !ok {
+					return nil, fmt.Errorf("default is not a string array")
+				}
+				val = append(val, strVal)
 			}
 		}
 		return val, nil

diff --git a/pkg/devspace/pipeline/pipeline.go b/pkg/devspace/pipeline/pipeline.go
@@ -423,22 +423,44 @@ func (p *pipeline) startNewDependency(ctx devspacecontext.Context, dependency ty
 }
 
 func applyFlags(ctx devspacecontext.Context, pipeline *latest.Pipeline, setFlags []string) (devspacecontext.Context, error) {
-	newFlags := map[string]string{}
+	defaultFlags := map[string]string{}
 	for _, flag := range pipeline.Flags {
 		val, err := GetDefaultValue(flag)
 		if err != nil {
 			return nil, err
 		}
 
-		newFlags[flag.Name] = fmt.Sprintf("%v", val)
+		switch flag.Type {
+		case latest.PipelineFlagTypeStringArray:
+			defaultFlags[flag.Name] = fmt.Sprintf("%v", strings.Join(val.([]string), " "))
+		default:
+			defaultFlags[flag.Name] = fmt.Sprintf("%v", val)
+		}
 	}
+
+	newFlags := map[string]string{}
 	for _, v := range setFlags {
 		splitted := strings.Split(v, "=")
 		if len(splitted) <= 1 {
 			return nil, fmt.Errorf("error parsing flag %s: expected format flag=value", v)
 		}
 
-		newFlags[splitted[0]] = strings.Join(splitted[1:], "=")
+		flagName := splitted[0]
+		flagVal := strings.Join(splitted[1:], "=")
+		flagVals := strings.Join(strings.Split(flagVal, ","), " ")
+
+		if newFlags[flagName] != "" {
+			newFlags[flagName] = strings.Join([]string{newFlags[flagName], flagVals}, " ")
+		} else {
+			newFlags[flagName] = flagVals
+		}
+
+	}
+
+	for name, value := range defaultFlags {
+		if _, ok := newFlags[name]; !ok {
+			newFlags[name] = value
+		}
 	}
 
 	return ctx.WithContext(values.WithFlagsMap(ctx.Context(), newFlags)), nil