From b419b2c87869a27a693a68909359103d0df1eb98 Mon Sep 17 00:00:00 2001 From: Khurram Baig Date: Fri, 5 Apr 2024 20:14:39 +0530 Subject: [PATCH] Add a feature flag to disable inline spec By default the inline specs will be enabled. Only if flag is set to true, inline spec would be disabled. This is to increase security of pipelines. --- config/config-feature-flags.yaml | 6 ++ docs/additional-configs.md | 26 ++++++ docs/pipeline-api.md | 32 +++++++- pkg/apis/config/feature_flags.go | 20 +++++ pkg/apis/config/feature_flags_test.go | 12 ++- .../testdata/feature-flags-all-flags-set.yaml | 1 + pkg/apis/pipeline/v1/openapi_generated.go | 10 ++- pkg/apis/pipeline/v1/pipeline_types.go | 4 + pkg/apis/pipeline/v1/pipeline_validation.go | 21 +++++ .../pipeline/v1/pipeline_validation_test.go | 80 +++++++++++++++++++ pkg/apis/pipeline/v1/pipelinerun_types.go | 2 + .../pipeline/v1/pipelinerun_validation.go | 6 ++ .../v1/pipelinerun_validation_test.go | 39 +++++++++ pkg/apis/pipeline/v1/swagger.json | 6 +- pkg/apis/pipeline/v1/taskrun_types.go | 2 + pkg/apis/pipeline/v1/taskrun_validation.go | 4 + .../pipeline/v1/taskrun_validation_test.go | 36 +++++++++ .../pipeline/v1beta1/openapi_generated.go | 10 ++- pkg/apis/pipeline/v1beta1/pipeline_types.go | 4 + .../pipeline/v1beta1/pipeline_validation.go | 21 +++++ .../v1beta1/pipeline_validation_test.go | 80 +++++++++++++++++++ .../pipeline/v1beta1/pipelinerun_types.go | 2 + .../v1beta1/pipelinerun_validation.go | 6 ++ .../v1beta1/pipelinerun_validation_test.go | 39 +++++++++ pkg/apis/pipeline/v1beta1/swagger.json | 6 +- pkg/apis/pipeline/v1beta1/taskrun_types.go | 2 + .../pipeline/v1beta1/taskrun_validation.go | 4 + .../v1beta1/taskrun_validation_test.go | 36 +++++++++ pkg/reconciler/taskrun/taskrun_test.go | 3 + 29 files changed, 503 insertions(+), 17 deletions(-) diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index 6cb5073da37..bea6994cc99 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -131,3 +131,9 @@ data: enable-artifacts: "false" # Setting this flag to "true" will enable the built-in param input validation via param enum. enable-param-enum: "false" + # Setting this flag to "pipeline,pipelinerun,taskrun" will prevent users from creating + # embedded spec Taskruns or Pipelineruns for Pipeline, Pipelinerun and taskrun + # respectively. We can specify "pipeline" to disable for Pipeline resource only. + # "pipelinerun" for Pipelinerun and "taskrun" for Taskrun. Or a combination of + # these. + disable-inline-spec: "" diff --git a/docs/additional-configs.md b/docs/additional-configs.md index 8193f333b7d..be60c6c3b2d 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -32,6 +32,7 @@ installation. - [Verify Tekton Resources](#verify-tekton-resources) - [Pipelinerun with Affinity Assistant](#pipelineruns-with-affinity-assistant) - [TaskRuns with `imagePullBackOff` Timeout](#taskruns-with-imagepullbackoff-timeout) + - [Disabling Inline Spec in TaskRun and PipelineRun](#disabling-inline-spec-in-taskrun-and-pipelinerun) - [Next steps](#next-steps) @@ -694,6 +695,31 @@ data: default-imagepullbackoff-timeout: "5m" ``` +## Disabling Inline Spec in Pipeline, TaskRun and PipelineRun + +Tekton users may embed the specification of a `Task` (via `taskSpec`) or a `Pipeline` (via `pipelineSpec`) as an alternative to referring to an external resource via `taskRef` and `pipelineRef` respectively. This behaviour can be selectively disabled for three Tekton resources: `TaskRun`, `PipelineRun` and `Pipeline`. + + In certain clusters and scenarios, an admin might want to disable the customisation of `Tasks` and `Pipelines` and only allow users to run pre-defined resources. To achieve that the admin should disable embedded specification via the `disable-inline-spec` flag, and remote resolvers too. + +To disable inline specification, set the `disable-inline-spec` flag to `"pipeline,pipelinerun,taskrun"` +in the `feature-flags` configmap. +```yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: feature-flags + namespace: tekton-pipelines + labels: + app.kubernetes.io/instance: default + app.kubernetes.io/part-of: tekton-pipelines +data: + disable-inline-spec: "pipeline,pipelinerun,taskrun" +``` + +Inline specifications can be disabled for specific resources only. To achieve that, set the disable-inline-spec flag to a comma-separated list of the desired resources. Valid values are pipeline, pipelinerun and taskrun. + +The default value of disable-inline-spec is "", which means inline specification is enabled in all cases. + ## Next steps To get started with Tekton check the [Introductory tutorials][quickstarts], diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index e0b52e34073..c8e57271de6 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -733,6 +733,8 @@ PipelineSpec (Optional) +

Specifying PipelineSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -1155,6 +1157,8 @@ TaskSpec (Optional) +

Specifying PipelineSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -2349,6 +2353,8 @@ PipelineSpec (Optional) +

Specifying PipelineSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -2887,7 +2893,9 @@ EmbeddedTask (Optional) -

TaskSpec is a specification of a task

+

TaskSpec is a specification of a task +Specifying TaskSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -3014,7 +3022,9 @@ PipelineSpec (Optional)

PipelineSpec is a specification of a pipeline -Note: PipelineSpec is in preview mode and not yet supported

+Note: PipelineSpec is in preview mode and not yet supported +Specifying PipelineSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -5530,6 +5540,8 @@ TaskSpec (Optional) +

Specifying PipelineSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -9026,6 +9038,8 @@ PipelineSpec (Optional) +

Specifying PipelineSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -9524,6 +9538,8 @@ TaskSpec (Optional) +

Specifying PipelineSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -11197,6 +11213,8 @@ PipelineSpec (Optional) +

Specifying PipelineSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -11828,7 +11846,9 @@ EmbeddedTask (Optional) -

TaskSpec is a specification of a task

+

TaskSpec is a specification of a task +Specifying TaskSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -11969,7 +11989,9 @@ PipelineSpec (Optional)

PipelineSpec is a specification of a pipeline -Note: PipelineSpec is in preview mode and not yet supported

+Note: PipelineSpec is in preview mode and not yet supported +Specifying TaskSpec can be disabled by setting +disable-inline-spec feature flag..

@@ -14997,6 +15019,8 @@ TaskSpec (Optional) +

Specifying PipelineSpec can be disabled by setting +disable-inline-spec feature flag..

diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index deda086727f..f8c057801a0 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -68,6 +68,8 @@ const ( DefaultRunningInEnvWithInjectedSidecars = true // DefaultAwaitSidecarReadiness is the default value for "await-sidecar-readiness". DefaultAwaitSidecarReadiness = true + // DefaultDisableInlineSpec is the default value of "disable-inline-spec" + DefaultDisableInlineSpec = "" // DefaultRequireGitSSHSecretKnownHosts is the default value for "require-git-ssh-secret-known-hosts". DefaultRequireGitSSHSecretKnownHosts = false // DefaultEnableTektonOciBundles is the default value for "enable-tekton-oci-bundles". @@ -107,6 +109,10 @@ const ( // EnableParamEnum is the flag to enabled enum in params EnableParamEnum = "enable-param-enum" + // DisableInlineSpec is the flag to disable embedded spec + // in Taskrun or Pipelinerun + DisableInlineSpec = "disable-inline-spec" + disableAffinityAssistantKey = "disable-affinity-assistant" disableCredsInitKey = "disable-creds-init" runningInEnvWithInjectedSidecarsKey = "running-in-environment-with-injected-sidecars" @@ -193,6 +199,7 @@ type FeatureFlags struct { EnableStepActions bool EnableParamEnum bool EnableArtifacts bool + DisableInlineSpec string } // GetFeatureFlagsConfigName returns the name of the configmap containing all @@ -291,6 +298,10 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) { if err := setPerFeatureFlag(EnableArtifacts, DefaultEnableArtifacts, &tc.EnableArtifacts); err != nil { return nil, err } + if err := setFeatureInlineSpec(cfgMap, DisableInlineSpec, DefaultDisableInlineSpec, &tc.DisableInlineSpec); err != nil { + return nil, err + } + // Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if // enable-api-fields is "alpha". If enable-api-fields is not "alpha" then fall back to the value of // each feature's individual flag. @@ -364,6 +375,15 @@ func setEnforceNonFalsifiability(cfgMap map[string]string, feature *string) erro } } +func setFeatureInlineSpec(cfgMap map[string]string, key string, defaultValue string, feature *string) error { + if cfg, ok := cfgMap[key]; ok { + *feature = cfg + return nil + } + *feature = strings.ReplaceAll(defaultValue, " ", "") + return nil +} + // setResultExtractionMethod sets the "results-from" flag based on the content of a given map. // If the feature gate is invalid or missing then an error is returned. func setResultExtractionMethod(cfgMap map[string]string, defaultValue string, feature *string) error { diff --git a/pkg/apis/config/feature_flags_test.go b/pkg/apis/config/feature_flags_test.go index 5cf208ba594..0cefc572368 100644 --- a/pkg/apis/config/feature_flags_test.go +++ b/pkg/apis/config/feature_flags_test.go @@ -57,6 +57,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled, EnableStepActions: config.DefaultEnableStepActions.Enabled, EnableParamEnum: config.DefaultEnableParamEnum.Enabled, + DisableInlineSpec: config.DefaultDisableInlineSpec, }, fileName: config.GetFeatureFlagsConfigName(), }, @@ -81,6 +82,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableStepActions: true, EnableArtifacts: true, EnableParamEnum: true, + DisableInlineSpec: "pipeline,pipelinerun,taskrun", }, fileName: "feature-flags-all-flags-set", }, @@ -107,6 +109,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled, EnableStepActions: config.DefaultEnableStepActions.Enabled, EnableParamEnum: config.DefaultEnableParamEnum.Enabled, + DisableInlineSpec: config.DefaultDisableInlineSpec, }, fileName: "feature-flags-enable-api-fields-overrides-bundles-and-custom-tasks", }, @@ -128,6 +131,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { SetSecurityContext: config.DefaultSetSecurityContext, Coschedule: config.DefaultCoschedule, EnableParamEnum: config.DefaultEnableParamEnum.Enabled, + DisableInlineSpec: config.DefaultDisableInlineSpec, }, fileName: "feature-flags-bundles-and-custom-tasks", }, @@ -149,6 +153,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { SetSecurityContext: config.DefaultSetSecurityContext, Coschedule: config.DefaultCoschedule, EnableParamEnum: config.DefaultEnableParamEnum.Enabled, + DisableInlineSpec: config.DefaultDisableInlineSpec, }, fileName: "feature-flags-beta-api-fields", }, @@ -169,6 +174,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled, EnableStepActions: config.DefaultEnableStepActions.Enabled, EnableParamEnum: config.DefaultEnableParamEnum.Enabled, + DisableInlineSpec: config.DefaultDisableInlineSpec, }, fileName: "feature-flags-enforce-nonfalsifiability-spire", }, @@ -188,6 +194,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) { EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled, EnableStepActions: config.DefaultEnableStepActions.Enabled, EnableParamEnum: config.DefaultEnableParamEnum.Enabled, + DisableInlineSpec: config.DefaultDisableInlineSpec, }, fileName: "feature-flags-results-via-sidecar-logs", }, @@ -224,6 +231,7 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) { EnableCELInWhenExpression: config.DefaultEnableCELInWhenExpression.Enabled, EnableStepActions: config.DefaultEnableStepActions.Enabled, EnableParamEnum: config.DefaultEnableParamEnum.Enabled, + DisableInlineSpec: config.DefaultDisableInlineSpec, } verifyConfigFileWithExpectedFeatureFlagsConfig(t, FeatureFlagsConfigEmptyName, expectedConfig) } @@ -311,7 +319,9 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) { t.Run(tc.fileName, func(t *testing.T) { cm := test.ConfigMapFromTestFile(t, tc.fileName) _, err := config.NewFeatureFlagsFromConfigMap(cm) - if d := cmp.Diff(tc.want, err.Error()); d != "" { + if err == nil { + t.Error("failed to get:", tc.want) + } else if d := cmp.Diff(tc.want, err.Error()); d != "" { t.Errorf("failed to get expected error; diff:\n%s", diff.PrintWantGot(d)) } }) diff --git a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml index 51f312c52b0..6b539bc16da 100644 --- a/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml +++ b/pkg/apis/config/testdata/feature-flags-all-flags-set.yaml @@ -36,3 +36,4 @@ data: enable-step-actions: "true" enable-param-enum: "true" enable-artifacts: "true" + disable-inline-spec: "pipeline,pipelinerun,taskrun" diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index 67a9508011b..f9380013f7d 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -1349,7 +1349,8 @@ func schema_pkg_apis_pipeline_v1_PipelineRunSpec(ref common.ReferenceCallback) c }, "pipelineSpec": { SchemaProps: spec.SchemaProps{ - Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PipelineSpec"), + Description: "Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PipelineSpec"), }, }, "params": { @@ -1923,7 +1924,7 @@ func schema_pkg_apis_pipeline_v1_PipelineTask(ref common.ReferenceCallback) comm }, "taskSpec": { SchemaProps: spec.SchemaProps{ - Description: "TaskSpec is a specification of a task", + Description: "TaskSpec is a specification of a task Specifying TaskSpec can be disabled by setting `disable-inline-spec` feature flag..", Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.EmbeddedTask"), }, }, @@ -2026,7 +2027,7 @@ func schema_pkg_apis_pipeline_v1_PipelineTask(ref common.ReferenceCallback) comm }, "pipelineSpec": { SchemaProps: spec.SchemaProps{ - Description: "PipelineSpec is a specification of a pipeline Note: PipelineSpec is in preview mode and not yet supported", + Description: "PipelineSpec is a specification of a pipeline Note: PipelineSpec is in preview mode and not yet supported Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.PipelineSpec"), }, }, @@ -3941,7 +3942,8 @@ func schema_pkg_apis_pipeline_v1_TaskRunSpec(ref common.ReferenceCallback) commo }, "taskSpec": { SchemaProps: spec.SchemaProps{ - Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskSpec"), + Description: "Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.TaskSpec"), }, }, "status": { diff --git a/pkg/apis/pipeline/v1/pipeline_types.go b/pkg/apis/pipeline/v1/pipeline_types.go index 3cbc5295149..4d6e20d6e93 100644 --- a/pkg/apis/pipeline/v1/pipeline_types.go +++ b/pkg/apis/pipeline/v1/pipeline_types.go @@ -199,6 +199,8 @@ type PipelineTask struct { TaskRef *TaskRef `json:"taskRef,omitempty"` // TaskSpec is a specification of a task + // Specifying TaskSpec can be disabled by setting + // `disable-inline-spec` feature flag.. // +optional TaskSpec *EmbeddedTask `json:"taskSpec,omitempty"` @@ -243,6 +245,8 @@ type PipelineTask struct { // PipelineSpec is a specification of a pipeline // Note: PipelineSpec is in preview mode and not yet supported + // Specifying PipelineSpec can be disabled by setting + // `disable-inline-spec` feature flag.. // +optional PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"` diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 5ff738ad372..a0b65f7787c 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -19,6 +19,7 @@ package v1 import ( "context" "fmt" + "slices" "strings" "github.com/tektoncd/pipeline/pkg/apis/config" @@ -177,6 +178,8 @@ func (pt PipelineTask) ValidateName() *apis.FieldError { func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(pt.validateRefOrSpec(ctx)) + errs = errs.Also(pt.validateEnabledInlineSpec(ctx)) + errs = errs.Also(pt.validateEmbeddedOrType()) // taskKinds contains the kinds when the apiVersion is not set, they are not custom tasks, // if apiVersion is set they are custom tasks. @@ -271,6 +274,24 @@ func (pt *PipelineTask) validateWorkspaces(workspaceNames sets.String) (errs *ap return errs } +// validateEnabledInlineSpec validates that pipelineSpec or taskSpec is allowed by checking +// disable-inline-spec field +func (pt PipelineTask) validateEnabledInlineSpec(ctx context.Context) (errs *apis.FieldError) { + if pt.TaskSpec != nil { + if slices.Contains(strings.Split( + config.FromContextOrDefaults(ctx).FeatureFlags.DisableInlineSpec, ","), "pipeline") { + errs = errs.Also(apis.ErrDisallowedFields("taskSpec")) + } + } + if pt.PipelineSpec != nil { + if slices.Contains(strings.Split( + config.FromContextOrDefaults(ctx).FeatureFlags.DisableInlineSpec, ","), "pipeline") { + errs = errs.Also(apis.ErrDisallowedFields("pipelineSpec")) + } + } + return errs +} + // validateRefOrSpec validates at least one of taskRef or taskSpec or pipelineRef or pipelineSpec is specified func (pt PipelineTask) validateRefOrSpec(ctx context.Context) (errs *apis.FieldError) { // collect all the specified specifications diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index b226aad2bca..446c8177066 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -285,6 +285,86 @@ func TestPipeline_Validate_Failure(t *testing.T) { Message: `expected exactly one, got multiple`, Paths: []string{"spec.tasks[0].pipelineRef, spec.tasks[0].pipelineSpec"}, }, + }, { + name: "pipelineSpec when disable-inline-spec", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Tasks: []PipelineTask{ + { + Name: "foo", + PipelineSpec: &PipelineSpec{Description: "foo-pipeline-description"}, + }, + }, + }, + }, + expectedError: *apis.ErrDisallowedFields("spec.tasks[0]" + "." + "pipelineSpec"), + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "disable-inline-spec": "pipeline", + "enable-api-fields": "alpha"}) + }, + }, { + name: "pipelineSpec when disable-inline-spec all", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Tasks: []PipelineTask{ + { + Name: "foo", + PipelineSpec: &PipelineSpec{Description: "foo-pipeline-description"}, + }, + }, + }, + }, + expectedError: *apis.ErrDisallowedFields("spec.tasks[0]" + "." + "pipelineSpec"), + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "disable-inline-spec": "pipeline,taskrun,pipelinerun", + "enable-api-fields": "alpha"}) + }, + }, { + name: "taskSpec when disable-inline-spec", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Description: "inline task", + Tasks: []PipelineTask{{ + Name: "task-spec", + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + }, + expectedError: *apis.ErrDisallowedFields("spec.tasks[0]" + "." + "taskSpec"), + wc: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "pipeline", + }, + }) + }, + }, { + name: "taskSpec when disable-inline-spec all", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Description: "inline task", + Tasks: []PipelineTask{{ + Name: "task-spec", + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + }, + expectedError: *apis.ErrDisallowedFields("spec.tasks[0]" + "." + "taskSpec"), + wc: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "pipeline,pipelinerun,taskrun", + }, + }) + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1/pipelinerun_types.go b/pkg/apis/pipeline/v1/pipelinerun_types.go index 9c9bcd85566..f171719d6a0 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1/pipelinerun_types.go @@ -249,6 +249,8 @@ func (pr *PipelineRun) HasVolumeClaimTemplate() bool { type PipelineRunSpec struct { // +optional PipelineRef *PipelineRef `json:"pipelineRef,omitempty"` + // Specifying PipelineSpec can be disabled by setting + // `disable-inline-spec` feature flag.. // +optional PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"` // Params is a list of parameter names and values. diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation.go b/pkg/apis/pipeline/v1/pipelinerun_validation.go index aea583b940b..d45b00ab8e3 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1/pipelinerun_validation.go @@ -19,6 +19,8 @@ package v1 import ( "context" "fmt" + "slices" + "strings" "time" "github.com/tektoncd/pipeline/pkg/apis/config" @@ -68,6 +70,10 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) // Validate PipelineSpec if it's present if ps.PipelineSpec != nil { + if slices.Contains(strings.Split( + config.FromContextOrDefaults(ctx).FeatureFlags.DisableInlineSpec, ","), "pipelinerun") { + errs = errs.Also(apis.ErrDisallowedFields("pipelineSpec")) + } errs = errs.Also(ps.PipelineSpec.Validate(ctx).ViaField("pipelineSpec")) } diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1/pipelinerun_validation_test.go index 825002d0a36..8e7f24a0e87 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1/pipelinerun_validation_test.go @@ -22,6 +22,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" @@ -877,6 +878,44 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { }}}, }, wantErr: apis.ErrMultipleOneOf("pipelineRef", "pipelineSpec"), + }, { + name: "pipelineSpec when inline disabled all", + spec: v1.PipelineRunSpec{ + PipelineSpec: &v1.PipelineSpec{ + Tasks: []v1.PipelineTask{{ + Name: "mytask", + TaskRef: &v1.TaskRef{ + Name: "mytask", + }, + }}}, + }, + wantErr: apis.ErrDisallowedFields("pipelineSpec"), + withContext: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "taskrun,pipelinerun,pipeline", + }, + }) + }, + }, { + name: "pipelineSpec when inline disabled", + spec: v1.PipelineRunSpec{ + PipelineSpec: &v1.PipelineSpec{ + Tasks: []v1.PipelineTask{{ + Name: "mytask", + TaskRef: &v1.TaskRef{ + Name: "mytask", + }, + }}}, + }, + wantErr: apis.ErrDisallowedFields("pipelineSpec"), + withContext: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "pipelinerun", + }, + }) + }, }, { name: "workspaces may only appear once", spec: v1.PipelineRunSpec{ diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index e014703014c..8993af2ffff 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -669,6 +669,7 @@ "$ref": "#/definitions/v1.PipelineRef" }, "pipelineSpec": { + "description": "Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", "$ref": "#/definitions/v1.PipelineSpec" }, "status": { @@ -969,7 +970,7 @@ "$ref": "#/definitions/v1.PipelineRef" }, "pipelineSpec": { - "description": "PipelineSpec is a specification of a pipeline Note: PipelineSpec is in preview mode and not yet supported", + "description": "PipelineSpec is a specification of a pipeline Note: PipelineSpec is in preview mode and not yet supported Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", "$ref": "#/definitions/v1.PipelineSpec" }, "retries": { @@ -991,7 +992,7 @@ "$ref": "#/definitions/v1.TaskRef" }, "taskSpec": { - "description": "TaskSpec is a specification of a task", + "description": "TaskSpec is a specification of a task Specifying TaskSpec can be disabled by setting `disable-inline-spec` feature flag..", "$ref": "#/definitions/v1.EmbeddedTask" }, "timeout": { @@ -2074,6 +2075,7 @@ "$ref": "#/definitions/v1.TaskRef" }, "taskSpec": { + "description": "Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", "$ref": "#/definitions/v1.TaskSpec" }, "timeout": { diff --git a/pkg/apis/pipeline/v1/taskrun_types.go b/pkg/apis/pipeline/v1/taskrun_types.go index d2d017a0f95..615eaaa788c 100644 --- a/pkg/apis/pipeline/v1/taskrun_types.go +++ b/pkg/apis/pipeline/v1/taskrun_types.go @@ -43,6 +43,8 @@ type TaskRunSpec struct { // no more than one of the TaskRef and TaskSpec may be specified. // +optional TaskRef *TaskRef `json:"taskRef,omitempty"` + // Specifying PipelineSpec can be disabled by setting + // `disable-inline-spec` feature flag.. // +optional TaskSpec *TaskSpec `json:"taskSpec,omitempty"` // Used for cancelling a TaskRun (and maybe more later on) diff --git a/pkg/apis/pipeline/v1/taskrun_validation.go b/pkg/apis/pipeline/v1/taskrun_validation.go index 9ad832ae324..771d684d40c 100644 --- a/pkg/apis/pipeline/v1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1/taskrun_validation.go @@ -63,6 +63,10 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) { } // Validate TaskSpec if it's present. if ts.TaskSpec != nil { + if slices.Contains(strings.Split( + config.FromContextOrDefaults(ctx).FeatureFlags.DisableInlineSpec, ","), "taskrun") { + errs = errs.Also(apis.ErrDisallowedFields("taskSpec")) + } errs = errs.Also(ts.TaskSpec.Validate(ctx).ViaField("taskSpec")) } diff --git a/pkg/apis/pipeline/v1/taskrun_validation_test.go b/pkg/apis/pipeline/v1/taskrun_validation_test.go index 9652b2d4130..978b3164a1c 100644 --- a/pkg/apis/pipeline/v1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1/taskrun_validation_test.go @@ -548,6 +548,42 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { }, }, wantErr: apis.ErrMultipleOneOf("taskRef", "taskSpec"), + }, { + name: "taskspec when inline disabled", + spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Name: "mystep", + Image: "myimage", + }}, + }, + }, + wantErr: apis.ErrDisallowedFields("taskSpec"), + wc: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "taskrun", + }, + }) + }, + }, { + name: "taskspec when inline disabled all", + spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Name: "mystep", + Image: "myimage", + }}, + }, + }, + wantErr: apis.ErrDisallowedFields("taskSpec"), + wc: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "taskrun,pipelinerun,pipeline", + }, + }) + }, }, { name: "negative pipeline timeout", spec: v1.TaskRunSpec{ diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 72277eb914e..d938565b86d 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -1994,7 +1994,8 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRunSpec(ref common.ReferenceCallba }, "pipelineSpec": { SchemaProps: spec.SchemaProps{ - Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineSpec"), + Description: "Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineSpec"), }, }, "resources": { @@ -2673,7 +2674,7 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineTask(ref common.ReferenceCallback) }, "taskSpec": { SchemaProps: spec.SchemaProps{ - Description: "TaskSpec is a specification of a task", + Description: "TaskSpec is a specification of a task Specifying TaskSpec can be disabled by setting `disable-inline-spec` feature flag..", Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.EmbeddedTask"), }, }, @@ -2782,7 +2783,7 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineTask(ref common.ReferenceCallback) }, "pipelineSpec": { SchemaProps: spec.SchemaProps{ - Description: "PipelineSpec is a specification of a pipeline Note: PipelineSpec is in preview mode and not yet supported", + Description: "PipelineSpec is a specification of a pipeline Note: PipelineSpec is in preview mode and not yet supported Specifying TaskSpec can be disabled by setting `disable-inline-spec` feature flag..", Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.PipelineSpec"), }, }, @@ -5203,7 +5204,8 @@ func schema_pkg_apis_pipeline_v1beta1_TaskRunSpec(ref common.ReferenceCallback) }, "taskSpec": { SchemaProps: spec.SchemaProps{ - Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskSpec"), + Description: "Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.TaskSpec"), }, }, "status": { diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types.go b/pkg/apis/pipeline/v1beta1/pipeline_types.go index 256f24f1dcf..804bcbd34f4 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types.go @@ -209,6 +209,8 @@ type PipelineTask struct { TaskRef *TaskRef `json:"taskRef,omitempty"` // TaskSpec is a specification of a task + // Specifying TaskSpec can be disabled by setting + // `disable-inline-spec` feature flag.. // +optional TaskSpec *EmbeddedTask `json:"taskSpec,omitempty"` @@ -257,6 +259,8 @@ type PipelineTask struct { // PipelineSpec is a specification of a pipeline // Note: PipelineSpec is in preview mode and not yet supported + // Specifying TaskSpec can be disabled by setting + // `disable-inline-spec` feature flag.. // +optional PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"` diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index ac5b5c5c797..f1c34eee5e5 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/utils/strings/slices" "knative.dev/pkg/apis" "knative.dev/pkg/webhook/resourcesemantics" ) @@ -179,6 +180,8 @@ func (pt PipelineTask) ValidateName() *apis.FieldError { func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(pt.validateRefOrSpec(ctx)) + errs = errs.Also(pt.validateEnabledInlineSpec(ctx)) + errs = errs.Also(pt.validateEmbeddedOrType()) if pt.Resources != nil { @@ -222,6 +225,24 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) { return //nolint:nakedret } +// validateEnabledInlineSpec validates that pipelineSpec or taskSpec is allowed by checking +// disable-inline-spec field +func (pt PipelineTask) validateEnabledInlineSpec(ctx context.Context) (errs *apis.FieldError) { + if pt.TaskSpec != nil { + if slices.Contains(strings.Split( + config.FromContextOrDefaults(ctx).FeatureFlags.DisableInlineSpec, ","), "pipeline") { + errs = errs.Also(apis.ErrDisallowedFields("taskSpec")) + } + } + if pt.PipelineSpec != nil { + if slices.Contains(strings.Split( + config.FromContextOrDefaults(ctx).FeatureFlags.DisableInlineSpec, ","), "pipeline") { + errs = errs.Also(apis.ErrDisallowedFields("pipelineSpec")) + } + } + return errs +} + func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldError) { if pt.IsMatrixed() { // This is a beta feature and will fail validation if it's used in a pipeline spec diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 602ea92ddd5..adb5810009b 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -287,6 +287,86 @@ func TestPipeline_Validate_Failure(t *testing.T) { Message: `expected exactly one, got multiple`, Paths: []string{"spec.tasks[0].pipelineRef, spec.tasks[0].pipelineSpec"}, }, + }, { + name: "pipelineSpec when disable-inline-spec", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Tasks: []PipelineTask{ + { + Name: "foo", + PipelineSpec: &PipelineSpec{Description: "foo-pipeline-description"}, + }, + }, + }, + }, + expectedError: *apis.ErrDisallowedFields("spec.tasks[0]" + "." + "pipelineSpec"), + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "disable-inline-spec": "pipeline", + "enable-api-fields": "alpha"}) + }, + }, { + name: "pipelineSpec when disable-inline-spec all", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Tasks: []PipelineTask{ + { + Name: "foo", + PipelineSpec: &PipelineSpec{Description: "foo-pipeline-description"}, + }, + }, + }, + }, + expectedError: *apis.ErrDisallowedFields("spec.tasks[0]" + "." + "pipelineSpec"), + wc: func(ctx context.Context) context.Context { + return cfgtesting.SetFeatureFlags(ctx, t, + map[string]string{ + "disable-inline-spec": "pipeline,taskrun,pipelinerun", + "enable-api-fields": "alpha"}) + }, + }, { + name: "taskSpec when disable-inline-spec", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Description: "inline task", + Tasks: []PipelineTask{{ + Name: "task-spec", + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + }, + expectedError: *apis.ErrDisallowedFields("spec.tasks[0]" + "." + "taskSpec"), + wc: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "pipeline", + }, + }) + }, + }, { + name: "taskSpec when disable-inline-spec all", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Description: "inline task", + Tasks: []PipelineTask{{ + Name: "task-spec", + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, + }}, + }, + }, + expectedError: *apis.ErrDisallowedFields("spec.tasks[0]" + "." + "taskSpec"), + wc: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "pipeline,pipelinerun,taskrun", + }, + }) + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index c3a111a978b..453c3778c06 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -252,6 +252,8 @@ func (pr *PipelineRun) HasVolumeClaimTemplate() bool { type PipelineRunSpec struct { // +optional PipelineRef *PipelineRef `json:"pipelineRef,omitempty"` + // Specifying PipelineSpec can be disabled by setting + // `disable-inline-spec` feature flag.. // +optional PipelineSpec *PipelineSpec `json:"pipelineSpec,omitempty"` // Resources is a list of bindings specifying which actual instances of diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index f7c434eb7b4..7113b653620 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -19,6 +19,7 @@ package v1beta1 import ( "context" "fmt" + "strings" "time" "github.com/tektoncd/pipeline/pkg/apis/config" @@ -27,6 +28,7 @@ import ( admissionregistrationv1 "k8s.io/api/admissionregistration/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/strings/slices" "knative.dev/pkg/apis" "knative.dev/pkg/webhook/resourcesemantics" ) @@ -73,6 +75,10 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) // Validate PipelineSpec if it's present if ps.PipelineSpec != nil { + if slices.Contains(strings.Split( + config.FromContextOrDefaults(ctx).FeatureFlags.DisableInlineSpec, ","), "pipelinerun") { + errs = errs.Also(apis.ErrDisallowedFields("pipelineSpec")) + } errs = errs.Also(ps.PipelineSpec.Validate(ctx).ViaField("pipelineSpec")) } diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index 61cdd7bfe47..15e044591a4 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -22,6 +22,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/config" cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" @@ -996,6 +997,44 @@ func TestPipelineRunSpec_Invalidate(t *testing.T) { }}}, }, wantErr: apis.ErrMultipleOneOf("pipelineRef", "pipelineSpec"), + }, { + name: "pipelineSpec when inline disabled all", + spec: v1beta1.PipelineRunSpec{ + PipelineSpec: &v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "mytask", + TaskRef: &v1beta1.TaskRef{ + Name: "mytask", + }, + }}}, + }, + wantErr: apis.ErrDisallowedFields("pipelineSpec"), + withContext: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "taskrun,pipelinerun,pipeline", + }, + }) + }, + }, { + name: "pipelineSpec when inline disabled", + spec: v1beta1.PipelineRunSpec{ + PipelineSpec: &v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "mytask", + TaskRef: &v1beta1.TaskRef{ + Name: "mytask", + }, + }}}, + }, + wantErr: apis.ErrDisallowedFields("pipelineSpec"), + withContext: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "pipelinerun", + }, + }) + }, }, { name: "workspaces may only appear once", spec: v1beta1.PipelineRunSpec{ diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 4fd2b41fd3f..622b1d680c3 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -1010,6 +1010,7 @@ "$ref": "#/definitions/v1beta1.PipelineRef" }, "pipelineSpec": { + "description": "Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", "$ref": "#/definitions/v1beta1.PipelineSpec" }, "podTemplate": { @@ -1362,7 +1363,7 @@ "$ref": "#/definitions/v1beta1.PipelineRef" }, "pipelineSpec": { - "description": "PipelineSpec is a specification of a pipeline Note: PipelineSpec is in preview mode and not yet supported", + "description": "PipelineSpec is a specification of a pipeline Note: PipelineSpec is in preview mode and not yet supported Specifying TaskSpec can be disabled by setting `disable-inline-spec` feature flag..", "$ref": "#/definitions/v1beta1.PipelineSpec" }, "resources": { @@ -1388,7 +1389,7 @@ "$ref": "#/definitions/v1beta1.TaskRef" }, "taskSpec": { - "description": "TaskSpec is a specification of a task", + "description": "TaskSpec is a specification of a task Specifying TaskSpec can be disabled by setting `disable-inline-spec` feature flag..", "$ref": "#/definitions/v1beta1.EmbeddedTask" }, "timeout": { @@ -2892,6 +2893,7 @@ "$ref": "#/definitions/v1beta1.TaskRef" }, "taskSpec": { + "description": "Specifying PipelineSpec can be disabled by setting `disable-inline-spec` feature flag..", "$ref": "#/definitions/v1beta1.TaskSpec" }, "timeout": { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go index 4d6e13ee544..551a5856ed6 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -48,6 +48,8 @@ type TaskRunSpec struct { // no more than one of the TaskRef and TaskSpec may be specified. // +optional TaskRef *TaskRef `json:"taskRef,omitempty"` + // Specifying PipelineSpec can be disabled by setting + // `disable-inline-spec` feature flag.. // +optional TaskSpec *TaskSpec `json:"taskSpec,omitempty"` // Used for cancelling a TaskRun (and maybe more later on) diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index 8dd96f0ef99..b44b3d42e2c 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -63,6 +63,10 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) { } // Validate TaskSpec if it's present. if ts.TaskSpec != nil { + if slices.Contains(strings.Split( + config.FromContextOrDefaults(ctx).FeatureFlags.DisableInlineSpec, ","), "taskrun") { + errs = errs.Also(apis.ErrDisallowedFields("taskSpec")) + } errs = errs.Also(ts.TaskSpec.Validate(ctx).ViaField("taskSpec")) } diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index 3005c14c357..3bab6c0e77c 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -565,6 +565,42 @@ func TestTaskRunSpec_Invalidate(t *testing.T) { StatusMessage: v1beta1.TaskRunSpecStatusMessage(invalidStatusMessage), }, wantErr: apis.ErrInvalidValue("statusMessage should not be set if status is not set, but it is currently set to "+invalidStatusMessage, "statusMessage"), + }, { + name: "taskspec when inline disabled", + spec: v1beta1.TaskRunSpec{ + TaskSpec: &v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{ + Name: "mystep", + Image: "myimage", + }}, + }, + }, + wantErr: apis.ErrDisallowedFields("taskSpec"), + wc: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "taskrun", + }, + }) + }, + }, { + name: "taskspec when inline disabled all", + spec: v1beta1.TaskRunSpec{ + TaskSpec: &v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{ + Name: "mystep", + Image: "myimage", + }}, + }, + }, + wantErr: apis.ErrDisallowedFields("taskSpec"), + wc: func(ctx context.Context) context.Context { + return config.ToContext(ctx, &config.Config{ + FeatureFlags: &config.FeatureFlags{ + DisableInlineSpec: "taskrun,pipelinerun,pipeline", + }, + }) + }, }, { name: "invalid taskspec", spec: v1beta1.TaskRunSpec{ diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 6f9fbd919c4..31f2ba22d1e 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -1704,6 +1704,7 @@ status: ResultExtractionMethod: "termination-message" MaxResultSize: 4096 Coschedule: "workspaces" + DisableInlineSpec: "" provenance: featureFlags: RunningInEnvWithInjectedSidecars: true @@ -1716,6 +1717,7 @@ status: ResultExtractionMethod: "termination-message" MaxResultSize: 4096 Coschedule: "workspaces" + DisableInlineSpec: "" `, pipelineErrors.UserErrorLabel, pipelineErrors.UserErrorLabel)) reconciliatonError = errors.New("1 error occurred:\n\t* Provided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"") toBeRetriedTaskRun = parse.MustParseV1TaskRun(t, ` @@ -1769,6 +1771,7 @@ status: ResultExtractionMethod: "termination-message" MaxResultSize: 4096 Coschedule: "workspaces" + DisableInlineSpec: "" `) toBeRetriedWithResultsTaskRun = parse.MustParseV1TaskRun(t, ` metadata: