From 7d020a93de4198db2804a5c90dd6189193b9a88b Mon Sep 17 00:00:00 2001
From: Michael Sauter <michael.sauter@boehringer-ingelheim.com>
Date: Thu, 2 Mar 2023 19:48:48 +0100
Subject: [PATCH] Set dependency on start task for all parallel tasks

Fixes #671
---
 CHANGELOG.md                      |  1 +
 internal/manager/pipeline.go      | 10 ++++-
 internal/manager/pipeline_test.go | 68 +++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4a4465c8..d8442929 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@ listed in the changelog.
 ### Fixed
 
 - `ods-start` is unable to cleanup workspace for some storage configurations due to changes in `ods-package-image` ([#672](https://github.com/opendevstack/ods-pipeline/issues/672))
+- `runAfter: [start]` not set for all parallel tasks ([#671](https://github.com/opendevstack/ods-pipeline/issues/671))
 
 ## [0.10.0] - 2023-02-27
 
diff --git a/internal/manager/pipeline.go b/internal/manager/pipeline.go
index 5114634f..b469ca5c 100644
--- a/internal/manager/pipeline.go
+++ b/internal/manager/pipeline.go
@@ -143,8 +143,16 @@ func assemblePipelineSpec(cfg PipelineConfig, taskKind tekton.TaskKind, taskSuff
 			tektonStringParam("version", "$(params.version)"),
 		},
 	})
+
 	if len(cfg.Tasks) > 0 {
-		cfg.Tasks[0].RunAfter = append(cfg.Tasks[0].RunAfter, "start")
+		// Add "start" to runAfter of the first configured task, and to each further task
+		// that does not set runAfter until we hit a task that does.
+		for i := range cfg.Tasks {
+			if i > 0 && len(cfg.Tasks[i].RunAfter) > 0 {
+				break
+			}
+			cfg.Tasks[i].RunAfter = append(cfg.Tasks[i].RunAfter, "start")
+		}
 		tasks = append(tasks, cfg.Tasks...)
 	}
 
diff --git a/internal/manager/pipeline_test.go b/internal/manager/pipeline_test.go
index 366141e2..f74258bf 100644
--- a/internal/manager/pipeline_test.go
+++ b/internal/manager/pipeline_test.go
@@ -209,3 +209,71 @@ func TestAssemblePipeline(t *testing.T) {
 		t.Fatalf("expected (-want +got):\n%s", diff)
 	}
 }
+
+func TestTasksRunAfterInjection(t *testing.T) {
+	tests := map[string]struct {
+		cfgTasks []tekton.PipelineTask
+		want     []tekton.PipelineTask
+	}{
+		"one build task": {
+			cfgTasks: []tekton.PipelineTask{
+				{Name: "build"},
+				{Name: "package-image", RunAfter: []string{"build"}},
+			},
+			want: []tekton.PipelineTask{
+				{Name: "start"},
+				{Name: "build", RunAfter: []string{"start"}},
+				{Name: "package-image", RunAfter: []string{"build"}},
+			},
+		},
+		"parallel build tasks": {
+			cfgTasks: []tekton.PipelineTask{
+				{Name: "build-one"},
+				{Name: "build-two"},
+				{Name: "package-image", RunAfter: []string{"build-one", "build-two"}},
+			},
+			want: []tekton.PipelineTask{
+				{Name: "start"},
+				{Name: "build-one", RunAfter: []string{"start"}},
+				{Name: "build-two", RunAfter: []string{"start"}},
+				{Name: "package-image", RunAfter: []string{"build-one", "build-two"}},
+			},
+		},
+		"no configured tasks": {
+			cfgTasks: []tekton.PipelineTask{},
+			want: []tekton.PipelineTask{
+				{Name: "start"},
+			},
+		},
+		"only parallel tasks": {
+			cfgTasks: []tekton.PipelineTask{
+				{Name: "build-one"},
+				{Name: "build-two"},
+			},
+			want: []tekton.PipelineTask{
+				{Name: "start"},
+				{Name: "build-one", RunAfter: []string{"start"}},
+				{Name: "build-two", RunAfter: []string{"start"}},
+			},
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			cfg := PipelineConfig{Tasks: tc.cfgTasks}
+			got := assemblePipelineSpec(cfg, tekton.NamespacedTaskKind, "")
+			wantRunAfter := [][]string{}
+			for _, task := range tc.want {
+				wantRunAfter = append(wantRunAfter, task.RunAfter)
+			}
+			gotRunAfter := [][]string{}
+			for _, task := range got.Tasks {
+				gotRunAfter = append(gotRunAfter, task.RunAfter)
+			}
+			if diff := cmp.Diff(wantRunAfter, gotRunAfter); diff != "" {
+				t.Fatalf("expected (-want +got):\n%s", diff)
+			}
+		})
+	}
+
+}