From 3ddba8bea0267c200b7c9cbd431e35b1698aa134 Mon Sep 17 00:00:00 2001 From: sm43 Date: Tue, 27 Jul 2021 10:14:29 +0530 Subject: [PATCH 1/2] Passes pipelines from bundles through defaulting before execution for pipeline from yaml webhook does the defaulting but for pipeline fetched from bundles, defaulting was not taking place due to which validation error use to occur at execution. This adds the defaulting for pipelines fetched from bundles. Signed-off-by: Shivam Mukhade --- internal/builder/v1alpha1/pipeline.go | 8 ++++++ .../pipeline/v1beta1/pipeline_interface.go | 6 ++++- .../pipelinerun/pipelinerun_test.go | 1 + .../pipelinerun/resources/pipelineref.go | 2 ++ .../pipelinerun/resources/pipelineref_test.go | 27 +++++++++++++++++++ 5 files changed, 43 insertions(+), 1 deletion(-) diff --git a/internal/builder/v1alpha1/pipeline.go b/internal/builder/v1alpha1/pipeline.go index fbad8813645..b36723dd65d 100644 --- a/internal/builder/v1alpha1/pipeline.go +++ b/internal/builder/v1alpha1/pipeline.go @@ -76,6 +76,14 @@ func Pipeline(name string, ops ...PipelineOp) *v1alpha1.Pipeline { return p } +// PipelineType will add a TypeMeta to the pipeline's definition. +func PipelineType(t *v1alpha1.Pipeline) { + t.TypeMeta = metav1.TypeMeta{ + Kind: "Pipeline", + APIVersion: "tekton.dev/v1alpha1", + } +} + // PipelineNamespace sets the namespace on the Pipeline func PipelineNamespace(namespace string) PipelineOp { return func(t *v1alpha1.Pipeline) { diff --git a/pkg/apis/pipeline/v1beta1/pipeline_interface.go b/pkg/apis/pipeline/v1beta1/pipeline_interface.go index eb1e30a45eb..fb21e16dafe 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_interface.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_interface.go @@ -16,10 +16,14 @@ limitations under the License. package v1beta1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" +) // PipelineObject is implemented by Pipeline and ClusterPipeline type PipelineObject interface { + apis.Defaultable PipelineMetadata() metav1.ObjectMeta PipelineSpec() PipelineSpec Copy() PipelineObject diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 47b774d2d41..2f1bb9c8e14 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -5881,6 +5881,7 @@ func TestReconcile_RemotePipelineRef(t *testing.T) { Resources: &v1beta1.TaskRunResources{}, Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute}, TaskRef: &v1beta1.TaskRef{ + Kind: "Task", Name: "unit-test-task", Bundle: ref, }, diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index 3d250861bf4..ce14a37ec85 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -69,12 +69,14 @@ func GetPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekton clien return nil, err } if pipeline, ok := obj.(v1beta1.PipelineObject); ok { + pipeline.SetDefaults(ctx) return pipeline, nil } if pipeline, ok := obj.(*v1alpha1.Pipeline); ok { betaPipeline := &v1beta1.Pipeline{} err := pipeline.ConvertTo(ctx, betaPipeline) + betaPipeline.SetDefaults(ctx) return betaPipeline, err } diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go index 66733012d6c..e9fdd941b05 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref_test.go @@ -24,8 +24,10 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-containerregistry/pkg/registry" + ta "github.com/tektoncd/pipeline/internal/builder/v1alpha1" tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" "github.com/tektoncd/pipeline/pkg/apis/config" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/client/clientset/versioned/fake" "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources" @@ -146,6 +148,31 @@ func TestGetPipelineFunc(t *testing.T) { Name: "simple", }, expected: tb.Pipeline("simple", tb.PipelineType, tb.PipelineNamespace("default"), tb.PipelineSpec(tb.PipelineTask("something", "something"))), + }, { + name: "remote-pipeline-without-defaults", + localPipelines: []runtime.Object{simplePipeline}, + remotePipelines: []runtime.Object{ + tb.Pipeline("simple", tb.PipelineType, tb.PipelineNamespace("default"), + tb.PipelineSpec(tb.PipelineTask("something", "something"), tb.PipelineParamSpec("foo", ""))), + dummyPipeline}, + ref: &v1beta1.PipelineRef{ + Name: "simple", + Bundle: u.Host + "/remote-pipeline-without-defaults", + }, + expected: tb.Pipeline("simple", tb.PipelineType, tb.PipelineNamespace("default"), + tb.PipelineSpec(tb.PipelineTask("something", "something", tb.PipelineTaskRefKind(v1beta1.NamespacedTaskKind)), tb.PipelineParamSpec("foo", v1beta1.ParamTypeString))), + }, { + name: "remote-v1alpha1-pipeline-without-defaults", + localPipelines: []runtime.Object{simplePipeline}, + remotePipelines: []runtime.Object{ + ta.Pipeline("simple", ta.PipelineType, ta.PipelineNamespace("default"), + ta.PipelineSpec(ta.PipelineTask("something", "something"), ta.PipelineParamSpec("foo", "")))}, + ref: &v1alpha1.PipelineRef{ + Name: "simple", + Bundle: u.Host + "/remote-v1alpha1-pipeline-without-defaults", + }, + expected: tb.Pipeline("simple", tb.PipelineNamespace("default"), + tb.PipelineSpec(tb.PipelineTask("something", "something", tb.PipelineTaskRefKind(v1beta1.NamespacedTaskKind)), tb.PipelineParamSpec("foo", v1beta1.ParamTypeString))), }} for _, tc := range testcases { From 56ed95765e61d698c9f494e09aa930185869d9a1 Mon Sep 17 00:00:00 2001 From: sm43 Date: Tue, 27 Jul 2021 10:36:58 +0530 Subject: [PATCH 2/2] Passes tasks from bundles through defaulting before execution for tasks from yaml webhook does the defaulting but for tasks fetched from bundles, defaulting was not taking place due to which validation error use to occur at execution. This adds the defaulting for tasks fetched from bundles. Signed-off-by: Shivam Mukhade --- pkg/apis/pipeline/v1beta1/task_interface.go | 6 ++++- pkg/reconciler/taskrun/resources/taskref.go | 3 +++ .../taskrun/resources/taskref_test.go | 25 +++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/pkg/apis/pipeline/v1beta1/task_interface.go b/pkg/apis/pipeline/v1beta1/task_interface.go index 9d321238300..ac018081be0 100644 --- a/pkg/apis/pipeline/v1beta1/task_interface.go +++ b/pkg/apis/pipeline/v1beta1/task_interface.go @@ -16,10 +16,14 @@ limitations under the License. package v1beta1 -import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" +) // TaskObject is implemented by Task and ClusterTask type TaskObject interface { + apis.Defaultable TaskMetadata() metav1.ObjectMeta TaskSpec() TaskSpec Copy() TaskObject diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index 6ba0f1d9f50..a6caeb4fc2c 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -96,6 +96,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset // If the resolved object is already a v1beta1.{Cluster}Task, it should be returnable as a // v1beta1.TaskObject. if ti, ok := obj.(v1beta1.TaskObject); ok { + ti.SetDefaults(ctx) return ti, nil } @@ -105,10 +106,12 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset case *v1alpha1.Task: betaTask := &v1beta1.Task{} err := tt.ConvertTo(ctx, betaTask) + betaTask.SetDefaults(ctx) return betaTask, err case *v1alpha1.ClusterTask: betaTask := &v1beta1.ClusterTask{} err := tt.ConvertTo(ctx, betaTask) + betaTask.SetDefaults(ctx) return betaTask, err } diff --git a/pkg/reconciler/taskrun/resources/taskref_test.go b/pkg/reconciler/taskrun/resources/taskref_test.go index 1ae56a49b8c..691b016da86 100644 --- a/pkg/reconciler/taskrun/resources/taskref_test.go +++ b/pkg/reconciler/taskrun/resources/taskref_test.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-containerregistry/pkg/registry" + ta "github.com/tektoncd/pipeline/internal/builder/v1alpha1" tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" @@ -153,6 +154,30 @@ func TestGetTaskFunc(t *testing.T) { }, expected: tb.Task("simple", tb.TaskType), expectedKind: v1beta1.NamespacedTaskKind, + }, { + name: "remote-task-without-defaults", + localTasks: []runtime.Object{}, + remoteTasks: []runtime.Object{ + tb.Task("simple", tb.TaskType, tb.TaskNamespace("default"), tb.TaskSpec(tb.TaskParam("foo", ""), tb.Step("something"))), + }, + ref: &v1beta1.TaskRef{ + Name: "simple", + Bundle: u.Host + "/remote-task-without-defaults", + }, + expected: tb.Task("simple", tb.TaskType, tb.TaskNamespace("default"), tb.TaskSpec(tb.TaskParam("foo", v1beta1.ParamTypeString), tb.Step("something"))), + expectedKind: v1beta1.NamespacedTaskKind, + }, { + name: "remote-v1alpha1-task-without-defaults", + localTasks: []runtime.Object{}, + remoteTasks: []runtime.Object{ + ta.Task("simple", ta.TaskType, ta.TaskNamespace("default"), ta.TaskSpec(ta.TaskParam("foo", ""), ta.Step("something"))), + }, + ref: &v1alpha1.TaskRef{ + Name: "simple", + Bundle: u.Host + "/remote-v1alpha1-task-without-defaults", + }, + expected: tb.Task("simple", tb.TaskNamespace("default"), tb.TaskSpec(tb.TaskParam("foo", v1alpha1.ParamTypeString), tb.Step("something"))), + expectedKind: v1beta1.NamespacedTaskKind, }, { name: "local-task", localTasks: []runtime.Object{