diff --git a/pkg/apis/pipeline/v1alpha1/conversion_error.go b/pkg/apis/pipeline/v1alpha1/conversion_error.go index 9df97adc2d0..fae79e6595d 100644 --- a/pkg/apis/pipeline/v1alpha1/conversion_error.go +++ b/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 diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_conversion.go b/pkg/apis/pipeline/v1alpha1/pipeline_conversion.go index 4ce6ec730c8..a7a30a378e7 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_conversion.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_conversion.go @@ -19,6 +19,7 @@ package v1alpha1 import ( "context" + "encoding/json" "fmt" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" @@ -26,6 +27,7 @@ import ( ) 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 } diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_conversion_test.go b/pkg/apis/pipeline/v1alpha1/pipeline_conversion_test.go index 6585cf87309..7bab36f7130 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_conversion_test.go +++ b/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)) } }) } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 0db801cff12..67be1b00a0f 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/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 diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 3d2a529b314..b66cf0722e5 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/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") }