Skip to content

Commit

Permalink
remove beta flag check for v1beta1 object param,results and array result
Browse files Browse the repository at this point in the history
This commit removes the beta feature flag check for v1beta1 object
param,results and array result. Beta features are considered to be
'stable' for v1beta1 CRDs, so we shouldn't check them.

Signed-off-by: Yongxuan Zhang [email protected]
  • Loading branch information
Yongxuanzhang committed May 17, 2023
1 parent 5abf7c2 commit 4ebcc01
Show file tree
Hide file tree
Showing 9 changed files with 5 additions and 134 deletions.
6 changes: 3 additions & 3 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1335,8 +1335,7 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := config.EnableAlphaAPIFields(context.Background())
ctx = config.SkipValidationDueToPropagatedParametersAndWorkspaces(ctx, false)
ctx := config.SkipValidationDueToPropagatedParametersAndWorkspaces(context.Background(), false)
err := ValidatePipelineParameterVariables(ctx, tt.tasks, tt.params)
if err != nil {
t.Errorf("Pipeline.ValidatePipelineParameterVariables() returned error for valid pipeline parameters: %v", err)
Expand Down Expand Up @@ -1676,7 +1675,8 @@ func TestValidatePipelineDeclaredParameterUsage_Failure(t *testing.T) {
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validatePipelineTaskParameterUsage(tt.tasks, tt.params)
ctx := config.SkipValidationDueToPropagatedParametersAndWorkspaces(context.Background(), false)
err := ValidatePipelineParameterVariables(ctx, tt.tasks, tt.params)
if err == nil {
t.Errorf("Pipeline.ValidatePipelineParameterVariables() did not return error for invalid pipeline parameters")
}
Expand Down
15 changes: 0 additions & 15 deletions pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,6 @@ func TestPipelineRef_Invalid(t *testing.T) {
},
wantErr: apis.ErrMultipleOneOf("bundle", "params").Also(apis.ErrMissingField("resolver")),
withContext: enableTektonOCIBundles(t),
}, {
name: "pipelineref param object requires alpha",
ref: &v1beta1.PipelineRef{
ResolverRef: v1beta1.ResolverRef{
Resolver: "some-resolver",
Params: v1beta1.Params{{
Name: "foo",
Value: v1beta1.ParamValue{
Type: v1beta1.ParamTypeObject,
ObjectVal: map[string]string{"bar": "baz"},
},
}},
},
},
wantErr: apis.ErrGeneric("object type parameter requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\""),
}}

for _, tc := range tests {
Expand Down
9 changes: 0 additions & 9 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ func TestPipelineRun_Invalid(t *testing.T) {
Message: `non-existent variable in "$(params.pipeline-words.not-hello)"`,
Paths: []string{"spec.steps[0].args[0]"},
},
wc: config.EnableAlphaAPIFields,
}, {
name: "propagating object params with pipelinespec and taskspec params not provided",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -211,7 +210,6 @@ func TestPipelineRun_Invalid(t *testing.T) {
Message: `non-existent variable in "$(params.pipeline-words.not-hello)"`,
Paths: []string{"spec.steps[0].args[0]"},
},
wc: config.EnableAlphaAPIFields,
}, {
name: "propagating object params with pipelinespec and taskspec params provided in taskrun",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -256,7 +254,6 @@ func TestPipelineRun_Invalid(t *testing.T) {
Message: `non-existent variable in "$(params.pipeline-words.not-hello)"`,
Paths: []string{"spec.steps[0].args[0]"},
},
wc: config.EnableAlphaAPIFields,
}, {
name: "propagating params with pipelinespec and taskspec",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -352,7 +349,6 @@ func TestPipelineRun_Invalid(t *testing.T) {
Message: `non-existent variable in "$(params.pipeline-words.not-hello)"`,
Paths: []string{"spec.steps[0].args[0]"},
},
wc: config.EnableAlphaAPIFields,
}, {
name: "duplicate param names",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -613,7 +609,6 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
wc: config.EnableAlphaAPIFields,
}, {
name: "propagating params with pipelinespec and taskspec",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -643,7 +638,6 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
wc: config.EnableAlphaAPIFields,
}, {
name: "propagating object params with pipelinespec and taskspec",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -673,7 +667,6 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
wc: config.EnableAlphaAPIFields,
}, {
name: "propagating object params no value in params but value in default",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -713,7 +706,6 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
wc: config.EnableAlphaAPIFields,
}, {
name: "propagating object params with some params defined in taskspec only",
pr: v1beta1.PipelineRun{
Expand Down Expand Up @@ -754,7 +746,6 @@ func TestPipelineRun_Validate(t *testing.T) {
},
},
},
wc: config.EnableAlphaAPIFields,
}, {
name: "propagating workspaces",
pr: v1beta1.PipelineRun{
Expand Down
6 changes: 0 additions & 6 deletions pkg/apis/pipeline/v1beta1/result_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ import (
"context"
"fmt"

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

Expand All @@ -29,14 +27,10 @@ func (tr TaskResult) Validate(ctx context.Context) (errs *apis.FieldError) {
}

switch {
// Object results is a beta feature - make sure the feature flag is set to "beta"
case tr.Type == ResultsTypeObject:
errs := validateObjectResult(tr)
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.BetaAPIFields))
return errs
// Array results is a beta feature - make sure the feature flag is set to "beta"
case tr.Type == ResultsTypeArray:
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "results type", config.BetaAPIFields))
return errs
// Resources created before the result. Type was introduced may not have Type set
// and should be considered valid
Expand Down
70 changes: 2 additions & 68 deletions pkg/apis/pipeline/v1beta1/result_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,51 +22,35 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/pipeline/test/diff"
"knative.dev/pkg/apis"
)

func TestResultsValidate(t *testing.T) {
tests := []struct {
name string
Result v1beta1.TaskResult
apiFields string
name string
Result v1beta1.TaskResult
}{{
name: "valid result type empty",
Result: v1beta1.TaskResult{
Name: "MY-RESULT",
Description: "my great result",
},
apiFields: "stable",
}, {
name: "valid result type string",
Result: v1beta1.TaskResult{
Name: "MY-RESULT",
Type: v1beta1.ResultsTypeString,
Description: "my great result",
},

apiFields: "stable",
}, {
name: "valid result type array",
Result: v1beta1.TaskResult{
Name: "MY-RESULT",
Type: v1beta1.ResultsTypeArray,
Description: "my great result",
},

apiFields: "alpha",
}, {
name: "valid result type array",
Result: v1beta1.TaskResult{
Name: "MY-RESULT",
Type: v1beta1.ResultsTypeArray,
Description: "my great result",
},

apiFields: config.BetaAPIFields,
}, {
name: "valid result type object",
Result: v1beta1.TaskResult{
Expand All @@ -75,17 +59,10 @@ func TestResultsValidate(t *testing.T) {
Description: "my great result",
Properties: map[string]v1beta1.PropertySpec{"hello": {Type: v1beta1.ParamTypeString}},
},
apiFields: "alpha",
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
if tt.apiFields == "alpha" {
ctx = config.EnableAlphaAPIFields(ctx)
}
if tt.apiFields == config.BetaAPIFields {
ctx = config.EnableBetaAPIFields(ctx)
}
if err := tt.Result.Validate(ctx); err != nil {
t.Errorf("TaskSpec.Validate() = %v", err)
}
Expand All @@ -97,7 +74,6 @@ func TestResultsValidateError(t *testing.T) {
tests := []struct {
name string
Result v1beta1.TaskResult
apiFields string
expectedError apis.FieldError
}{{
name: "invalid result type in stable",
Expand All @@ -106,48 +82,11 @@ func TestResultsValidateError(t *testing.T) {
Type: "wrong",
Description: "my great result",
},
apiFields: "stable",
expectedError: apis.FieldError{
Message: `invalid value: wrong`,
Paths: []string{"type"},
Details: "type must be string",
},
}, {
name: "invalid result type in alpha",
Result: v1beta1.TaskResult{
Name: "MY-RESULT",
Type: "wrong",
Description: "my great result",
},
apiFields: "alpha",
expectedError: apis.FieldError{
Message: `invalid value: wrong`,
Paths: []string{"type"},
Details: "type must be string",
},
}, {
name: "invalid array result type in stable",
Result: v1beta1.TaskResult{
Name: "MY-RESULT",
Type: v1beta1.ResultsTypeArray,
Description: "my great result",
},
apiFields: "stable",
expectedError: apis.FieldError{
Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"",
},
}, {
name: "invalid object result type in stable",
Result: v1beta1.TaskResult{
Name: "MY-RESULT",
Type: v1beta1.ResultsTypeObject,
Description: "my great result",
Properties: map[string]v1beta1.PropertySpec{"hello": {Type: v1beta1.ParamTypeString}},
},
apiFields: "stable",
expectedError: apis.FieldError{
Message: "results type requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"",
},
}, {
name: "invalid object properties type",
Result: v1beta1.TaskResult{
Expand All @@ -156,7 +95,6 @@ func TestResultsValidateError(t *testing.T) {
Description: "my great result",
Properties: map[string]v1beta1.PropertySpec{"hello": {Type: "wrong type"}},
},
apiFields: "alpha",
expectedError: apis.FieldError{
Message: "The value type specified for these keys [hello] is invalid, the type must be string",
Paths: []string{"MY-RESULT.properties"},
Expand All @@ -168,7 +106,6 @@ func TestResultsValidateError(t *testing.T) {
Type: v1beta1.ResultsTypeObject,
Description: "my great result",
},
apiFields: "alpha",
expectedError: apis.FieldError{
Message: "missing field(s)",
Paths: []string{"MY-RESULT.properties"},
Expand All @@ -177,9 +114,6 @@ func TestResultsValidateError(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
if tt.apiFields == "alpha" {
ctx = config.EnableAlphaAPIFields(ctx)
}
err := tt.Result.Validate(ctx)
if err == nil {
t.Fatalf("Expected an error, got nothing for %v", tt.Result)
Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1beta1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,11 +325,6 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
// ValidateParameterTypes validates all the types within a slice of ParamSpecs
func ValidateParameterTypes(ctx context.Context, params []ParamSpec) (errs *apis.FieldError) {
for _, p := range params {
if p.Type == ParamTypeObject {
// Object type parameter is a beta feature and will fail validation if it's used in a task spec
// when the enable-api-fields feature gate is not "alpha" or "beta".
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields))
}
errs = errs.Also(p.ValidateType(ctx))
}
return errs
Expand Down
15 changes: 0 additions & 15 deletions pkg/apis/pipeline/v1beta1/taskref_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,21 +164,6 @@ func TestTaskRef_Invalid(t *testing.T) {
},
wantErr: apis.ErrMultipleOneOf("bundle", "params").Also(apis.ErrMissingField("resolver")),
wc: enableTektonOCIBundles(t),
}, {
name: "taskref param object requires alpha",
taskRef: &v1beta1.TaskRef{
ResolverRef: v1beta1.ResolverRef{
Resolver: "some-resolver",
Params: v1beta1.Params{{
Name: "foo",
Value: v1beta1.ParamValue{
Type: v1beta1.ParamTypeObject,
ObjectVal: map[string]string{"bar": "baz"},
},
}},
},
},
wantErr: apis.ErrGeneric("object type parameter requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\""),
}}
for _, ts := range tests {
t.Run(ts.name, func(t *testing.T) {
Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1beta1/taskrun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,6 @@ func ValidateWorkspaceBindings(ctx context.Context, wb []WorkspaceBinding) (errs
func ValidateParameters(ctx context.Context, params Params) (errs *apis.FieldError) {
var names []string
for _, p := range params {
if p.Value.Type == ParamTypeObject {
// Object type parameter is a beta feature and will fail validation if it's used in a taskrun spec
// when the enable-api-fields feature gate is not "alpha" or "beta".
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields))
}
names = append(names, p.Name)
}
return errs.Also(validateNoDuplicateNames(names, false))
Expand Down
Loading

0 comments on commit 4ebcc01

Please sign in to comment.