Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TEP-0090: Matrix - Minimal Status is Required #5019

Merged
merged 1 commit into from
Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ 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`.
>
> :warning: This feature is in a preview mode.
> It is still in a very early stage of development and is not yet fully functional.
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,9 @@ 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(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
39 changes: 36 additions & 3 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -684,9 +684,10 @@ func TestPipelineTaskList_Validate(t *testing.T) {

func TestPipelineTask_validateMatrix(t *testing.T) {
tests := []struct {
name string
pt *PipelineTask
wantErrs *apis.FieldError
name string
pt *PipelineTask
embeddedStatus string
wantErrs *apis.FieldError
}{{
name: "parameter duplicated in matrix and params",
pt: &PipelineTask{
Expand Down Expand Up @@ -770,11 +771,43 @@ func TestPipelineTask_validateMatrix(t *testing.T) {
Name: "browser", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"chrome", "firefox"}},
}},
},
}, {
name: "pipeline has a matrix but embedded status is full",
pt: &PipelineTask{
Name: "task",
Matrix: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}, {
Name: "barfoo", Value: ArrayOrString{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: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}, {
Name: "barfoo", Value: ArrayOrString{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: 2 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2749,6 +2749,7 @@ 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 @@ -2861,6 +2862,7 @@ 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: 36 additions & 0 deletions pkg/apis/pipeline/v1beta1/status_validation.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
Copyright 2022 The Tekton Authors

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1beta1

import (
"context"
"fmt"

"github.com/tektoncd/pipeline/pkg/apis/config"
"knative.dev/pkg/apis"
)

// ValidateEmbeddedStatus checks that the embedded-status feature gate is set to the wantEmbeddedStatus value and,
// if not, returns an error stating which feature is dependent on the status and what the current status actually is.
func ValidateEmbeddedStatus(ctx context.Context, featureName, wantEmbeddedStatus string) *apis.FieldError {
embeddedStatus := config.FromContextOrDefaults(ctx).FeatureFlags.EmbeddedStatus
if embeddedStatus != wantEmbeddedStatus {
message := fmt.Sprintf(`%s requires "embedded-status" feature gate to be %q but it is %q`, featureName, wantEmbeddedStatus, embeddedStatus)
return apis.ErrGeneric(message)
}
return nil
}
58 changes: 58 additions & 0 deletions pkg/apis/pipeline/v1beta1/status_validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
Copyright 2022 The Tekton Authors

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package v1beta1

import (
"context"
"testing"

"github.com/tektoncd/pipeline/pkg/apis/config"
)

func TestValidateEmbeddedStatus(t *testing.T) {
status := "minimal"
flags, err := config.NewFeatureFlagsFromMap(map[string]string{
"embedded-status": status,
})
if err != nil {
t.Fatalf("error creating feature flags from map: %v", err)
}
cfg := &config.Config{
FeatureFlags: flags,
}
ctx := config.ToContext(context.Background(), cfg)
if err := ValidateEmbeddedStatus(ctx, "test feature", status); err != nil {
t.Errorf("unexpected error for compatible feature gates: %q", err)
}
}

func TestValidateEmbeddedStatusError(t *testing.T) {
flags, err := config.NewFeatureFlagsFromMap(map[string]string{
"embedded-status": config.FullEmbeddedStatus,
})
if err != nil {
t.Fatalf("error creating feature flags from map: %v", err)
}
cfg := &config.Config{
FeatureFlags: flags,
}
ctx := config.ToContext(context.Background(), cfg)
err = ValidateEmbeddedStatus(ctx, "test feature", config.MinimalEmbeddedStatus)
if err == nil {
t.Errorf("error expected for incompatible feature gates")
}
}
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7614,7 +7614,7 @@ spec:
`),
}

cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())}
cms := []*corev1.ConfigMap{withEmbeddedStatus(withEnabledAlphaAPIFields(newFeatureFlagsConfigMap()), config.MinimalEmbeddedStatus)}
cms = append(cms, withMaxMatrixCombinationsCount(newDefaultsConfigMap(), 10))

tests := []struct {
Expand Down