Skip to content

Commit 5d403c6

Browse files
JeromeJutekton-robot
JeromeJu
authored and
tekton-robot
committed
Refactor version.ValidateEnabledAPIFields to config pkg
This commit refactors the ValidateEnabledAPIFields function to the config package. The validation previously lived in v1beta1 and it was moved to the version pkg which functions as API version conversion utils rather than for feature versions. This commit moves it to the config pkg since it is actually more tied with config validation of the feature flag. version.ValidateEnabledAPIFields has been deprecated and aliased to keep backwards compatibility. /kind misc
1 parent d26a45c commit 5d403c6

14 files changed

+194
-65
lines changed

‎pkg/apis/version/version_validation.go‎ renamed to ‎pkg/apis/config/featureflags_validation.go‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,40 +14,39 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package version
17+
package config
1818

1919
import (
2020
"context"
2121
"fmt"
2222

23-
"github.com/tektoncd/pipeline/pkg/apis/config"
2423
"knative.dev/pkg/apis"
2524
)
2625

2726
// ValidateEnabledAPIFields checks that the enable-api-fields feature gate is set
2827
// to a version at most as stable as wantVersion, if not, returns an error stating which feature
2928
// is dependent on the version and what the current version actually is.
3029
func ValidateEnabledAPIFields(ctx context.Context, featureName string, wantVersion string) *apis.FieldError {
31-
currentVersion := config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields
30+
currentVersion := FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields
3231
var errs *apis.FieldError
3332
message := `%s requires "enable-api-fields" feature gate to be %s but it is %q`
3433
switch wantVersion {
35-
case config.StableAPIFields:
34+
case StableAPIFields:
3635
// If the feature is stable, it doesn't matter what the current version is
37-
case config.BetaAPIFields:
36+
case BetaAPIFields:
3837
// If the feature requires "beta" fields to be enabled, the current version may be "beta" or "alpha"
39-
if currentVersion != config.BetaAPIFields && currentVersion != config.AlphaAPIFields {
40-
message = fmt.Sprintf(message, featureName, fmt.Sprintf("%q or %q", config.AlphaAPIFields, config.BetaAPIFields), currentVersion)
38+
if currentVersion != BetaAPIFields && currentVersion != AlphaAPIFields {
39+
message = fmt.Sprintf(message, featureName, fmt.Sprintf("%q or %q", AlphaAPIFields, BetaAPIFields), currentVersion)
4140
errs = apis.ErrGeneric(message)
4241
}
43-
case config.AlphaAPIFields:
42+
case AlphaAPIFields:
4443
// If the feature requires "alpha" fields to be enabled, the current version must be "alpha"
4544
if currentVersion != wantVersion {
46-
message = fmt.Sprintf(message, featureName, fmt.Sprintf("%q", config.AlphaAPIFields), currentVersion)
45+
message = fmt.Sprintf(message, featureName, fmt.Sprintf("%q", AlphaAPIFields), currentVersion)
4746
errs = apis.ErrGeneric(message)
4847
}
4948
default:
50-
errs = apis.ErrGeneric("invalid wantVersion %s, must be one of (%s, %s, %s)", wantVersion, config.AlphaAPIFields, config.BetaAPIFields, config.StableAPIFields)
49+
errs = apis.ErrGeneric("invalid wantVersion %s, must be one of (%s, %s, %s)", wantVersion, AlphaAPIFields, BetaAPIFields, StableAPIFields)
5150
}
5251
return errs
5352
}

‎pkg/apis/config/featureflags_validation_test.go‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
Copyright 2021 The Tekton Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package config_test
18+
19+
import (
20+
"context"
21+
"testing"
22+
23+
"github.com/tektoncd/pipeline/pkg/apis/config"
24+
)
25+
26+
func TestValidateEnabledAPIFields(t *testing.T) {
27+
tcs := []struct {
28+
name string
29+
wantVersion string
30+
currentVersion string
31+
}{{
32+
name: "alpha fields w/ alpha",
33+
wantVersion: "alpha",
34+
currentVersion: "alpha",
35+
}, {
36+
name: "beta fields w/ alpha",
37+
wantVersion: "beta",
38+
currentVersion: "alpha",
39+
}, {
40+
name: "beta fields w/ beta",
41+
wantVersion: "beta",
42+
currentVersion: "beta",
43+
}, {
44+
name: "stable fields w/ alpha",
45+
wantVersion: "stable",
46+
currentVersion: "alpha",
47+
}, {
48+
name: "stable fields w/ beta",
49+
wantVersion: "stable",
50+
currentVersion: "beta",
51+
}, {
52+
name: "stable fields w/ stable",
53+
wantVersion: "stable",
54+
currentVersion: "stable",
55+
}}
56+
for _, tc := range tcs {
57+
t.Run(tc.name, func(t *testing.T) {
58+
flags, err := config.NewFeatureFlagsFromMap(map[string]string{
59+
"enable-api-fields": tc.currentVersion,
60+
})
61+
if err != nil {
62+
t.Fatalf("error creating feature flags from map: %v", err)
63+
}
64+
cfg := &config.Config{
65+
FeatureFlags: flags,
66+
}
67+
ctx := config.ToContext(context.Background(), cfg)
68+
if err := config.ValidateEnabledAPIFields(ctx, "test feature", tc.wantVersion); err != nil {
69+
t.Errorf("unexpected error for compatible feature gates: %q", err)
70+
}
71+
})
72+
}
73+
}
74+
75+
func TestValidateEnabledAPIFieldsError(t *testing.T) {
76+
tcs := []struct {
77+
name string
78+
wantVersion string
79+
currentVersion string
80+
}{{
81+
name: "alpha fields w/ stable",
82+
wantVersion: "alpha",
83+
currentVersion: "stable",
84+
}, {
85+
name: "alpha fields w/ beta",
86+
wantVersion: "alpha",
87+
currentVersion: "beta",
88+
}, {
89+
name: "beta fields w/ stable",
90+
wantVersion: "beta",
91+
currentVersion: "stable",
92+
}, {
93+
name: "invalid wantVersion",
94+
wantVersion: "foo",
95+
currentVersion: "stable",
96+
}}
97+
for _, tc := range tcs {
98+
t.Run(tc.name, func(t *testing.T) {
99+
flags, err := config.NewFeatureFlagsFromMap(map[string]string{
100+
"enable-api-fields": tc.currentVersion,
101+
})
102+
if err != nil {
103+
t.Fatalf("error creating feature flags from map: %v", err)
104+
}
105+
cfg := &config.Config{
106+
FeatureFlags: flags,
107+
}
108+
ctx := config.ToContext(context.Background(), cfg)
109+
fieldErr := config.ValidateEnabledAPIFields(ctx, "test feature", tc.wantVersion)
110+
111+
if fieldErr == nil {
112+
t.Errorf("error expected for incompatible feature gates")
113+
}
114+
})
115+
}
116+
}

‎pkg/apis/pipeline/v1/pipeline_validation.go‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323

2424
"github.com/tektoncd/pipeline/pkg/apis/config"
2525
"github.com/tektoncd/pipeline/pkg/apis/validate"
26-
"github.com/tektoncd/pipeline/pkg/apis/version"
2726
"github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag"
2827
"github.com/tektoncd/pipeline/pkg/substitution"
2928
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
@@ -103,21 +102,21 @@ func (ps *PipelineSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError
103102
// Object parameters
104103
for i, p := range ps.Params {
105104
if p.Type == ParamTypeObject {
106-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
105+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
107106
}
108107
}
109108
// Indexing into array parameters
110109
arrayParamIndexingRefs := ps.GetIndexingReferencesToArrayParams()
111110
if len(arrayParamIndexingRefs) != 0 {
112-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields))
111+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields))
113112
}
114113
// array and object results
115114
for i, result := range ps.Results {
116115
switch result.Type {
117116
case ResultsTypeObject:
118-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
117+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
119118
case ResultsTypeArray:
120-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
119+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
121120
case ResultsTypeString:
122121
default:
123122
}
@@ -139,10 +138,10 @@ func (pt *PipelineTask) validateBetaFields(ctx context.Context) *apis.FieldError
139138
if pt.TaskRef != nil {
140139
// Resolvers
141140
if pt.TaskRef.Resolver != "" {
142-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.resolver", config.BetaAPIFields))
141+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "taskref.resolver", config.BetaAPIFields))
143142
}
144143
if len(pt.TaskRef.Params) > 0 {
145-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.params", config.BetaAPIFields))
144+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "taskref.params", config.BetaAPIFields))
146145
}
147146
} else if pt.TaskSpec != nil {
148147
errs = errs.Also(pt.TaskSpec.ValidateBetaFields(ctx))
@@ -232,7 +231,7 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr
232231
if pt.IsMatrixed() {
233232
// This is an alpha feature and will fail validation if it's used in a pipeline spec
234233
// when the enable-api-fields feature gate is anything but "alpha".
235-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields))
234+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields))
236235
errs = errs.Also(pt.Matrix.validateCombinationsCount(ctx))
237236
errs = errs.Also(pt.Matrix.validateUniqueParams())
238237
}
@@ -307,11 +306,11 @@ func (pt PipelineTask) validateRefOrSpec(ctx context.Context) (errs *apis.FieldE
307306
nonNilFields = append(nonNilFields, taskSpec)
308307
}
309308
if pt.PipelineRef != nil {
310-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, pipelineRef, config.AlphaAPIFields))
309+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, pipelineRef, config.AlphaAPIFields))
311310
nonNilFields = append(nonNilFields, pipelineRef)
312311
}
313312
if pt.PipelineSpec != nil {
314-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, pipelineSpec, config.AlphaAPIFields))
313+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, pipelineSpec, config.AlphaAPIFields))
315314
nonNilFields = append(nonNilFields, pipelineSpec)
316315
}
317316

‎pkg/apis/pipeline/v1/pipelineref_validation.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121

2222
"github.com/tektoncd/pipeline/pkg/apis/config"
23-
"github.com/tektoncd/pipeline/pkg/apis/version"
2423
"knative.dev/pkg/apis"
2524
)
2625

@@ -33,13 +32,13 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) {
3332

3433
if ref.Resolver != "" || ref.Params != nil {
3534
if ref.Resolver != "" {
36-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
35+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
3736
if ref.Name != "" {
3837
errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver"))
3938
}
4039
}
4140
if ref.Params != nil {
42-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver params", config.BetaAPIFields).ViaField("params"))
41+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver params", config.BetaAPIFields).ViaField("params"))
4342
if ref.Name != "" {
4443
errs = errs.Also(apis.ErrMultipleOneOf("name", "params"))
4544
}

‎pkg/apis/pipeline/v1/pipelinerun_validation.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323

2424
"github.com/tektoncd/pipeline/pkg/apis/config"
2525
"github.com/tektoncd/pipeline/pkg/apis/validate"
26-
"github.com/tektoncd/pipeline/pkg/apis/version"
2726
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
2827
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2928
"knative.dev/pkg/apis"
@@ -283,15 +282,15 @@ func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorM
283282

284283
func validateTaskRunSpec(ctx context.Context, trs PipelineTaskRunSpec) (errs *apis.FieldError) {
285284
if trs.StepSpecs != nil {
286-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "stepSpecs", config.AlphaAPIFields).ViaField("stepSpecs"))
285+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "stepSpecs", config.AlphaAPIFields).ViaField("stepSpecs"))
287286
errs = errs.Also(validateStepSpecs(trs.StepSpecs).ViaField("stepSpecs"))
288287
}
289288
if trs.SidecarSpecs != nil {
290-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "sidecarSpecs", config.AlphaAPIFields).ViaField("sidecarSpecs"))
289+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "sidecarSpecs", config.AlphaAPIFields).ViaField("sidecarSpecs"))
291290
errs = errs.Also(validateSidecarSpecs(trs.SidecarSpecs).ViaField("sidecarSpecs"))
292291
}
293292
if trs.ComputeResources != nil {
294-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "computeResources", config.AlphaAPIFields).ViaField("computeResources"))
293+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "computeResources", config.AlphaAPIFields).ViaField("computeResources"))
295294
errs = errs.Also(validateTaskRunComputeResources(trs.ComputeResources, trs.StepSpecs))
296295
}
297296
if trs.PodTemplate != nil {

‎pkg/apis/pipeline/v1/task_validation.go‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727
"github.com/tektoncd/pipeline/pkg/apis/config"
2828
"github.com/tektoncd/pipeline/pkg/apis/pipeline"
2929
"github.com/tektoncd/pipeline/pkg/apis/validate"
30-
"github.com/tektoncd/pipeline/pkg/apis/version"
3130
"github.com/tektoncd/pipeline/pkg/substitution"
3231
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
3332
corev1 "k8s.io/api/core/v1"
@@ -109,21 +108,21 @@ func (ts *TaskSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError {
109108
// Object parameters
110109
for i, p := range ts.Params {
111110
if p.Type == ParamTypeObject {
112-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
111+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i))
113112
}
114113
}
115114
// Indexing into array parameters
116115
arrayIndexParamRefs := ts.GetIndexingReferencesToArrayParams()
117116
if len(arrayIndexParamRefs) != 0 {
118-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields))
117+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields))
119118
}
120119
// Array and object results
121120
for i, result := range ts.Results {
122121
switch result.Type {
123122
case ResultsTypeObject:
124-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
123+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i))
125124
case ResultsTypeArray:
126-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
125+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i))
127126
case ResultsTypeString:
128127
default:
129128
}
@@ -222,7 +221,7 @@ func validateWorkspaceUsages(ctx context.Context, ts *TaskSpec) (errs *apis.Fiel
222221

223222
for stepIdx, step := range steps {
224223
if len(step.Workspaces) != 0 {
225-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "step workspaces", config.BetaAPIFields).ViaIndex(stepIdx).ViaField("steps"))
224+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step workspaces", config.BetaAPIFields).ViaIndex(stepIdx).ViaField("steps"))
226225
}
227226
for workspaceIdx, w := range step.Workspaces {
228227
if !wsNames.Has(w.Name) {
@@ -233,7 +232,7 @@ func validateWorkspaceUsages(ctx context.Context, ts *TaskSpec) (errs *apis.Fiel
233232

234233
for sidecarIdx, sidecar := range sidecars {
235234
if len(sidecar.Workspaces) != 0 {
236-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "sidecar workspaces", config.BetaAPIFields).ViaIndex(sidecarIdx).ViaField("sidecars"))
235+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "sidecar workspaces", config.BetaAPIFields).ViaIndex(sidecarIdx).ViaField("sidecars"))
237236
}
238237
for workspaceIdx, w := range sidecar.Workspaces {
239238
if !wsNames.Has(w.Name) {
@@ -325,19 +324,19 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
325324
if s.Script != "" {
326325
cleaned := strings.TrimSpace(s.Script)
327326
if strings.HasPrefix(cleaned, "#!win") {
328-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "windows script support", config.AlphaAPIFields).ViaField("script"))
327+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "windows script support", config.AlphaAPIFields).ViaField("script"))
329328
}
330329
}
331330

332331
// StdoutConfig is an alpha feature and will fail validation if it's used in a task spec
333332
// when the enable-api-fields feature gate is not "alpha".
334333
if s.StdoutConfig != nil {
335-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "step stdout stream support", config.AlphaAPIFields).ViaField("stdoutconfig"))
334+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stdout stream support", config.AlphaAPIFields).ViaField("stdoutconfig"))
336335
}
337336
// StderrConfig is an alpha feature and will fail validation if it's used in a task spec
338337
// when the enable-api-fields feature gate is not "alpha".
339338
if s.StderrConfig != nil {
340-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "step stderr stream support", config.AlphaAPIFields).ViaField("stderrconfig"))
339+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stderr stream support", config.AlphaAPIFields).ViaField("stderrconfig"))
341340
}
342341
return errs
343342
}

‎pkg/apis/pipeline/v1/taskref_validation.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"strings"
2222

2323
"github.com/tektoncd/pipeline/pkg/apis/config"
24-
"github.com/tektoncd/pipeline/pkg/apis/version"
2524
"k8s.io/apimachinery/pkg/util/validation"
2625
"knative.dev/pkg/apis"
2726
)
@@ -36,13 +35,13 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) {
3635
switch {
3736
case ref.Resolver != "" || ref.Params != nil:
3837
if ref.Resolver != "" {
39-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
38+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver"))
4039
if ref.Name != "" {
4140
errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver"))
4241
}
4342
}
4443
if ref.Params != nil {
45-
errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver params", config.BetaAPIFields).ViaField("params"))
44+
errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver params", config.BetaAPIFields).ViaField("params"))
4645
if ref.Name != "" {
4746
errs = errs.Also(apis.ErrMultipleOneOf("name", "params"))
4847
}

0 commit comments

Comments
 (0)