Skip to content

Commit

Permalink
adding finally type at the pipeline level
Browse files Browse the repository at this point in the history
these changes are adding finally type and its validation, does not
implement this new functionality.
  • Loading branch information
pritidesai committed May 28, 2020
1 parent 3554a30 commit 59ebb4a
Show file tree
Hide file tree
Showing 11 changed files with 887 additions and 33 deletions.
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/conversion_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ const (
// resources when they cannot be converted to warn of a forthcoming
// breakage.
ConditionTypeConvertible apis.ConditionType = v1beta1.ConditionTypeConvertible
// Conversion Error Field for finally
ConversionErrorFinally = "Finally"
// Conversion Error descriptive message for finally
ConversionErrorFinallyMsg = ConversionErrorFinally + " is not available in v1alpha1"
)

// CannotConvertError is returned when a field cannot be converted.
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipeline_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func (source *PipelineSpec) ConvertTo(ctx context.Context, sink *v1beta1.Pipelin
}
}
}
sink.Finally = nil
return nil
}

Expand Down Expand Up @@ -97,6 +98,10 @@ 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(ConversionErrorFinally, ConversionErrorFinallyMsg)
}
return nil
}

Expand Down
120 changes: 99 additions & 21 deletions pkg/apis/pipeline/v1alpha1/pipeline_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,12 @@ func TestPipelineConversionBadType(t *testing.T) {
}
}

func TestPipelineConversion(t *testing.T) {
func TestPipelineConversion_Success(t *testing.T) {
versions := []apis.Convertible{&v1beta1.Pipeline{}}

tests := []struct {
name string
in *Pipeline
wantErr bool
name string
in *Pipeline
}{{
name: "simple conversion",
in: &Pipeline{
Expand Down Expand Up @@ -114,7 +113,38 @@ func TestPipelineConversion(t *testing.T) {
}},
},
},
}, {
}}

for _, test := range tests {
for _, version := range versions {
t.Run(test.name, func(t *testing.T) {
ver := version
// convert v1alpha1 Pipeline to v1beta1 Pipeline
if err := test.in.ConvertTo(context.Background(), ver); err != nil {
t.Errorf("ConvertTo() = %v", err)
}
got := &Pipeline{}
// converting it back to v1alpha1 pipeline and storing it in got variable to compare with original input
if err := got.ConvertFrom(context.Background(), ver); err != nil {
t.Errorf("ConvertFrom() = %v", err)
}
// compare origin input and roundtrip Pipeline i.e. v1alpha1 pipeline converted to v1beta1 and then converted back to v1alpha1
// this check is making sure that we do not end up with different object than what we started with
if d := cmp.Diff(test.in, got); d != "" {
t.Errorf("roundtrip %s", diff.PrintWantGot(d))
}
})
}
}
}

func TestPipelineConversion_Failure(t *testing.T) {
versions := []apis.Convertible{&v1beta1.Pipeline{}}

tests := []struct {
name string
in *Pipeline
}{{
name: "simple conversion with task spec error",
in: &Pipeline{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -152,29 +182,77 @@ func TestPipelineConversion(t *testing.T) {
}},
},
},
wantErr: true,
}}

for _, test := range tests {
for _, version := range versions {
t.Run(test.name, func(t *testing.T) {
ver := version
if err := test.in.ConvertTo(context.Background(), ver); err != nil {
if !test.wantErr {
t.Errorf("ConvertTo() = %v", err)
}
return
}
t.Logf("ConvertTo() = %#v", ver)
got := &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.in, got); d != "" {
t.Errorf("roundtrip %s", diff.PrintWantGot(d))
if err := test.in.ConvertTo(context.Background(), ver); err == nil {
t.Errorf("Expected ConvertTo to fail but did not produce any error")
}
return
})
}
}
}

func TestPipelineConversionFromWithFinally(t *testing.T) {
versions := []apis.Convertible{&v1beta1.Pipeline{}}
p := &Pipeline{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
Generation: 1,
},
Spec: PipelineSpec{
Tasks: []PipelineTask{{Name: "mytask", TaskRef: &TaskRef{Name: "task"}}},
},
}
for _, version := range versions {
t.Run("finally not available in v1alpha1", func(t *testing.T) {
ver := version
// convert v1alpha1 to v1beta1
if err := p.ConvertTo(context.Background(), ver); err != nil {
t.Errorf("ConvertTo() = %v", err)
}
t.Logf("ConvertTo() = %#v", ver)
// 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{}
if err := got.ConvertFrom(context.Background(), source); err != nil {
cce, ok := err.(*CannotConvertError)
// conversion error contains the field name which resulted in the failure and should be equal to "Finally" here
if ok && cce.Field == ConversionErrorFinally {
return
}
t.Errorf("ConvertFrom() should have failed")
}
})
}
}

func TestPipelineConversionFromBetaToAlphaWithFinally_Failure(t *testing.T) {
p := &v1beta1.Pipeline{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
Generation: 1,
},
Spec: v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{{Name: "mytask", TaskRef: &TaskRef{Name: "task"}}},
Finally: []v1beta1.PipelineTask{{Name: "mytask", 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 contains the field name which resulted in the failure and should be equal to "Finally" here
if ok && cce.Field == ConversionErrorFinally {
return
}
t.Errorf("ConvertFrom() should have failed")
}
})
}
118 changes: 118 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipeline_defaults_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
Copyright 2020 The Tekton Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1alpha1_test

import (
"context"
"testing"

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"

"github.com/google/go-cmp/cmp"
)

func TestPipeline_SetDefaults(t *testing.T) {
cases := []struct {
desc string
p *v1alpha1.Pipeline
want *v1alpha1.Pipeline
}{{
desc: "empty pipeline",
p: &v1alpha1.Pipeline{},
want: &v1alpha1.Pipeline{},
}}
for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
ctx := context.Background()
tc.p.SetDefaults(ctx)
if diff := cmp.Diff(tc.want, tc.p); diff != "" {
t.Errorf("Mismatch of Pipeline: %s", diff)
}
})
}
}

func TestPipelineSpec_SetDefaults(t *testing.T) {
cases := []struct {
desc string
ps *v1alpha1.PipelineSpec
want *v1alpha1.PipelineSpec
}{{
desc: "empty pipelineSpec",
ps: &v1alpha1.PipelineSpec{},
want: &v1alpha1.PipelineSpec{},
}, {
desc: "pipeline task - default task kind must be " + string(v1alpha1.NamespacedTaskKind),
ps: &v1alpha1.PipelineSpec{
Tasks: []v1alpha1.PipelineTask{{
Name: "foo", TaskRef: &v1alpha1.TaskRef{Name: "foo-task"},
}},
},
want: &v1alpha1.PipelineSpec{
Tasks: []v1alpha1.PipelineTask{{
Name: "foo", TaskRef: &v1alpha1.TaskRef{Name: "foo-task", Kind: v1alpha1.NamespacedTaskKind},
}},
},
}, {
desc: "param type - default param type must be " + string(v1alpha1.ParamTypeString),
ps: &v1alpha1.PipelineSpec{
Params: []v1alpha1.ParamSpec{{
Name: "string-param",
}},
},
want: &v1alpha1.PipelineSpec{
Params: []v1alpha1.ParamSpec{{
Name: "string-param", Type: v1alpha1.ParamTypeString,
}},
},
}, {
desc: "pipeline task with taskSpec - default param type must be " + string(v1alpha1.ParamTypeString),
ps: &v1alpha1.PipelineSpec{
Tasks: []v1alpha1.PipelineTask{{
Name: "foo", TaskSpec: &v1alpha1.TaskSpec{
TaskSpec: v1beta1.TaskSpec{
Params: []v1alpha1.ParamSpec{{
Name: "string-param",
}},
},
},
}},
},
want: &v1alpha1.PipelineSpec{
Tasks: []v1alpha1.PipelineTask{{
Name: "foo", TaskSpec: &v1alpha1.TaskSpec{
TaskSpec: v1beta1.TaskSpec{
Params: []v1alpha1.ParamSpec{{
Name: "string-param",
Type: v1alpha1.ParamTypeString,
}},
},
},
}},
},
}}
for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
ctx := context.Background()
tc.ps.SetDefaults(ctx)
if diff := cmp.Diff(tc.want, tc.ps); diff != "" {
t.Errorf("Mismatch of PipelineSpec: %s", diff)
}
})
}
}
10 changes: 10 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,14 @@ func (ps *PipelineSpec) SetDefaults(ctx context.Context) {
for i := range ps.Params {
ps.Params[i].SetDefaults(ctx)
}
for _, ft := range ps.Finally {
if ft.TaskRef != nil {
if ft.TaskRef.Kind == "" {
ft.TaskRef.Kind = NamespacedTaskKind
}
}
if ft.TaskSpec != nil {
ft.TaskSpec.SetDefaults(ctx)
}
}
}
Loading

0 comments on commit 59ebb4a

Please sign in to comment.