[TEP-0104] Add Step-level Validation

Step-level validation is added to ensure no step-level resource requirements will be configured when task-level is specified.
tektoncd · Jul 4, 2022 · 7a5f2a7 · 7a5f2a7
1 parent 65d3bf3
commit 7a5f2a7
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 5 deletions.
diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go
@@ -561,6 +561,7 @@ func (pr *PipelineRun) GetTaskRunSpec(pipelineTaskName string) PipelineTaskRunSp
 			s.StepOverrides = task.StepOverrides
 			s.SidecarOverrides = task.SidecarOverrides
 			s.Metadata = task.Metadata
+			s.ComputeResources = task.ComputeResources
 		}
 	}
 	return s

diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go
@@ -194,7 +194,7 @@ func validateTaskRunSpec(ctx context.Context, trs PipelineTaskRunSpec) (errs *ap
 	}
 	if trs.ComputeResources != nil {
 		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "computeResources", config.AlphaAPIFields).ViaField("computeResources"))
-		errs = errs.Also(validateTaskRunComputeResources(trs.ComputeResources, trs.StepOverrides))
+		errs = errs.Also(validateStepOverridesComputeResources(trs.ComputeResources, trs.StepOverrides))
 	}
 	return errs
 }
diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go
@@ -75,7 +75,8 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	}
 	if ts.ComputeResources != nil {
 		errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "computeResources", config.AlphaAPIFields).ViaField("computeResources"))
-		errs = errs.Also(validateTaskRunComputeResources(ts.ComputeResources, ts.StepOverrides))
+		errs = errs.Also(validateStepComputeResources(ts.ComputeResources, ts.TaskSpec))
+		errs = errs.Also(validateStepOverridesComputeResources(ts.ComputeResources, ts.StepOverrides))
 	}
 
 	if ts.Status != "" {
@@ -93,6 +94,22 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	return errs
 }
 
+// validateStepComputeResources validates compute resources are not configured in TaskRunSpec.Step
+func validateStepComputeResources(computeResources *corev1.ResourceRequirements, taskSpec *TaskSpec) (errs *apis.FieldError) {
+	if taskSpec == nil {
+		return nil
+	}
+	for _, step := range taskSpec.Steps {
+		if step.Resources.Limits != nil || step.Resources.Requests != nil {
+			return apis.ErrMultipleOneOf(
+				"step.resources",
+				"computeResources",
+			)
+		}
+	}
+	return nil
+}
+
 // validateDebug
 func validateDebug(db *TaskRunDebug) (errs *apis.FieldError) {
 	breakpointOnFailure := "onFailure"
@@ -145,10 +162,10 @@ func validateStepOverrides(overrides []TaskRunStepOverride) (errs *apis.FieldErr
 	return errs
 }
 
-// validateTaskRunComputeResources ensures that compute resources are not configured at both the step level and the task level
-func validateTaskRunComputeResources(computeResources *corev1.ResourceRequirements, overrides []TaskRunStepOverride) (errs *apis.FieldError) {
+// validateStepOverridesComputeResources validates compute resources are not configured in TaskRunSpec.StepOverrides
+func validateStepOverridesComputeResources(computeResources *corev1.ResourceRequirements, overrides []TaskRunStepOverride) (errs *apis.FieldError) {
 	for _, override := range overrides {
-		if override.Resources.Size() != 0 && computeResources != nil {
+		if override.Resources.Limits != nil || override.Resources.Requests != nil {
 			return apis.ErrMultipleOneOf(
 				"stepOverrides.resources",
 				"computeResources",

diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go
@@ -538,6 +538,29 @@ func TestTaskRunSpec_Invalidate(t *testing.T) {
 			"computeResources",
 		),
 		wc: enableAlphaAPIFields,
+	}, {
+		name: "invalid both step-level (step.resources) and task-level (spec.computeResources) resource requirements",
+		spec: v1beta1.TaskRunSpec{
+			TaskSpec: &v1beta1.TaskSpec{
+				Steps: []v1beta1.Step{{
+					Name:  "foo-step",
+					Image: "foo-image",
+					Resources: corev1.ResourceRequirements{
+						Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("1Gi")},
+					},
+				}},
+			},
+			ComputeResources: &corev1.ResourceRequirements{
+				Requests: corev1.ResourceList{
+					corev1.ResourceMemory: corev1resources.MustParse("2Gi"),
+				},
+			},
+		},
+		wantErr: apis.ErrMultipleOneOf(
+			"step.resources",
+			"computeResources",
+		),
+		wc: enableAlphaAPIFields,
 	}, {
 		name: "computeResources disallowed without alpha feature gate",
 		spec: v1beta1.TaskRunSpec{