Losslessly roundtrip Pipelines with Finally from beta to alpha and back

pkg/apis/pipeline/v1alpha1/conversion_error.go

@@ -34,6 +34,3 @@ const (
 type CannotConvertError = v1beta1.CannotConvertError
 
 var _ error = (*CannotConvertError)(nil)
-
-// convertErrorf creates a CannotConvertError from the field name and format string.
-var convertErrorf = v1beta1.ConvertErrorf

pkg/apis/pipeline/v1alpha1/pipeline_conversion.go

@@ -19,13 +19,15 @@ package v1alpha1
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"knative.dev/pkg/apis"
 )
 
 const FinallyFieldName = "finally"
+const finallyAnnotationKey = "tekton.dev/v1beta1Finally"
 
 var _ apis.Convertible = (*Pipeline)(nil)
 
@@ -34,10 +36,29 @@ func (source *Pipeline) ConvertTo(ctx context.Context, obj apis.Convertible) err
 	switch sink := obj.(type) {
 	case *v1beta1.Pipeline:
 		sink.ObjectMeta = source.ObjectMeta
-		return source.Spec.ConvertTo(ctx, &sink.Spec)
+		if err := source.Spec.ConvertTo(ctx, &sink.Spec); err != nil {
+			return err
+		}
+		if source.Annotations != nil {
+			if _, ok := source.Annotations[finallyAnnotationKey]; ok {
+				finally := []v1beta1.PipelineTask{}
+				if err := json.Unmarshal([]byte(source.Annotations[finallyAnnotationKey]), &finally); err != nil {
+					return fmt.Errorf("error converting finally annotation into beta field: %w", err)
+				}
+				if err := v1beta1.ValidatePipelineTasks(ctx, sink.Spec.Tasks, finally); err != nil {
+					return fmt.Errorf("error converting finally annotation into beta field: %w", err)
+				}
+				delete(sink.ObjectMeta.Annotations, finallyAnnotationKey)
+				if len(sink.ObjectMeta.Annotations) == 0 {
+					sink.ObjectMeta.Annotations = nil
+				}
+				sink.Spec.Finally = finally
+			}
+		}
 	default:
 		return fmt.Errorf("unknown version, got: %T", sink)
 	}
+	return nil
 }
 
 func (source *PipelineSpec) ConvertTo(ctx context.Context, sink *v1beta1.PipelineSpec) error {
@@ -81,6 +102,16 @@ func (sink *Pipeline) ConvertFrom(ctx context.Context, obj apis.Convertible) err
 	switch source := obj.(type) {
 	case *v1beta1.Pipeline:
 		sink.ObjectMeta = source.ObjectMeta
+		if len(source.Spec.Finally) != 0 {
+			b, err := json.Marshal(source.Spec.Finally)
+			if err != nil {
+				return err
+			}
+			if sink.ObjectMeta.Annotations == nil {
+				sink.ObjectMeta.Annotations = make(map[string]string)
+			}
+			sink.ObjectMeta.Annotations[finallyAnnotationKey] = string(b)
+		}
 		return sink.Spec.ConvertFrom(ctx, source.Spec)
 	default:
 		return fmt.Errorf("unknown version, got: %T", sink)
@@ -100,10 +131,6 @@ func (sink *PipelineSpec) ConvertFrom(ctx context.Context, source v1beta1.Pipeli
 			}
 		}
 	}
-	// finally clause was introduced in v1beta1 and not available in v1alpha1
-	if len(source.Finally) > 0 {
-		return convertErrorf(FinallyFieldName, ConversionErrorFieldNotAvailableMsg)
-	}
 	return nil
 }
 

pkg/apis/pipeline/v1alpha1/pipeline_conversion_test.go

@@ -215,7 +215,6 @@ func TestPipelineConversionFromWithFinally(t *testing.T) {
 			if err := p.ConvertTo(context.Background(), ver); err != nil {
 				t.Errorf("ConvertTo() = %v", err)
 			}
-			// modify ver to introduce new field which causes failure to convert v1beta1 to v1alpha1
 			source := ver
 			source.(*v1beta1.Pipeline).Spec.Finally = []v1beta1.PipelineTask{{Name: "finaltask", TaskRef: &TaskRef{Name: "task"}}}
 			got := &Pipeline{}
@@ -231,7 +230,7 @@ func TestPipelineConversionFromWithFinally(t *testing.T) {
 	}
 }
 
-func TestPipelineConversionFromBetaToAlphaWithFinally_Failure(t *testing.T) {
+func TestPipelineConversionFromBetaToAlphaWithFinally(t *testing.T) {
 	p := &v1beta1.Pipeline{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:       "foo",
@@ -240,18 +239,20 @@ func TestPipelineConversionFromBetaToAlphaWithFinally_Failure(t *testing.T) {
 		},
 		Spec: v1beta1.PipelineSpec{
 			Tasks:   []v1beta1.PipelineTask{{Name: "mytask", TaskRef: &TaskRef{Name: "task"}}},
-			Finally: []v1beta1.PipelineTask{{Name: "mytask", TaskRef: &TaskRef{Name: "task"}}},
+			Finally: []v1beta1.PipelineTask{{Name: "myfinallytask", TaskRef: &TaskRef{Name: "task"}}},
 		},
 	}
-	t.Run("finally not available in v1alpha1", func(t *testing.T) {
-		got := &Pipeline{}
-		if err := got.ConvertFrom(context.Background(), p); err != nil {
-			cce, ok := err.(*CannotConvertError)
-			// conversion error (cce) contains the field name which resulted in the failure and should be equal to "finally" here
-			if ok && cce.Field == FinallyFieldName {
-				return
-			}
-			t.Errorf("ConvertFrom() should have failed")
+	t.Run("finally stored by v1alpha1 and rehydrated for v1beta1", func(t *testing.T) {
+		downgrade := &Pipeline{}
+		if err := downgrade.ConvertFrom(context.Background(), p); err != nil {
+			t.Errorf("error converting from v1beta1 with finally field to v1alpha1 with finally annotation: %v", err)
+		}
+		upgrade := &v1beta1.Pipeline{}
+		if err := downgrade.ConvertTo(context.Background(), upgrade); err != nil {
+			t.Errorf("error converting from v1alpha1 with finally annotation to v1beta1 with finally field: %v", err)
+		}
+		if d := cmp.Diff(p, upgrade); d != "" {
+			t.Errorf("unexpected difference between v1beta1 with finally field and round-tripped v1beta1 with finally field: %s", diff.PrintWantGot(d))
 		}
 	})
 }

pkg/apis/pipeline/v1beta1/pipeline_validation.go

@@ -49,7 +49,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 		errs = errs.Also(apis.ErrGeneric("expected at least one, got none", "description", "params", "resources", "tasks", "workspaces"))
 	}
 	// PipelineTask must have a valid unique label and at least one of taskRef or taskSpec should be specified
-	errs = errs.Also(validatePipelineTasks(ctx, ps.Tasks, ps.Finally))
+	errs = errs.Also(ValidatePipelineTasks(ctx, ps.Tasks, ps.Finally))
 	// All declared resources should be used, and the Pipeline shouldn't try to use any resources
 	// that aren't declared
 	errs = errs.Also(validateDeclaredResources(ps.Resources, ps.Tasks, ps.Finally))
@@ -76,7 +76,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 
 // validatePipelineTasks ensures that pipeline tasks has unique label, pipeline tasks has specified one of
 // taskRef or taskSpec, and in case of a pipeline task with taskRef, it has a reference to a valid task (task name)
-func validatePipelineTasks(ctx context.Context, tasks []PipelineTask, finalTasks []PipelineTask) *apis.FieldError {
+func ValidatePipelineTasks(ctx context.Context, tasks []PipelineTask, finalTasks []PipelineTask) *apis.FieldError {
 	// Names cannot be duplicated
 	taskNames := sets.NewString()
 	var errs *apis.FieldError

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go

@@ -767,7 +767,7 @@ func TestValidatePipelineTasks_Success(t *testing.T) {
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			err := validatePipelineTasks(context.Background(), tt.tasks, []PipelineTask{})
+			err := ValidatePipelineTasks(context.Background(), tt.tasks, []PipelineTask{})
 			if err != nil {
 				t.Errorf("Pipeline.validatePipelineTasks() returned error for valid pipeline tasks: %v", err)
 			}
@@ -927,7 +927,7 @@ func TestValidatePipelineTasks_Failure(t *testing.T) {
 			if tt.wc != nil {
 				ctx = tt.wc(ctx)
 			}
-			err := validatePipelineTasks(ctx, tt.tasks, []PipelineTask{})
+			err := ValidatePipelineTasks(ctx, tt.tasks, []PipelineTask{})
 			if err == nil {
 				t.Error("Pipeline.validatePipelineTasks() did not return error for invalid pipeline tasks")
 			}