From 9668bb2225f2e22e3697203d642c3974ed544915 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Fri, 7 Feb 2020 08:30:00 +0100 Subject: [PATCH] =?UTF-8?q?Remove=20deprecated=20fields=20from=20PipelineR?= =?UTF-8?q?esourceResult=20=F0=9F=8D=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `Name` and `Digest` field have been deprecated in a previous release. This removes the fields from the struct. See https://github.com/tektoncd/pipeline/issues/1392 for the inital issue. It also reduce code duplication by aliasing some types between v1alpha1 and v1alpha2. Signed-off-by: Vincent Demeester --- cmd/imagedigestexporter/main.go | 6 --- docs/resources.md | 8 ++-- examples/taskruns/cloud-event.yaml | 4 +- .../pipeline/v1alpha1/pipelinerun_types.go | 8 +--- pkg/apis/pipeline/v1alpha1/resource_types.go | 13 +------ .../v1alpha1/zz_generated.deepcopy.go | 37 +------------------ pkg/apis/pipeline/v1alpha2/resource_types.go | 4 ++ pkg/reconciler/taskrun/taskrun_test.go | 23 +++++++----- pkg/termination/parse_test.go | 5 ++- pkg/termination/write_test.go | 2 +- 10 files changed, 33 insertions(+), 77 deletions(-) diff --git a/cmd/imagedigestexporter/main.go b/cmd/imagedigestexporter/main.go index 6e98dcfa2c3..c0fd7472f81 100644 --- 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 index 300a3cdcf23..05311ccadf2 100644 --- 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 index 36afb255d66..006c3a94ca7 100644 --- 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 index 2fa1e22c8f9..b10f8053e91 100644 --- 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 index f32704bf552..904c883a25f 100644 --- 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 index 99fbf8460c0..4065c6ae435 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -512,7 +512,7 @@ func (in *PipelineResourceBinding) DeepCopyInto(out *PipelineResourceBinding) { *out = *in if in.ResourceRef != nil { in, out := &in.ResourceRef, &out.ResourceRef - *out = new(PipelineResourceRef) + *out = new(v1alpha2.PipelineResourceRef) **out = **in } if in.ResourceSpec != nil { @@ -533,39 +533,6 @@ func (in *PipelineResourceBinding) DeepCopy() *PipelineResourceBinding { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PipelineResourceRef) DeepCopyInto(out *PipelineResourceRef) { - *out = *in - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PipelineResourceRef. -func (in *PipelineResourceRef) DeepCopy() *PipelineResourceRef { - if in == nil { - return nil - } - out := new(PipelineResourceRef) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PipelineResourceResult) DeepCopyInto(out *PipelineResourceResult) { - *out = *in - out.ResourceRef = in.ResourceRef - return -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PipelineResourceResult. -func (in *PipelineResourceResult) DeepCopy() *PipelineResourceResult { - if in == nil { - return nil - } - out := new(PipelineResourceResult) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PipelineRun) DeepCopyInto(out *PipelineRun) { *out = *in @@ -1295,7 +1262,7 @@ func (in *TaskRunStatusFields) DeepCopyInto(out *TaskRunStatusFields) { } if in.ResourcesResult != nil { in, out := &in.ResourcesResult, &out.ResourcesResult - *out = make([]PipelineResourceResult, len(*in)) + *out = make([]v1alpha2.PipelineResourceResult, len(*in)) copy(*out, *in) } if in.TaskRunResults != nil { diff --git a/pkg/apis/pipeline/v1alpha2/resource_types.go b/pkg/apis/pipeline/v1alpha2/resource_types.go index 3e5d48e38ae..b579a245169 100644 --- 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 index 4daf8bcfbba..a81bdcac8dc 100644 --- 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 index da95cc6682f..57169f21cbc 100644 --- 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 index 1ef34e5f1b5..c6165fc272e 100644 --- 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) }