Recover Conversion Functions from Pipeline Resources

When we reverted removal of `PipelineResources` [related fields](#6436), we did not recover the conversion functions. As a result, when migrating Tekton Chains to watch `v1` objects, we run into Issue #7105. This PR recovers the conversion functions so that we can continue to convert PipelineResources related fields.
tektoncd · Sep 25, 2023 · 0e1b1a3 · 0e1b1a3
1 parent 6fdc3c0
commit 0e1b1a3
Show file tree

Hide file tree

Showing 10 changed files with 363 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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))
+				}
+			})
+		}
+	}
+}