Remove deprecated fields from PipelineResourceResult 🍠

The `Name` and `Digest` field have been deprecated in a previous release. This removes the fields from the struct. See tektoncd#1392 for the inital issue. It also reduce code duplication by aliasing some types between v1alpha1 and v1alpha2. Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
vdemeester · Feb 7, 2020 · 9668bb2 · 9668bb2
1 parent 259cf76
commit 9668bb2
Show file tree

Hide file tree

Showing 10 changed files with 33 additions and 77 deletions.
diff --git a/cmd/imagedigestexporter/main.go b/cmd/imagedigestexporter/main.go
@@ -62,12 +62,6 @@ func main() {
 		if err != nil {
 			logger.Fatalf("Unexpected error getting image digest for %s: %v", imageResource.Name, err)
 		}
-		// We need to write both the old Name/Digest style and the new Key/Value styles.
-		output = append(output, v1alpha1.PipelineResourceResult{
-			Name:   imageResource.Name,
-			Digest: digest.String(),
-		})
-
 		output = append(output, v1alpha1.PipelineResourceResult{
 			Key:   "digest",
 			Value: digest.String(),

diff --git a/docs/resources.md b/docs/resources.md
@@ -478,7 +478,7 @@ URLs should be of the form: https://github.com/tektoncd/pipeline/pull/1
 
 The PullRequest resource works with self hosted or enterprise GitHub/GitLab
 instances. Simply provide the pull request URL and set the `provider` parameter.
-If you need to skip certificate validation set the `insecure-skip-tls-verify` 
+If you need to skip certificate validation set the `insecure-skip-tls-verify`
 parameter to `"true"`.
 
 ```yaml
@@ -574,8 +574,10 @@ for example:
 status:
     ...
     resourcesResult:
-    - digest: sha256:eed29cd0b6feeb1a92bc3c4f977fd203c63b376a638731c88cacefe3adb1c660
-      name: skaffold-image-leeroy-web
+    - key: "digest"
+      value: "sha256:eed29cd0b6feeb1a92bc3c4f977fd203c63b376a638731c88cacefe3adb1c660"
+      resourceRef:
+        name: skaffold-image-leeroy-web
     ...
 ```
 

diff --git a/examples/taskruns/cloud-event.yaml b/examples/taskruns/cloud-event.yaml
@@ -140,8 +140,8 @@ spec:
         if response.status == 200:
           print("Got it!")
           taskrun = json.loads(response.read().decode('utf-8'))
-          digest = taskrun['taskRun']['status']['resourcesResult'][0]['digest']
-          image_name = taskrun['taskRun']['status']['resourcesResult'][0]['name']
+          digest = taskrun['taskRun']['status']['resourcesResult'][0]['value']
+          image_name = taskrun['taskRun']['status']['resourcesResult'][0]['resourceRef']['name']
           print("Got digest %s for image %s" % (digest, image_name))
           if image_name == "myimage" and digest:
             break

diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go
@@ -106,13 +106,7 @@ const (
 )
 
 // PipelineResourceRef can be used to refer to a specific instance of a Resource
-type PipelineResourceRef struct {
-	// Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names
-	Name string `json:"name,omitempty"`
-	// API version of the referent
-	// +optional
-	APIVersion string `json:"apiVersion,omitempty"`
-}
+type PipelineResourceRef = v1alpha2.PipelineResourceRef
 
 // PipelineRef can be used to refer to a specific instance of a Pipeline.
 // Copied from CrossVersionObjectReference: https://github.com/kubernetes/kubernetes/blob/169df7434155cbbc22f1532cba8e0a9588e29ad8/pkg/apis/autoscaling/types.go#L64

diff --git a/pkg/apis/pipeline/v1alpha1/resource_types.go b/pkg/apis/pipeline/v1alpha1/resource_types.go
@@ -115,19 +115,10 @@ type PipelineResourceBinding struct {
 }
 
 // PipelineResourceResult used to export the image name and digest as json
-type PipelineResourceResult struct {
-	// Name and Digest are deprecated.
-	Name   string `json:"name"`
-	Digest string `json:"digest"`
-	// These will replace Name and Digest (https://github.com/tektoncd/pipeline/issues/1392)
-	Key         string              `json:"key"`
-	Value       string              `json:"value"`
-	ResourceRef PipelineResourceRef `json:"resourceRef,omitempty"`
-	ResultType  ResultType          `json:"type,omitempty"`
-}
+type PipelineResourceResult = v1alpha2.PipelineResourceResult
 
 // ResultType used to find out whether a PipelineResourceResult is from a task result or not
-type ResultType string
+type ResultType = v1alpha2.ResultType
 
 // ResourceFromType returns an instance of the correct PipelineResource object type which can be
 // used to add input and output containers as well as volumes to a TaskRun's pod in order to realize

diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/apis/pipeline/v1alpha2/resource_types.go b/pkg/apis/pipeline/v1alpha2/resource_types.go
@@ -112,8 +112,12 @@ type PipelineResourceResult struct {
 	Key         string              `json:"key"`
 	Value       string              `json:"value"`
 	ResourceRef PipelineResourceRef `json:"resourceRef,omitempty"`
+	ResultType  ResultType          `json:"type,omitempty"`
 }
 
+// ResultType used to find out whether a PipelineResourceResult is from a task result or not
+type ResultType string
+
 // PipelineResourceRef can be used to refer to a specific instance of a Resource
 type PipelineResourceRef struct {
 	// Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names

diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go
@@ -1643,14 +1643,15 @@ func TestUpdateTaskRunResourceResult(t *testing.T) {
 			ContainerStatuses: []corev1.ContainerStatus{{
 				State: corev1.ContainerState{
 					Terminated: &corev1.ContainerStateTerminated{
-						Message: `[{"name":"source-image","digest":"sha256:1234"}]`,
+						Message: `[{"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
 					},
 				},
 			}},
 		},
 		want: []v1alpha1.PipelineResourceResult{{
-			Name:   "source-image",
-			Digest: "sha256:1234",
+			Key:         "digest",
+			Value:       "sha256:1234",
+			ResourceRef: v1alpha1.PipelineResourceRef{Name: "source-image"},
 		}},
 	}} {
 		t.Run(c.desc, func(t *testing.T) {
@@ -1683,7 +1684,7 @@ func TestUpdateTaskRunResult(t *testing.T) {
 			ContainerStatuses: []corev1.ContainerStatus{{
 				State: corev1.ContainerState{
 					Terminated: &corev1.ContainerStateTerminated{
-						Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"name":"source-image","digest":"sha256:1234", "type": "PipelineResourceResult"}]`,
+						Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}, "type": "PipelineResourceResult"}]`,
 					},
 				},
 			}},
@@ -1693,9 +1694,10 @@ func TestUpdateTaskRunResult(t *testing.T) {
 			Value: "resultValue",
 		}},
 		want: []v1alpha1.PipelineResourceResult{{
-			Name:       "source-image",
-			Digest:     "sha256:1234",
-			ResultType: "PipelineResourceResult",
+			Key:         "digest",
+			Value:       "sha256:1234",
+			ResourceRef: v1alpha1.PipelineResourceRef{Name: "source-image"},
+			ResultType:  "PipelineResourceResult",
 		}},
 	}} {
 		t.Run(c.desc, func(t *testing.T) {
@@ -1730,7 +1732,7 @@ func TestUpdateTaskRunResult2(t *testing.T) {
 			ContainerStatuses: []corev1.ContainerStatus{{
 				State: corev1.ContainerState{
 					Terminated: &corev1.ContainerStateTerminated{
-						Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"name":"source-image","digest":"sha256:1234"}]`,
+						Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
 					},
 				},
 			}},
@@ -1740,8 +1742,9 @@ func TestUpdateTaskRunResult2(t *testing.T) {
 			Value: "resultValue",
 		}},
 		want: []v1alpha1.PipelineResourceResult{{
-			Name:   "source-image",
-			Digest: "sha256:1234",
+			Key:         "digest",
+			Value:       "sha256:1234",
+			ResourceRef: v1alpha1.PipelineResourceRef{Name: "source-image"},
 		}},
 	}} {
 		t.Run(c.desc, func(t *testing.T) {

diff --git a/pkg/termination/parse_test.go b/pkg/termination/parse_test.go
@@ -28,9 +28,10 @@ func TestParseMessage(t *testing.T) {
 		want      []v1alpha1.PipelineResourceResult
 	}{{
 		desc: "valid message",
-		msg:  `[{"digest":"foo"},{"key":"foo","value":"bar"}]`,
+		msg:  `[{"key": "digest","value":"hereisthedigest"},{"key":"foo","value":"bar"}]`,
 		want: []v1alpha1.PipelineResourceResult{{
-			Digest: "foo",
+			Key:   "digest",
+			Value: "hereisthedigest",
 		}, {
 			Key:   "foo",
 			Value: "bar",

diff --git a/pkg/termination/write_test.go b/pkg/termination/write_test.go
@@ -59,7 +59,7 @@ func TestExistingFile(t *testing.T) {
 	if fileContents, err := ioutil.ReadFile(tmpFile.Name()); err != nil {
 		logger.Fatalf("Unexpected error reading %v: %v", tmpFile.Name(), err)
 	} else {
-		want := `[{"name":"","digest":"","key":"key1","value":"hello","resourceRef":{}},{"name":"","digest":"","key":"key2","value":"world","resourceRef":{}}]`
+		want := `[{"key":"key1","value":"hello","resourceRef":{}},{"key":"key2","value":"world","resourceRef":{}}]`
 		if d := cmp.Diff(want, string(fileContents)); d != "" {
 			t.Fatalf("Diff(-want, got): %s", d)
 		}