remove results from status when type mismatched and add more logs

This commit fixes #6023. Before this commit we only log the result name
and needed types, but omit the want types and the invalid results are in
status. This commit adds the want types in error message and remove
invalid results from status.

Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com

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
@@ -4894,15 +4886,15 @@ spec:
     results:
       - name: result1
     steps:
-    - script: echo foo >> $(results.result1.path)  
+    - script: echo foo >> $(results.result1.path)
       image: myimage
       name: mycontainer
 status:
   taskSpec:
     results:
       - name: result1
     steps:
-    - script: echo foo >> $(results.result1.path)  
+    - script: echo foo >> $(results.result1.path)
       image: myimage
       name: mycontainer
 `)
@@ -5084,6 +5076,9 @@ status:
     - name: aResult
       type: string
       value: aResultValue
+    - name: objectResult
+      type: string
+      value: objectResultValue
 `)
 
 	taskRunResultsObjectMissKey := parse.MustParseV1beta1TaskRun(t, `
@@ -5098,8 +5093,8 @@ status:
     - name: aResult
       type: array
       value:
-       - 1
-       - 2
+       - "1"
+       - "2"
     - name: objectResult
       type: object
       value:
@@ -5125,16 +5120,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)
@@ -5149,6 +5156,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)

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.