Skip to content

Commit

Permalink
TEP-0061 (1/3), Allow custom task to be embedded.
Browse files Browse the repository at this point in the history
    This is first in the three part series of PRs for implementing the TEP-0061.
    #Changes

   1. API changes, This PR adds new APIs i.e. adds a field `Spec *v1beta1.EmbeddedTask` to `RunSpec`
   2. An embedded task will accept 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.
   4. Unit tests to cover the newly added fields and above mentioned validations.
  • Loading branch information
ScrapCodes committed Apr 29, 2021
1 parent 3dc52df commit d06d8dd
Show file tree
Hide file tree
Showing 12 changed files with 499 additions and 35 deletions.
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/run_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ type RunSpec struct {
// +optional
Ref *TaskRef `json:"ref,omitempty"`

// Spec is a specification of a custom task
// +optional
Spec *v1beta1.EmbeddedTask `json:"spec,omitempty"`

// +optional
Params []v1beta1.Param `json:"params,omitempty"`

Expand Down
23 changes: 16 additions & 7 deletions pkg/apis/pipeline/v1alpha1/run_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,25 @@ func (rs *RunSpec) Validate(ctx context.Context) *apis.FieldError {
return apis.ErrMissingField("spec")
}

if rs.Ref == nil {
return apis.ErrMissingField("spec.ref")
if rs.Ref == nil && rs.Spec == nil {
return apis.ErrMissingOneOf("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
}
Expand Down
41 changes: 37 additions & 4 deletions pkg/apis/pipeline/v1alpha1/run_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha1_test

import (
"context"
"k8s.io/apimachinery/pkg/runtime"
"testing"

"github.com/google/go-cmp/cmp"
Expand All @@ -43,18 +44,19 @@ 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: "missing apiVersion in Ref",
run: &v1alpha1.Run{
ObjectMeta: metav1.ObjectMeta{
Name: "temp",
Expand All @@ -67,7 +69,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",
Expand All @@ -80,6 +82,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: &v1beta1.EmbeddedTask{
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: &v1beta1.EmbeddedTask{
TypeMeta: runtime.TypeMeta{
APIVersion: "apiVersion",
Kind: "",
},
},
},
},
want: apis.ErrMissingField("spec.spec.kind"),
}, {
name: "non-unique params",
run: &v1alpha1.Run{
Expand Down
21 changes: 20 additions & 1 deletion pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -109,6 +110,12 @@ type PipelineTaskMetadata struct {
}

type EmbeddedTask struct {
// +optional
runtime.TypeMeta `json:",inline,omitempty"`

// +optional
Spec runtime.RawExtension `json:"spec,omitempty"`

// +optional
Metadata PipelineTaskMetadata `json:"metadata,omitempty"`

Expand Down Expand Up @@ -185,9 +192,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 {
Expand Down Expand Up @@ -274,6 +291,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())
Expand Down
34 changes: 34 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1beta1

import (
"context"
"k8s.io/apimachinery/pkg/runtime"
"testing"
"time"

Expand Down Expand Up @@ -139,6 +140,39 @@ 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{
Expand Down
20 changes: 20 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1beta1

import (
"context"
"k8s.io/apimachinery/pkg/runtime"
"testing"
"time"

Expand Down Expand Up @@ -54,6 +55,25 @@ 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{
Expand Down
25 changes: 20 additions & 5 deletions pkg/controller/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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
}
}
Loading

0 comments on commit d06d8dd

Please sign in to comment.