Skip to content

Commit 909a650

Browse files
wazerysami-wazery
authored andcommitted
Migrate CRD, RBAC, and Webhook generators to centralized featuregate package
- Remove duplicated code across all three generators - Update type references and method calls - All tests pass with backward compatibility maintained
1 parent baf14ee commit 909a650

17 files changed

+117
-445
lines changed

go.mod

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ require (
1010
github.com/onsi/gomega v1.38.0
1111
github.com/spf13/cobra v1.9.1
1212
github.com/spf13/pflag v1.0.7
13-
github.com/stretchr/testify v1.10.0
1413
golang.org/x/tools v0.36.0
1514
golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated
1615
gopkg.in/yaml.v2 v2.4.0
@@ -56,7 +55,6 @@ require (
5655
github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect
5756
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
5857
github.com/nxadm/tail v1.4.8 // indirect
59-
github.com/pmezard/go-difflib v1.0.0 // indirect
6058
github.com/prometheus/client_golang v1.22.0 // indirect
6159
github.com/prometheus/client_model v0.6.1 // indirect
6260
github.com/prometheus/common v0.62.0 // indirect

pkg/crd/gen.go

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -26,53 +26,13 @@ import (
2626
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2727
"k8s.io/apimachinery/pkg/runtime/schema"
2828
crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
29+
"sigs.k8s.io/controller-tools/pkg/featuregate"
2930
"sigs.k8s.io/controller-tools/pkg/genall"
3031
"sigs.k8s.io/controller-tools/pkg/loader"
3132
"sigs.k8s.io/controller-tools/pkg/markers"
3233
"sigs.k8s.io/controller-tools/pkg/version"
3334
)
3435

35-
// FeatureGateMap represents a map of feature gate names to their enabled status.
36-
type FeatureGateMap map[string]bool
37-
38-
// parseFeatureGates parses a feature gates string in the format "gate1=true,gate2=false"
39-
// and returns a FeatureGateMap. Supports comma-separated key=value pairs.
40-
// and returns a FeatureGateMap.
41-
func parseFeatureGates(featureGatesStr string) (FeatureGateMap, error) {
42-
gates := make(FeatureGateMap)
43-
if featureGatesStr == "" {
44-
return gates, nil
45-
}
46-
47-
pairs := strings.Split(featureGatesStr, ",")
48-
for _, pair := range pairs {
49-
parts := strings.Split(strings.TrimSpace(pair), "=")
50-
if len(parts) != 2 {
51-
return nil, fmt.Errorf("invalid feature gate format: %s (expected format: gate1=true,gate2=false)", pair)
52-
}
53-
54-
name := strings.TrimSpace(parts[0])
55-
valueStr := strings.TrimSpace(parts[1])
56-
57-
switch valueStr {
58-
case "true":
59-
gates[name] = true
60-
case "false":
61-
gates[name] = false
62-
default:
63-
return nil, fmt.Errorf("invalid feature gate value for %s: %s (must be 'true' or 'false')", name, valueStr)
64-
}
65-
}
66-
67-
return gates, nil
68-
}
69-
70-
// isFeatureGateEnabled checks if a feature gate is enabled.
71-
func (fg FeatureGateMap) isEnabled(gateName string) bool {
72-
enabled, exists := fg[gateName]
73-
return exists && enabled
74-
}
75-
7636
// The identifier for v1 CustomResourceDefinitions.
7737
const v1 = "v1"
7838

@@ -175,7 +135,7 @@ func transformPreserveUnknownFields(value bool) func(map[string]interface{}) err
175135
}
176136

177137
func (g Generator) Generate(ctx *genall.GenerationContext) error {
178-
featureGates, err := parseFeatureGates(g.FeatureGates)
138+
featureGates, err := featuregate.ParseFeatureGates(g.FeatureGates, true)
179139
if err != nil {
180140
return fmt.Errorf("invalid feature gates: %w", err)
181141
}

pkg/crd/markers/validation.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,18 +127,14 @@ var ValidationIshMarkers = []*definitionWithHelp{
127127
WithHelp(XPreserveUnknownFields{}.Help()),
128128
must(markers.MakeDefinition("kubebuilder:pruning:PreserveUnknownFields", markers.DescribesType, XPreserveUnknownFields{})).
129129
WithHelp(XPreserveUnknownFields{}.Help()),
130+
131+
must(markers.MakeDefinition("kubebuilder:feature-gate", markers.DescribesField, FeatureGate(""))).
132+
WithHelp(FeatureGate("").Help()),
130133
}
131134

132135
func init() {
133136
AllDefinitions = append(AllDefinitions, ValidationMarkers...)
134137

135-
// Add FeatureGate markers
136-
featureGateMarkers := []*definitionWithHelp{
137-
must(markers.MakeDefinition("kubebuilder:feature-gate", markers.DescribesField, FeatureGate(""))).WithHelp(FeatureGate("").Help()),
138-
must(markers.MakeDefinition("kubebuilder:feature-gate", markers.DescribesType, FeatureGate(""))).WithHelp(FeatureGate("").Help()),
139-
}
140-
AllDefinitions = append(AllDefinitions, featureGateMarkers...)
141-
142138
for _, def := range ValidationMarkers {
143139
typDef := def.clone()
144140
typDef.Target = markers.DescribesType

pkg/crd/parser.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2323
"k8s.io/apimachinery/pkg/runtime/schema"
24+
"sigs.k8s.io/controller-tools/pkg/featuregate"
2425
"sigs.k8s.io/controller-tools/pkg/internal/crd"
2526
"sigs.k8s.io/controller-tools/pkg/loader"
2627
"sigs.k8s.io/controller-tools/pkg/markers"
@@ -94,7 +95,7 @@ type Parser struct {
9495
GenerateEmbeddedObjectMeta bool
9596

9697
// FeatureGates specifies which feature gates are enabled for conditional field inclusion
97-
FeatureGates FeatureGateMap
98+
FeatureGates featuregate.FeatureGateMap
9899
}
99100

100101
func (p *Parser) init() {

pkg/crd/schema.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2929
"k8s.io/apimachinery/pkg/util/sets"
3030
crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
31+
"sigs.k8s.io/controller-tools/pkg/featuregate"
3132
"sigs.k8s.io/controller-tools/pkg/loader"
3233
"sigs.k8s.io/controller-tools/pkg/markers"
3334
)
@@ -72,12 +73,12 @@ type schemaContext struct {
7273

7374
allowDangerousTypes bool
7475
ignoreUnexportedFields bool
75-
featureGates FeatureGateMap
76+
featureGates featuregate.FeatureGateMap
7677
}
7778

7879
// newSchemaContext constructs a new schemaContext for the given package and schema requester.
7980
// It must have type info added before use via ForInfo.
80-
func newSchemaContext(pkg *loader.Package, req schemaRequester, allowDangerousTypes, ignoreUnexportedFields bool, featureGates FeatureGateMap) *schemaContext {
81+
func newSchemaContext(pkg *loader.Package, req schemaRequester, allowDangerousTypes, ignoreUnexportedFields bool, featureGates featuregate.FeatureGateMap) *schemaContext {
8182
pkg.NeedTypesInfo()
8283
return &schemaContext{
8384
pkg: pkg,
@@ -435,7 +436,7 @@ func structToSchema(ctx *schemaContext, structType *ast.StructType) *apiext.JSON
435436
if featureGateMarker := field.Markers.Get("kubebuilder:feature-gate"); featureGateMarker != nil {
436437
if featureGate, ok := featureGateMarker.(crdmarkers.FeatureGate); ok {
437438
gateName := string(featureGate)
438-
if !ctx.featureGates.isEnabled(gateName) {
439+
if !ctx.featureGates.IsEnabled(gateName) {
439440
// Skip this field as its feature gate is not enabled
440441
continue
441442
}

pkg/crd/schema_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
pkgstest "golang.org/x/tools/go/packages/packagestest"
2727
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2828
crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
29+
"sigs.k8s.io/controller-tools/pkg/featuregate"
2930
testloader "sigs.k8s.io/controller-tools/pkg/loader/testutils"
3031
"sigs.k8s.io/controller-tools/pkg/markers"
3132
)
@@ -64,7 +65,7 @@ func transform(t *testing.T, expr string) *apiext.JSONSchemaProps {
6465
pkg.NeedTypesInfo()
6566
failIfErrors(t, pkg.Errors)
6667

67-
schemaContext := newSchemaContext(pkg, nil, true, false, FeatureGateMap{}).ForInfo(&markers.TypeInfo{})
68+
schemaContext := newSchemaContext(pkg, nil, true, false, featuregate.FeatureGateMap{}).ForInfo(&markers.TypeInfo{})
6869
// yick: grab the only type definition
6970
definedType := pkg.Syntax[0].Decls[0].(*ast.GenDecl).Specs[0].(*ast.TypeSpec).Type
7071
result := typeToSchema(schemaContext, definedType)

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: 6 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ package featuregate
1919
import (
2020
"testing"
2121

22-
"github.com/stretchr/testify/assert"
22+
"github.com/onsi/gomega"
2323
)
2424

2525
func TestParseFeatureGates(t *testing.T) {
@@ -118,63 +118,18 @@ func TestParseFeatureGates(t *testing.T) {
118118

119119
for _, tt := range tests {
120120
t.Run(tt.name, func(t *testing.T) {
121+
g := gomega.NewWithT(t)
121122
result, err := ParseFeatureGates(tt.input, tt.strict)
122123

123124
if tt.expectError {
124-
assert.Error(t, err)
125+
g.Expect(err).To(gomega.HaveOccurred())
125126
if tt.errorContains != "" {
126-
assert.Contains(t, err.Error(), tt.errorContains)
127+
g.Expect(err.Error()).To(gomega.ContainSubstring(tt.errorContains))
127128
}
128129
} else {
129-
assert.NoError(t, err)
130-
assert.Equal(t, tt.expected, result)
130+
g.Expect(err).NotTo(gomega.HaveOccurred())
131+
g.Expect(result).To(gomega.Equal(tt.expected))
131132
}
132133
})
133134
}
134135
}
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

0 commit comments

Comments
 (0)