From 0e1b1a31c3f8d4be882e33171b3ce037cf1f5851 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Fri, 19 May 2023 14:41:02 -0400 Subject: [PATCH] Recover Conversion Functions from Pipeline Resources When we reverted removal of `PipelineResources` [related fields](https://github.com/tektoncd/pipeline/pull/6436), we did not recover the conversion functions. As a result, when migrating Tekton Chains to watch `v1` objects, we run into Issue https://github.com/tektoncd/pipeline/issues/7105. This PR recovers the conversion functions so that we can continue to convert PipelineResources related fields. --- .../pipeline/v1beta1/pipeline_conversion.go | 26 ++++++ .../v1beta1/pipeline_conversion_test.go | 68 ++++++++++++++++ .../v1beta1/pipelinerun_conversion.go | 26 ++++++ .../v1beta1/pipelinerun_conversion_test.go | 36 +++++++++ pkg/apis/pipeline/v1beta1/resource_types.go | 10 +++ pkg/apis/pipeline/v1beta1/task_conversion.go | 30 ++++++- .../pipeline/v1beta1/task_conversion_test.go | 79 +++++++++++++++++++ .../pipeline/v1beta1/taskrun_conversion.go | 25 ++++++ .../v1beta1/taskrun_conversion_test.go | 50 ++++++++++++ .../v1alpha1/pipeline_resource_types.go | 14 ++++ 10 files changed, 363 insertions(+), 1 deletion(-) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_conversion.go b/pkg/apis/pipeline/v1beta1/pipeline_conversion.go index d6b4c770ee2..c4a993fa869 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_conversion.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_conversion.go @@ -23,6 +23,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/pkg/apis/version" "knative.dev/pkg/apis" ) @@ -36,6 +37,9 @@ func (p *Pipeline) ConvertTo(ctx context.Context, to apis.Convertible) error { switch sink := to.(type) { case *v1.Pipeline: sink.ObjectMeta = p.ObjectMeta + if err := serializePipelineResources(&sink.ObjectMeta, &p.Spec); err != nil { + return err + } return p.Spec.ConvertTo(ctx, &sink.Spec, &sink.ObjectMeta) default: return fmt.Errorf("unknown version, got: %T", sink) @@ -90,6 +94,9 @@ func (p *Pipeline) ConvertFrom(ctx context.Context, from apis.Convertible) error switch source := from.(type) { case *v1.Pipeline: p.ObjectMeta = source.ObjectMeta + if err := deserializePipelineResources(&p.ObjectMeta, &p.Spec); err != nil { + return err + } return p.Spec.ConvertFrom(ctx, &source.Spec, &p.ObjectMeta) default: return fmt.Errorf("unknown version, got: %T", p) @@ -321,3 +328,22 @@ func (ptm *PipelineTaskMetadata) convertFrom(ctx context.Context, source v1.Pipe ptm.Labels = source.Labels ptm.Annotations = source.Labels } + +func serializePipelineResources(meta *metav1.ObjectMeta, spec *PipelineSpec) error { + if spec.Resources == nil { + return nil + } + return version.SerializeToMetadata(meta, spec.Resources, resourcesAnnotationKey) +} + +func deserializePipelineResources(meta *metav1.ObjectMeta, spec *PipelineSpec) error { + resources := &[]PipelineDeclaredResource{} + err := version.DeserializeFromMetadata(meta, resources, resourcesAnnotationKey) + if err != nil { + return err + } + if len(*resources) != 0 { + spec.Resources = *resources + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go b/pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go index 5d31b6e106f..7159723a961 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go @@ -244,3 +244,71 @@ func TestPipelineConversion(t *testing.T) { }) } } + +func TestPipelineConversionFromDeprecated(t *testing.T) { + tests := []struct { + name string + in *v1beta1.Pipeline + want *v1beta1.Pipeline + }{{ + name: "pipeline resources", + in: &v1beta1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: v1beta1.PipelineSpec{ + Resources: []v1beta1.PipelineDeclaredResource{ + { + Name: "1st pipeline resource", + Type: v1beta1.PipelineResourceTypeGit, + Optional: true, + }, { + Name: "2nd pipeline resource", + Type: v1beta1.PipelineResourceTypeGit, + Optional: false, + }, + }, + }}, + want: &v1beta1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: v1beta1.PipelineSpec{ + Resources: []v1beta1.PipelineDeclaredResource{ + { + Name: "1st pipeline resource", + Type: v1beta1.PipelineResourceTypeGit, + Optional: true, + }, { + Name: "2nd pipeline resource", + Type: v1beta1.PipelineResourceTypeGit, + Optional: false, + }, + }, + }, + }, + }} + + for _, test := range tests { + versions := []apis.Convertible{&v1.Pipeline{}} + for _, version := range versions { + t.Run(test.name, func(t *testing.T) { + ver := version + if err := test.in.ConvertTo(context.Background(), ver); err != nil { + t.Errorf("ConvertTo() = %v", err) + } + t.Logf("ConvertTo() = %#v", ver) + got := &v1beta1.Pipeline{} + if err := got.ConvertFrom(context.Background(), ver); err != nil { + t.Errorf("ConvertFrom() = %v", err) + } + t.Logf("ConvertFrom() = %#v", got) + if d := cmp.Diff(test.want, got); d != "" { + t.Errorf("roundtrip %s", diff.PrintWantGot(d)) + } + }) + } + } +} diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_conversion.go b/pkg/apis/pipeline/v1beta1/pipelinerun_conversion.go index 0b3f45274ca..d03c18a8eec 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_conversion.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_conversion.go @@ -23,6 +23,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/pkg/apis/version" "knative.dev/pkg/apis" ) @@ -36,6 +37,9 @@ func (pr *PipelineRun) ConvertTo(ctx context.Context, to apis.Convertible) error switch sink := to.(type) { case *v1.PipelineRun: sink.ObjectMeta = pr.ObjectMeta + if err := serializePipelineRunResources(&sink.ObjectMeta, &pr.Spec); err != nil { + return err + } if err := pr.Status.convertTo(ctx, &sink.Status, &sink.ObjectMeta); err != nil { return err } @@ -96,6 +100,9 @@ func (pr *PipelineRun) ConvertFrom(ctx context.Context, from apis.Convertible) e switch source := from.(type) { case *v1.PipelineRun: pr.ObjectMeta = source.ObjectMeta + if err := deserializePipelineRunResources(&pr.ObjectMeta, &pr.Spec); err != nil { + return err + } if err := pr.Status.convertFrom(ctx, &source.Status, &pr.ObjectMeta); err != nil { return err } @@ -345,3 +352,22 @@ func (csr *ChildStatusReference) convertFrom(ctx context.Context, source v1.Chil csr.WhenExpressions = append(csr.WhenExpressions, new) } } + +func serializePipelineRunResources(meta *metav1.ObjectMeta, spec *PipelineRunSpec) error { + if spec.Resources == nil { + return nil + } + return version.SerializeToMetadata(meta, spec.Resources, resourcesAnnotationKey) +} + +func deserializePipelineRunResources(meta *metav1.ObjectMeta, spec *PipelineRunSpec) error { + resources := []PipelineResourceBinding{} + err := version.DeserializeFromMetadata(meta, &resources, resourcesAnnotationKey) + if err != nil { + return err + } + if len(resources) != 0 { + spec.Resources = resources + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go index 0dc99e2b518..305337e646b 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go @@ -401,6 +401,42 @@ func TestPipelineRunConversionFromDeprecated(t *testing.T) { in *v1beta1.PipelineRun want *v1beta1.PipelineRun }{{ + name: "resources", + in: &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: v1beta1.PipelineRunSpec{ + Resources: []v1beta1.PipelineResourceBinding{ + { + Name: "git-resource", + ResourceRef: &v1beta1.PipelineResourceRef{Name: "sweet-resource"}, + }, { + Name: "image-resource", + ResourceRef: &v1beta1.PipelineResourceRef{Name: "sweet-resource2"}, + }, + }, + }, + }, + want: &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + }, + Spec: v1beta1.PipelineRunSpec{ + Resources: []v1beta1.PipelineResourceBinding{ + { + Name: "git-resource", + ResourceRef: &v1beta1.PipelineResourceRef{Name: "sweet-resource"}, + }, { + Name: "image-resource", + ResourceRef: &v1beta1.PipelineResourceRef{Name: "sweet-resource2"}, + }, + }, + }, + }, + }, { name: "timeout to timeouts", in: &v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/apis/pipeline/v1beta1/resource_types.go b/pkg/apis/pipeline/v1beta1/resource_types.go index 0e5ec62de3a..e230dc661cb 100644 --- a/pkg/apis/pipeline/v1beta1/resource_types.go +++ b/pkg/apis/pipeline/v1beta1/resource_types.go @@ -47,6 +47,16 @@ type ResourceParam = resource.ResourceParam // Deprecated: Unused, preserved only for backwards compatibility type PipelineResourceType = resource.PipelineResourceType +const ( + // PipelineResourceTypeGit indicates that this source is a GitHub repo. + // Deprecated: Unused, preserved only for backwards compatibility + PipelineResourceTypeGit PipelineResourceType = resource.PipelineResourceTypeGit + + // PipelineResourceTypeStorage indicates that this source is a storage blob resource. + // Deprecated: Unused, preserved only for backwards compatibility + PipelineResourceTypeStorage PipelineResourceType = resource.PipelineResourceTypeStorage +) + // PipelineDeclaredResource is used by a Pipeline to declare the types of the // PipelineResources that it will required to run and names which can be used to // refer to these PipelineResources in PipelineTaskResourceBindings. diff --git a/pkg/apis/pipeline/v1beta1/task_conversion.go b/pkg/apis/pipeline/v1beta1/task_conversion.go index 694e72dda26..0c3df76ed3b 100644 --- a/pkg/apis/pipeline/v1beta1/task_conversion.go +++ b/pkg/apis/pipeline/v1beta1/task_conversion.go @@ -53,7 +53,10 @@ import ( // "deprecatedSteps":[{"tty":true}], // }, // }` -const TaskDeprecationsAnnotationKey = "tekton.dev/v1beta1.task-deprecations" +const ( + TaskDeprecationsAnnotationKey = "tekton.dev/v1beta1.task-deprecations" + resourcesAnnotationKey = "tekton.dev/v1beta1Resources" +) var _ apis.Convertible = (*Task)(nil) @@ -65,6 +68,9 @@ func (t *Task) ConvertTo(ctx context.Context, to apis.Convertible) error { switch sink := to.(type) { case *v1.Task: sink.ObjectMeta = t.ObjectMeta + if err := serializeResources(&sink.ObjectMeta, &t.Spec); err != nil { + return err + } return t.Spec.ConvertTo(ctx, &sink.Spec, &sink.ObjectMeta, t.Name) default: return fmt.Errorf("unknown version, got: %T", sink) @@ -125,6 +131,9 @@ func (t *Task) ConvertFrom(ctx context.Context, from apis.Convertible) error { switch source := from.(type) { case *v1.Task: t.ObjectMeta = source.ObjectMeta + if err := deserializeResources(&t.ObjectMeta, &t.Spec); err != nil { + return err + } return t.Spec.ConvertFrom(ctx, &source.Spec, &t.ObjectMeta, t.Name) default: return fmt.Errorf("unknown version, got: %T", t) @@ -313,3 +322,22 @@ func retrieveTaskDeprecation(spec *TaskSpec) *taskDeprecation { DeprecatedStepTemplate: dst, } } + +func serializeResources(meta *metav1.ObjectMeta, spec *TaskSpec) error { + if spec.Resources == nil { + return nil + } + return version.SerializeToMetadata(meta, spec.Resources, resourcesAnnotationKey) +} + +func deserializeResources(meta *metav1.ObjectMeta, spec *TaskSpec) error { + resources := &TaskResources{} + err := version.DeserializeFromMetadata(meta, resources, resourcesAnnotationKey) + if err != nil { + return err + } + if resources.Inputs != nil || resources.Outputs != nil { + spec.Resources = resources + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/task_conversion_test.go b/pkg/apis/pipeline/v1beta1/task_conversion_test.go index 6195d1ab0a6..629ed816c7a 100644 --- a/pkg/apis/pipeline/v1beta1/task_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/task_conversion_test.go @@ -27,6 +27,7 @@ import ( "github.com/tektoncd/pipeline/test/diff" "github.com/tektoncd/pipeline/test/parse" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" ) func TestTaskConversionBadType(t *testing.T) { @@ -326,3 +327,81 @@ spec: }) } } + +func TestTaskConversionFromDeprecated(t *testing.T) { + tests := []struct { + name string + in *v1beta1.Task + want *v1beta1.Task + }{{ + name: "input resources", + in: &v1beta1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + Generation: 1, + }, + Spec: v1beta1.TaskSpec{ + Resources: &v1beta1.TaskResources{ + Inputs: []v1beta1.TaskResource{{v1beta1.ResourceDeclaration{Name: "input-resource"}}}, + }, + }, + }, + want: &v1beta1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + Generation: 1, + }, + Spec: v1beta1.TaskSpec{ + Resources: &v1beta1.TaskResources{ + Inputs: []v1beta1.TaskResource{{v1beta1.ResourceDeclaration{Name: "input-resource"}}}, + }, + }, + }, + }, { + name: "output resources", + in: &v1beta1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + Generation: 1, + }, + Spec: v1beta1.TaskSpec{ + Resources: &v1beta1.TaskResources{ + Outputs: []v1beta1.TaskResource{{v1beta1.ResourceDeclaration{Name: "output-resource"}}}, + }, + }, + }, + want: &v1beta1.Task{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "bar", + Generation: 1, + }, + Spec: v1beta1.TaskSpec{Resources: &v1beta1.TaskResources{ + Outputs: []v1beta1.TaskResource{{v1beta1.ResourceDeclaration{Name: "output-resource"}}}, + }}, + }, + }} + for _, test := range tests { + versions := []apis.Convertible{&v1.Task{}} + for _, version := range versions { + t.Run(test.name, func(t *testing.T) { + ver := version + if err := test.in.ConvertTo(context.Background(), ver); err != nil { + t.Errorf("ConvertTo() = %v", err) + } + t.Logf("ConvertTo() = %#v", ver) + got := &v1beta1.Task{} + if err := got.ConvertFrom(context.Background(), ver); err != nil { + t.Errorf("ConvertFrom() = %v", err) + } + t.Logf("ConvertFrom() = %#v", got) + if d := cmp.Diff(test.want, got); d != "" { + t.Errorf("roundtrip %s", diff.PrintWantGot(d)) + } + }) + } + } +} diff --git a/pkg/apis/pipeline/v1beta1/taskrun_conversion.go b/pkg/apis/pipeline/v1beta1/taskrun_conversion.go index d97da847a3a..59a33850f9b 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_conversion.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_conversion.go @@ -44,6 +44,9 @@ 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, &sink.ObjectMeta); err != nil { return err } @@ -115,6 +118,9 @@ 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, &tr.ObjectMeta); err != nil { return err } @@ -366,3 +372,22 @@ 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 index 4097bc48d28..eb530d34c5a 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go @@ -410,6 +410,56 @@ 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/resource/v1alpha1/pipeline_resource_types.go b/pkg/apis/resource/v1alpha1/pipeline_resource_types.go index 6cde87cb8a9..dffaf6db06c 100644 --- a/pkg/apis/resource/v1alpha1/pipeline_resource_types.go +++ b/pkg/apis/resource/v1alpha1/pipeline_resource_types.go @@ -26,6 +26,20 @@ import ( // Deprecated: Unused, preserved only for backwards compatibility type PipelineResourceType = string +const ( + // PipelineResourceTypeGit indicates that this source is a GitHub repo. + // Deprecated: Unused, preserved only for backwards compatibility + PipelineResourceTypeGit PipelineResourceType = "git" + + // PipelineResourceTypeStorage indicates that this source is a storage blob resource. + // Deprecated: Unused, preserved only for backwards compatibility + PipelineResourceTypeStorage PipelineResourceType = "storage" + + // PipelineResourceTypeGCS is the subtype for the GCSResources, which is backed by a GCS blob/directory. + // Deprecated: Unused, preserved only for backwards compatibility + PipelineResourceTypeGCS PipelineResourceType = "gcs" +) + // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +genclient:noStatus