From 6fbf98f1b95c230da310ef882e9afc345357d579 Mon Sep 17 00:00:00 2001
From: Anbraten <anton@ju60.de>
Date: Mon, 1 Jan 2024 11:03:31 +0100
Subject: [PATCH] Fix slice unmarshaling (#3097)

closes #3055
---
 pipeline/frontend/yaml/compiler/dag.go        |  2 +-
 pipeline/frontend/yaml/compiler/dag_test.go   | 19 ++++++++++
 pipeline/frontend/yaml/parse_test.go          | 35 +++++++++++++++++++
 .../yaml/types/base/base_types_test.go        | 19 ++++++----
 pipeline/frontend/yaml/types/base/slice.go    |  2 +-
 5 files changed, 69 insertions(+), 8 deletions(-)

diff --git a/pipeline/frontend/yaml/compiler/dag.go b/pipeline/frontend/yaml/compiler/dag.go
index 1ce6a9f73..45d4292e8 100644
--- a/pipeline/frontend/yaml/compiler/dag.go
+++ b/pipeline/frontend/yaml/compiler/dag.go
@@ -43,7 +43,7 @@ func newDAGCompiler(steps []*dagCompilerStep, prefix string) dagCompiler {
 
 func (c dagCompiler) isDAG() bool {
 	for _, v := range c.steps {
-		if len(v.dependsOn) != 0 {
+		if v.dependsOn != nil {
 			return true
 		}
 	}
diff --git a/pipeline/frontend/yaml/compiler/dag_test.go b/pipeline/frontend/yaml/compiler/dag_test.go
index 86131b1d0..ae774617c 100644
--- a/pipeline/frontend/yaml/compiler/dag_test.go
+++ b/pipeline/frontend/yaml/compiler/dag_test.go
@@ -149,3 +149,22 @@ func TestConvertDAGToStages(t *testing.T) {
 		}},
 	}}, stages)
 }
+
+func TestIsDag(t *testing.T) {
+	steps := []*dagCompilerStep{
+		{
+			step: &backend_types.Step{},
+		},
+	}
+	c := newDAGCompiler(steps, "")
+	assert.False(t, c.isDAG())
+
+	steps = []*dagCompilerStep{
+		{
+			step:      &backend_types.Step{},
+			dependsOn: []string{},
+		},
+	}
+	c = newDAGCompiler(steps, "")
+	assert.True(t, c.isDAG())
+}
diff --git a/pipeline/frontend/yaml/parse_test.go b/pipeline/frontend/yaml/parse_test.go
index bfecc9e16..ea679411b 100644
--- a/pipeline/frontend/yaml/parse_test.go
+++ b/pipeline/frontend/yaml/parse_test.go
@@ -249,3 +249,38 @@ steps:
     when:
       event: success
 `
+
+var sampleSliceYaml = `
+steps:
+  nil_slice:
+    image: plugins/slack
+  empty_slice:
+    image: plugins/slack
+    depends_on: []
+`
+
+func TestSlice(t *testing.T) {
+	g := goblin.Goblin(t)
+
+	g.Describe("Parser", func() {
+		g.It("should marshal a not set slice to nil", func() {
+			out, err := ParseString(sampleSliceYaml)
+			if err != nil {
+				g.Fail(err)
+			}
+
+			g.Assert(out.Steps.ContainerList[0].DependsOn).IsNil()
+			g.Assert(len(out.Steps.ContainerList[0].DependsOn)).Equal(0)
+		})
+
+		g.It("should marshal an empty slice", func() {
+			out, err := ParseString(sampleSliceYaml)
+			if err != nil {
+				g.Fail(err)
+			}
+
+			g.Assert(out.Steps.ContainerList[1].DependsOn).IsNotNil()
+			g.Assert(len(out.Steps.ContainerList[1].DependsOn)).Equal(0)
+		})
+	})
+}
diff --git a/pipeline/frontend/yaml/types/base/base_types_test.go b/pipeline/frontend/yaml/types/base/base_types_test.go
index 98accdfab..e21d68cda 100644
--- a/pipeline/frontend/yaml/types/base/base_types_test.go
+++ b/pipeline/frontend/yaml/types/base/base_types_test.go
@@ -49,20 +49,27 @@ type StructStringOrSlice struct {
 }
 
 func TestStringOrSliceYaml(t *testing.T) {
-	str := `{foo: [bar, baz]}`
-
+	str := `{foo: [bar, "baz"]}`
 	s := StructStringOrSlice{}
 	assert.NoError(t, yaml.Unmarshal([]byte(str), &s))
-
 	assert.Equal(t, StringOrSlice{"bar", "baz"}, s.Foo)
 
 	d, err := yaml.Marshal(&s)
 	assert.NoError(t, err)
 
-	s2 := StructStringOrSlice{}
-	assert.NoError(t, yaml.Unmarshal(d, &s2))
+	s = StructStringOrSlice{}
+	assert.NoError(t, yaml.Unmarshal(d, &s))
+	assert.Equal(t, StringOrSlice{"bar", "baz"}, s.Foo)
 
-	assert.Equal(t, StringOrSlice{"bar", "baz"}, s2.Foo)
+	str = `{foo: []}`
+	s = StructStringOrSlice{}
+	assert.NoError(t, yaml.Unmarshal([]byte(str), &s))
+	assert.Equal(t, StringOrSlice{}, s.Foo)
+
+	str = `{}`
+	s = StructStringOrSlice{}
+	assert.NoError(t, yaml.Unmarshal([]byte(str), &s))
+	assert.Nil(t, s.Foo)
 }
 
 type StructSliceorMap struct {
diff --git a/pipeline/frontend/yaml/types/base/slice.go b/pipeline/frontend/yaml/types/base/slice.go
index 7c964ace6..0d2685f55 100644
--- a/pipeline/frontend/yaml/types/base/slice.go
+++ b/pipeline/frontend/yaml/types/base/slice.go
@@ -45,7 +45,7 @@ func (s *StringOrSlice) UnmarshalYAML(unmarshal func(any) error) error {
 }
 
 func toStrings(s []any) ([]string, error) {
-	if len(s) == 0 {
+	if s == nil {
 		return nil, nil
 	}
 	r := make([]string, len(s))