Skip to content

Commit

Permalink
Add TaskSpec to taskRun to allow users to run task with one file
Browse files Browse the repository at this point in the history
- users can specifiy TaskRef or TaskSpec, but not both
- update type
- add validation and tests
- update taskRef everywhere to be a pointer since its now optional

Signed-off-by: Nader Ziada <[email protected]>
  • Loading branch information
nader-ziada committed Nov 22, 2018
1 parent 9aa252b commit 4af81da
Show file tree
Hide file tree
Showing 12 changed files with 136 additions and 37 deletions.
6 changes: 5 additions & 1 deletion pkg/apis/pipeline/v1alpha1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ var _ webhook.GenericCRD = (*TaskRun)(nil)

// TaskRunSpec defines the desired state of TaskRun
type TaskRunSpec struct {
TaskRef TaskRef `json:"taskRef"`
Trigger TaskTrigger `json:"trigger"`
// +optional
Inputs TaskRunInputs `json:"inputs,omitempty"`
Expand All @@ -44,6 +43,11 @@ type TaskRunSpec struct {
Generation int64 `json:"generation,omitempty"`
// +optional
ServiceAccount string `json:"serviceAccount"`
// no more than one of the TaskRef and TaskSpec may be specified.
// +optional
TaskRef *TaskRef `json:"taskRef"`
//+optional
TaskSpec *TaskSpec `json:"taskSpec"`
}

// TaskRunInputs holds the input values that this task was invoked with.
Expand Down
12 changes: 9 additions & 3 deletions pkg/apis/pipeline/v1alpha1/taskrun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,15 @@ func (ts *TaskRunSpec) Validate() *apis.FieldError {
return apis.ErrMissingField("spec")
}

// Check for TaskRef
if ts.TaskRef.Name == "" {
return apis.ErrMissingField("spec.taskref.name")
// can't have both taskRef and taskSpec at the same time
if (ts.TaskRef != nil && ts.TaskRef.Name != "") && ts.TaskSpec != nil {
return apis.ErrDisallowedFields("spec.taskSpec")
}

// Check that one of TaskRef and TaskSpec is present
if (ts.TaskRef == nil || (ts.TaskRef != nil && ts.TaskRef.Name == "")) && ts.TaskSpec == nil {
fields := []string{"spec.taskref.name", "spec.taskspec"}
return apis.ErrMissingField(fields...)
}

// Check for Trigger
Expand Down
52 changes: 45 additions & 7 deletions pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/knative/pkg/apis"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -53,7 +54,7 @@ func TestTaskRun_Validate(t *testing.T) {
Name: "taskname",
},
Spec: TaskRunSpec{
TaskRef: TaskRef{
TaskRef: &TaskRef{
Name: "taskrefname",
},
Trigger: TaskTrigger{
Expand Down Expand Up @@ -83,19 +84,19 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
{
name: "invalid taskref name",
spec: TaskRunSpec{
TaskRef: TaskRef{},
TaskRef: &TaskRef{},
Trigger: TaskTrigger{
TriggerRef: TaskTriggerRef{
Type: TaskTriggerTypeManual,
},
},
},
wantErr: apis.ErrMissingField("spec.taskref.name"),
wantErr: apis.ErrMissingField("spec.taskref.name, spec.taskspec"),
},
{
name: "invalid taskref type",
spec: TaskRunSpec{
TaskRef: TaskRef{
TaskRef: &TaskRef{
Name: "taskrefname",
},
Trigger: TaskTrigger{
Expand All @@ -109,7 +110,7 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
{
name: "invalid taskref",
spec: TaskRunSpec{
TaskRef: TaskRef{
TaskRef: &TaskRef{
Name: "taskrefname",
},
Trigger: TaskTrigger{
Expand All @@ -121,6 +122,26 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
},
wantErr: apis.ErrMissingField("spec.trigger.triggerref.name"),
},
{
name: "invalid taskref and taskspec together",
spec: TaskRunSpec{
TaskRef: &TaskRef{
Name: "taskrefname",
},
TaskSpec: &TaskSpec{
Steps: []corev1.Container{{
Name: "mystep",
Image: "myimage",
}},
},
Trigger: TaskTrigger{
TriggerRef: TaskTriggerRef{
Type: "manual",
},
},
},
wantErr: apis.ErrDisallowedFields("spec.taskSpec"),
},
}

for _, ts := range tests {
Expand All @@ -141,7 +162,7 @@ func TestTaskRunSpec_Validate(t *testing.T) {
{
name: "task trigger run type",
spec: TaskRunSpec{
TaskRef: TaskRef{
TaskRef: &TaskRef{
Name: "taskrefname",
},
Trigger: TaskTrigger{
Expand All @@ -155,7 +176,7 @@ func TestTaskRunSpec_Validate(t *testing.T) {
{
name: "task trigger run type with different capitalization",
spec: TaskRunSpec{
TaskRef: TaskRef{
TaskRef: &TaskRef{
Name: "taskrefname",
},
Trigger: TaskTrigger{
Expand All @@ -166,6 +187,23 @@ func TestTaskRunSpec_Validate(t *testing.T) {
},
},
},
{
name: "taskspec without a taskRef",
spec: TaskRunSpec{
TaskSpec: &TaskSpec{
Steps: []corev1.Container{{
Name: "mystep",
Image: "myimage",
}},
},
Trigger: TaskTrigger{
TriggerRef: TaskTriggerRef{
Type: "PiPeLiNeRuN",
Name: "testtrigger",
},
},
},
},
}

for _, ts := range tests {
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ func (c *Reconciler) createTaskRun(t *v1alpha1.Task, trName string, pr *v1alpha1
},
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: t.Name,
},
Inputs: v1alpha1.TaskRunInputs{
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func TestReconcile(t *testing.T) {
},
Spec: v1alpha1.TaskRunSpec{
ServiceAccount: "test-sa",
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "unit-test-task",
},
Inputs: v1alpha1.TaskRunInputs{
Expand Down
59 changes: 55 additions & 4 deletions pkg/reconciler/v1alpha1/taskrun/resources/input_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func TestAddResourceToBuild(t *testing.T) {
Namespace: "marshmallow",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "simpleTask",
},
Inputs: v1alpha1.TaskRunInputs{
Expand Down Expand Up @@ -214,7 +214,58 @@ func TestAddResourceToBuild(t *testing.T) {
Namespace: "marshmallow",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "simpleTask",
},
Inputs: v1alpha1.TaskRunInputs{

Resources: []v1alpha1.TaskRunResource{{
ResourceRef: v1alpha1.PipelineResourceRef{
Name: "the-git",
},
Name: "workspace",
}},
},
},
},
build: build(),
wantErr: false,
want: &buildv1alpha1.Build{
TypeMeta: metav1.TypeMeta{
Kind: "Build",
APIVersion: "build.knative.dev/v1alpha1"},
ObjectMeta: metav1.ObjectMeta{
Name: "build-from-repo",
Namespace: "marshmallow",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "pipeline.knative.dev/v1alpha1",
Kind: "TaskRun",
Name: "build-from-repo-run",
Controller: &boolTrue,
BlockOwnerDeletion: &boolTrue,
},
},
},
Spec: buildv1alpha1.BuildSpec{
Source: &buildv1alpha1.SourceSpec{
Git: &buildv1alpha1.GitSourceSpec{
Url: "https://github.com/grafeas/kritis",
Revision: "branch",
},
},
},
},
}, {
desc: "set revision to default value",
task: task,
taskRun: &v1alpha1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "build-from-repo-run",
Namespace: "marshmallow",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: &v1alpha1.TaskRef{
Name: "simpleTask",
},
Inputs: v1alpha1.TaskRunInputs{
Expand Down Expand Up @@ -301,7 +352,7 @@ func TestAddResourceToBuild(t *testing.T) {
Namespace: "marshmallow",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "build-from-repo",
},
Inputs: v1alpha1.TaskRunInputs{
Expand Down Expand Up @@ -367,7 +418,7 @@ func TestAddResourceToBuild(t *testing.T) {
Namespace: "marshmallow",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "build-from-repo",
},
Inputs: v1alpha1.TaskRunInputs{
Expand Down
16 changes: 8 additions & 8 deletions pkg/reconciler/v1alpha1/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func TestReconcile(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: simpleTask.Name,
APIVersion: "a1",
},
Expand All @@ -242,7 +242,7 @@ func TestReconcile(t *testing.T) {
},
Spec: v1alpha1.TaskRunSpec{
ServiceAccount: "test-sa",
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: saTask.Name,
APIVersion: "a1",
},
Expand All @@ -253,7 +253,7 @@ func TestReconcile(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: templatedTask.Name,
APIVersion: "a1",
},
Expand Down Expand Up @@ -290,7 +290,7 @@ func TestReconcile(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: defaultTemplatedTask.Name,
APIVersion: "a1",
},
Expand Down Expand Up @@ -318,7 +318,7 @@ func TestReconcile(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: defaultTemplatedTask.Name,
APIVersion: "a1",
},
Expand Down Expand Up @@ -591,7 +591,7 @@ func TestReconcile_InvalidTaskRuns(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "notask",
APIVersion: "a1",
},
Expand Down Expand Up @@ -652,7 +652,7 @@ func TestReconcileBuildFetchError(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "test-task",
APIVersion: "a1",
},
Expand Down Expand Up @@ -689,7 +689,7 @@ func TestReconcileBuildUpdateStatus(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "test-task",
APIVersion: "a1",
},
Expand Down
14 changes: 7 additions & 7 deletions pkg/reconciler/v1alpha1/taskrun/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func Test_ValidTaskRunTask(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "task-valid-input",
},
Inputs: v1alpha1.TaskRunInputs{
Expand Down Expand Up @@ -111,7 +111,7 @@ func Test_InvalidTaskRunTask(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "unit-test-task",
},
Inputs: v1alpha1.TaskRunInputs{
Expand All @@ -131,7 +131,7 @@ func Test_InvalidTaskRunTask(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "unit-test-task",
},
Outputs: v1alpha1.TaskRunOutputs{
Expand All @@ -151,7 +151,7 @@ func Test_InvalidTaskRunTask(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "unit-test-wrong-input",
},
Inputs: v1alpha1.TaskRunInputs{
Expand All @@ -171,7 +171,7 @@ func Test_InvalidTaskRunTask(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "unit-test-task",
},
Outputs: v1alpha1.TaskRunOutputs{
Expand All @@ -191,7 +191,7 @@ func Test_InvalidTaskRunTask(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "unit-task-multiple-params",
},
Inputs: v1alpha1.TaskRunInputs{
Expand All @@ -209,7 +209,7 @@ func Test_InvalidTaskRunTask(t *testing.T) {
Namespace: "foo",
},
Spec: v1alpha1.TaskRunSpec{
TaskRef: v1alpha1.TaskRef{
TaskRef: &v1alpha1.TaskRef{
Name: "unit-task-bad-resourcetype",
},
Inputs: v1alpha1.TaskRunInputs{
Expand Down
Loading

0 comments on commit 4af81da

Please sign in to comment.