From d779a3124a204a6b34081b1361f383888eb5530f Mon Sep 17 00:00:00 2001 From: Prashant Sharma Date: Mon, 5 Apr 2021 20:06:08 +0530 Subject: [PATCH 1/4] TEP-0061, Allow custom task to be embedded. It is now possible to embed the spec of a custom task in a Run resource, whether stand-alone in Run or embedded in a Pipeline. e.g. in a Run using example TaskLoop controller in tektoncd/experimental/task-loop/ apiVersion: tekton.dev/v1alpha1 kind: Run metadata: name: simpletasklooprun spec: params: - name: word value: - jump - land - name: suffix value: ing spec: apiVersion: custom.tekton.dev/v1alpha1 kind: TaskLoop spec: # This is a new field, which comprises of custom-task spec. taskRef: name: simpletask iterateParam: word timeout: 60s retries: 2 1. API changes, This PR adds new APIs i.e. adds a field `Spec EmbeddedRunSpec` to `RunSpec` 2. An embedded task will accepts new field `Spec` with type `runtime.RawExtension` in addition to `ApiVersion` and `Kind` fields of type string (as part of `runtime.TypeMeta`) 3. Validation changes, in addition to adding support for `Run.RunSpec.Spec` the validations will be changed to support "One of `Run.RunSpec.Spec` or `Run.RunSpec.Ref` " only and not both as part of a single API request to kubernetes. Developers of custom controllers (existing and new), who would like to support embedded spec for their custom task, need to implement the logic required to extract, validate and use the custom task spec from the new RunSpec.Spec field. Please review the documentation in docs/runs.md and docs/pipelines.md --- internal/builder/v1beta1/pipeline_test.go | 4 +- pkg/apis/pipeline/v1alpha1/run_types.go | 19 +- pkg/apis/pipeline/v1alpha1/run_validation.go | 24 +- .../pipeline/v1alpha1/run_validation_test.go | 76 ++++++- pkg/apis/pipeline/v1beta1/pipeline_types.go | 23 +- .../pipeline/v1beta1/pipeline_types_test.go | 32 +++ .../v1beta1/pipeline_validation_test.go | 19 ++ pkg/controller/filter.go | 25 +- pkg/controller/filter_test.go | 214 +++++++++++++++++- pkg/reconciler/pipelinerun/pipelinerun.go | 18 ++ .../pipelinerun/pipelinerun_test.go | 67 +++++- .../resources/pipelinerunresolution.go | 15 +- .../resources/pipelinerunresolution_test.go | 71 +++++- test/custom_task_test.go | 13 ++ 14 files changed, 580 insertions(+), 40 deletions(-) diff --git a/internal/builder/v1beta1/pipeline_test.go b/internal/builder/v1beta1/pipeline_test.go index ea6981d3669..e0e10fbcda9 100644 --- a/internal/builder/v1beta1/pipeline_test.go +++ b/internal/builder/v1beta1/pipeline_test.go @@ -156,10 +156,10 @@ func TestPipeline(t *testing.T) { Labels: map[string]string{"label": "labelvalue"}, Annotations: map[string]string{"annotation": "annotationvalue"}, }, + TaskSpec: getTaskSpec(), }, - }, - }, + }}, Workspaces: []v1beta1.PipelineWorkspaceDeclaration{{ Name: "workspace1", }}, diff --git a/pkg/apis/pipeline/v1alpha1/run_types.go b/pkg/apis/pipeline/v1alpha1/run_types.go index 19a45c5d1d7..bba19a2a82f 100644 --- a/pkg/apis/pipeline/v1alpha1/run_types.go +++ b/pkg/apis/pipeline/v1alpha1/run_types.go @@ -20,9 +20,10 @@ import ( "fmt" "github.com/tektoncd/pipeline/pkg/apis/pipeline" - v1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" runv1alpha1 "github.com/tektoncd/pipeline/pkg/apis/run/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -36,11 +37,27 @@ var ( } ) +// EmbeddedRunSpec allows custom task definitions to be embedded +type EmbeddedRunSpec struct { + runtime.TypeMeta `json:",inline"` + + // +optional + Metadata v1beta1.PipelineTaskMetadata `json:"metadata,omitempty"` + + // Spec is a specification of a custom task + // +optional + Spec runtime.RawExtension `json:"spec,omitempty"` +} + // RunSpec defines the desired state of Run type RunSpec struct { // +optional Ref *TaskRef `json:"ref,omitempty"` + // Spec is a specification of a custom task + // +optional + Spec *EmbeddedRunSpec `json:"spec,omitempty"` + // +optional Params []v1beta1.Param `json:"params,omitempty"` diff --git a/pkg/apis/pipeline/v1alpha1/run_validation.go b/pkg/apis/pipeline/v1alpha1/run_validation.go index 5a68359f172..8ad179c85b4 100644 --- a/pkg/apis/pipeline/v1alpha1/run_validation.go +++ b/pkg/apis/pipeline/v1alpha1/run_validation.go @@ -36,20 +36,30 @@ func (r *Run) Validate(ctx context.Context) *apis.FieldError { // Validate Run spec func (rs *RunSpec) Validate(ctx context.Context) *apis.FieldError { + // this covers the case rs.Ref == nil && rs.Spec == nil if equality.Semantic.DeepEqual(rs, &RunSpec{}) { return apis.ErrMissingField("spec") } - if rs.Ref == nil { - return apis.ErrMissingField("spec.ref") + if rs.Ref != nil && rs.Spec != nil { + return apis.ErrMultipleOneOf("spec.ref", "spec.spec") } - if rs.Ref.APIVersion == "" { - return apis.ErrMissingField("spec.ref.apiVersion") + if rs.Ref != nil { + if rs.Ref.APIVersion == "" { + return apis.ErrMissingField("spec.ref.apiVersion") + } + if rs.Ref.Kind == "" { + return apis.ErrMissingField("spec.ref.kind") + } } - if rs.Ref.Kind == "" { - return apis.ErrMissingField("spec.ref.kind") + if rs.Spec != nil { + if rs.Spec.APIVersion == "" { + return apis.ErrMissingField("spec.spec.apiVersion") + } + if rs.Spec.Kind == "" { + return apis.ErrMissingField("spec.spec.kind") + } } - if err := validateParameters("spec.params", rs.Params); err != nil { return err } diff --git a/pkg/apis/pipeline/v1alpha1/run_validation_test.go b/pkg/apis/pipeline/v1alpha1/run_validation_test.go index 230a2bec615..15c79d1505b 100644 --- a/pkg/apis/pipeline/v1alpha1/run_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/run_validation_test.go @@ -26,6 +26,7 @@ import ( "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "knative.dev/pkg/apis" ) @@ -43,18 +44,39 @@ func TestRun_Invalid(t *testing.T) { }, want: apis.ErrMissingField("spec"), }, { - name: "missing ref", + name: "Empty spec", run: &v1alpha1.Run{ ObjectMeta: metav1.ObjectMeta{ Name: "temp", }, Spec: v1alpha1.RunSpec{ - Ref: nil, + Ref: nil, + Spec: nil, }, }, want: apis.ErrMissingField("spec"), }, { - name: "missing apiVersion", + name: "Both Ref and Spec", + run: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Name: "temp", + }, + Spec: v1alpha1.RunSpec{ + Ref: &v1alpha1.TaskRef{ + APIVersion: "apiVersion", + Kind: "kind", + }, + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "apiVersion", + Kind: "kind", + }, + }, + }, + }, + want: apis.ErrMultipleOneOf("spec.ref", "spec.spec"), + }, { + name: "missing apiVersion in Ref", run: &v1alpha1.Run{ ObjectMeta: metav1.ObjectMeta{ Name: "temp", @@ -67,7 +89,7 @@ func TestRun_Invalid(t *testing.T) { }, want: apis.ErrMissingField("spec.ref.apiVersion"), }, { - name: "missing kind", + name: "missing kind in Ref", run: &v1alpha1.Run{ ObjectMeta: metav1.ObjectMeta{ Name: "temp", @@ -80,6 +102,37 @@ func TestRun_Invalid(t *testing.T) { }, }, want: apis.ErrMissingField("spec.ref.kind"), + }, { + name: "missing apiVersion in Spec", + run: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Name: "temp", + }, + Spec: v1alpha1.RunSpec{ + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "", + }, + }, + }, + }, + want: apis.ErrMissingField("spec.spec.apiVersion"), + }, { + name: "missing kind in Spec", + run: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Name: "temp", + }, + Spec: v1alpha1.RunSpec{ + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "apiVersion", + Kind: "", + }, + }, + }, + }, + want: apis.ErrMissingField("spec.spec.kind"), }, { name: "non-unique params", run: &v1alpha1.Run{ @@ -142,6 +195,21 @@ func TestRun_Valid(t *testing.T) { }, }, }, + }, { + name: "Spec with valid ApiVersion and Kind", + run: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Name: "temp", + }, + Spec: v1alpha1.RunSpec{ + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "apiVersion", + Kind: "kind", + }, + }, + }, + }, }, { name: "unique params", run: &v1alpha1.Run{ diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types.go b/pkg/apis/pipeline/v1beta1/pipeline_types.go index 6d633f23c3a..b022f62dd82 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types.go @@ -26,6 +26,7 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" @@ -114,10 +115,18 @@ type PipelineTaskMetadata struct { } type EmbeddedTask struct { + // +optional + runtime.TypeMeta `json:",inline,omitempty"` + + // Spec is a specification of a custom task + // +optional + Spec runtime.RawExtension `json:"spec,omitempty"` + // +optional Metadata PipelineTaskMetadata `json:"metadata,omitempty"` // TaskSpec is a specification of a task + // +optional TaskSpec `json:",inline,omitempty"` } @@ -190,9 +199,19 @@ func (pt PipelineTask) validateRefOrSpec() (errs *apis.FieldError) { // validateCustomTask validates custom task specifications - checking kind and fail if not yet supported features specified func (pt PipelineTask) validateCustomTask() (errs *apis.FieldError) { - if pt.TaskRef.Kind == "" { + if pt.TaskRef != nil && pt.TaskRef.Kind == "" { errs = errs.Also(apis.ErrInvalidValue("custom task ref must specify kind", "taskRef.kind")) } + if pt.TaskSpec != nil && pt.TaskSpec.Kind == "" { + errs = errs.Also(apis.ErrInvalidValue("custom task spec must specify kind", "taskSpec.kind")) + } + if pt.TaskRef != nil && pt.TaskRef.APIVersion == "" { + errs = errs.Also(apis.ErrInvalidValue("custom task ref must specify apiVersion", "taskRef.apiVersion")) + } + if pt.TaskSpec != nil && pt.TaskSpec.APIVersion == "" { + errs = errs.Also(apis.ErrInvalidValue("custom task spec must specify apiVersion", "taskSpec.apiVersion")) + } + // Conditions are deprecated so the effort to support them with custom tasks is not justified. // When expressions should be used instead. if len(pt.Conditions) > 0 { @@ -279,6 +298,8 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) { switch { case cfg.FeatureFlags.EnableCustomTasks && pt.TaskRef != nil && pt.TaskRef.APIVersion != "": errs = errs.Also(pt.validateCustomTask()) + case cfg.FeatureFlags.EnableCustomTasks && pt.TaskSpec != nil && pt.TaskSpec.APIVersion != "": + errs = errs.Also(pt.validateCustomTask()) // If EnableTektonOCIBundles feature flag is on, validate bundle specifications case cfg.FeatureFlags.EnableTektonOCIBundles && pt.TaskRef != nil && pt.TaskRef.Bundle != "": errs = errs.Also(pt.validateBundle()) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 1e80ad7c1fc..ed0a3159f4f 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -25,6 +25,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/tektoncd/pipeline/test/diff" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" "knative.dev/pkg/apis" ) @@ -139,6 +140,37 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { Message: `invalid value: custom task ref must specify kind`, Paths: []string{"taskRef.kind"}, }, + }, { + name: "custom task - taskSpec without kind", + task: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example.dev/v0", + Kind: "", + }, + }}, + expectedError: apis.FieldError{ + Message: `invalid value: custom task spec must specify kind`, + Paths: []string{"taskSpec.kind"}, + }, + }, { + name: "custom task - taskSpec without apiVersion", + task: PipelineTask{Name: "foo", TaskSpec: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "", + Kind: "some-kind", + }, + }}, + expectedError: apis.FieldError{ + Message: `invalid value: custom task spec must specify apiVersion`, + Paths: []string{"taskSpec.apiVersion"}, + }, + }, { + name: "custom task - taskRef without apiVersion", + task: PipelineTask{Name: "foo", TaskRef: &TaskRef{APIVersion: "", Kind: "some-kind", Name: ""}}, + expectedError: apis.FieldError{ + Message: `invalid value: custom task ref must specify apiVersion`, + Paths: []string{"taskRef.apiVersion"}, + }, }, { name: "custom task doesn't support conditions", task: PipelineTask{ diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 165cfdf5dfb..fb39e31b189 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -27,6 +27,7 @@ import ( "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/selection" "knative.dev/pkg/apis" logtesting "knative.dev/pkg/logging/testing" @@ -54,6 +55,24 @@ func TestPipeline_Validate_Success(t *testing.T) { }, }, wc: enableFeatures(t, []string{"enable-custom-tasks"}), + }, { + name: "pipelinetask custom task spec", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Tasks: []PipelineTask{{Name: "foo", + TaskSpec: &EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example.dev/v0", + Kind: "Example", + }, + Spec: runtime.RawExtension{ + Raw: []byte(`{"field1":123,"field2":"value"}`), + }}, + }}, + }, + }, + wc: enableFeatures(t, []string{"enable-custom-tasks"}), }, { name: "valid pipeline with params, resources, workspaces, task results, and pipeline results", p: &Pipeline{ diff --git a/pkg/controller/filter.go b/pkg/controller/filter.go index 659cd6211f4..092b937c073 100644 --- a/pkg/controller/filter.go +++ b/pkg/controller/filter.go @@ -44,13 +44,18 @@ func FilterRunRef(apiVersion, kind string) func(interface{}) bool { // Ignore. return false } - if r == nil || r.Spec.Ref == nil { + if r == nil || (r.Spec.Ref == nil && r.Spec.Spec == nil) { // These are invalid, but just in case they get // created somehow, don't panic. return false } - - return r.Spec.Ref.APIVersion == apiVersion && r.Spec.Ref.Kind == v1alpha1.TaskKind(kind) + result := false + if r.Spec.Ref != nil { + result = r.Spec.Ref.APIVersion == apiVersion && r.Spec.Ref.Kind == v1alpha1.TaskKind(kind) + } else if r.Spec.Spec != nil { + result = r.Spec.Spec.APIVersion == apiVersion && r.Spec.Spec.Kind == kind + } + return result } } @@ -82,10 +87,20 @@ func FilterOwnerRunRef(runLister listersalpha.RunLister, apiVersion, kind string if err != nil { return false } - if run.Spec.Ref == nil { + if run.Spec.Ref == nil && run.Spec.Spec == nil { // These are invalid, but just in case they get created somehow, don't panic. return false } - return run.Spec.Ref.APIVersion == apiVersion && run.Spec.Ref.Kind == v1alpha1.TaskKind(kind) + if run.Spec.Ref != nil && run.Spec.Spec != nil { + // These are invalid. + return false + } + result := false + if run.Spec.Ref != nil { + result = run.Spec.Ref.APIVersion == apiVersion && run.Spec.Ref.Kind == v1alpha1.TaskKind(kind) + } else if run.Spec.Spec != nil { + result = run.Spec.Spec.APIVersion == apiVersion && run.Spec.Spec.Kind == kind + } + return result } } diff --git a/pkg/controller/filter_test.go b/pkg/controller/filter_test.go index bef4158665f..53e7d63acfa 100644 --- a/pkg/controller/filter_test.go +++ b/pkg/controller/filter_test.go @@ -25,6 +25,7 @@ import ( fakeruninformer "github.com/tektoncd/pipeline/pkg/client/injection/informers/pipeline/v1alpha1/run/fake" "github.com/tektoncd/pipeline/pkg/controller" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" rtesting "knative.dev/pkg/reconciler/testing" ) @@ -51,15 +52,33 @@ func TestFilterRunRef(t *testing.T) { in: (*v1alpha1.Run)(nil), want: false, }, { - desc: "nil ref", + desc: "nil ref and spec", in: &v1alpha1.Run{ Spec: v1alpha1.RunSpec{ - Ref: nil, + Ref: nil, + Spec: nil, }, }, want: false, }, { - desc: "Run without matching apiVersion", + desc: "both ref and spec", + in: &v1alpha1.Run{ + Spec: v1alpha1.RunSpec{ + Ref: &v1alpha1.TaskRef{ + APIVersion: "not-matching", + Kind: kind, + }, + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: apiVersion, + Kind: kind, + }, + }, + }, + }, + want: false, + }, { + desc: "Run without matching apiVersion in taskRef", in: &v1alpha1.Run{ Spec: v1alpha1.RunSpec{ Ref: &v1alpha1.TaskRef{ @@ -70,7 +89,7 @@ func TestFilterRunRef(t *testing.T) { }, want: false, }, { - desc: "Run without matching kind", + desc: "Run without matching kind in taskRef", in: &v1alpha1.Run{ Spec: v1alpha1.RunSpec{ Ref: &v1alpha1.TaskRef{ @@ -81,7 +100,7 @@ func TestFilterRunRef(t *testing.T) { }, want: false, }, { - desc: "Run with matching apiVersion and kind", + desc: "Run with matching apiVersion and kind in taskRef", in: &v1alpha1.Run{ Spec: v1alpha1.RunSpec{ Ref: &v1alpha1.TaskRef{ @@ -92,7 +111,46 @@ func TestFilterRunRef(t *testing.T) { }, want: true, }, { - desc: "Run with matching apiVersion and kind and name", + desc: "Run with matching apiVersion and kind in taskSpec", + in: &v1alpha1.Run{ + Spec: v1alpha1.RunSpec{ + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: apiVersion, + Kind: kind, + }, + }, + }, + }, + want: true, + }, { + desc: "Run without matching kind for taskSpec", + in: &v1alpha1.Run{ + Spec: v1alpha1.RunSpec{ + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: apiVersion, + Kind: "not-matching", + }, + }, + }, + }, + want: false, + }, { + desc: "Run without matching apiVersion for taskSpec", + in: &v1alpha1.Run{ + Spec: v1alpha1.RunSpec{ + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "not-matching", + Kind: kind, + }, + }, + }, + }, + want: false, + }, { + desc: "Run with matching apiVersion and kind and name for taskRef", in: &v1alpha1.Run{ Spec: v1alpha1.RunSpec{ Ref: &v1alpha1.TaskRef{ @@ -120,7 +178,7 @@ func TestFilterOwnerRunRef(t *testing.T) { owner *v1alpha1.Run want bool }{{ - desc: "Owner is a Run that references a matching apiVersion and kind", + desc: "Owner is a Run for taskRef that references a matching apiVersion and kind", in: &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: "some-taskrun", @@ -151,7 +209,40 @@ func TestFilterOwnerRunRef(t *testing.T) { }, want: true, }, { - desc: "Owner is a Run that references a non-matching apiversion", + desc: "Owner is a Run for taskSpec that references a matching apiVersion and kind", + in: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-taskrun", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: pipeline.RunControllerName, + Name: "some-run", + Controller: &trueB, + }}, + }, + }, + owner: &v1alpha1.Run{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: pipeline.RunControllerName, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-run", + Namespace: "default", + }, + Spec: v1alpha1.RunSpec{ + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: apiVersion, + Kind: kind, + }, + }, + }, + }, + want: true, + }, { + desc: "Owner is a Run for taskRef that references a non-matching apiversion", in: &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: "some-taskrun", @@ -182,7 +273,40 @@ func TestFilterOwnerRunRef(t *testing.T) { }, want: false, }, { - desc: "Owner is a Run that references a non-matching kind", + desc: "Owner is a Run for taskSpec that references a non-matching apiversion", + in: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-taskrun", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: pipeline.RunControllerName, + Name: "some-other-run", + Controller: &trueB, + }}, + }, + }, + owner: &v1alpha1.Run{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: pipeline.RunControllerName, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-other-run", + Namespace: "default", + }, + Spec: v1alpha1.RunSpec{ + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: apiVersion2, + Kind: kind, + }, + }, + }, + }, + want: false, + }, { + desc: "Owner is a Run for taskRef that references a non-matching kind", in: &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: "some-taskrun", @@ -213,7 +337,40 @@ func TestFilterOwnerRunRef(t *testing.T) { }, want: false, }, { - desc: "Owner is a Run that with a missing ref", + desc: "Owner is a Run for taskSpec that references a non-matching kind", + in: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-taskrun", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: pipeline.RunControllerName, + Name: "some-other-run2", + Controller: &trueB, + }}, + }, + }, + owner: &v1alpha1.Run{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: pipeline.RunControllerName, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-other-run2", + Namespace: "default", + }, + Spec: v1alpha1.RunSpec{ + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: apiVersion, + Kind: kind2, + }, + }, + }, + }, + want: false, + }, { + desc: "Owner is a Run with a missing ref and spec", in: &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: "some-taskrun", @@ -238,6 +395,43 @@ func TestFilterOwnerRunRef(t *testing.T) { Spec: v1alpha1.RunSpec{}, // missing ref (illegal) }, want: false, + }, { + desc: "Owner is a Run with both ref and spec with matching apiversion and kind", + in: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-taskrun", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: pipeline.RunControllerName, + Name: "some-strange-run", + Controller: &trueB, + }}, + }, + }, + owner: &v1alpha1.Run{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.SchemeGroupVersion.String(), + Kind: pipeline.RunControllerName, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "some-strange-run", + Namespace: "default", + }, + Spec: v1alpha1.RunSpec{ + Ref: &v1alpha1.TaskRef{ + APIVersion: apiVersion, + Kind: kind, + }, + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: apiVersion, + Kind: kind, + }, + }, + }, + }, + want: false, }, { desc: "Owner is not a Run", in: &v1beta1.TaskRun{ diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 37cd22bf2de..169de75ff4d 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -18,6 +18,7 @@ package pipelinerun import ( "context" + "encoding/json" "fmt" "path/filepath" "reflect" @@ -51,6 +52,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes" "knative.dev/pkg/apis" "knative.dev/pkg/controller" @@ -765,6 +767,22 @@ func (c *Reconciler) createRun(ctx context.Context, rprt *resources.ResolvedPipe }, } + if rprt.PipelineTask.TaskSpec != nil { + j, err := json.Marshal(rprt.PipelineTask.TaskSpec.Spec) + if err != nil { + return nil, err + } + r.Spec.Spec = &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: rprt.PipelineTask.TaskSpec.APIVersion, + Kind: rprt.PipelineTask.TaskSpec.Kind, + }, + Metadata: rprt.PipelineTask.TaskSpec.Metadata, + Spec: runtime.RawExtension{ + Raw: j, + }, + } + } var pipelinePVCWorkspaceName string var err error r.Spec.Workspaces, pipelinePVCWorkspaceName, err = getTaskrunWorkspaces(pr, rprt) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 0ab2076c74b..8863eada046 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -470,7 +470,7 @@ func TestReconcile_CustomTask(t *testing.T) { pr *v1beta1.PipelineRun wantRun *v1alpha1.Run }{{ - name: "simple custom task", + name: "simple custom task with taskRef", pr: &v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ Name: pipelineRunName, @@ -522,6 +522,69 @@ func TestReconcile_CustomTask(t *testing.T) { ServiceAccountName: "default", }, }, + }, { + name: "simple custom task with taskSpec", + pr: &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: pipelineRunName, + Namespace: namespace, + }, + Spec: v1beta1.PipelineRunSpec{ + PipelineSpec: &v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: pipelineTaskName, + Params: []v1beta1.Param{{ + Name: "param1", + Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "value1"}, + }}, + TaskSpec: &v1beta1.EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example.dev/v0", + Kind: "Example", + }, + Spec: runtime.RawExtension{ + Raw: []byte(`{"field1":123,"field2":"value"}`), + }, + }, + }}, + }, + }, + }, + wantRun: &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pipelinerun-custom-task-mz4c7", + Namespace: namespace, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "tekton.dev/v1beta1", + Kind: "PipelineRun", + Name: pipelineRunName, + Controller: &trueb, + BlockOwnerDeletion: &trueb, + }}, + Labels: map[string]string{ + "tekton.dev/pipeline": pipelineRunName, + "tekton.dev/pipelineRun": pipelineRunName, + "tekton.dev/pipelineTask": pipelineTaskName, + }, + Annotations: map[string]string{}, + }, + Spec: v1alpha1.RunSpec{ + Params: []v1beta1.Param{{ + Name: "param1", + Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "value1"}, + }}, + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example.dev/v0", + Kind: "Example", + }, + Spec: runtime.RawExtension{ + Raw: []byte(`{"field1":123,"field2":"value"}`), + }, + }, + ServiceAccountName: "default", + }, + }, }, { name: "custom task with workspace", pr: &v1beta1.PipelineRun{ @@ -557,7 +620,7 @@ func TestReconcile_CustomTask(t *testing.T) { }, wantRun: &v1alpha1.Run{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-pipelinerun-custom-task-mz4c7", + Name: "test-pipelinerun-custom-task-mssqb", Namespace: namespace, OwnerReferences: []metav1.OwnerReference{{ APIVersion: "tekton.dev/v1beta1", diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index bb915008214..4b9e8d2aff9 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -339,6 +339,15 @@ func ValidateServiceaccountMapping(p *v1beta1.PipelineSpec, pr *v1beta1.Pipeline return nil } +func isCustomTask(ctx context.Context, rprt ResolvedPipelineRunTask) bool { + isTaskRefCustomTask := rprt.PipelineTask.TaskRef != nil && rprt.PipelineTask.TaskRef.APIVersion != "" && + rprt.PipelineTask.TaskRef.Kind != "" + isTaskSpecCustomTask := rprt.PipelineTask.TaskSpec != nil && rprt.PipelineTask.TaskSpec.APIVersion != "" && + rprt.PipelineTask.TaskSpec.Kind != "" + cfg := config.FromContextOrDefaults(ctx) + return cfg.FeatureFlags.EnableCustomTasks && (isTaskRefCustomTask || isTaskSpecCustomTask) +} + // ResolvePipelineRunTask retrieves a single Task's instance using the getTask to fetch // the spec. If it is unable to retrieve an instance of a referenced Task, it will return // an error, otherwise it returns a list of all of the Tasks retrieved. It will retrieve @@ -357,11 +366,7 @@ func ResolvePipelineRunTask( rprt := ResolvedPipelineRunTask{ PipelineTask: &task, } - - cfg := config.FromContextOrDefaults(ctx) - rprt.CustomTask = cfg.FeatureFlags.EnableCustomTasks && rprt.PipelineTask.TaskRef != nil && - rprt.PipelineTask.TaskRef.APIVersion != "" && rprt.PipelineTask.TaskRef.Kind != "" - + rprt.CustomTask = isCustomTask(ctx, rprt) if rprt.IsCustomTask() { rprt.RunName = getRunName(pipelineRun.Status.Runs, task.Name, pipelineRun.Name) run, err := getRun(rprt.RunName) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index d287ea61742..2791c95a1e9 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -37,6 +37,7 @@ import ( corev1 "k8s.io/api/core/v1" kerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/selection" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" @@ -1199,6 +1200,14 @@ func TestResolvePipelineRun_CustomTask(t *testing.T) { pts := []v1beta1.PipelineTask{{ Name: "customtask", TaskRef: &v1beta1.TaskRef{APIVersion: "example.dev/v0", Kind: "Example"}, + }, { + Name: "customtask-spec", + TaskSpec: &v1beta1.EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example.dev/v0", + Kind: "Example", + }, + }, }, { Name: "run-exists", TaskRef: &v1beta1.TaskRef{APIVersion: "example.dev/v0", Kind: "Example"}, @@ -1208,7 +1217,7 @@ func TestResolvePipelineRun_CustomTask(t *testing.T) { } run := &v1alpha1.Run{ObjectMeta: metav1.ObjectMeta{Name: "run-exists-abcde"}} getRun := func(name string) (*v1alpha1.Run, error) { - if name == "pipelinerun-run-exists-mz4c7" { + if name == "pipelinerun-run-exists-mssqb" { return run, nil } return nil, kerrors.NewNotFound(v1beta1.Resource("run"), name) @@ -1240,7 +1249,12 @@ func TestResolvePipelineRun_CustomTask(t *testing.T) { }, { PipelineTask: &pts[1], CustomTask: true, - RunName: "pipelinerun-run-exists-mz4c7", + RunName: "pipelinerun-customtask-spec-mz4c7", + Run: nil, + }, { + PipelineTask: &pts[2], + CustomTask: true, + RunName: "pipelinerun-run-exists-mssqb", Run: run, }} if d := cmp.Diff(expectedState, pipelineState); d != "" { @@ -2165,7 +2179,40 @@ func TestIsCustomTask(t *testing.T) { pt v1beta1.PipelineTask want bool }{{ - name: "custom", + name: "custom taskSpec", + pt: v1beta1.PipelineTask{ + TaskSpec: &v1beta1.EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example.dev/v0", + Kind: "Sample", + }, + }, + }, + want: true, + }, { + name: "custom taskSpec missing kind", + pt: v1beta1.PipelineTask{ + TaskSpec: &v1beta1.EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "example.dev/v0", + Kind: "", + }, + }, + }, + want: false, + }, { + name: "custom taskSpec missing apiVersion", + pt: v1beta1.PipelineTask{ + TaskSpec: &v1beta1.EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: "", + Kind: "Sample", + }, + }, + }, + want: false, + }, { + name: "custom taskRef", pt: v1beta1.PipelineTask{ TaskRef: &v1beta1.TaskRef{ APIVersion: "example.dev/v0", @@ -2173,6 +2220,24 @@ func TestIsCustomTask(t *testing.T) { }, }, want: true, + }, { + name: "custom taskRef missing kind", + pt: v1beta1.PipelineTask{ + TaskRef: &v1beta1.TaskRef{ + APIVersion: "example.dev/v0", + Kind: "", + }, + }, + want: false, + }, { + name: "custom taskRef missing apiVersion", + pt: v1beta1.PipelineTask{ + TaskRef: &v1beta1.TaskRef{ + APIVersion: "", + Kind: "Sample", + }, + }, + want: false, }, { name: "non-custom taskref", pt: v1beta1.PipelineTask{ diff --git a/test/custom_task_test.go b/test/custom_task_test.go index ac76f818b86..6e6b93e06ed 100644 --- a/test/custom_task_test.go +++ b/test/custom_task_test.go @@ -66,6 +66,19 @@ func TestCustomTask(t *testing.T) { APIVersion: apiVersion, Kind: kind, }, + }, { + Name: "custom-task-spec", + TaskSpec: &v1beta1.EmbeddedTask{ + EmbeddedSpec: v1beta1.EmbeddedSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: apiVersion, + Kind: kind, + }, + Spec: runtime.RawExtension{ + Raw: customTaskRawSpec, + }, + }, + }, }, { Name: "result-consumer", Params: []v1beta1.Param{{ From dcdfe076efdfb19f42e7d2fc42b020e39b189f0a Mon Sep 17 00:00:00 2001 From: Prashant Sharma Date: Sat, 1 May 2021 14:01:49 +0530 Subject: [PATCH 2/4] TEP-0061, Added documentation for embedding spec in custom-task --- docs/pipelines.md | 25 +++++++++ docs/runs.md | 56 ++++++++++++++++++- .../pipelinerun/pipelinerun_test.go | 2 + 3 files changed, 82 insertions(+), 1 deletion(-) diff --git a/docs/pipelines.md b/docs/pipelines.md index 13f4d14e040..e8c30fdc08d 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -1179,6 +1179,31 @@ If the `taskRef` specifies a name, the custom task controller should look up the If the `taskRef` does not specify a name, the custom task controller might support some default behavior for executing unnamed tasks. +### Specifying a Custom Task Spec in-line (or embedded) + +```yaml +spec: + tasks: + - name: run-custom-task + taskSpec: + apiVersion: example.dev/v1alpha1 + kind: Example + spec: + field1: value1 + field2: value2 +``` + +If the custom task controller supports the in-line or embedded task spec, this will create a `Run` of a custom task of +type `Example` in the `example.dev` API group with the version `v1alpha1`. + +If the `taskSpec` is not supported, the custom task controller should produce proper validation errors. + +Please take a look at the +[developer guide for custom controllers supporting `taskSpec`.](runs.md#developer-guide-for-custom-controllers-supporting-spec) + +`taskSpec` support for `pipelineRun` was designed and discussed in +[TEP-0061](https://github.com/tektoncd/community/blob/main/teps/0061-allow-custom-task-to-be-embedded-in-pipeline.md) + ### Specifying parameters If a custom task supports [`parameters`](tasks.md#parameters), you can use the diff --git a/docs/runs.md b/docs/runs.md index daea2d8fd7b..d4df1efe441 100644 --- a/docs/runs.md +++ b/docs/runs.md @@ -47,8 +47,10 @@ A `Run` definition supports the following fields: - [`metadata`][kubernetes-overview] - Specifies the metadata that uniquely identifies the `Run`, such as a `name`. - [`spec`][kubernetes-overview] - Specifies the configuration for the `Run`. - - [`ref`](#specifying-the-target-custom-task) - Specifies the type and + - [`ref`](#1-specifying-the-target-custom-task-with-ref) - Specifies the type and (optionally) name of the custom task type to execute. + - [`spec`](#2-specifying-the-target-custom-task-by-embedding-its-spec) - Embed the custom task resource spec + directly in a `Run`. - Optional: - [`params`](#specifying-parameters) - Specifies the desired execution parameters for the custom task. @@ -64,6 +66,19 @@ A `Run` definition supports the following fields: ### Specifying the target Custom Task +A custom task resource's `Spec` may be directly embedded in the `Run` or it may +be referred to by a `Ref`. But, not both at the same time. + +1. [Specifying the target Custom Task with ref](#1-specifying-the-target-custom-task-with-ref) + Referring a custom task (i.e. `Ref` ) promotes reuse of custom task definitions. + +2. [Specifying the target Custom Task by embedding its spec](#2-specifying-the-target-custom-task-by-embedding-its-spec) + Embedding a custom task (i.e. `Spec` ) helps in avoiding name collisions with other users within the same namespace. + Additionally, in a pipeline with multiple embedded custom tasks, the details of entire pipeline can be fetched in a + single API request. + +### 1. Specifying the target Custom Task with ref + To specify the custom task type you want to execute in your `Run`, use the `ref` field as shown below: @@ -99,6 +114,45 @@ In either case, if the named resource cannot be found, or if unnamed tasks are not supported, the custom task controller should update the `Run`'s status to indicate the error. +### 2. Specifying the target Custom Task by embedding its spec + +To specify the custom task spec, it can be embedded directly into a +`Run`'s spec as shown below: + +```yaml +apiVersion: tekton.dev/v1alpha1 +kind: Run +metadata: + name: embedded-run +spec: + spec: + apiVersion: example.dev/v1alpha1 + kind: Example + spec: + field1: value1 + field2: value2 +``` + +This initiates the execution of a `Run` of a custom task of type `Example`, in +the `example.dev` API group, with the version `v1alpha1`. + +#### Developer guide for custom controllers supporting `spec`. + +1. A custom controller may or may not support a `Spec`. In cases where it is + not supported the custom controller should respond with proper validation error. + +2. Validation of the fields of the custom task is delegated to the custom task controller. It is recommended to + implement validations as asynchronous + (i.e. at reconcile time), rather than part of the webhook. Using a webhook for validation is problematic because, it + is not possible to filter custom task resource objects before validation step, as a result each custom task resource + has to undergo validation by all the installed custom task controllers. + +3. A custom task may have an empty spec, but cannot have an empty + `ApiVersion` and `Kind`. Custom task controllers should handle + an empty spec, either with a default behaviour, in a case no default + behaviour is supported then, appropriate validation error should be + updated to the `Run`'s status. + ### Specifying `Parameters` If a custom task supports [`parameters`](tasks.md#parameters), you can use the diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 8863eada046..a1d429b4093 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -542,6 +542,7 @@ func TestReconcile_CustomTask(t *testing.T) { APIVersion: "example.dev/v0", Kind: "Example", }, + Metadata: v1beta1.PipelineTaskMetadata{Labels: map[string]string{"test-label": "test"}}, Spec: runtime.RawExtension{ Raw: []byte(`{"field1":123,"field2":"value"}`), }, @@ -578,6 +579,7 @@ func TestReconcile_CustomTask(t *testing.T) { APIVersion: "example.dev/v0", Kind: "Example", }, + Metadata: v1beta1.PipelineTaskMetadata{Labels: map[string]string{"test-label": "test"}}, Spec: runtime.RawExtension{ Raw: []byte(`{"field1":123,"field2":"value"}`), }, From 239f98c8ac00ba18e1e4407d8c24425b84f3df1b Mon Sep 17 00:00:00 2001 From: Prashant Sharma Date: Mon, 3 May 2021 13:32:09 +0530 Subject: [PATCH 3/4] TEP-0061, updated custom-task e2e to test embedded custom task --- test/custom_task_test.go | 149 ++++++++++++++++++++++----------------- 1 file changed, 84 insertions(+), 65 deletions(-) diff --git a/test/custom_task_test.go b/test/custom_task_test.go index 6e6b93e06ed..7cf8c5dfbeb 100644 --- a/test/custom_task_test.go +++ b/test/custom_task_test.go @@ -20,6 +20,7 @@ package test import ( "context" + "strings" "testing" "time" @@ -29,6 +30,7 @@ import ( "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "knative.dev/pkg/apis" duckv1 "knative.dev/pkg/apis/duck/v1" knativetest "knative.dev/pkg/test" @@ -51,7 +53,8 @@ func TestCustomTask(t *testing.T) { c, namespace := setup(ctx, t, requireAnyGate(supportedFeatureGates)) knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) defer tearDown(ctx, t, c, namespace) - + customTaskRawSpec := []byte(`{"field1":123,"field2":"value"}`) + metadataLabel := map[string]string{"test-label": "test"} // Create a PipelineRun that runs a Custom Task. pipelineRunName := "custom-task-pipeline" if _, err := c.PipelineRunClient.Create( @@ -61,7 +64,7 @@ func TestCustomTask(t *testing.T) { Spec: v1beta1.PipelineRunSpec{ PipelineSpec: &v1beta1.PipelineSpec{ Tasks: []v1beta1.PipelineTask{{ - Name: "custom-task", + Name: "custom-task-ref", TaskRef: &v1beta1.TaskRef{ APIVersion: apiVersion, Kind: kind, @@ -69,35 +72,41 @@ func TestCustomTask(t *testing.T) { }, { Name: "custom-task-spec", TaskSpec: &v1beta1.EmbeddedTask{ - EmbeddedSpec: v1beta1.EmbeddedSpec{ - TypeMeta: runtime.TypeMeta{ - APIVersion: apiVersion, - Kind: kind, - }, - Spec: runtime.RawExtension{ - Raw: customTaskRawSpec, - }, + TypeMeta: runtime.TypeMeta{ + APIVersion: apiVersion, + Kind: kind, + }, + Metadata: v1beta1.PipelineTaskMetadata{Labels: metadataLabel}, + Spec: runtime.RawExtension{ + Raw: customTaskRawSpec, }, }, }, { Name: "result-consumer", Params: []v1beta1.Param{{ - Name: "input-result-from-custom-task", Value: *v1beta1.NewArrayOrString("$(tasks.custom-task.results.runResult)"), + Name: "input-result-from-custom-task-ref", Value: *v1beta1.NewArrayOrString("$(tasks.custom-task-ref.results.runResult)"), + }, { + Name: "input-result-from-custom-task-spec", Value: *v1beta1.NewArrayOrString("$(tasks.custom-task-spec.results.runResult)"), }}, TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: v1beta1.TaskSpec{ Params: []v1beta1.ParamSpec{{ - Name: "input-result-from-custom-task", Type: v1beta1.ParamTypeString, + Name: "input-result-from-custom-task-ref", Type: v1beta1.ParamTypeString, + }, { + Name: "input-result-from-custom-task-spec", Type: v1beta1.ParamTypeString, }}, Steps: []v1beta1.Step{{Container: corev1.Container{ Image: "ubuntu", Command: []string{"/bin/bash"}, - Args: []string{"-c", "echo $(input-result-from-custom-task)"}, + Args: []string{"-c", "echo $(input-result-from-custom-task-ref) $(input-result-from-custom-task-spec)"}, }}}, }}, }}, Results: []v1beta1.PipelineResult{{ - Name: "prResult", - Value: "$(tasks.custom-task.results.runResult)", + Name: "prResult-ref", + Value: "$(tasks.custom-task-ref.results.runResult)", + }, { + Name: "prResult-spec", + Value: "$(tasks.custom-task-spec.results.runResult)", }}, }, }, @@ -118,53 +127,58 @@ func TestCustomTask(t *testing.T) { } // Get the Run name. - if len(pr.Status.Runs) != 1 { - t.Fatalf("PipelineRun had unexpected .status.runs; got %d, want 1", len(pr.Status.Runs)) - } - var runName string - for k := range pr.Status.Runs { - runName = k - break - } - - // Get the Run. - r, err := c.RunClient.Get(ctx, runName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Failed to get Run %q: %v", runName, err) - } - if r.IsDone() { - t.Fatalf("Run unexpectedly done: %v", r.Status.GetCondition(apis.ConditionSucceeded)) - } - - // Simulate a Custom Task controller updating the Run to done/successful. - r.Status = v1alpha1.RunStatus{ - Status: duckv1.Status{ - Conditions: duckv1.Conditions{{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, - }}, - }, - RunStatusFields: v1alpha1.RunStatusFields{ - Results: []v1alpha1.RunResult{{ - Name: "runResult", - Value: "aResultValue", - }}, - }, - } - - if _, err := c.RunClient.UpdateStatus(ctx, r, metav1.UpdateOptions{}); err != nil { - t.Fatalf("Failed to update Run to successful: %v", err) - } - - // Get the Run. - r, err = c.RunClient.Get(ctx, runName, metav1.GetOptions{}) - if err != nil { - t.Fatalf("Failed to get Run %q: %v", runName, err) - } - if !r.IsDone() { - t.Fatalf("Run unexpectedly not done after update (UpdateStatus didn't work): %v", r.Status) + if len(pr.Status.Runs) != 2 { + t.Fatalf("PipelineRun had unexpected .status.runs; got %d, want 2", len(pr.Status.Runs)) + } + + for runName := range pr.Status.Runs { + // Get the Run. + r, err := c.RunClient.Get(ctx, runName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failed to get Run %q: %v", runName, err) + } + if r.IsDone() { + t.Fatalf("Run unexpectedly done: %v", r.Status.GetCondition(apis.ConditionSucceeded)) + } + + // Simulate a Custom Task controller updating the Run to done/successful. + r.Status = v1alpha1.RunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + }}, + }, + RunStatusFields: v1alpha1.RunStatusFields{ + Results: []v1alpha1.RunResult{{ + Name: "runResult", + Value: "aResultValue", + }}, + }, + } + + if _, err := c.RunClient.UpdateStatus(ctx, r, metav1.UpdateOptions{}); err != nil { + t.Fatalf("Failed to update Run to successful: %v", err) + } + + // Get the Run. + r, err = c.RunClient.Get(ctx, runName, metav1.GetOptions{}) + + if strings.Contains(runName, "custom-task-spec") { + if d := cmp.Diff(customTaskRawSpec, r.Spec.Spec.Spec.Raw); d != "" { + t.Fatalf("Unexpected value of Spec.Raw: %s", diff.PrintWantGot(d)) + } + if d := cmp.Diff(metadataLabel, r.Spec.Spec.Metadata.Labels); d != "" { + t.Fatalf("Unexpected value of Metadata.Labels: %s", diff.PrintWantGot(d)) + } + } + if err != nil { + t.Fatalf("Failed to get Run %q: %v", runName, err) + } + if !r.IsDone() { + t.Fatalf("Run unexpectedly not done after update (UpdateStatus didn't work): %v", r.Status) + } } - // Wait for the PipelineRun to become done/successful. if err := WaitForPipelineRunState(ctx, c, pipelineRunName, time.Minute, PipelineRunSucceed(pipelineRunName), "PipelineRunCompleted"); err != nil { t.Fatalf("Waiting for PipelineRun to complete successfully: %v", err) @@ -194,7 +208,9 @@ func TestCustomTask(t *testing.T) { // Validate the task's result reference to the custom task's result was resolved. expectedTaskRunParams := []v1beta1.Param{{ - Name: "input-result-from-custom-task", Value: *v1beta1.NewArrayOrString("aResultValue"), + Name: "input-result-from-custom-task-ref", Value: *v1beta1.NewArrayOrString("aResultValue"), + }, { + Name: "input-result-from-custom-task-spec", Value: *v1beta1.NewArrayOrString("aResultValue"), }} if d := cmp.Diff(expectedTaskRunParams, taskRun.Spec.Params); d != "" { @@ -204,12 +220,15 @@ func TestCustomTask(t *testing.T) { // Validate that the pipeline's result reference to the custom task's result was resolved. expectedPipelineResults := []v1beta1.PipelineRunResult{{ - Name: "prResult", + Name: "prResult-ref", + Value: "aResultValue", + }, { + Name: "prResult-spec", Value: "aResultValue", }} - if len(pr.Status.PipelineResults) == 0 { - t.Fatalf("Expected PipelineResults but there are none") + if len(pr.Status.PipelineResults) != 2 { + t.Fatalf("Expected 2 PipelineResults but there are %d.", len(pr.Status.PipelineResults)) } if d := cmp.Diff(expectedPipelineResults, pr.Status.PipelineResults); d != "" { t.Fatalf("Unexpected PipelineResults: %s", diff.PrintWantGot(d)) From 4e7a9ffb4bc8891577319c586ace35afc353d1df Mon Sep 17 00:00:00 2001 From: Prashant Sharma Date: Tue, 25 May 2021 14:09:07 +0530 Subject: [PATCH 4/4] Ran hack/update-codegen.sh --- .../v1alpha1/zz_generated.deepcopy.go | 24 +++++++++++++++++++ .../pipeline/v1beta1/openapi_generated.go | 21 +++++++++++++++- pkg/apis/pipeline/v1beta1/swagger.json | 11 +++++++++ .../pipeline/v1beta1/zz_generated.deepcopy.go | 2 ++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index c5e48955b6d..a9c35363f31 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -197,6 +197,25 @@ func (in *ConditionSpec) DeepCopy() *ConditionSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EmbeddedRunSpec) DeepCopyInto(out *EmbeddedRunSpec) { + *out = *in + out.TypeMeta = in.TypeMeta + in.Metadata.DeepCopyInto(&out.Metadata) + in.Spec.DeepCopyInto(&out.Spec) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EmbeddedRunSpec. +func (in *EmbeddedRunSpec) DeepCopy() *EmbeddedRunSpec { + if in == nil { + return nil + } + out := new(EmbeddedRunSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Inputs) DeepCopyInto(out *Inputs) { *out = *in @@ -679,6 +698,11 @@ func (in *RunSpec) DeepCopyInto(out *RunSpec) { *out = new(v1beta1.TaskRef) **out = **in } + if in.Spec != nil { + in, out := &in.Spec, &out.Spec + *out = new(EmbeddedRunSpec) + (*in).DeepCopyInto(*out) + } if in.Params != nil { in, out := &in.Params, &out.Params *out = make([]v1beta1.Param, len(*in)) diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 8b47e4b412a..5f0f5783413 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -687,6 +687,25 @@ func schema_pkg_apis_pipeline_v1beta1_EmbeddedTask(ref common.ReferenceCallback) SchemaProps: spec.SchemaProps{ Type: []string{"object"}, Properties: map[string]spec.Schema{ + "apiVersion": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "kind": { + SchemaProps: spec.SchemaProps{ + Type: []string{"string"}, + Format: "", + }, + }, + "spec": { + SchemaProps: spec.SchemaProps{ + Description: "Spec is a specification of a custom task", + Default: map[string]interface{}{}, + Ref: ref("k8s.io/apimachinery/pkg/runtime.RawExtension"), + }, + }, "metadata": { SchemaProps: spec.SchemaProps{ Default: map[string]interface{}{}, @@ -800,7 +819,7 @@ func schema_pkg_apis_pipeline_v1beta1_EmbeddedTask(ref common.ReferenceCallback) }, }, Dependencies: []string{ - "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ParamSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineTaskMetadata", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Sidecar", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Step", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResources", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResult", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WorkspaceDeclaration", "k8s.io/api/core/v1.Container", "k8s.io/api/core/v1.Volume"}, + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ParamSpec", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineTaskMetadata", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Sidecar", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.Step", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResources", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskResult", "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.WorkspaceDeclaration", "k8s.io/api/core/v1.Container", "k8s.io/api/core/v1.Volume", "k8s.io/apimachinery/pkg/runtime.RawExtension"}, } } diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 3eec4b4948b..14b6b0efe9d 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -505,10 +505,16 @@ "v1beta1.EmbeddedTask": { "type": "object", "properties": { + "apiVersion": { + "type": "string" + }, "description": { "description": "Description is a user-facing description of the task that may be used to populate a UI.", "type": "string" }, + "kind": { + "type": "string" + }, "metadata": { "default": {}, "$ref": "#/definitions/v1beta1.PipelineTaskMetadata" @@ -541,6 +547,11 @@ "$ref": "#/definitions/v1beta1.Sidecar" } }, + "spec": { + "description": "Spec is a specification of a custom task", + "default": {}, + "$ref": "#/definitions/k8s.io.apimachinery.pkg.runtime.RawExtension" + }, "stepTemplate": { "description": "StepTemplate can be used as the basis for all step containers within the Task, so that the steps inherit settings on the base container.", "$ref": "#/definitions/v1.Container" diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 3a7d13b84a2..66c65a2ef44 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -229,6 +229,8 @@ func (in *ConditionCheckStatusFields) DeepCopy() *ConditionCheckStatusFields { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *EmbeddedTask) DeepCopyInto(out *EmbeddedTask) { *out = *in + out.TypeMeta = in.TypeMeta + in.Spec.DeepCopyInto(&out.Spec) in.Metadata.DeepCopyInto(&out.Metadata) in.TaskSpec.DeepCopyInto(&out.TaskSpec) return