From 7df47850fc058d682bcc4f93744183364862af0a Mon Sep 17 00:00:00 2001
From: Khurram Baig <kbaig@redhat.com>
Date: Mon, 1 Jul 2024 22:31:36 +0530
Subject: [PATCH] Disable Matrix Validation for non Matrix Pipelines

This disables the Matrix Validation for Results in non Matrixed Pipelines.
This ensure we don't encounter any crash or such for these pipelines like
did at issue #8086
---
 pkg/apis/pipeline/v1/pipeline_validation.go | 13 ++++-
 test/e2e-tests-upgrade.sh                   | 16 ++++++
 test/upgrade/resultRefResources.yaml        | 64 +++++++++++++++++++++
 test/upgrade/resultRefRuns.yaml             | 14 +++++
 4 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 test/upgrade/resultRefResources.yaml
 create mode 100644 test/upgrade/resultRefRuns.yaml

diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go
index 43e56d102f5..09d3a7c6875 100644
--- a/pkg/apis/pipeline/v1/pipeline_validation.go
+++ b/pkg/apis/pipeline/v1/pipeline_validation.go
@@ -779,10 +779,16 @@ func validateGraph(tasks []PipelineTask) (errs *apis.FieldError) {
 }
 
 func validateMatrix(ctx context.Context, tasks []PipelineTask) (errs *apis.FieldError) {
+	hasMatrix := false
 	for idx, task := range tasks {
-		errs = errs.Also(task.validateMatrix(ctx).ViaIndex(idx))
+		if task.IsMatrixed() {
+			errs = errs.Also(task.validateMatrix(ctx).ViaIndex(idx))
+			hasMatrix = true
+		}
+	}
+	if hasMatrix {
+		errs = errs.Also(validateTaskResultsFromMatrixedPipelineTasksConsumed(tasks))
 	}
-	errs = errs.Also(validateTaskResultsFromMatrixedPipelineTasksConsumed(tasks))
 	return errs
 }
 
@@ -812,6 +818,9 @@ func validateMatrixedPipelineTaskConsumed(expressions []string, taskMapping map[
 	for _, expression := range expressions {
 		// ie. "tasks.<pipelineTaskName>.results.<resultName>[*]"
 		subExpressions := strings.Split(expression, ".")
+		if len(subExpressions) < 2 {
+			continue
+		}
 		pipelineTask := subExpressions[1] // pipelineTaskName
 		taskConsumed := taskMapping[pipelineTask]
 		if taskConsumed.IsMatrixed() {
diff --git a/test/e2e-tests-upgrade.sh b/test/e2e-tests-upgrade.sh
index d88b0e0ddc2..e96ffdbfec8 100755
--- a/test/e2e-tests-upgrade.sh
+++ b/test/e2e-tests-upgrade.sh
@@ -82,14 +82,30 @@ header "Create resources at previous release version"
 kubectl create namespace upgrade
 trap "kubectl delete namespace upgrade" EXIT
 kubectl create -f ./test/upgrade/simpleResources.yaml || fail_test
+kubectl create -f ./test/upgrade/resultRefResources.yaml || fail_test
 
 # Upgrade to the current release.
 header "Upgrade to the current release of Tekton pipeline"
 install_pipeline_crd
 
+initRestartCount=$(kubectl get pods -o=jsonpath='{.items[?(@.metadata.labels.app=="tekton-pipelines-controller")].status.containerStatuses[0].restartCount}' -n ${SYSTEM_NAMESPACE})
+
 # Create runs from the Task and Pipeline resources created at the previous release version
 header "Creating TaskRuns and PipelineRuns referencing existing Tasks and Pipelines"
 kubectl create -f ./test/upgrade/simpleRuns.yaml || fail_test
+kubectl create -f ./test/upgrade/resultRefRuns.yaml || fail_test
+
+for i in $(kc get pr -o=custom-columns=NAMENAME:.metadata.name --no-headers);
+      do kubectl wait --for=jsonpath='{.status.conditions[0].reason}'=Succeeded pipelinerun $i;
+done
+
+curRestartCount=$(kubectl get pods -o=jsonpath='{.items[?(@.metadata.labels.app=="tekton-pipelines-controller")].status.containerStatuses[0].restartCount}' -n ${SYSTEM_NAMESPACE})
+
+if [ ${initRestartCount} -ne ${curRestartCount} ];
+   echo "controller restarted"
+   then fail_test;
+fi
+
 
 # Run the post-integration tests. We do not need to install the resources again, since
 # they are installed before the upgrade. We verify if they still work, after going through
diff --git a/test/upgrade/resultRefResources.yaml b/test/upgrade/resultRefResources.yaml
new file mode 100644
index 00000000000..29bfed5bcbd
--- /dev/null
+++ b/test/upgrade/resultRefResources.yaml
@@ -0,0 +1,64 @@
+apiVersion: tekton.dev/v1
+kind: Task
+metadata:
+  name: add-task
+spec:
+  params:
+    - name: first
+      description: the first operand
+    - name: second
+      description: the second operand
+  results:
+    - name: sum
+      description: the sum of the first and second operand
+  steps:
+    - name: add
+      image: alpine
+      env:
+        - name: OP1
+          value: $(params.first)
+        - name: OP2
+          value: $(params.second)
+      command: ["/bin/sh", "-c"]
+      args:
+        - echo -n $((${OP1}+${OP2})) | tee $(results.sum.path);
+---
+apiVersion: tekton.dev/v1
+kind: Pipeline
+metadata:
+  name: sum-three-pipeline
+spec:
+  params:
+    - name: first
+      description: the first operand
+    - name: second
+      description: the second operand
+    - name: third
+      description: the third operand
+  tasks:
+    - name: first-add
+      taskRef:
+        name: add-task
+      params:
+        - name: first
+          value: $(params.first)
+        - name: second
+          value: $(params.second)
+    - name: second-add
+      taskRef:
+        name: add-task
+      params:
+        - name: first
+          value: $(tasks.first-add.results.sum)
+        - name: second
+          value: $(tasks.first-add.results.sum) - $(sum)
+  results:
+    - name: sum
+      description: the sum of all three operands
+      value: $(tasks.second-add.results.sum)
+    - name: partial-sum
+      description: the sum of first two operands
+      value: $(tasks.first-add.results.sum)
+    - name: all-sum
+      description: the sum of everything
+      value: $(tasks.second-add.results.sum)-$(tasks.first-add.results.sum)
diff --git a/test/upgrade/resultRefRuns.yaml b/test/upgrade/resultRefRuns.yaml
new file mode 100644
index 00000000000..149d4705e69
--- /dev/null
+++ b/test/upgrade/resultRefRuns.yaml
@@ -0,0 +1,14 @@
+apiVersion: tekton.dev/v1
+kind: PipelineRun
+metadata:
+  name: resultRefRun
+spec:
+  pipelineRef:
+    name: sum-three-pipeline
+  params:
+    - name: first
+      value: "2"
+    - name: second
+      value: "10"
+    - name: third
+      value: "10"