From 81dcdea2bee6f4d2adbe2083eced7a6f4176616c Mon Sep 17 00:00:00 2001
From: Zav Shotan <3694482+LamaAni@users.noreply.github.com>
Date: Mon, 13 Jun 2022 17:13:09 -0400
Subject: [PATCH] Add support for steps to be a list (instead of dict) (#826)

- Support for pipeline/containers as list
- Support for container name in logs (step.Name)

Co-authored-by: Zav Shotan <zshotan@bloomberg.net>
Co-authored-by: 6543 <6543@obermui.de>
---
 pipeline/frontend/yaml/compiler/convert.go   |  9 +++-
 pipeline/frontend/yaml/container.go          | 45 ++++++++++++++------
 pipeline/frontend/yaml/linter/linter_test.go | 36 ++++++++++++----
 pipeline/schema/schema.json                  |  9 ++--
 4 files changed, 73 insertions(+), 26 deletions(-)

diff --git a/pipeline/frontend/yaml/compiler/convert.go b/pipeline/frontend/yaml/compiler/convert.go
index 3e1fb6c5b..52befda18 100644
--- a/pipeline/frontend/yaml/compiler/convert.go
+++ b/pipeline/frontend/yaml/compiler/convert.go
@@ -145,8 +145,15 @@ func (c *Compiler) createProcess(name string, container *yaml.Container, section
 		cpuSet = c.reslimit.CPUSet
 	}
 
+	stepName := container.Name
+	if len(stepName) == 0 {
+		stepName = name
+	} else {
+		stepName = section + "." + stepName
+	}
+
 	return &backend.Step{
-		Name:         name,
+		Name:         stepName,
 		Alias:        container.Name,
 		Image:        image,
 		Pull:         container.Pull,
diff --git a/pipeline/frontend/yaml/container.go b/pipeline/frontend/yaml/container.go
index f7c24f1a7..5e5287f69 100644
--- a/pipeline/frontend/yaml/container.go
+++ b/pipeline/frontend/yaml/container.go
@@ -65,26 +65,47 @@ type (
 
 // UnmarshalYAML implements the Unmarshaler interface.
 func (c *Containers) UnmarshalYAML(value *yaml.Node) error {
-	containers := map[string]Container{}
-	err := value.Decode(&containers)
-	if err != nil {
-		return err
-	}
+	switch value.Kind {
+	// We support mapps ...
+	case yaml.MappingNode:
+		c.Containers = make([]*Container, 0, len(value.Content)/2+1)
+		// We cannot use decode on specific values
+		// since if we try to load from a map, the order
+		// will not be kept. Therefore use value.Content
+		// and take the map values i%2=1
+		for i, n := range value.Content {
+			if i%2 == 1 {
+				container := &Container{}
+				if err := n.Decode(container); err != nil {
+					return err
+				}
 
-	for i, n := range value.Content {
-		if i%2 == 1 {
-			container := Container{}
-			err := n.Decode(&container)
-			if err != nil {
+				if container.Name == "" {
+					container.Name = fmt.Sprintf("%v", value.Content[i-1].Value)
+				}
+
+				c.Containers = append(c.Containers, container)
+			}
+		}
+
+	// ... and lists
+	case yaml.SequenceNode:
+		c.Containers = make([]*Container, 0, len(value.Content))
+		for i, n := range value.Content {
+			container := &Container{}
+			if err := n.Decode(container); err != nil {
 				return err
 			}
 
 			if container.Name == "" {
-				container.Name = fmt.Sprintf("%v", value.Content[i-1].Value)
+				container.Name = fmt.Sprintf("step-%d", i)
 			}
 
-			c.Containers = append(c.Containers, &container)
+			c.Containers = append(c.Containers, container)
 		}
+
+	default:
+		return fmt.Errorf("yaml node type[%d]: '%s' not supported", value.Kind, value.Tag)
 	}
 
 	return nil
diff --git a/pipeline/frontend/yaml/linter/linter_test.go b/pipeline/frontend/yaml/linter/linter_test.go
index e6c6a56a3..1fe247066 100644
--- a/pipeline/frontend/yaml/linter/linter_test.go
+++ b/pipeline/frontend/yaml/linter/linter_test.go
@@ -7,7 +7,7 @@ import (
 )
 
 func TestLint(t *testing.T) {
-	testdata := `
+	testdatas := []struct{ Title, Data string }{{Title: "map", Data: `
 pipeline:
   build:
     image: docker
@@ -28,14 +28,34 @@ services:
     image: redis
     entrypoint: [ /bin/redis-server ]
     command: [ -v ]
-`
+`}, {Title: "list", Data: `
+pipeline:
+  - name: build
+    image: docker
+    privileged: true
+    network_mode: host
+    volumes:
+      - /tmp:/tmp
+    commands:
+      - go build
+      - go test
+  - name: publish
+    image: plugins/docker
+    repo: foo/bar
+    settings:
+      foo: bar
+`}}
 
-	conf, err := yaml.ParseString(testdata)
-	if err != nil {
-		t.Fatalf("Cannot unmarshal yaml %q. Error: %s", testdata, err)
-	}
-	if err := New(WithTrusted(true)).Lint(conf); err != nil {
-		t.Errorf("Expected lint returns no errors, got %q", err)
+	for _, testd := range testdatas {
+		t.Run(testd.Title, func(t *testing.T) {
+			conf, err := yaml.ParseString(testd.Data)
+			if err != nil {
+				t.Fatalf("Cannot unmarshal yaml %q. Error: %s", testd, err)
+			}
+			if err := New(WithTrusted(true)).Lint(conf); err != nil {
+				t.Errorf("Expected lint returns no errors, got %q", err)
+			}
+		})
 	}
 }
 
diff --git a/pipeline/schema/schema.json b/pipeline/schema/schema.json
index dc980af4c..78f421358 100644
--- a/pipeline/schema/schema.json
+++ b/pipeline/schema/schema.json
@@ -90,11 +90,10 @@
     },
     "pipeline": {
       "description": "The pipeline section defines a list of steps which will be executed serially, in the order in which they are defined. Read more: https://woodpecker-ci.org/docs/usage/pipeline-syntax",
-      "type": "object",
-      "additionalProperties": {
-        "$ref": "#/definitions/step"
-      },
-      "minProperties": 1
+      "oneOf": [
+        { "type": "object", "additionalProperties": { "$ref": "#/definitions/step" }, "minProperties": 1 },
+        { "type": "array", "items": { "$ref": "#/definitions/step" }, "minLength": 1 }
+      ]
     },
     "step": {
       "description": "Every step of your pipeline executes arbitrary commands inside a specified docker container. Read more: https://woodpecker-ci.org/docs/usage/pipeline-syntax#steps",