Skip to content

Commit

Permalink
WIP remove embedded-status flag
Browse files Browse the repository at this point in the history
  • Loading branch information
JeromeJu committed Jan 27, 2023
1 parent 60815be commit 419a1ef
Show file tree
Hide file tree
Showing 31 changed files with 359 additions and 2,679 deletions.
5 changes: 0 additions & 5 deletions config/config-feature-flags.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,6 @@ data:
# in the TaskRun/PipelineRun such as the source from where a remote Task/Pipeline
# definition was fetched.
enable-provenance-in-status: "false"
# Setting this flag to "full" to enable full embedding of `TaskRun` and `Run` statuses in the
# `PipelineRun` status. Set it to "minimal" to populate the `ChildReferences` field in the
# `PipelineRun` status with name, kind, and API version information for each `TaskRun` and
# `Run` in the `PipelineRun` instead. Set it to "both" to do both.
embedded-status: "minimal"
# Setting this flag will determine the version for custom tasks created by PipelineRuns.
# Acceptable values are "v1beta1" and "v1alpha1".
# The default is "v1beta1".
Expand Down
5 changes: 0 additions & 5 deletions docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -470,11 +470,6 @@ The default is `false`. For more information, see the [associated issue](https:/
most stable features to be used. Set it to "alpha" to allow [alpha
features](#alpha-features) to be used.

- `embedded-status`: set this flag to "full" to enable full embedding of `TaskRun` and `Run` statuses in the
`PipelineRun` status. Set it to "minimal" to populate the `ChildReferences` field in the `PipelineRun` status with
name, kind, and API version information for each `TaskRun` and `Run` in the `PipelineRun` instead. Set it to "both" to
do both. For more information, see [Configuring usage of `TaskRun` and `Run` embedded statuses](pipelineruns.md#configuring-usage-of-taskrun-and-run-embedded-statuses).

- `resource-verification-mode`: Setting this flag to "enforce" will enforce verification of tasks/pipeline. Failing to verify will fail the taskrun/pipelinerun. "warn" will only log the err message and "skip" will skip the whole verification.
- `results-from`: set this flag to "termination-message" to use the container's termination message to fetch results from. This is the default method of extracting results. Set it to "sidecar-logs" to enable use of a results sidecar logs to extract results instead of termination message.

Expand Down
1 change: 0 additions & 1 deletion docs/matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ Documentation for specifying `Matrix` in a `Pipeline`:

> :seedling: **`Matrix` is an [alpha](install.md#alpha-features) feature.**
> The `enable-api-fields` feature flag must be set to `"alpha"` to specify `Matrix` in a `PipelineTask`.
> The `embedded-status` feature flag must be set to `"minimal"` to specify `Matrix` in a `PipelineTask`.
## Configuring a Matrix

Expand Down
30 changes: 0 additions & 30 deletions pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,6 @@ const (
AlphaAPIFields = "alpha"
// BetaAPIFields is the value used for "enable-api-fields" when beta APIs should be usable as well.
BetaAPIFields = "beta"
// FullEmbeddedStatus is the value used for "embedded-status" when the full statuses of TaskRuns and Runs should be
// embedded in PipelineRunStatusFields, but ChildReferences should not be used.
FullEmbeddedStatus = "full"
// BothEmbeddedStatus is the value used for "embedded-status" when full embedded statuses of TaskRuns and Runs as
// well as ChildReferences should be used in PipelineRunStatusFields.
BothEmbeddedStatus = "both"
// MinimalEmbeddedStatus is the value used for "embedded-status" when only ChildReferences should be used in
// PipelineRunStatusFields.
MinimalEmbeddedStatus = "minimal"
// EnforceResourceVerificationMode is the value used for "resource-verification-mode" when verification is applied and fail the
// TaskRun or PipelineRun when verification fails
EnforceResourceVerificationMode = "enforce"
Expand Down Expand Up @@ -76,8 +67,6 @@ const (
DefaultEnableAPIFields = StableAPIFields
// DefaultSendCloudEventsForRuns is the default value for "send-cloudevents-for-runs".
DefaultSendCloudEventsForRuns = false
// DefaultEmbeddedStatus is the default value for "embedded-status".
DefaultEmbeddedStatus = MinimalEmbeddedStatus
// DefaultEnableSpire is the default value for "enable-spire".
DefaultEnableSpire = false
// DefaultResourceVerificationMode is the default value for "resource-verification-mode".
Expand Down Expand Up @@ -175,9 +164,6 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
if err := setFeature(sendCloudEventsForRuns, DefaultSendCloudEventsForRuns, &tc.SendCloudEventsForRuns); err != nil {
return nil, err
}
if err := setEmbeddedStatus(cfgMap, DefaultEmbeddedStatus, &tc.EmbeddedStatus); err != nil {
return nil, err
}
if err := setResourceVerificationMode(cfgMap, DefaultResourceVerificationMode, &tc.ResourceVerificationMode); err != nil {
return nil, err
}
Expand Down Expand Up @@ -230,22 +216,6 @@ func setEnabledAPIFields(cfgMap map[string]string, defaultValue string, feature
return nil
}

// setEmbeddedStatus sets the "embedded-status" flag based on the content of a given map.
// If the feature gate is invalid or missing then an error is returned.
func setEmbeddedStatus(cfgMap map[string]string, defaultValue string, feature *string) error {
value := defaultValue
if cfg, ok := cfgMap[embeddedStatus]; ok {
value = strings.ToLower(cfg)
}
switch value {
case FullEmbeddedStatus, BothEmbeddedStatus, MinimalEmbeddedStatus:
*feature = value
default:
return fmt.Errorf("invalid value for feature flag %q: %q", embeddedStatus, value)
}
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 {
Expand Down
10 changes: 0 additions & 10 deletions pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles,
EnableAPIFields: config.DefaultEnableAPIFields,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus,
ResultExtractionMethod: config.DefaultResultExtractionMethod,
Expand All @@ -65,7 +64,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
EnableTektonOCIBundles: true,
EnableAPIFields: "alpha",
SendCloudEventsForRuns: true,
EmbeddedStatus: "both",
EnableSpire: true,
ResourceVerificationMode: "enforce",
EnableProvenanceInStatus: true,
Expand All @@ -89,7 +87,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness,
RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
ResultExtractionMethod: config.DefaultResultExtractionMethod,
MaxResultSize: config.DefaultMaxResultSize,
Expand All @@ -108,7 +105,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness,
RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
ResultExtractionMethod: config.DefaultResultExtractionMethod,
MaxResultSize: config.DefaultMaxResultSize,
Expand All @@ -127,7 +123,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness,
RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
ResultExtractionMethod: config.DefaultResultExtractionMethod,
MaxResultSize: config.DefaultMaxResultSize,
Expand All @@ -138,7 +133,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
{
expectedConfig: &config.FeatureFlags{
EnableAPIFields: "stable",
EmbeddedStatus: config.DefaultEmbeddedStatus,
EnableSpire: true,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
Expand All @@ -152,7 +146,6 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
{
expectedConfig: &config.FeatureFlags{
EnableAPIFields: "stable",
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness,
Expand Down Expand Up @@ -184,7 +177,6 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) {
EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles,
EnableAPIFields: config.DefaultEnableAPIFields,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
EnableSpire: config.DefaultEnableSpire,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
EnableProvenanceInStatus: config.DefaultEnableProvenanceInStatus,
Expand Down Expand Up @@ -229,8 +221,6 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) {
fileName: "feature-flags-invalid-boolean",
}, {
fileName: "feature-flags-invalid-enable-api-fields",
}, {
fileName: "feature-flags-invalid-embedded-status",
}, {
fileName: "feature-flags-invalid-resource-verification-mode",
}, {
Expand Down
1 change: 0 additions & 1 deletion pkg/apis/config/testdata/feature-flags-all-flags-set.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ data:
enable-custom-tasks: "true"
enable-api-fields: "alpha"
send-cloudevents-for-runs: "true"
embedded-status: "both"
enable-spire: "true"
resource-verification-mode: "enforce"
enable-provenance-in-status: "true"
Expand Down
24 changes: 0 additions & 24 deletions pkg/apis/config/testing/feature_flags.go

This file was deleted.

3 changes: 0 additions & 3 deletions pkg/apis/pipeline/v1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,9 +283,6 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr
// This is an alpha feature and will fail validation if it's used in a pipeline spec
// when the enable-api-fields feature gate is anything but "alpha".
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields))
// Matrix requires "embedded-status" feature gate to be set to "minimal", and will fail
// validation if it is anything but "minimal".
errs = errs.Also(ValidateEmbeddedStatus(ctx, "matrix", config.MinimalEmbeddedStatus))
errs = errs.Also(pt.validateMatrixCombinationsCount(ctx))
}
errs = errs.Also(validateParameterInOneOfMatrixOrParams(pt.Matrix, pt.Params))
Expand Down
43 changes: 5 additions & 38 deletions pkg/apis/pipeline/v1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,10 +591,10 @@ func TestPipelineTaskList_Deps(t *testing.T) {

func TestPipelineTask_validateMatrix(t *testing.T) {
tests := []struct {
name string
pt *PipelineTask
embeddedStatus string
wantErrs *apis.FieldError
name string
pt *PipelineTask

wantErrs *apis.FieldError
}{{
name: "parameter duplicated in matrix and params",
pt: &PipelineTask{
Expand Down Expand Up @@ -681,45 +681,12 @@ func TestPipelineTask_validateMatrix(t *testing.T) {
Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "firefox"}},
}}},
},
}, {
name: "pipeline has a matrix but embedded status is full",
pt: &PipelineTask{
Name: "task",
Matrix: &Matrix{
Params: []Param{{
Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}, {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}}},
},
embeddedStatus: config.FullEmbeddedStatus,
wantErrs: &apis.FieldError{
Message: "matrix requires \"embedded-status\" feature gate to be \"minimal\" but it is \"full\"",
},
}, {
name: "pipeline has a matrix but embedded status is both",
pt: &PipelineTask{
Name: "task",
Matrix: &Matrix{
Params: []Param{{
Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}, {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}}},
},
embeddedStatus: config.BothEmbeddedStatus,
wantErrs: &apis.FieldError{
Message: "matrix requires \"embedded-status\" feature gate to be \"minimal\" but it is \"both\"",
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.embeddedStatus == "" {
tt.embeddedStatus = config.MinimalEmbeddedStatus
}

featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": "alpha",
"embedded-status": tt.embeddedStatus,
})
defaults := &config.Defaults{
DefaultMaxMatrixCombinationsCount: 4,
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/pipeline/v1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2891,7 +2891,6 @@ func TestMatrixIncompatibleAPIVersions(t *testing.T) {
ps := tt.spec
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": version,
"embedded-status": "minimal",
})
defaults := &config.Defaults{
DefaultMaxMatrixCombinationsCount: 4,
Expand Down Expand Up @@ -3008,7 +3007,6 @@ func Test_validateMatrix(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": "alpha",
"embedded-status": "minimal",
})
defaults := &config.Defaults{
DefaultMaxMatrixCombinationsCount: 4,
Expand Down
36 changes: 0 additions & 36 deletions pkg/apis/pipeline/v1/status_validation.go

This file was deleted.

58 changes: 0 additions & 58 deletions pkg/apis/pipeline/v1/status_validation_test.go

This file was deleted.

3 changes: 0 additions & 3 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,6 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr
// This is an alpha feature and will fail validation if it's used in a pipeline spec
// when the enable-api-fields feature gate is anything but "alpha".
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields))
// Matrix requires "embedded-status" feature gate to be set to "minimal", and will fail
// validation if it is anything but "minimal".
errs = errs.Also(ValidateEmbeddedStatus(ctx, "matrix", config.MinimalEmbeddedStatus))
errs = errs.Also(pt.validateMatrixCombinationsCount(ctx))
}
errs = errs.Also(validateParameterInOneOfMatrixOrParams(pt.Matrix, pt.Params))
Expand Down
Loading

0 comments on commit 419a1ef

Please sign in to comment.