Skip to content

Commit

Permalink
Fail taskrun when results validation fails
Browse files Browse the repository at this point in the history
This commit adds the ReasonFailedValidation into taskrun status and fail
the taskrun when results validation fails. Before this commit users can
only see error messages in logs but the taskrun are completed like no
errors happen.
  • Loading branch information
Yongxuanzhang committed Jul 22, 2022
1 parent 94055d9 commit 2dec008
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 0 deletions.
1 change: 1 addition & 0 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun, rtr *re
}

if err := validateTaskRunResults(tr, rtr.TaskSpec); err != nil {
tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err)
return err
}

Expand Down
108 changes: 108 additions & 0 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,21 @@ var (
Steps: []v1beta1.Step{simpleStep},
},
}
resultsTask = &v1beta1.Task{
ObjectMeta: objectMeta("test-results-task", "foo"),
Spec: v1beta1.TaskSpec{
Steps: []v1beta1.Step{simpleStep},
Results: []v1beta1.TaskResult{{
Name: "aResult",
Type: v1beta1.ResultsTypeArray,
}, {
Name: "objectResult",
Type: v1beta1.ResultsTypeObject,
Properties: map[string]v1beta1.PropertySpec{"url": {Type: "string"}, "commit": {Type: "string"}},
},
},
},
}

clustertask = &v1beta1.ClusterTask{
ObjectMeta: metav1.ObjectMeta{Name: "test-cluster-task"},
Expand Down Expand Up @@ -4657,3 +4672,96 @@ func objectMeta(name, ns string) metav1.ObjectMeta {
Annotations: map[string]string{},
}
}

func TestReconcile_validateTaskRunResults(t *testing.T) {
taskRunResultsTypeMismatched := parse.MustParseTaskRun(t, `
metadata:
name: test-taskrun-results-type-mismatched
namespace: foo
spec:
taskRef:
name: test-results-task
status:
taskResults:
- name: aResult
type: string
value: aResultValue
`)

taskRunResultsObjectMissKey := parse.MustParseTaskRun(t, `
metadata:
name: test-taskrun-results-object-miss-key
namespace: foo
spec:
taskRef:
name: test-results-task
status:
taskResults:
- name: aResult
type: array
value:
- 1
- 2
- name: objectResult
type: object
value:
url: abc
`)

taskruns := []*v1beta1.TaskRun{
taskRunResultsTypeMismatched, taskRunResultsObjectMissKey,
}

d := test.Data{
TaskRuns: taskruns,
Tasks: []*v1beta1.Task{resultsTask},
ConfigMaps: []*corev1.ConfigMap{{
ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()},
Data: map[string]string{
"enable-api-fields": config.AlphaAPIFields,
},
}},
}
for _, tc := range []struct {
name string
taskRun *v1beta1.TaskRun
wantFailedReason string
expectedError error
}{{
name: "taskrun results type mismatched",
taskRun: taskRunResultsTypeMismatched,
wantFailedReason: podconvert.ReasonFailedValidation,
expectedError: fmt.Errorf("1 error occurred:\n\t* missmatched Types for these results, map[aResult:[array]]\n\n"),
}, {
name: "taskrun results object miss key",
taskRun: taskRunResultsObjectMissKey,
wantFailedReason: podconvert.ReasonFailedValidation,
expectedError: fmt.Errorf("1 error occurred:\n\t* missing keys for these results which are required in TaskResult's properties map[objectResult:[commit]]\n\n"),
}} {
t.Run(tc.name, func(t *testing.T) {
testAssets, cancel := getTaskRunController(t, d)
defer cancel()
c := testAssets.Controller
clients := testAssets.Clients
saName := tc.taskRun.Spec.ServiceAccountName
if saName == "" {
saName = "default"
}
createServiceAccount(t, testAssets, saName, tc.taskRun.Namespace)

err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.taskRun))
if d := cmp.Diff(err.Error(), tc.expectedError.Error()); d != "" {
t.Errorf("Expected: %v, but Got: %v", tc.expectedError, err)
}
tr, err := clients.Pipeline.TektonV1beta1().TaskRuns(tc.taskRun.Namespace).Get(testAssets.Ctx, tc.taskRun.Name, metav1.GetOptions{})
if err != nil {
t.Fatalf("getting updated taskrun: %v", err)
}
condition := tr.Status.GetCondition(apis.ConditionSucceeded)
if condition.Type != apis.ConditionSucceeded || condition.Status != corev1.ConditionFalse || condition.Reason != tc.wantFailedReason {
t.Errorf("Expected TaskRun to \"%s\" but it did not. Final conditions were:\n%#v", tc.wantFailedReason, tr.Status.Conditions)
}

})
}
}

0 comments on commit 2dec008

Please sign in to comment.