Skip to content

Commit baf14ee

Browse files
author
sami-wazery
committed
Add centralized feature gate package
Introduces pkg/featuregate to eliminate code duplication across CRD, RBAC, and Webhook generators. This addresses the main architectural concerns raised in PR #1259 review. Features: - Centralized parsing and validation logic - Strict mode for unknown gate validation - Support for complex AND/OR expressions - Backward compatible legacy functions - Uses k8s.io/apimachinery/pkg/util/sets - Comprehensive test coverage The package provides both legacy functions for backward compatibility and a new Registry API for strict validation of known feature gates.
1 parent cc41ffa commit baf14ee

File tree

10 files changed

+1370
-0
lines changed

10 files changed

+1370
-0
lines changed

‎pkg/featuregate/doc.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/*
2+
Copyright 2024 The Kubernetes 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+
/*
18+
Package featuregate provides a centralized implementation for feature gate functionality
19+
across all controller-tools generators.
20+
21+
This package addresses the code duplication that existed in CRD, RBAC, and Webhook generators
22+
by providing a unified API for:
23+
24+
- Parsing feature gate configurations from CLI parameters
25+
- Validating feature gate expressions with strict or legacy modes
26+
- Evaluating complex boolean expressions (AND, OR logic)
27+
- Managing known feature gates with a registry
28+
29+
# Basic Usage
30+
31+
The simplest way to use this package is with the legacy functions that maintain
32+
backward compatibility:
33+
34+
gates := featuregate.ParseFeatureGatesLegacy("alpha=true,beta=false")
35+
evaluator := featuregate.NewFeatureGateEvaluator(gates)
36+
37+
if evaluator.EvaluateExpression("alpha|beta") {
38+
// Include the feature
39+
}
40+
41+
# Expression Syntax
42+
43+
Feature gate expressions support the following formats:
44+
45+
- Empty string: "" (always evaluates to true - no gating)
46+
- Single gate: "alpha" (true if alpha=true)
47+
- OR logic: "alpha|beta" (true if either alpha=true OR beta=true)
48+
- AND logic: "alpha&beta" (true if both alpha=true AND beta=true)
49+
50+
Multiple gates are supported:
51+
- "alpha|beta|gamma" (true if any gate is enabled)
52+
- "alpha&beta&gamma" (true if all gates are enabled)
53+
54+
Mixing AND and OR operators in the same expression is not allowed.
55+
56+
# Strict Validation
57+
58+
For new implementations requiring strict validation:
59+
60+
registry := featuregate.NewRegistry([]string{"alpha", "beta"}, true)
61+
evaluator, err := registry.CreateEvaluator("alpha=true,beta=false")
62+
if err != nil {
63+
// Handle parsing error
64+
}
65+
66+
err = registry.ValidateExpression("alpha|unknown")
67+
if err != nil {
68+
// Handle unknown gate error
69+
}
70+
71+
# Migration from Existing Code
72+
73+
This package provides legacy functions that match the behavior of existing
74+
implementations in CRD, RBAC, and Webhook generators:
75+
76+
- ParseFeatureGatesLegacy() replaces individual parseFeatureGates() functions
77+
- ValidateFeatureGateExpressionLegacy() replaces individual validateFeatureGateExpression() functions
78+
- FeatureGateEvaluator.EvaluateExpression() replaces individual shouldInclude*() functions
79+
80+
The FeatureGateMap type is compatible with existing map[string]bool usage patterns.
81+
*/
82+
package featuregate

‎pkg/featuregate/evaluator.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
Copyright 2024 The Kubernetes 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 featuregate
18+
19+
import (
20+
"strings"
21+
)
22+
23+
// evaluateAndExpression evaluates an AND expression where all gates must be enabled.
24+
// Format: "gate1&gate2&gate3" - returns true only if all gates are enabled.
25+
func (fge *FeatureGateEvaluator) evaluateAndExpression(expr string) bool {
26+
gates := strings.Split(expr, "&")
27+
for _, gate := range gates {
28+
gate = strings.TrimSpace(gate)
29+
if !fge.gates.IsEnabled(gate) {
30+
return false
31+
}
32+
}
33+
return true
34+
}
35+
36+
// evaluateOrExpression evaluates an OR expression where any gate can be enabled.
37+
// Format: "gate1|gate2|gate3" - returns true if any gate is enabled.
38+
func (fge *FeatureGateEvaluator) evaluateOrExpression(expr string) bool {
39+
gates := strings.Split(expr, "|")
40+
for _, gate := range gates {
41+
gate = strings.TrimSpace(gate)
42+
if fge.gates.IsEnabled(gate) {
43+
return true
44+
}
45+
}
46+
return false
47+
}
48+
49+
// hasAndOperator checks if the expression contains AND operators.
50+
func hasAndOperator(expr string) bool {
51+
return strings.Contains(expr, "&")
52+
}
53+
54+
// hasOrOperator checks if the expression contains OR operators.
55+
func hasOrOperator(expr string) bool {
56+
return strings.Contains(expr, "|")
57+
}

‎pkg/featuregate/parser.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/*
2+
Copyright 2024 The Kubernetes 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 featuregate
18+
19+
import (
20+
"fmt"
21+
"strings"
22+
)
23+
24+
// ParseFeatureGates parses a comma-separated feature gate string into a FeatureGateMap.
25+
// Format: "gate1=true,gate2=false,gate3=true"
26+
//
27+
// With strict validation enabled, this function will return an error for:
28+
// - Invalid format (missing = or wrong number of parts)
29+
// - Invalid values (anything other than "true" or "false")
30+
//
31+
// Returns a FeatureGateMap and an error if parsing fails with strict validation.
32+
func ParseFeatureGates(featureGates string, strict bool) (FeatureGateMap, error) {
33+
gates := make(FeatureGateMap)
34+
if featureGates == "" {
35+
return gates, nil
36+
}
37+
38+
pairs := strings.Split(featureGates, ",")
39+
for _, pair := range pairs {
40+
parts := strings.Split(strings.TrimSpace(pair), "=")
41+
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
47+
}
48+
49+
gateName := strings.TrimSpace(parts[0])
50+
gateValue := strings.TrimSpace(parts[1])
51+
52+
switch gateValue {
53+
case "true":
54+
gates[gateName] = true
55+
case "false":
56+
gates[gateName] = false
57+
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
63+
}
64+
}
65+
66+
return gates, nil
67+
}
68+
69+
// ParseFeatureGatesLegacy provides backward compatibility with the existing non-strict parsing behavior.
70+
// This is used by existing implementations that silently ignore invalid entries.
71+
func ParseFeatureGatesLegacy(featureGates string) FeatureGateMap {
72+
gates, _ := ParseFeatureGates(featureGates, false)
73+
return gates
74+
}

‎pkg/featuregate/parser_test.go

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
/*
2+
Copyright 2024 The Kubernetes 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 featuregate
18+
19+
import (
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
)
24+
25+
func TestParseFeatureGates(t *testing.T) {
26+
tests := []struct {
27+
name string
28+
input string
29+
strict bool
30+
expected FeatureGateMap
31+
expectError bool
32+
errorContains string
33+
}{
34+
{
35+
name: "empty string",
36+
input: "",
37+
strict: true,
38+
expected: FeatureGateMap{},
39+
},
40+
{
41+
name: "single gate enabled",
42+
input: "alpha=true",
43+
strict: true,
44+
expected: FeatureGateMap{
45+
"alpha": true,
46+
},
47+
},
48+
{
49+
name: "single gate disabled",
50+
input: "alpha=false",
51+
strict: true,
52+
expected: FeatureGateMap{
53+
"alpha": false,
54+
},
55+
},
56+
{
57+
name: "multiple gates",
58+
input: "alpha=true,beta=false,gamma=true",
59+
strict: true,
60+
expected: FeatureGateMap{
61+
"alpha": true,
62+
"beta": false,
63+
"gamma": true,
64+
},
65+
},
66+
{
67+
name: "gates with spaces",
68+
input: " alpha = true , beta = false ",
69+
strict: true,
70+
expected: FeatureGateMap{
71+
"alpha": true,
72+
"beta": false,
73+
},
74+
},
75+
{
76+
name: "invalid format strict mode",
77+
input: "alpha=true,invalid,beta=false",
78+
strict: true,
79+
expectError: true,
80+
errorContains: "invalid feature gate format",
81+
},
82+
{
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",
93+
input: "alpha=true,beta=maybe",
94+
strict: true,
95+
expectError: true,
96+
errorContains: "invalid feature gate value",
97+
},
98+
{
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,
111+
expected: FeatureGateMap{
112+
"v1beta1": true,
113+
"my-feature": false,
114+
"under_score": true,
115+
},
116+
},
117+
}
118+
119+
for _, tt := range tests {
120+
t.Run(tt.name, func(t *testing.T) {
121+
result, err := ParseFeatureGates(tt.input, tt.strict)
122+
123+
if tt.expectError {
124+
assert.Error(t, err)
125+
if tt.errorContains != "" {
126+
assert.Contains(t, err.Error(), tt.errorContains)
127+
}
128+
} else {
129+
assert.NoError(t, err)
130+
assert.Equal(t, tt.expected, result)
131+
}
132+
})
133+
}
134+
}
135+
136+
func TestParseFeatureGatesLegacy(t *testing.T) {
137+
tests := []struct {
138+
name string
139+
input string
140+
expected FeatureGateMap
141+
}{
142+
{
143+
name: "empty string",
144+
input: "",
145+
expected: FeatureGateMap{},
146+
},
147+
{
148+
name: "valid gates",
149+
input: "alpha=true,beta=false",
150+
expected: FeatureGateMap{
151+
"alpha": true,
152+
"beta": false,
153+
},
154+
},
155+
{
156+
name: "invalid format ignored",
157+
input: "alpha=true,invalid,beta=false",
158+
expected: FeatureGateMap{
159+
"alpha": true,
160+
"beta": false,
161+
},
162+
},
163+
{
164+
name: "invalid values default to false",
165+
input: "alpha=true,beta=maybe,gamma=false",
166+
expected: FeatureGateMap{
167+
"alpha": true,
168+
"beta": false,
169+
"gamma": false,
170+
},
171+
},
172+
}
173+
174+
for _, tt := range tests {
175+
t.Run(tt.name, func(t *testing.T) {
176+
result := ParseFeatureGatesLegacy(tt.input)
177+
assert.Equal(t, tt.expected, result)
178+
})
179+
}
180+
}

0 commit comments

Comments
 (0)