Tombstone ResourcesResult field with the removal of PipelineResources

The ResourcesResult field of TaskRun is tombstoned along with the migration of PipelineResources and is kept only for backward compatibility.
tektoncd · Apr 6, 2023 · 1d73a55 · 1d73a55
1 parent 351d201
commit 1d73a55
Show file tree

Hide file tree

Showing 10 changed files with 21 additions and 133 deletions.
diff --git a/docs/deprecations.md b/docs/deprecations.md
@@ -53,4 +53,6 @@ See [TEP-0074](https://github.com/tektoncd/community/blob/main/teps/0074-depreca
 
 - The artifacts bucket/pvc setup by the `pkg/artifacts` package related with Storage PipelineResources
 
-- The generic pipelineResources functions including inputs and outputs resources and the `from` type
+- The generic pipelineResources functions including inputs and outputs resources and the `from` type
+
+- [TaskRun.Status.ResourcesResult is deprecated and tombstoned #6301](https://github.com/tektoncd/pipeline/issues/6325)
diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md
@@ -13973,8 +13973,9 @@ All TaskRunStatus stored in RetriesStatus will have no date within the RetriesSt
 </td>
 <td>
 <em>(Optional)</em>
-<p>Results from Resources built during the TaskRun. currently includes
-the digest of build container images</p>
+<p>Results from Resources built during the TaskRun.
+This is tomb-stoned along with the removal of pipelineResources
+Deprecated: this field is not populated and is preserved only for backwards compatibility</p>
 </td>
 </tr>
 <tr>

diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go
diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go
@@ -97,11 +97,6 @@ var (
 					ImageID:       "image-id",
 					ContainerName: "sidecar-error",
 				}},
-				ResourcesResult: []v1beta1.RunResult{{
-					Key:          "digest",
-					Value:        "sha256:1234",
-					ResourceName: "source-image",
-				}},
 			},
 		},
 		WhenExpressions: v1beta1.WhenExpressions{{

diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json
@@ -2849,7 +2849,7 @@
           "$ref": "#/definitions/v1beta1.Provenance"
         },
         "resourcesResult": {
-          "description": "Results from Resources built during the TaskRun. currently includes the digest of build container images",
+          "description": "Results from Resources built during the TaskRun. This is tomb-stoned along with the removal of pipelineResources Deprecated: this field is not populated and is preserved only for backwards compatibility",
           "type": "array",
           "items": {
             "default": {},
@@ -2941,7 +2941,7 @@
           "$ref": "#/definitions/v1beta1.Provenance"
         },
         "resourcesResult": {
-          "description": "Results from Resources built during the TaskRun. currently includes the digest of build container images",
+          "description": "Results from Resources built during the TaskRun. This is tomb-stoned along with the removal of pipelineResources Deprecated: this field is not populated and is preserved only for backwards compatibility",
           "type": "array",
           "items": {
             "default": {},

diff --git a/pkg/apis/pipeline/v1beta1/taskrun_conversion.go b/pkg/apis/pipeline/v1beta1/taskrun_conversion.go
@@ -44,9 +44,6 @@ func (tr *TaskRun) ConvertTo(ctx context.Context, to apis.Convertible) error {
 		if err := serializeTaskRunCloudEvents(&sink.ObjectMeta, &tr.Status); err != nil {
 			return err
 		}
-		if err := serializeTaskRunResourcesResult(&sink.ObjectMeta, &tr.Status); err != nil {
-			return err
-		}
 		if err := tr.Status.ConvertTo(ctx, &sink.Status); err != nil {
 			return err
 		}
@@ -118,9 +115,6 @@ func (tr *TaskRun) ConvertFrom(ctx context.Context, from apis.Convertible) error
 		if err := deserializeTaskRunCloudEvents(&tr.ObjectMeta, &tr.Status); err != nil {
 			return err
 		}
-		if err := deserializeTaskRunResourcesResult(&tr.ObjectMeta, &tr.Status); err != nil {
-			return err
-		}
 		if err := tr.Status.ConvertFrom(ctx, source.Status); err != nil {
 			return err
 		}
@@ -372,22 +366,3 @@ func deserializeTaskRunCloudEvents(meta *metav1.ObjectMeta, status *TaskRunStatu
 	}
 	return nil
 }
-
-func serializeTaskRunResourcesResult(meta *metav1.ObjectMeta, status *TaskRunStatus) error {
-	if status.ResourcesResult == nil {
-		return nil
-	}
-	return version.SerializeToMetadata(meta, status.ResourcesResult, resourcesResultAnnotationKey)
-}
-
-func deserializeTaskRunResourcesResult(meta *metav1.ObjectMeta, status *TaskRunStatus) error {
-	resourcesResult := []RunResult{}
-	err := version.DeserializeFromMetadata(meta, &resourcesResult, resourcesResultAnnotationKey)
-	if err != nil {
-		return err
-	}
-	if len(resourcesResult) != 0 {
-		status.ResourcesResult = resourcesResult
-	}
-	return nil
-}
diff --git a/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go b/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go
@@ -377,56 +377,6 @@ func TestTaskRunConversionFromDeprecated(t *testing.T) {
 				},
 			},
 		},
-	}, {
-		name: "resourcesResult",
-		in: &v1beta1.TaskRun{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "foo",
-				Namespace: "bar",
-			},
-			Spec: v1beta1.TaskRunSpec{
-				TaskRef: &v1beta1.TaskRef{
-					Name: "test-resources-result",
-				},
-			},
-			Status: v1beta1.TaskRunStatus{
-				TaskRunStatusFields: v1beta1.TaskRunStatusFields{
-					ResourcesResult: []v1beta1.RunResult{{
-						Key:          "digest",
-						Value:        "sha256:1234",
-						ResourceName: "source-image",
-					}, {
-						Key:          "digest-11",
-						Value:        "sha256:1234",
-						ResourceName: "source-image",
-					}},
-				},
-			},
-		},
-		want: &v1beta1.TaskRun{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "foo",
-				Namespace: "bar",
-			},
-			Spec: v1beta1.TaskRunSpec{
-				TaskRef: &v1beta1.TaskRef{
-					Name: "test-resources-result",
-				},
-			},
-			Status: v1beta1.TaskRunStatus{
-				TaskRunStatusFields: v1beta1.TaskRunStatusFields{
-					ResourcesResult: []v1beta1.RunResult{{
-						Key:          "digest",
-						Value:        "sha256:1234",
-						ResourceName: "source-image",
-					}, {
-						Key:          "digest-11",
-						Value:        "sha256:1234",
-						ResourceName: "source-image",
-					}},
-				},
-			},
-		},
 	}}
 	for _, test := range tests {
 		versions := []apis.Convertible{&v1.TaskRun{}}

diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go
@@ -250,8 +250,9 @@ type TaskRunStatusFields struct {
 	// +listType=atomic
 	RetriesStatus []TaskRunStatus `json:"retriesStatus,omitempty"`
 
-	// Results from Resources built during the TaskRun. currently includes
-	// the digest of build container images
+	// Results from Resources built during the TaskRun.
+	// This is tomb-stoned along with the removal of pipelineResources
+	// Deprecated: this field is not populated and is preserved only for backwards compatibility
 	// +optional
 	// +listType=atomic
 	ResourcesResult []RunResult `json:"resourcesResult,omitempty"`

diff --git a/pkg/pod/status.go b/pkg/pod/status.go
@@ -181,10 +181,9 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
 		}
 
 		// populate task run CRD with results from sidecar logs
-		taskResults, RunResults, _ := filterResultsAndResources(sidecarLogResults, specResults)
+		taskResults, _ := filterResults(sidecarLogResults, specResults)
 		if tr.IsSuccessful() {
 			trs.TaskRunResults = append(trs.TaskRunResults, taskResults...)
-			trs.ResourcesResult = append(trs.ResourcesResult, RunResults...)
 		}
 	}
 	// Continue with extraction of termination messages
@@ -208,10 +207,9 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
 					merr = multierror.Append(merr, err)
 				}
 
-				taskResults, RunResults, filteredResults := filterResultsAndResources(results, specResults)
+				taskResults, filteredResults := filterResults(results, specResults)
 				if tr.IsSuccessful() {
 					trs.TaskRunResults = append(trs.TaskRunResults, taskResults...)
-					trs.ResourcesResult = append(trs.ResourcesResult, RunResults...)
 				}
 				msg, err = createMessageFromResults(filteredResults)
 				if err != nil {
@@ -261,9 +259,11 @@ func createMessageFromResults(results []v1beta1.RunResult) (string, error) {
 	return string(bytes), nil
 }
 
-func filterResultsAndResources(results []v1beta1.RunResult, specResults []v1beta1.TaskResult) ([]v1beta1.TaskRunResult, []v1beta1.RunResult, []v1beta1.RunResult) {
+// filterResults filters the RunResults and TaskResults based on the results declared in the task spec.
+// It returns a slice of any of the input results that are defined in the task spec, converted to TaskRunResults,
+// and a slice of any of the RunResults that don't represent internal values (i.e. those that should not be displayed in the TaskRun status.
+func filterResults(results []v1beta1.RunResult, specResults []v1beta1.TaskResult) ([]v1beta1.TaskRunResult, []v1beta1.RunResult) {
 	var taskResults []v1beta1.TaskRunResult
-	var RunResults []v1beta1.RunResult
 	var filteredResults []v1beta1.RunResult
 	neededTypes := make(map[string]v1beta1.ResultsType)
 	for _, r := range specResults {
@@ -297,12 +297,11 @@ func filterResultsAndResources(results []v1beta1.RunResult, specResults []v1beta
 			// Internal messages are ignored because they're not used as external result
 			continue
 		default:
-			RunResults = append(RunResults, r)
 			filteredResults = append(filteredResults, r)
 		}
 	}
 
-	return taskResults, RunResults, filteredResults
+	return taskResults, filteredResults
 }
 
 func removeDuplicateResults(taskRunResult []v1beta1.TaskRunResult) []v1beta1.TaskRunResult {

diff --git a/pkg/pod/status_test.go b/pkg/pod/status_test.go
@@ -647,7 +647,7 @@ func TestMakeTaskRunStatus(t *testing.T) {
 			},
 		},
 	}, {
-		desc: "image resource updated",
+		desc: "image resource that should not populate resourcesResult",
 		podStatus: corev1.PodStatus{
 			Phase: corev1.PodSucceeded,
 			ContainerStatuses: []corev1.ContainerStatus{{
@@ -671,11 +671,6 @@ func TestMakeTaskRunStatus(t *testing.T) {
 					ContainerName: "step-foo",
 				}},
 				Sidecars: []v1beta1.SidecarState{},
-				ResourcesResult: []v1beta1.RunResult{{
-					Key:          "digest",
-					Value:        "sha256:12345",
-					ResourceName: "source-image",
-				}},
 				// We don't actually care about the time, just that it's not nil
 				CompletionTime: &metav1.Time{Time: time.Now()},
 			},
@@ -705,11 +700,6 @@ func TestMakeTaskRunStatus(t *testing.T) {
 					ContainerName: "step-bar",
 				}},
 				Sidecars: []v1beta1.SidecarState{},
-				ResourcesResult: []v1beta1.RunResult{{
-					Key:          "digest",
-					Value:        "sha256:1234",
-					ResourceName: "source-image",
-				}},
 				TaskRunResults: []v1beta1.TaskRunResult{{
 					Name:  "resultName",
 					Type:  v1beta1.ResultsTypeString,
@@ -744,11 +734,6 @@ func TestMakeTaskRunStatus(t *testing.T) {
 					ContainerName: "step-banana",
 				}},
 				Sidecars: []v1beta1.SidecarState{},
-				ResourcesResult: []v1beta1.RunResult{{
-					Key:          "digest",
-					Value:        "sha256:1234",
-					ResourceName: "source-image",
-				}},
 				TaskRunResults: []v1beta1.TaskRunResult{{
 					Name:  "resultName",
 					Type:  v1beta1.ResultsTypeString,
@@ -1297,11 +1282,6 @@ func TestMakeTaskRunStatusAlpha(t *testing.T) {
 					ContainerName: "step-bar",
 				}},
 				Sidecars: []v1beta1.SidecarState{},
-				ResourcesResult: []v1beta1.RunResult{{
-					Key:          "digest",
-					Value:        "sha256:1234",
-					ResourceName: "source-image",
-				}},
 				TaskRunResults: []v1beta1.TaskRunResult{{
 					Name:  "resultName",
 					Type:  v1beta1.ResultsTypeString,
@@ -1344,11 +1324,6 @@ func TestMakeTaskRunStatusAlpha(t *testing.T) {
 					ContainerName: "step-bar",
 				}},
 				Sidecars: []v1beta1.SidecarState{},
-				ResourcesResult: []v1beta1.RunResult{{
-					Key:          "digest",
-					Value:        "sha256:1234",
-					ResourceName: "source-image",
-				}},
 				TaskRunResults: []v1beta1.TaskRunResult{{
 					Name:  "resultName",
 					Type:  v1beta1.ResultsTypeString,
@@ -1391,11 +1366,6 @@ func TestMakeTaskRunStatusAlpha(t *testing.T) {
 					ContainerName: "step-bar",
 				}},
 				Sidecars: []v1beta1.SidecarState{},
-				ResourcesResult: []v1beta1.RunResult{{
-					Key:          "digest",
-					Value:        "sha256:1234",
-					ResourceName: "source-image",
-				}},
 				TaskRunResults: []v1beta1.TaskRunResult{{
 					Name:  "resultName",
 					Type:  v1beta1.ResultsTypeArray,
@@ -1438,11 +1408,6 @@ func TestMakeTaskRunStatusAlpha(t *testing.T) {
 					ContainerName: "step-bar",
 				}},
 				Sidecars: []v1beta1.SidecarState{},
-				ResourcesResult: []v1beta1.RunResult{{
-					Key:          "digest",
-					Value:        "sha256:1234",
-					ResourceName: "source-image",
-				}},
 				TaskRunResults: []v1beta1.TaskRunResult{{
 					Name:  "resultName",
 					Type:  v1beta1.ResultsTypeObject,