Skip to content

Commit f369e27

Browse files
author
sami-wazery
committed
Remove legacy backward compatibility functions
Since feature gates are a new feature, no need for legacy functions: - Remove ParseFeatureGatesLegacy(), ValidateFeatureGateExpressionLegacy(), NewLegacyRegistry() - Update all usages to standard functions with non-strict validation - Clean up documentation and tests
1 parent 2c4f6d0 commit f369e27

File tree

11 files changed

+66
-188
lines changed

11 files changed

+66
-188
lines changed

pkg/featuregate/doc.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,20 @@ This package addresses the code duplication that existed in CRD, RBAC, and Webho
2222
by providing a unified API for:
2323
2424
- Parsing feature gate configurations from CLI parameters
25-
- Validating feature gate expressions with strict or legacy modes
25+
- Validating feature gate expressions with strict validation
2626
- Evaluating complex boolean expressions (AND, OR logic)
2727
- Managing known feature gates with a registry
2828
2929
# Basic Usage
3030
31-
The simplest way to use this package is with the legacy functions that maintain
32-
backward compatibility:
31+
The simplest way to use this package is:
3332
34-
gates := featuregate.ParseFeatureGatesLegacy("alpha=true,beta=false")
33+
gates, err := featuregate.ParseFeatureGates("alpha=true,beta=false", false)
34+
if err != nil {
35+
// handle error
36+
}
3537
evaluator := featuregate.NewFeatureGateEvaluator(gates)
36-
38+
3739
if evaluator.EvaluateExpression("alpha|beta") {
3840
// Include the feature
3941
}
@@ -62,19 +64,19 @@ For new implementations requiring strict validation:
6264
if err != nil {
6365
// Handle parsing error
6466
}
65-
67+
6668
err = registry.ValidateExpression("alpha|unknown")
6769
if err != nil {
6870
// Handle unknown gate error
6971
}
7072
71-
# Migration from Existing Code
73+
# Integration
7274
73-
This package provides legacy functions that match the behavior of existing
74-
implementations in CRD, RBAC, and Webhook generators:
75+
This package provides functions that centralize the feature gate logic
76+
previously duplicated across CRD, RBAC, and Webhook generators:
7577
76-
- ParseFeatureGatesLegacy() replaces individual parseFeatureGates() functions
77-
- ValidateFeatureGateExpressionLegacy() replaces individual validateFeatureGateExpression() functions
78+
- ParseFeatureGates() replaces individual parseFeatureGates() functions
79+
- ValidateFeatureGateExpression() replaces individual validateFeatureGateExpression() functions
7880
- FeatureGateEvaluator.EvaluateExpression() replaces individual shouldInclude*() functions
7981
8082
The FeatureGateMap type is compatible with existing map[string]bool usage patterns.

pkg/featuregate/parser.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,3 @@ func ParseFeatureGates(featureGates string, strict bool) (FeatureGateMap, error)
6565

6666
return gates, nil
6767
}
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: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -132,49 +132,3 @@ func TestParseFeatureGates(t *testing.T) {
132132
})
133133
}
134134
}
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-
}

pkg/featuregate/registry.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,6 @@ func NewRegistry(knownGates []string, strict bool) *Registry {
4040
}
4141
}
4242

43-
// NewLegacyRegistry creates a registry that maintains backward compatibility
44-
// with the existing behavior (no strict validation).
45-
func NewLegacyRegistry() *Registry {
46-
return &Registry{
47-
knownGates: sets.New[string](),
48-
strict: false,
49-
}
50-
}
51-
5243
// ParseAndValidate parses feature gates and validates expressions in one step.
5344
func (r *Registry) ParseAndValidate(featureGatesStr string, expression string) (FeatureGateMap, error) {
5445
// Parse the feature gates

pkg/featuregate/registry_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,6 @@ func TestNewRegistry(t *testing.T) {
3434
assert.False(t, registry.IsKnownGate("unknown"))
3535
}
3636

37-
func TestNewLegacyRegistry(t *testing.T) {
38-
registry := NewLegacyRegistry()
39-
40-
assert.False(t, registry.strict)
41-
assert.Equal(t, 0, registry.knownGates.Len())
42-
}
43-
4437
func TestRegistry_ParseAndValidate(t *testing.T) {
4538
registry := NewRegistry([]string{"alpha", "beta"}, true)
4639

pkg/featuregate/validator.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,6 @@ func ValidateFeatureGateExpression(expr string, knownGates sets.Set[string], str
6060
return nil
6161
}
6262

63-
// ValidateFeatureGateExpressionLegacy provides backward compatibility with the existing validation behavior.
64-
// This does not perform strict validation of gate names.
65-
func ValidateFeatureGateExpressionLegacy(expr string) error {
66-
return ValidateFeatureGateExpression(expr, nil, false)
67-
}
68-
6963
// isValidCharacter checks if a character is valid in a feature gate expression.
7064
func isValidCharacter(char rune) bool {
7165
return (char >= 'a' && char <= 'z') ||

pkg/featuregate/validator_test.go

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -166,63 +166,6 @@ func TestValidateFeatureGateExpression(t *testing.T) {
166166
}
167167
}
168168

169-
func TestValidateFeatureGateExpressionLegacy(t *testing.T) {
170-
tests := []struct {
171-
name string
172-
expr string
173-
expectError bool
174-
errorContains string
175-
}{
176-
{
177-
name: "empty expression",
178-
expr: "",
179-
},
180-
{
181-
name: "simple gate name",
182-
expr: "alpha",
183-
},
184-
{
185-
name: "OR expression",
186-
expr: "alpha|beta",
187-
},
188-
{
189-
name: "AND expression",
190-
expr: "alpha&beta",
191-
},
192-
{
193-
name: "mixed operators",
194-
expr: "alpha&beta|gamma",
195-
expectError: true,
196-
errorContains: "cannot mix '&' and '|' operators",
197-
},
198-
{
199-
name: "invalid character",
200-
expr: "alpha@beta",
201-
expectError: true,
202-
errorContains: "invalid character '@'",
203-
},
204-
{
205-
name: "unknown gate - no error in legacy mode",
206-
expr: "unknown_gate",
207-
},
208-
}
209-
210-
for _, tt := range tests {
211-
t.Run(tt.name, func(t *testing.T) {
212-
err := ValidateFeatureGateExpressionLegacy(tt.expr)
213-
214-
if tt.expectError {
215-
assert.Error(t, err)
216-
if tt.errorContains != "" {
217-
assert.Contains(t, err.Error(), tt.errorContains)
218-
}
219-
} else {
220-
assert.NoError(t, err)
221-
}
222-
})
223-
}
224-
}
225-
226169
func TestExtractGateNames(t *testing.T) {
227170
tests := []struct {
228171
name string

pkg/rbac/advanced_feature_gates_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func TestFeatureGateValidation(t *testing.T) {
129129

130130
for _, tt := range tests {
131131
t.Run(tt.name, func(t *testing.T) {
132-
err := featuregate.ValidateFeatureGateExpressionLegacy(tt.expression)
132+
err := featuregate.ValidateFeatureGateExpression(tt.expression, nil, false)
133133
if tt.shouldError {
134134
if err == nil {
135135
t.Errorf("Expected error for expression %s, but got none", tt.expression)

pkg/rbac/parser.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ import (
2727
"sort"
2828
"strings"
2929

30-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3130
rbacv1 "k8s.io/api/rbac/v1"
31+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3232

3333
"sigs.k8s.io/controller-tools/pkg/featuregate"
3434
"sigs.k8s.io/controller-tools/pkg/genall"
@@ -197,9 +197,12 @@ func (Generator) RegisterMarkers(into *markers.Registry) error {
197197
// The order of the objs in the returned slice is stable and determined by their namespaces.
198198
func GenerateRoles(ctx *genall.GenerationContext, roleName string, featureGates string) ([]interface{}, error) {
199199
// Parse feature gates using the centralized package
200-
enabledGates := featuregate.ParseFeatureGatesLegacy(featureGates)
200+
enabledGates, err := featuregate.ParseFeatureGates(featureGates, false)
201+
if err != nil {
202+
return nil, fmt.Errorf("failed to parse feature gates: %w", err)
203+
}
201204
evaluator := featuregate.NewFeatureGateEvaluator(enabledGates)
202-
205+
203206
rulesByNSResource := make(map[string][]*Rule)
204207
for _, root := range ctx.Roots {
205208
markerSet, err := markers.PackageMarkers(ctx.Collector, root)
@@ -212,7 +215,7 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string, featureGates
212215
rule := markerValue.(Rule)
213216

214217
// Validate feature gate expression syntax using centralized package
215-
if err := featuregate.ValidateFeatureGateExpressionLegacy(rule.FeatureGate); err != nil {
218+
if err := featuregate.ValidateFeatureGateExpression(rule.FeatureGate, nil, false); err != nil {
216219
return nil, fmt.Errorf("invalid feature gate expression in RBAC rule: %w", err)
217220
}
218221

0 commit comments

Comments
 (0)