Skip to content

Commit 76e419f

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

21 files changed

+99
-289
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/featuregate.go

Lines changed: 0 additions & 36 deletions
This file was deleted.

pkg/crd/markers/validation.go

Lines changed: 28 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() {
@@ -399,6 +399,31 @@ type ExactlyOneOf []string
399399
// +controllertools:marker:generateHelp:category="CRD validation"
400400
type AtLeastOneOf []string
401401

402+
// FeatureGate marks a field or type to be conditionally included based on feature gate enablement.
403+
//
404+
// Fields or types marked with +kubebuilder:featuregate will only be included in generated CRDs
405+
// when the specified feature gate expression evaluates to true via the crd:featureGates parameter.
406+
//
407+
// Supported formats:
408+
// - Single gate: +kubebuilder:featuregate=alpha
409+
// - OR expression: +kubebuilder:featuregate=alpha|beta (true if ANY gate is enabled)
410+
// - AND expression: +kubebuilder:featuregate=alpha&beta (true if ALL gates are enabled)
411+
// - Mixed with precedence: +kubebuilder:featuregate=alpha&beta|gamma (equivalent to (alpha&beta)|gamma)
412+
// - Explicit precedence: +kubebuilder:featuregate=(alpha|beta)&gamma
413+
//
414+
// Operator precedence follows standard conventions: & (AND) has higher precedence than | (OR).
415+
// Use parentheses for explicit grouping when needed.
416+
// +controllertools:marker:generateHelp:category="CRD feature gates"
417+
type FeatureGate string
418+
419+
// ApplyToSchema does nothing for feature gates - they are processed by the generator
420+
// to conditionally include/exclude fields and types.
421+
func (FeatureGate) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
422+
// Feature gates don't modify the schema directly.
423+
// They are processed by the generator to conditionally include/exclude fields.
424+
return nil
425+
}
426+
402427
func (m Maximum) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
403428
if !hasNumericType(schema) {
404429
return fmt.Errorf("must apply maximum to a numeric value, found %s", schema.Type)
@@ -756,44 +781,3 @@ func fieldsToOneOfCelRuleStr(fields []string) string {
756781
}
757782

758783
// +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

0 commit comments

Comments
 (0)