Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix re-applying v1beta1 TaskRun failing 🚢 #2285

Merged
merged 1 commit into from
Mar 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ go 1.13

require (
cloud.google.com/go v0.47.0 // indirect
cloud.google.com/go/storage v1.0.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Me neither.. go mod wonders… it comes, it goes those two things.. I have no idea why…

contrib.go.opencensus.io/exporter/stackdriver v0.12.8 // indirect
github.com/GoogleCloudPlatform/cloud-builders/gcs-fetcher v0.0.0-20191203181535-308b93ad1f39
github.com/cloudevents/sdk-go/v2 v2.0.0-preview6
Expand Down Expand Up @@ -43,7 +42,6 @@ require (
golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
golang.org/x/tools v0.0.0-20200214144324-88be01311a71 // indirect
gomodules.xyz/jsonpatch/v2 v2.1.0 // indirect
google.golang.org/api v0.15.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

google.golang.org/appengine v1.6.5 // indirect
k8s.io/api v0.17.3
k8s.io/apiextensions-apiserver v0.17.3 // indirect
Expand Down
55 changes: 29 additions & 26 deletions pkg/apis/pipeline/v1alpha1/taskrun_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,37 +58,40 @@ func (source *TaskRunSpec) ConvertTo(ctx context.Context, sink *v1beta1.TaskRunS
sink.Params = source.Params
sink.Resources = source.Resources
// Deprecated fields
if len(source.Inputs.Params) > 0 && len(source.Params) > 0 {
// This shouldn't happen as it shouldn't pass validation
return apis.ErrMultipleOneOf("inputs.params", "params")
}
if len(source.Inputs.Params) > 0 {
sink.Params = make([]v1beta1.Param, len(source.Inputs.Params))
for i, param := range source.Inputs.Params {
sink.Params[i] = *param.DeepCopy()
}
}
if len(source.Inputs.Resources) > 0 {
if sink.Resources == nil {
sink.Resources = &v1beta1.TaskRunResources{}
if source.Inputs != nil {
if len(source.Inputs.Params) > 0 && len(source.Params) > 0 {
// This shouldn't happen as it shouldn't pass validation
return apis.ErrMultipleOneOf("inputs.params", "params")
}
if len(source.Inputs.Resources) > 0 && source.Resources != nil && len(source.Resources.Inputs) > 0 {
// This shouldn't happen as it shouldn't pass validation but just in case
return apis.ErrMultipleOneOf("inputs.resources", "resources.inputs")
if len(source.Inputs.Params) > 0 {
sink.Params = make([]v1beta1.Param, len(source.Inputs.Params))
for i, param := range source.Inputs.Params {
sink.Params[i] = *param.DeepCopy()
}
}
sink.Resources.Inputs = make([]v1beta1.TaskResourceBinding, len(source.Inputs.Resources))
for i, resource := range source.Inputs.Resources {
sink.Resources.Inputs[i] = v1beta1.TaskResourceBinding{
PipelineResourceBinding: v1beta1.PipelineResourceBinding{
Name: resource.Name,
ResourceRef: resource.ResourceRef,
ResourceSpec: resource.ResourceSpec,
},
Paths: resource.Paths,
if len(source.Inputs.Resources) > 0 {
if sink.Resources == nil {
sink.Resources = &v1beta1.TaskRunResources{}
}
if len(source.Inputs.Resources) > 0 && source.Resources != nil && len(source.Resources.Inputs) > 0 {
// This shouldn't happen as it shouldn't pass validation but just in case
return apis.ErrMultipleOneOf("inputs.resources", "resources.inputs")
}
sink.Resources.Inputs = make([]v1beta1.TaskResourceBinding, len(source.Inputs.Resources))
for i, resource := range source.Inputs.Resources {
sink.Resources.Inputs[i] = v1beta1.TaskResourceBinding{
PipelineResourceBinding: v1beta1.PipelineResourceBinding{
Name: resource.Name,
ResourceRef: resource.ResourceRef,
ResourceSpec: resource.ResourceSpec,
},
Paths: resource.Paths,
}
}
}
}
if len(source.Outputs.Resources) > 0 {

if source.Outputs != nil && len(source.Outputs.Resources) > 0 {
if sink.Resources == nil {
sink.Resources = &v1beta1.TaskRunResources{}
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/apis/pipeline/v1alpha1/taskrun_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func TestTaskRunConversion(t *testing.T) {
Name: "p1",
Value: v1beta1.ArrayOrString{StringVal: "baz"},
}},
Inputs: TaskRunInputs{
Inputs: &TaskRunInputs{
Params: []Param{{
Name: "p2",
Value: v1beta1.ArrayOrString{StringVal: "bar"}},
Expand Down Expand Up @@ -215,7 +215,7 @@ func TestTaskRunConversion(t *testing.T) {
Paths: []string{"foo", "bar"},
}},
},
Inputs: TaskRunInputs{
Inputs: &TaskRunInputs{
Resources: []TaskResourceBinding{{
PipelineResourceBinding: PipelineResourceBinding{
Name: "i1",
Expand Down Expand Up @@ -260,7 +260,7 @@ func TestTaskRunConversion(t *testing.T) {
Paths: []string{"foo", "bar"},
}},
},
Outputs: TaskRunOutputs{
Outputs: &TaskRunOutputs{
Resources: []TaskResourceBinding{{
PipelineResourceBinding: PipelineResourceBinding{
Name: "o1",
Expand Down Expand Up @@ -314,7 +314,7 @@ func TestTaskRunConversionFromDeprecated(t *testing.T) {
Generation: 1,
},
Spec: TaskRunSpec{
Inputs: TaskRunInputs{
Inputs: &TaskRunInputs{
Params: []Param{{
Name: "p2",
Value: v1beta1.ArrayOrString{StringVal: "bar"}},
Expand Down Expand Up @@ -344,7 +344,7 @@ func TestTaskRunConversionFromDeprecated(t *testing.T) {
Generation: 1,
},
Spec: TaskRunSpec{
Inputs: TaskRunInputs{
Inputs: &TaskRunInputs{
Resources: []TaskResourceBinding{{
PipelineResourceBinding: PipelineResourceBinding{
Name: "i1",
Expand Down Expand Up @@ -382,7 +382,7 @@ func TestTaskRunConversionFromDeprecated(t *testing.T) {
Generation: 1,
},
Spec: TaskRunSpec{
Outputs: TaskRunOutputs{
Outputs: &TaskRunOutputs{
Resources: []TaskResourceBinding{{
PipelineResourceBinding: PipelineResourceBinding{
Name: "o1",
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1alpha1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ type TaskRunSpec struct {
Resources *v1beta1.TaskRunResources `json:"resources,omitempty"`
// Deprecated
// +optional
Inputs TaskRunInputs `json:"inputs,omitempty"`
Inputs *TaskRunInputs `json:"inputs,omitempty"`
// +optional
Outputs TaskRunOutputs `json:"outputs,omitempty"`
Outputs *TaskRunOutputs `json:"outputs,omitempty"`
}

// TaskRunSpecStatus defines the taskrun spec status the user can provide
Expand Down
12 changes: 8 additions & 4 deletions pkg/apis/pipeline/v1alpha1/taskrun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,18 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) *apis.FieldError {

// Deprecated
// check for input resources
if err := ts.Inputs.Validate(ctx, "spec.Inputs"); err != nil {
return err
if ts.Inputs != nil {
if err := ts.Inputs.Validate(ctx, "spec.Inputs"); err != nil {
return err
}
}

// Deprecated
// check for output resources
if err := ts.Outputs.Validate(ctx, "spec.Outputs"); err != nil {
return err
if ts.Outputs != nil {
if err := ts.Outputs.Validate(ctx, "spec.Outputs"); err != nil {
return err
}
}

// Validate Resources
Expand Down
12 changes: 10 additions & 2 deletions pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 8 additions & 6 deletions test/builder/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -774,11 +774,12 @@ func TaskRunResourcesOutput(name string, ops ...TaskResourceBindingOp) TaskRunRe
// Any number of TaskRunInputs modifier can be passed to transform it.
func TaskRunInputs(ops ...TaskRunInputsOp) TaskRunSpecOp {
return func(spec *v1alpha1.TaskRunSpec) {
inputs := &spec.Inputs
if spec.Inputs == nil {
spec.Inputs = &v1alpha1.TaskRunInputs{}
}
for _, op := range ops {
op(inputs)
op(spec.Inputs)
}
spec.Inputs = *inputs
}
}

Expand Down Expand Up @@ -843,11 +844,12 @@ func TaskResourceBindingPaths(paths ...string) TaskResourceBindingOp {
// Any number of TaskRunOutputs modifier can be passed to transform it.
func TaskRunOutputs(ops ...TaskRunOutputsOp) TaskRunSpecOp {
return func(spec *v1alpha1.TaskRunSpec) {
outputs := &spec.Outputs
if spec.Outputs == nil {
spec.Outputs = &v1alpha1.TaskRunOutputs{}
}
for _, op := range ops {
op(outputs)
op(spec.Outputs)
}
spec.Outputs = *outputs
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/builder/task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func TestTaskRunWithTaskRef(t *testing.T) {
Annotations: map[string]string{},
},
Spec: v1alpha1.TaskRunSpec{
Inputs: v1alpha1.TaskRunInputs{
Inputs: &v1alpha1.TaskRunInputs{
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
Name: "git-resource",
Expand All @@ -256,7 +256,7 @@ func TestTaskRunWithTaskRef(t *testing.T) {
Value: *tb.ArrayOrString("array", "values"),
}},
},
Outputs: v1alpha1.TaskRunOutputs{
Outputs: &v1alpha1.TaskRunOutputs{
Resources: []v1alpha1.TaskResourceBinding{{
PipelineResourceBinding: v1alpha1.PipelineResourceBinding{
Name: "git-resource",
Expand Down