Add check before creating task

Tasks with when expressions using variables that evaluated to false were
mistakenly executed because it missed an additional check to validate
that the task should be skipped after variable replacement.

Fixes tektoncd#3188

examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml

@@ -13,7 +13,7 @@ spec:
         This workspace will receive the cloned git repo and be passed
         to the next Task to create a file
   tasks:
-    - name: create-file
+    - name: create-file # when expression with parameter, evaluates to true
       when:
         - input: "$(params.path)"
           operator: in
@@ -56,7 +56,7 @@ spec:
               else
                 printf no | tee /tekton/results/exists
               fi
-    - name: echo-file-exists
+    - name: echo-file-exists # when expression with task result, evaluate to true
       when:
         - input: "$(tasks.check-file.results.exists)"
           operator: in
@@ -66,7 +66,17 @@ spec:
           - name: echo
             image: ubuntu
             script: 'echo file exists'
-    - name: task-should-be-skipped
+    - name: task-should-be-skipped-1 # when expression with task result, evaluate to false
+      when:
+        - input: "$(tasks.check-file.results.exists)"
+          operator: in
+          values: ["missing"]
+      taskSpec:
+        steps:
+          - name: echo
+            image: ubuntu
+            script: exit 1
+    - name: task-should-be-skipped-2 # when expression with parameter, evaluate to false 
       when:
         - input: "$(params.path)"
           operator: notin

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -522,7 +522,7 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip
 	resources.ApplyTaskResults(nextRprts, resolvedResultRefs)
 
 	for _, rprt := range nextRprts {
-		if rprt == nil {
+		if rprt == nil || rprt.Skip(pipelineState, d) {
 			continue
 		}
 		if rprt.ResolvedConditionChecks == nil || rprt.ResolvedConditionChecks.IsSuccess() {

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -2081,6 +2081,21 @@ func TestReconcileWithWhenExpressionsWithParameters(t *testing.T) {
 	if d := cmp.Diff(actualSkippedTasks, expectedSkippedTasks); d != "" {
 		t.Errorf("expected to find Skipped Tasks %v. Diff %s", expectedSkippedTasks, diff.PrintWantGot(d))
 	}
+
+	skippedTasks := []string{"hello-world-2"}
+	for _, skippedTask := range skippedTasks {
+		labelSelector := fmt.Sprintf("tekton.dev/pipelineTask=%s,tekton.dev/pipelineRun=test-pipeline-run-different-service-accs", skippedTask)
+		actualSkippedTask, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(metav1.ListOptions{
+			LabelSelector: labelSelector,
+			Limit:         1,
+		})
+		if err != nil {
+			t.Fatalf("Failure to list TaskRun's %s", err)
+		}
+		if len(actualSkippedTask.Items) != 0 {
+			t.Fatalf("Expected 0 TaskRuns got %d", len(actualSkippedTask.Items))
+		}
+	}
 }
 
 func TestReconcileWithWhenExpressionsWithTaskResults(t *testing.T) {
@@ -2189,6 +2204,21 @@ func TestReconcileWithWhenExpressionsWithTaskResults(t *testing.T) {
 	if d := cmp.Diff(actualSkippedTasks, expectedSkippedTasks); d != "" {
 		t.Errorf("expected to find Skipped Tasks %v. Diff %s", expectedSkippedTasks, diff.PrintWantGot(d))
 	}
+
+	skippedTasks := []string{"c-task", "d-task"}
+	for _, skippedTask := range skippedTasks {
+		labelSelector := fmt.Sprintf("tekton.dev/pipelineTask=%s,tekton.dev/pipelineRun=test-pipeline-run-different-service-accs", skippedTask)
+		actualSkippedTask, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(metav1.ListOptions{
+			LabelSelector: labelSelector,
+			Limit:         1,
+		})
+		if err != nil {
+			t.Fatalf("Failure to list TaskRun's %s", err)
+		}
+		if len(actualSkippedTask.Items) != 0 {
+			t.Fatalf("Expected 0 TaskRuns got %d", len(actualSkippedTask.Items))
+		}
+	}
 }
 
 // TestReconcileWithAffinityAssistantStatefulSet tests that given a pipelineRun with workspaces,