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 a new field i.e. 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 28, 2021
1 parent 3dc52df commit 716d12a
Show file tree
Hide file tree
Showing 11 changed files with 462 additions and 31 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.ErrMissingField("spec.ref or 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
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 716d12a

Please sign in to comment.