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 <smukhade@redhat.com>
imjasonh · Jul 27, 2021 · 22f383c · 22f383c
1 parent d03ba20
commit 22f383c
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 {