diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index 49434278be2..26aa8f57ad3 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -22,7 +22,6 @@ import ( "time" "github.com/tektoncd/pipeline/pkg/apis/config" - apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/validate" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" @@ -72,8 +71,6 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) } } - // This is an alpha feature and will fail validation if it's used in a pipelinerun spec - // when the enable-api-fields feature gate is anything but "alpha". if ps.Timeouts != nil { if ps.Timeout != nil { // can't have both at the same time @@ -153,7 +150,7 @@ func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorM if ps.Timeouts.Tasks.Duration > timeout { tasksTimeoutErr = true } - if ps.Timeouts.Tasks.Duration == apisconfig.NoTimeoutDuration && timeout != apisconfig.NoTimeoutDuration { + if ps.Timeouts.Tasks.Duration == config.NoTimeoutDuration && timeout != config.NoTimeoutDuration { tasksTimeoutErr = true tasksTimeoutStr += " (no timeout)" } @@ -168,7 +165,7 @@ func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorM if ps.Timeouts.Finally.Duration > timeout { finallyTimeoutErr = true } - if ps.Timeouts.Finally.Duration == apisconfig.NoTimeoutDuration && timeout != apisconfig.NoTimeoutDuration { + if ps.Timeouts.Finally.Duration == config.NoTimeoutDuration && timeout != config.NoTimeoutDuration { finallyTimeoutErr = true finallyTimeoutStr += " (no timeout)" } @@ -187,24 +184,17 @@ func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorM } func validateTaskRunSpec(ctx context.Context, trs PipelineTaskRunSpec) (errs *apis.FieldError) { - cfg := config.FromContextOrDefaults(ctx) - if cfg.FeatureFlags.EnableAPIFields == config.AlphaAPIFields { - if trs.StepOverrides != nil { - errs = errs.Also(validateStepOverrides(trs.StepOverrides).ViaField("stepOverrides")) - } - if trs.SidecarOverrides != nil { - errs = errs.Also(validateSidecarOverrides(trs.SidecarOverrides).ViaField("sidecarOverrides")) - } - if trs.ComputeResources != nil { - errs = errs.Also(validateTaskRunComputeResources(trs.ComputeResources, trs.StepOverrides)) - } - } else { - if trs.StepOverrides != nil { - errs = errs.Also(apis.ErrDisallowedFields("stepOverrides")) - } - if trs.SidecarOverrides != nil { - errs = errs.Also(apis.ErrDisallowedFields("sidecarOverrides")) - } + if trs.StepOverrides != nil { + errs = errs.Also(ValidateEnabledAPIFields(ctx, "stepOverrides", config.AlphaAPIFields).ViaField("stepOverrides")) + errs = errs.Also(validateStepOverrides(trs.StepOverrides).ViaField("stepOverrides")) + } + if trs.SidecarOverrides != nil { + errs = errs.Also(ValidateEnabledAPIFields(ctx, "sidecarOverrides", config.AlphaAPIFields).ViaField("sidecarOverrides")) + errs = errs.Also(validateSidecarOverrides(trs.SidecarOverrides).ViaField("sidecarOverrides")) + } + if trs.ComputeResources != nil { + errs = errs.Also(ValidateEnabledAPIFields(ctx, "computeResources", config.AlphaAPIFields).ViaField("computeResources")) + errs = errs.Also(validateTaskRunComputeResources(trs.ComputeResources, trs.StepOverrides)) } return errs } diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index ce82e89697a..cba5688677e 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -543,7 +543,7 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { }, }, }, - wantErr: apis.ErrDisallowedFields("stepOverrides").ViaIndex(0).ViaField("taskRunSpecs"), + wantErr: apis.ErrGeneric("stepOverrides requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"), }, { name: "sidecarOverride disallowed without alpha feature gate", spec: v1beta1.PipelineRunSpec{ @@ -560,7 +560,7 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { }, }, }, - wantErr: apis.ErrDisallowedFields("sidecarOverrides").ViaIndex(0).ViaField("taskRunSpecs"), + wantErr: apis.ErrGeneric("sidecarOverrides requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"), }, { name: "missing stepOverride name", spec: v1beta1.PipelineRunSpec{ @@ -634,6 +634,20 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { "taskRunSpecs[0].computeResources", ), withContext: enableAlphaAPIFields, + }, { + name: "computeResources disallowed without alpha feature gate", + spec: v1beta1.PipelineRunSpec{ + PipelineRef: &v1beta1.PipelineRef{Name: "foo"}, + TaskRunSpecs: []v1beta1.PipelineTaskRunSpec{ + { + PipelineTaskName: "bar", + ComputeResources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: corev1resources.MustParse("2Gi")}, + }, + }, + }, + }, + wantErr: apis.ErrGeneric("computeResources requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\"").ViaIndex(0).ViaField("taskRunSpecs"), }} for _, ps := range tests { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index 59bd4264d10..6f6cb4e4001 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -67,12 +67,15 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) { if ts.StepOverrides != nil { errs = errs.Also(ValidateEnabledAPIFields(ctx, "stepOverrides", config.AlphaAPIFields).ViaField("stepOverrides")) errs = errs.Also(validateStepOverrides(ts.StepOverrides).ViaField("stepOverrides")) - errs = errs.Also(validateTaskRunComputeResources(ts.ComputeResources, ts.StepOverrides)) } if ts.SidecarOverrides != nil { errs = errs.Also(ValidateEnabledAPIFields(ctx, "sidecarOverrides", config.AlphaAPIFields).ViaField("sidecarOverrides")) errs = errs.Also(validateSidecarOverrides(ts.SidecarOverrides).ViaField("sidecarOverrides")) } + if ts.ComputeResources != nil { + errs = errs.Also(ValidateEnabledAPIFields(ctx, "computeResources", config.AlphaAPIFields).ViaField("computeResources")) + errs = errs.Also(validateTaskRunComputeResources(ts.ComputeResources, ts.StepOverrides)) + } if ts.Status != "" { if ts.Status != TaskRunSpecStatusCancelled { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index cbd30e966bf..498f21936d0 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -538,6 +538,19 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { "computeResources", ), wc: enableAlphaAPIFields, + }, { + name: "computeResources disallowed without alpha feature gate", + spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{ + Name: "foo", + }, + ComputeResources: &corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceMemory: corev1resources.MustParse("2Gi"), + }, + }, + }, + wantErr: apis.ErrGeneric("computeResources requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""), }} for _, ts := range tests {