Skip to content

Commit 3be5d24

Browse files
committed
feat: finalize feature gate implementation with latest changes
1 parent 809dc79 commit 3be5d24

20 files changed

+74
-240
lines changed

cmd/controller-gen/workflow.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ func runGenerationForCombination(featureGates string, outputPath string, paths [
232232
fmt.Printf(" Generating: %s -> %s\n", featureGates, outputPath)
233233

234234
// Parse feature gates to validate them
235-
_, err := featuregate.ParseFeatureGates(featureGates, false)
235+
_, err := featuregate.ParseFeatureGates(featureGates)
236236
if err != nil {
237237
return fmt.Errorf("invalid feature gates %s: %w", featureGates, err)
238238
}

pkg/crd/gen.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func transformPreserveUnknownFields(value bool) func(map[string]interface{}) err
135135
}
136136

137137
func (g Generator) Generate(ctx *genall.GenerationContext) error {
138-
featureGates, err := featuregate.ParseFeatureGates(g.FeatureGates, true)
138+
featureGates, err := featuregate.ParseFeatureGates(g.FeatureGates)
139139
if err != nil {
140140
return fmt.Errorf("invalid feature gates: %w", err)
141141
}
@@ -336,8 +336,8 @@ func FindKubeKinds(parser *Parser, metav1Pkg *loader.Package, featureGates featu
336336

337337
// Check type-level feature gate marker
338338
if featureGateMarker := info.Markers.Get("kubebuilder:featuregate"); featureGateMarker != nil {
339-
if typeFeatureGate, ok := featureGateMarker.(crdmarkers.TypeFeatureGate); ok {
340-
gateName := string(typeFeatureGate)
339+
if featureGate, ok := featureGateMarker.(crdmarkers.FeatureGate); ok {
340+
gateName := string(featureGate)
341341
// Create evaluator to handle complex expressions (OR/AND logic)
342342
evaluator := featuregate.NewFeatureGateEvaluator(featureGates)
343343
if !evaluator.EvaluateExpression(gateName) {

pkg/crd/markers/crd.go

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,6 @@ var CRDMarkers = []*definitionWithHelp{
5757

5858
must(markers.MakeDefinition("kubebuilder:selectablefield", markers.DescribesType, SelectableField{})).
5959
WithHelp(SelectableField{}.Help()),
60-
61-
must(markers.MakeDefinition("kubebuilder:featuregate", markers.DescribesType, TypeFeatureGate(""))).
62-
WithHelp(TypeFeatureGate("").Help()),
6360
}
6461

6562
// TODO: categories and singular used to be annotations types
@@ -422,22 +419,3 @@ func (s SelectableField) ApplyToCRD(crd *apiext.CustomResourceDefinitionSpec, ve
422419

423420
return nil
424421
}
425-
426-
// +controllertools:marker:generateHelp:category="CRD feature gates"
427-
428-
// TypeFeatureGate marks an entire CRD type to be conditionally generated based on feature gate enablement.
429-
// Types marked with +kubebuilder:featuregate will only be included in generated CRDs
430-
// when the specified feature gate is enabled via the crd:featureGates parameter.
431-
//
432-
// Single gate format: +kubebuilder:featuregate=alpha
433-
// OR expression: +kubebuilder:featuregate=alpha|beta
434-
// AND expression: +kubebuilder:featuregate=alpha&beta
435-
// Complex expression: +kubebuilder:featuregate=(alpha&beta)|gamma
436-
type TypeFeatureGate string
437-
438-
// ApplyToCRD does nothing for type feature gates - they are processed by the generator
439-
// to conditionally include/exclude entire CRDs during generation.
440-
func (TypeFeatureGate) ApplyToCRD(crd *apiext.CustomResourceDefinitionSpec, version string) error {
441-
// Type feature gates are handled during CRD discovery/generation, not during CRD spec modification
442-
return nil
443-
}

pkg/crd/markers/validation.go

Lines changed: 3 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,11 @@ var ValidationIshMarkers = []*definitionWithHelp{
139139
must(markers.MakeAnyTypeDefinition("kubebuilder:title", markers.DescribesType, Title{})).
140140
WithHelp(Title{}.Help()),
141141

142+
// Feature gate markers for both fields and types (separate registrations)
142143
must(markers.MakeDefinition("kubebuilder:featuregate", markers.DescribesField, FeatureGate(""))).
143144
WithHelp(FeatureGate("").Help()),
144-
145-
must(markers.MakeDefinition("kubebuilder:validation:featureGate", markers.DescribesField, FeatureGateValidation{})).
146-
WithHelp(markers.SimpleHelp("CRD validation feature gates", "applies validation rules conditionally based on feature gate enablement.")),
145+
must(markers.MakeDefinition("kubebuilder:featuregate", markers.DescribesType, FeatureGate(""))).
146+
WithHelp(FeatureGate("").Help()),
147147
}
148148

149149
func init() {
@@ -756,44 +756,3 @@ func fieldsToOneOfCelRuleStr(fields []string) string {
756756
}
757757

758758
// +controllertools:marker:generateHelp:category="CRD validation feature gates"
759-
760-
// FeatureGateValidation marks a validation constraint to be conditionally applied based on feature gate enablement.
761-
// This allows validation rules to be enabled/disabled based on feature gates.
762-
// The validation parameter accepts the same values as standard kubebuilder:validation markers.
763-
//
764-
// Examples:
765-
// - +kubebuilder:validation:featureGate=alpha,rule="Maximum=100"
766-
// - +kubebuilder:validation:featureGate=alpha|beta,rule="MinLength=5"
767-
// - +kubebuilder:validation:featureGate=(alpha&beta)|gamma,rule="Pattern=^[a-z]+$"
768-
type FeatureGateValidation struct {
769-
// FeatureGate specifies the feature gate expression that must be satisfied
770-
// for this validation rule to be applied. Supports complex expressions with
771-
// AND (&), OR (|) operators and parentheses for precedence.
772-
FeatureGate string `marker:"featureGate"`
773-
774-
// Rule specifies the validation rule to apply when the feature gate is enabled.
775-
// This should be a valid validation marker rule (e.g., "Maximum=100", "MinLength=5").
776-
Rule string `marker:"rule"`
777-
}
778-
779-
// ApplyToSchema applies the validation rule to the schema if the feature gate is enabled.
780-
func (f FeatureGateValidation) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
781-
// This will be called by the schema generation with access to the context
782-
// The actual feature gate evaluation will be handled by the caller
783-
return fmt.Errorf("FeatureGateValidation cannot be applied directly - use feature gate context")
784-
}
785-
786-
// SupportsFeatureGate indicates this marker supports feature gates
787-
func (f FeatureGateValidation) SupportsFeatureGate() bool {
788-
return true
789-
}
790-
791-
// GetFeatureGate returns the feature gate expression
792-
func (f FeatureGateValidation) GetFeatureGate() string {
793-
return f.FeatureGate
794-
}
795-
796-
// GetValidationRule returns the validation rule to apply
797-
func (f FeatureGateValidation) GetValidationRule() string {
798-
return f.Rule
799-
}

pkg/crd/markers/zz_generated.markerhelp.go

Lines changed: 2 additions & 33 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/featuregate/doc.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ by providing a unified API for:
3030
3131
The simplest way to use this package is:
3232
33-
gates, err := featuregate.ParseFeatureGates("alpha=true,beta=false", false)
33+
gates, err := featuregate.ParseFeatureGates("alpha=true,beta=false")
3434
if err != nil {
3535
// handle error
3636
}

pkg/featuregate/evaluator.go

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -69,23 +69,13 @@ func (fge *FeatureGateEvaluator) evaluateOrExpression(expr string) bool {
6969
return false
7070
}
7171

72-
// hasAndOperator checks if the expression contains AND operators.
73-
func hasAndOperator(expr string) bool {
74-
return strings.Contains(expr, "&")
75-
}
76-
77-
// hasOrOperator checks if the expression contains OR operators.
78-
func hasOrOperator(expr string) bool {
79-
return strings.Contains(expr, "|")
80-
}
81-
82-
// evaluateComplexExpression evaluates complex feature gate expressions with parentheses.
83-
// Supports expressions like "(alpha&beta)|gamma" with proper precedence.
84-
func (fge *FeatureGateEvaluator) evaluateComplexExpression(expr string) bool {
72+
// evaluateSimpleExpression evaluates feature gate expressions with support for parentheses,
73+
// OR operations (lower precedence), and AND operations (higher precedence).
74+
func (fge *FeatureGateEvaluator) evaluateSimpleExpression(expr string) bool {
8575
// Remove all spaces for easier parsing
8676
expr = strings.ReplaceAll(expr, " ", "")
8777

88-
// Handle the expression recursively by evaluating parentheses first
78+
// Handle parentheses by evaluating them first (highest precedence)
8979
for strings.Contains(expr, "(") {
9080
// Find the innermost parentheses
9181
start := -1
@@ -108,13 +98,6 @@ func (fge *FeatureGateEvaluator) evaluateComplexExpression(expr string) bool {
10898
}
10999
}
110100

111-
// Now evaluate the remaining expression (which should be simple)
112-
return fge.evaluateSimpleExpression(expr)
113-
}
114-
115-
// evaluateSimpleExpression evaluates a simple expression without parentheses.
116-
// Handles OR operations which have lower precedence than AND operations.
117-
func (fge *FeatureGateEvaluator) evaluateSimpleExpression(expr string) bool {
118101
// Handle special boolean values from parenthetical evaluation
119102
if expr == boolTrueStr {
120103
return true

pkg/featuregate/evaluator_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,19 @@ var _ = Describe("FeatureGate Evaluator", func() {
167167
Expect(evaluator.EvaluateExpression("(alpha|gamma)&beta")).To(BeFalse())
168168
})
169169

170+
It("should handle mixed operators without parentheses using precedence (AND before OR)", func() {
171+
// alpha&gamma|beta = (alpha&gamma)|beta = (true&true)|false = true|false = true
172+
Expect(evaluator.EvaluateExpression("alpha&gamma|beta")).To(BeTrue())
173+
// alpha&beta|delta = (alpha&beta)|delta = (true&false)|false = false|false = false
174+
Expect(evaluator.EvaluateExpression("alpha&beta|delta")).To(BeFalse())
175+
// beta&delta|alpha = (beta&delta)|alpha = (false&false)|true = false|true = true
176+
Expect(evaluator.EvaluateExpression("beta&delta|alpha")).To(BeTrue())
177+
// More complex: alpha|beta&gamma = alpha|(beta&gamma) = true|(false&true) = true|false = true
178+
Expect(evaluator.EvaluateExpression("alpha|beta&gamma")).To(BeTrue())
179+
// More complex: beta|alpha&delta = beta|(alpha&delta) = false|(true&false) = false|false = false
180+
Expect(evaluator.EvaluateExpression("beta|alpha&delta")).To(BeFalse())
181+
})
182+
170183
It("should handle nested parentheses", func() {
171184
// ((alpha&gamma)|beta)&delta = ((true&true)|false)&false = (true|false)&false = true&false = false
172185
Expect(evaluator.EvaluateExpression("((alpha&gamma)|beta)&delta")).To(BeFalse())

pkg/featuregate/parser.go

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ import (
2424
// ParseFeatureGates parses a comma-separated feature gate string into a FeatureGateMap.
2525
// Format: "gate1=true,gate2=false,gate3=true"
2626
//
27-
// With strict validation enabled, this function will return an error for:
27+
// This function will return an error for:
2828
// - Invalid format (missing = or wrong number of parts)
2929
// - Invalid values (anything other than "true" or "false")
3030
//
31-
// Returns a FeatureGateMap and an error if parsing fails with strict validation.
32-
func ParseFeatureGates(featureGates string, strict bool) (FeatureGateMap, error) {
31+
// Returns a FeatureGateMap and an error if parsing fails.
32+
func ParseFeatureGates(featureGates string) (FeatureGateMap, error) {
3333
gates := make(FeatureGateMap)
3434
if featureGates == "" {
3535
return gates, nil
@@ -39,11 +39,7 @@ func ParseFeatureGates(featureGates string, strict bool) (FeatureGateMap, error)
3939
for _, pair := range pairs {
4040
parts := strings.Split(strings.TrimSpace(pair), "=")
4141
if len(parts) != 2 {
42-
if strict {
43-
return nil, fmt.Errorf("invalid feature gate format: %s (expected format: gate1=true,gate2=false)", pair)
44-
}
45-
// In non-strict mode, skip invalid entries
46-
continue
42+
return nil, fmt.Errorf("invalid feature gate format: %s (expected format: gate1=true,gate2=false)", pair)
4743
}
4844

4945
gateName := strings.TrimSpace(parts[0])
@@ -55,11 +51,7 @@ func ParseFeatureGates(featureGates string, strict bool) (FeatureGateMap, error)
5551
case "false":
5652
gates[gateName] = false
5753
default:
58-
if strict {
59-
return nil, fmt.Errorf("invalid feature gate value for %s: %s (must be 'true' or 'false')", gateName, gateValue)
60-
}
61-
// In non-strict mode, treat invalid values as false
62-
gates[gateName] = false
54+
return nil, fmt.Errorf("invalid feature gate value for %s: %s (must be 'true' or 'false')", gateName, gateValue)
6355
}
6456
}
6557

pkg/featuregate/parser_test.go

Lines changed: 13 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -26,88 +26,61 @@ func TestParseFeatureGates(t *testing.T) {
2626
tests := []struct {
2727
name string
2828
input string
29-
strict bool
3029
expected FeatureGateMap
3130
expectError bool
3231
errorContains string
3332
}{
3433
{
3534
name: "empty string",
3635
input: "",
37-
strict: true,
3836
expected: FeatureGateMap{},
3937
},
4038
{
41-
name: "single gate enabled",
42-
input: "alpha=true",
43-
strict: true,
39+
name: "single gate enabled",
40+
input: "alpha=true",
4441
expected: FeatureGateMap{
4542
"alpha": true,
4643
},
4744
},
4845
{
49-
name: "single gate disabled",
50-
input: "alpha=false",
51-
strict: true,
46+
name: "single gate disabled",
47+
input: "alpha=false",
5248
expected: FeatureGateMap{
5349
"alpha": false,
5450
},
5551
},
5652
{
57-
name: "multiple gates",
58-
input: "alpha=true,beta=false,gamma=true",
59-
strict: true,
53+
name: "multiple gates",
54+
input: "alpha=true,beta=false,gamma=true",
6055
expected: FeatureGateMap{
6156
"alpha": true,
6257
"beta": false,
6358
"gamma": true,
6459
},
6560
},
6661
{
67-
name: "gates with spaces",
68-
input: " alpha = true , beta = false ",
69-
strict: true,
62+
name: "gates with spaces",
63+
input: " alpha = true , beta = false ",
7064
expected: FeatureGateMap{
7165
"alpha": true,
7266
"beta": false,
7367
},
7468
},
7569
{
76-
name: "invalid format strict mode",
70+
name: "invalid format",
7771
input: "alpha=true,invalid,beta=false",
78-
strict: true,
7972
expectError: true,
8073
errorContains: "invalid feature gate format",
8174
},
8275
{
83-
name: "invalid format non-strict mode",
84-
input: "alpha=true,invalid,beta=false",
85-
strict: false,
86-
expected: FeatureGateMap{
87-
"alpha": true,
88-
"beta": false,
89-
},
90-
},
91-
{
92-
name: "invalid value strict mode",
76+
name: "invalid value",
9377
input: "alpha=true,beta=maybe",
94-
strict: true,
9578
expectError: true,
9679
errorContains: "invalid feature gate value",
9780
},
9881
{
99-
name: "invalid value non-strict mode",
100-
input: "alpha=true,beta=maybe",
101-
strict: false,
102-
expected: FeatureGateMap{
103-
"alpha": true,
104-
"beta": false, // Invalid values default to false
105-
},
106-
},
107-
{
108-
name: "complex gate names",
109-
input: "v1beta1=true,my-feature=false,under_score=true",
110-
strict: true,
82+
name: "complex gate names",
83+
input: "v1beta1=true,my-feature=false,under_score=true",
11184
expected: FeatureGateMap{
11285
"v1beta1": true,
11386
"my-feature": false,
@@ -119,7 +92,7 @@ func TestParseFeatureGates(t *testing.T) {
11992
for _, tt := range tests {
12093
t.Run(tt.name, func(t *testing.T) {
12194
g := gomega.NewWithT(t)
122-
result, err := ParseFeatureGates(tt.input, tt.strict)
95+
result, err := ParseFeatureGates(tt.input)
12396

12497
if tt.expectError {
12598
g.Expect(err).To(gomega.HaveOccurred())

0 commit comments

Comments
 (0)