Repository: tektoncd/pipeline (language: go)

Issue:
taskrunvalidationfailed - array results - results initialized with ambiguous failure message
# Expected Behavior

As a user, I expect the type of a result in the `taskRun` status matches with the type specified in the task definition. I do not expect to have the result set to `string` type by default. 

# Actual Behavior

I see two issues here when a result is defined as an array but it is initialized using a string value:

Issue 1: The taskRun results section lists the result of type string whereas it was originally defined as array in the task definition.

Issue 2: The error message in the status section of a taskRun is ambiguous. The error message needs to be a little more verbose and explicit. Also, the error message needs to list down the result names and their specified types. 

# Steps to Reproduce the Problem

1. Deploy Tekton Pipelines with the latest commit
2. Apply task, pipeline, and pipelineRun to the cluster.

```
$ kubectl create -f pipelinerun.yaml
pipeline.tekton.dev/mismatch-type-in-result created
task.tekton.dev/mismatch-type-task created
pipelinerun.tekton.dev/mismatch-type-in-result-pipelinerun-jq8mh created
```

```
apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
  name: mismatch-type-in-result
spec:
  tasks:
    - name: mismatch-type
      taskRef:
        name: mismatch-type-task
---

apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: mismatch-type-task
spec:
  results:
    - name: result1
      type: array
  steps:
    - name: write-invalid-result
      image: alpine:latest
      script: |
        #!/bin/sh
        echo -n "this is a string for a result with array type" | tee $(results.result1.path);
---

apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
  generateName: mismatch-type-in-result-pipelinerun-
spec:
  pipelineRef:
    name: mismatch-type-in-result
```

3. Check the taskRun status:

```
$ kubectl get tr
NAME                                                      SUCCEEDED   REASON                    STARTTIME   COMPLETIONTIME
mismatch-type-in-result-pipelinerun-jq8mh-mismatch-type   False       TaskRunValidationFailed   8s          3s
```

![Screen Shot 2023-01-23 at 6 44 18 PM](https://user-images.githubusercontent.com/206285/214203071-30c8866b-56f7-4187-89eb-987d404dd7f2.png)


# Additional Info

- Kubernetes version:

  **Output of `kubectl version`:**

```
Client Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.14", GitCommit:"3321ffc07d2f046afdf613796f9032f4460de093", GitTreeState:"clean", BuildDate:"2022-11-09T13:40:19Z", GoVersion:"go1.17.13", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.0", GitCommit:"ab69524f795c42094a6630298ff53f3c3ebab7f4", GitTreeState:"clean", BuildDate:"2021-12-07T18:09:57Z", GoVersion:"go1.17.3", Compiler:"gc", Platform:"linux/amd64"}

```

- Tekton Pipeline version:

  **Output of `tkn version` or `kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'`**

```
Client version: 0.23.1
Pipeline version: devel
```


<!-- Any other additional information -->



Discussion / hints:
/cc @Yongxuanzhang
Preparing for the promotion - #5688 


The same issue happens for object results.

```
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
  name: mismatch-type-task
spec:
  results:
    - name: result1
      type: array
    - name: result2
      type: object
      properties:
        hello:
          type: string
  steps:
    - name: write-invalid-result
      image: alpine:latest
      script: |
        #!/bin/sh
        echo -n "this is a string for a result with array type" | tee $(results.result1.path);
        echo -n "this is a string for a result with object type" | tee $(results.result2.path);
```

![Screen Shot 2023-01-23 at 7 32 08 PM](https://user-images.githubusercontent.com/206285/214206952-659e783d-90e0-47b2-9fd7-a795396cf2b4.png)

The message can be a little more descriptive and clear, for example:

```
Error: the result type does not match with the value its assigned to. The taskResult `result1` is defined as `array` but its assigned `string` value. The taskResult `result2` is defined as `object` but its assigned `string` value. 
```

/assign
Thank you! @pritidesai I'll take this

Fix patch:
diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go
index a9651ef52fb..31d4ecc2382 100644
--- a/pkg/reconciler/taskrun/validate_resources.go
+++ b/pkg/reconciler/taskrun/validate_resources.go
@@ -19,6 +19,8 @@ package taskrun
 import (
 	"context"
 	"fmt"
+	"sort"
+	"strings"
 
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
@@ -300,7 +302,12 @@ func validateTaskRunResults(tr *v1beta1.TaskRun, resolvedTaskSpec *v1beta1.TaskS
 
 	// When get the results, check if the type of result is the expected one
 	if missmatchedTypes := mismatchedTypesResults(tr, specResults); len(missmatchedTypes) != 0 {
-		return fmt.Errorf("missmatched Types for these results, %v", missmatchedTypes)
+		var s []string
+		for k, v := range missmatchedTypes {
+			s = append(s, fmt.Sprintf(" \"%v\": %v", k, v))
+		}
+		sort.Strings(s)
+		return fmt.Errorf("mismatched types: %v", strings.Join(s, ","))
 	}
 
 	// When get the results, for object value need to check if they have missing keys.
@@ -311,19 +318,28 @@ func validateTaskRunResults(tr *v1beta1.TaskRun, resolvedTaskSpec *v1beta1.TaskS
 }
 
 // mismatchedTypesResults checks and returns all the mismatched types of emitted results against specified results.
-func mismatchedTypesResults(tr *v1beta1.TaskRun, specResults []v1beta1.TaskResult) map[string][]string {
-	neededTypes := make(map[string][]string)
-	providedTypes := make(map[string][]string)
-	// collect needed keys for object results
+func mismatchedTypesResults(tr *v1beta1.TaskRun, specResults []v1beta1.TaskResult) map[string]string {
+	neededTypes := make(map[string]string)
+	mismatchedTypes := make(map[string]string)
+	var filteredResults []v1beta1.TaskRunResult
+	// collect needed types for results
 	for _, r := range specResults {
-		neededTypes[r.Name] = append(neededTypes[r.Name], string(r.Type))
+		neededTypes[r.Name] = string(r.Type)
 	}
 
-	// collect provided keys for object results
+	// collect mismatched types for results, and correct results in filteredResults
+	// TODO(#6097): Validate if the emitted results are defined in taskspec
 	for _, trr := range tr.Status.TaskRunResults {
-		providedTypes[trr.Name] = append(providedTypes[trr.Name], string(trr.Type))
+		needed, ok := neededTypes[trr.Name]
+		if ok && needed != string(trr.Type) {
+			mismatchedTypes[trr.Name] = fmt.Sprintf("task result is expected to be \"%v\" type but was initialized to a different type \"%v\"", needed, trr.Type)
+		} else {
+			filteredResults = append(filteredResults, trr)
+		}
 	}
-	return findMissingKeys(neededTypes, providedTypes)
+	// remove the mismatched results
+	tr.Status.TaskRunResults = filteredResults
+	return mismatchedTypes
 }
 
 // missingKeysofObjectResults checks and returns the missing keys of object results.


Test patch:
diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go
index fd51e281762..1faef279c9c 100644
--- a/pkg/reconciler/taskrun/taskrun_test.go
+++ b/pkg/reconciler/taskrun/taskrun_test.go
@@ -1877,31 +1877,23 @@ spec:
   taskRef:
     name: test-results-task
 status:
-  taskResults:
-  - name: aResult
-    type: string
-    value: aResultValue
   conditions:
   - reason: ToBeRetried
     status: Unknown
     type: Succeeded
-    message: "missmatched Types for these results, map[aResult:[array]]"
+    message: "mismatched types:  \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\""
   sideCars:
   retriesStatus:
   - conditions:
     - reason: TaskRunValidationFailed
       status: "False"
       type: "Succeeded"
-      message: "missmatched Types for these results, map[aResult:[array]]"
+      message: "mismatched types:  \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\""
     startTime: "2021-12-31T23:59:59Z"
     completionTime: "2022-01-01T00:00:00Z"
     podName: "test-taskrun-results-type-mismatched-pod"
-    taskResults:
-    - name: aResult
-      type: string
-      value: aResultValue
 `)
-		reconciliatonError = fmt.Errorf("1 error occurred:\n\t* missmatched Types for these results, map[aResult:[array]]")
+		reconciliatonError = fmt.Errorf("1 error occurred:\n\t* mismatched types:  \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"")
 		toBeRetriedTaskRun = parse.MustParseV1beta1TaskRun(t, `
 metadata:
   name: test-taskrun-to-be-retried
@@ -4892,7 +4884,7 @@ spec:
     results:
       - name: result1
     steps:
-    - script: echo foo >> $(results.result1.path)  
+    - script: echo foo >> $(results.result1.path)
       image: myimage
       name: mycontainer
 status:
@@ -4900,7 +4892,7 @@ status:
     results:
       - name: result1
     steps:
-    - script: echo foo >> $(results.result1.path)  
+    - script: echo foo >> $(results.result1.path)
       image: myimage
       name: mycontainer
 `)
@@ -5082,6 +5074,9 @@ status:
     - name: aResult
       type: string
       value: aResultValue
+    - name: objectResult
+      type: string
+      value: objectResultValue
 `)
 
 	taskRunResultsObjectMissKey := parse.MustParseV1beta1TaskRun(t, `
@@ -5096,8 +5091,8 @@ status:
     - name: aResult
       type: array
       value:
-       - 1
-       - 2
+       - "1"
+       - "2"
     - name: objectResult
       type: object
       value:
@@ -5123,16 +5118,28 @@ status:
 		taskRun          *v1beta1.TaskRun
 		wantFailedReason string
 		expectedError    error
+		expectedResults  []v1beta1.TaskRunResult
 	}{{
 		name:             "taskrun results type mismatched",
 		taskRun:          taskRunResultsTypeMismatched,
 		wantFailedReason: podconvert.ReasonFailedValidation,
-		expectedError:    fmt.Errorf("1 error occurred:\n\t* missmatched Types for these results, map[aResult:[array]]"),
+		expectedError:    fmt.Errorf("1 error occurred:\n\t* mismatched types:  \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\", \"objectResult\": task result is expected to be \"object\" type but was initialized to a different type \"string\""),
+		expectedResults:  nil,
 	}, {
 		name:             "taskrun results object miss key",
 		taskRun:          taskRunResultsObjectMissKey,
 		wantFailedReason: podconvert.ReasonFailedValidation,
 		expectedError:    fmt.Errorf("1 error occurred:\n\t* missing keys for these results which are required in TaskResult's properties map[objectResult:[commit]]"),
+		expectedResults: []v1beta1.TaskRunResult{
+			{
+				Name:  "aResult",
+				Type:  "array",
+				Value: *v1beta1.NewArrayOrString("1", "2"),
+			}, {
+				Name:  "objectResult",
+				Type:  "object",
+				Value: *v1beta1.NewObject(map[string]string{"url": "abc"}),
+			}},
 	}} {
 		t.Run(tc.name, func(t *testing.T) {
 			testAssets, cancel := getTaskRunController(t, d)
@@ -5147,6 +5154,9 @@ status:
 			if err != nil {
 				t.Fatalf("getting updated taskrun: %v", err)
 			}
+			if d := cmp.Diff(tr.Status.TaskRunResults, tc.expectedResults); d != "" {
+				t.Errorf("got unexpected results %s", diff.PrintWantGot(d))
+			}
 			condition := tr.Status.GetCondition(apis.ConditionSucceeded)
 			if condition.Type != apis.ConditionSucceeded || condition.Status != corev1.ConditionFalse || condition.Reason != tc.wantFailedReason {
 				t.Errorf("Expected TaskRun to fail with reason \"%s\" but it did not. Final conditions were:\n%#v", tc.wantFailedReason, tr.Status.Conditions)