Fix slice unmarshaling (woodpecker-ci#3097)

closes woodpecker-ci#3055
quintoandar · Feb 1, 2024 · 909dd0f · 909dd0f
1 parent d6d75ad
commit 909dd0f
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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))