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 20, 2020
1 parent 3554a30 commit ff08a8f
Show file tree
Hide file tree
Showing 11 changed files with 860 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 not available in v1alpha1
if len(source.Finally) > 0 {
return ConvertErrorf(ConversionErrorFinally, ConversionErrorFinallyMsg)
}
return nil
}

Expand Down
93 changes: 72 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,36 @@ func TestPipelineConversion(t *testing.T) {
}},
},
},
}, {
}}

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 {
t.Errorf("ConvertTo() = %v", err)
}
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))
}
})
}
}
}

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 +180,52 @@ 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")
}
})
}
}
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 ff08a8f

Please sign in to comment.