diff --git a/go.mod b/go.mod index c8b8821b1..518e5cae4 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/onsi/gomega v1.38.0 github.com/spf13/cobra v1.9.1 github.com/spf13/pflag v1.0.7 + github.com/stretchr/testify v1.10.0 golang.org/x/tools v0.36.0 golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated gopkg.in/yaml.v2 v2.4.0 @@ -55,6 +56,7 @@ require ( github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/nxadm/tail v1.4.8 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.22.0 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.62.0 // indirect diff --git a/pkg/crd/featuregate_integration_test.go b/pkg/crd/featuregate_integration_test.go new file mode 100644 index 000000000..52ea3b24b --- /dev/null +++ b/pkg/crd/featuregate_integration_test.go @@ -0,0 +1,155 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package crd_test + +import ( + "bytes" + "io" + "os" + "path/filepath" + + "github.com/google/go-cmp/cmp" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-tools/pkg/crd" + crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers" + "sigs.k8s.io/controller-tools/pkg/genall" + "sigs.k8s.io/controller-tools/pkg/loader" + "sigs.k8s.io/controller-tools/pkg/markers" +) + +var _ = Describe("CRD Feature Gate Generation", func() { + var ( + ctx *genall.GenerationContext + out *featureGateOutputRule + featureGateDir string + originalWorkingDir string + ) + + BeforeEach(func() { + var err error + originalWorkingDir, err = os.Getwd() + Expect(err).NotTo(HaveOccurred()) + + featureGateDir = filepath.Join(originalWorkingDir, "testdata", "featuregates") + + By("switching into featuregates testdata") + err = os.Chdir(featureGateDir) + Expect(err).NotTo(HaveOccurred()) + + By("loading the roots") + pkgs, err := loader.LoadRoots(".") + Expect(err).NotTo(HaveOccurred()) + Expect(pkgs).To(HaveLen(1)) + + out = &featureGateOutputRule{buf: &bytes.Buffer{}} + ctx = &genall.GenerationContext{ + Collector: &markers.Collector{Registry: &markers.Registry{}}, + Roots: pkgs, + Checker: &loader.TypeChecker{}, + OutputRule: out, + } + Expect(crdmarkers.Register(ctx.Collector.Registry)).To(Succeed()) + }) + + AfterEach(func() { + By("restoring original working directory") + err := os.Chdir(originalWorkingDir) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should not include feature-gated fields when no gates are enabled", func() { + By("calling the generator") + gen := &crd.Generator{ + CRDVersions: []string{"v1"}, + // No FeatureGates specified + } + Expect(gen.Generate(ctx)).NotTo(HaveOccurred()) + + By("loading the expected YAML") + expectedFile, err := os.ReadFile("output_none/_featuregatetests.yaml") + Expect(err).NotTo(HaveOccurred()) + + By("comparing the two") + Expect(out.buf.String()).To(Equal(string(expectedFile)), cmp.Diff(out.buf.String(), string(expectedFile))) + }) + + It("should include only alpha-gated fields when alpha gate is enabled", func() { + By("calling the generator") + gen := &crd.Generator{ + CRDVersions: []string{"v1"}, + FeatureGates: "alpha=true", + } + Expect(gen.Generate(ctx)).NotTo(HaveOccurred()) + + By("loading the expected YAML") + expectedFile, err := os.ReadFile("output_alpha/_featuregatetests.yaml") + Expect(err).NotTo(HaveOccurred()) + + By("comparing the two") + Expect(out.buf.String()).To(Equal(string(expectedFile)), cmp.Diff(out.buf.String(), string(expectedFile))) + }) + + It("should include only beta-gated fields when beta gate is enabled", func() { + By("calling the generator") + gen := &crd.Generator{ + CRDVersions: []string{"v1"}, + FeatureGates: "beta=true", + } + Expect(gen.Generate(ctx)).NotTo(HaveOccurred()) + + By("loading the expected YAML") + expectedFile, err := os.ReadFile("output_beta/_featuregatetests.yaml") + Expect(err).NotTo(HaveOccurred()) + + By("comparing the two") + Expect(out.buf.String()).To(Equal(string(expectedFile)), cmp.Diff(out.buf.String(), string(expectedFile))) + }) + + It("should include both feature-gated fields when both gates are enabled", func() { + By("calling the generator") + gen := &crd.Generator{ + CRDVersions: []string{"v1"}, + FeatureGates: "alpha=true,beta=true", + } + Expect(gen.Generate(ctx)).NotTo(HaveOccurred()) + + By("loading the expected YAML") + expectedFile, err := os.ReadFile("output_both/_featuregatetests.yaml") + Expect(err).NotTo(HaveOccurred()) + + By("comparing the two") + Expect(out.buf.String()).To(Equal(string(expectedFile)), cmp.Diff(out.buf.String(), string(expectedFile))) + }) +}) + +// Helper types for testing +type featureGateOutputRule struct { + buf *bytes.Buffer +} + +func (o *featureGateOutputRule) Open(_ *loader.Package, _ string) (io.WriteCloser, error) { + return featureGateNopCloser{o.buf}, nil +} + +type featureGateNopCloser struct { + io.Writer +} + +func (n featureGateNopCloser) Close() error { + return nil +} diff --git a/pkg/crd/gen.go b/pkg/crd/gen.go index 5fad65a71..ccfa6efad 100644 --- a/pkg/crd/gen.go +++ b/pkg/crd/gen.go @@ -32,6 +32,47 @@ import ( "sigs.k8s.io/controller-tools/pkg/version" ) +// FeatureGateMap represents a map of feature gate names to their enabled status. +type FeatureGateMap map[string]bool + +// parseFeatureGates parses a feature gates string in the format "gate1=true,gate2=false" +// and returns a FeatureGateMap. Supports comma-separated key=value pairs. +// and returns a FeatureGateMap. +func parseFeatureGates(featureGatesStr string) (FeatureGateMap, error) { + gates := make(FeatureGateMap) + if featureGatesStr == "" { + return gates, nil + } + + pairs := strings.Split(featureGatesStr, ",") + for _, pair := range pairs { + parts := strings.Split(strings.TrimSpace(pair), "=") + if len(parts) != 2 { + return nil, fmt.Errorf("invalid feature gate format: %s (expected format: gate1=true,gate2=false)", pair) + } + + name := strings.TrimSpace(parts[0]) + valueStr := strings.TrimSpace(parts[1]) + + switch valueStr { + case "true": + gates[name] = true + case "false": + gates[name] = false + default: + return nil, fmt.Errorf("invalid feature gate value for %s: %s (must be 'true' or 'false')", name, valueStr) + } + } + + return gates, nil +} + +// isFeatureGateEnabled checks if a feature gate is enabled. +func (fg FeatureGateMap) isEnabled(gateName string) bool { + enabled, exists := fg[gateName] + return exists && enabled +} + // The identifier for v1 CustomResourceDefinitions. const v1 = "v1" @@ -85,6 +126,16 @@ type Generator struct { // Year specifies the year to substitute for " YEAR" in the header file. Year string `marker:",optional"` + // FeatureGates specifies which feature gates are enabled for conditional field inclusion. + // + // Single gate format: "gatename=true" + // Multiple gates format: "gate1=true,gate2=false" (must use quoted strings for comma-separated values) + // + // Examples: + // controller-gen crd:featureGates="alpha=true" paths=./api/... + // controller-gen 'crd:featureGates="alpha=true,beta=false"' paths=./api/... + FeatureGates string `marker:",optional"` + // DeprecatedV1beta1CompatibilityPreserveUnknownFields indicates whether // or not we should turn off field pruning for this resource. // @@ -124,6 +175,11 @@ func transformPreserveUnknownFields(value bool) func(map[string]interface{}) err } func (g Generator) Generate(ctx *genall.GenerationContext) error { + featureGates, err := parseFeatureGates(g.FeatureGates) + if err != nil { + return fmt.Errorf("invalid feature gates: %w", err) + } + parser := &Parser{ Collector: ctx.Collector, Checker: ctx.Checker, @@ -132,6 +188,7 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error { AllowDangerousTypes: g.AllowDangerousTypes != nil && *g.AllowDangerousTypes, // Indicates the parser on whether to register the ObjectMeta type or not GenerateEmbeddedObjectMeta: g.GenerateEmbeddedObjectMeta != nil && *g.GenerateEmbeddedObjectMeta, + FeatureGates: featureGates, } AddKnownTypes(parser) diff --git a/pkg/crd/markers/featuregate.go b/pkg/crd/markers/featuregate.go new file mode 100644 index 000000000..f99e75c98 --- /dev/null +++ b/pkg/crd/markers/featuregate.go @@ -0,0 +1,36 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package markers + +import ( + apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" +) + +// +controllertools:marker:generateHelp:category="CRD feature gates" + +// FeatureGate marks a field to be conditionally included based on feature gate enablement. +// Fields marked with +kubebuilder:feature-gate will only be included in generated CRDs +// when the specified feature gate is enabled via the crd:featureGates parameter. +type FeatureGate string + +// ApplyToSchema does nothing for feature gates - they are processed by the generator +// to conditionally include/exclude fields. +func (FeatureGate) ApplyToSchema(schema *apiext.JSONSchemaProps, field string) error { + // Feature gates don't modify the schema directly. + // They are processed by the generator to conditionally include/exclude fields. + return nil +} diff --git a/pkg/crd/markers/validation.go b/pkg/crd/markers/validation.go index 839b07f6e..8b5d4a992 100644 --- a/pkg/crd/markers/validation.go +++ b/pkg/crd/markers/validation.go @@ -132,6 +132,13 @@ var ValidationIshMarkers = []*definitionWithHelp{ func init() { AllDefinitions = append(AllDefinitions, ValidationMarkers...) + // Add FeatureGate markers + featureGateMarkers := []*definitionWithHelp{ + must(markers.MakeDefinition("kubebuilder:feature-gate", markers.DescribesField, FeatureGate(""))).WithHelp(FeatureGate("").Help()), + must(markers.MakeDefinition("kubebuilder:feature-gate", markers.DescribesType, FeatureGate(""))).WithHelp(FeatureGate("").Help()), + } + AllDefinitions = append(AllDefinitions, featureGateMarkers...) + for _, def := range ValidationMarkers { typDef := def.clone() typDef.Target = markers.DescribesType diff --git a/pkg/crd/parser.go b/pkg/crd/parser.go index 3281f24a1..f4de13a6b 100644 --- a/pkg/crd/parser.go +++ b/pkg/crd/parser.go @@ -92,6 +92,9 @@ type Parser struct { // GenerateEmbeddedObjectMeta specifies if any embedded ObjectMeta should be generated GenerateEmbeddedObjectMeta bool + + // FeatureGates specifies which feature gates are enabled for conditional field inclusion + FeatureGates FeatureGateMap } func (p *Parser) init() { @@ -172,7 +175,7 @@ func (p *Parser) NeedSchemaFor(typ TypeIdent) { // avoid tripping recursive schemata, like ManagedFields, by adding an empty WIP schema p.Schemata[typ] = apiext.JSONSchemaProps{} - schemaCtx := newSchemaContext(typ.Package, p, p.AllowDangerousTypes, p.IgnoreUnexportedFields) + schemaCtx := newSchemaContext(typ.Package, p, p.AllowDangerousTypes, p.IgnoreUnexportedFields, p.FeatureGates) ctxForInfo := schemaCtx.ForInfo(info) pkgMarkers, err := markers.PackageMarkers(p.Collector, typ.Package) diff --git a/pkg/crd/schema.go b/pkg/crd/schema.go index efb09b7c9..ccfdbb9b8 100644 --- a/pkg/crd/schema.go +++ b/pkg/crd/schema.go @@ -72,17 +72,19 @@ type schemaContext struct { allowDangerousTypes bool ignoreUnexportedFields bool + featureGates FeatureGateMap } // newSchemaContext constructs a new schemaContext for the given package and schema requester. // It must have type info added before use via ForInfo. -func newSchemaContext(pkg *loader.Package, req schemaRequester, allowDangerousTypes, ignoreUnexportedFields bool) *schemaContext { +func newSchemaContext(pkg *loader.Package, req schemaRequester, allowDangerousTypes, ignoreUnexportedFields bool, featureGates FeatureGateMap) *schemaContext { pkg.NeedTypesInfo() return &schemaContext{ pkg: pkg, schemaRequester: req, allowDangerousTypes: allowDangerousTypes, ignoreUnexportedFields: ignoreUnexportedFields, + featureGates: featureGates, } } @@ -95,6 +97,7 @@ func (c *schemaContext) ForInfo(info *markers.TypeInfo) *schemaContext { schemaRequester: c.schemaRequester, allowDangerousTypes: c.allowDangerousTypes, ignoreUnexportedFields: c.ignoreUnexportedFields, + featureGates: c.featureGates, } } @@ -428,6 +431,17 @@ func structToSchema(ctx *schemaContext, structType *ast.StructType) *apiext.JSON continue } + // Check feature gate markers - skip field if feature gate is not enabled + if featureGateMarker := field.Markers.Get("kubebuilder:feature-gate"); featureGateMarker != nil { + if featureGate, ok := featureGateMarker.(crdmarkers.FeatureGate); ok { + gateName := string(featureGate) + if !ctx.featureGates.isEnabled(gateName) { + // Skip this field as its feature gate is not enabled + continue + } + } + } + jsonTag, hasTag := field.Tag.Lookup("json") if !hasTag { // if the field doesn't have a JSON tag, it doesn't belong in output (and shouldn't exist in a serialized type) diff --git a/pkg/crd/schema_test.go b/pkg/crd/schema_test.go index 9c0581d6f..c6f8011ce 100644 --- a/pkg/crd/schema_test.go +++ b/pkg/crd/schema_test.go @@ -64,7 +64,7 @@ func transform(t *testing.T, expr string) *apiext.JSONSchemaProps { pkg.NeedTypesInfo() failIfErrors(t, pkg.Errors) - schemaContext := newSchemaContext(pkg, nil, true, false).ForInfo(&markers.TypeInfo{}) + schemaContext := newSchemaContext(pkg, nil, true, false, FeatureGateMap{}).ForInfo(&markers.TypeInfo{}) // yick: grab the only type definition definedType := pkg.Syntax[0].Decls[0].(*ast.GenDecl).Specs[0].(*ast.TypeSpec).Type result := typeToSchema(schemaContext, definedType) diff --git a/pkg/crd/testdata/featuregates/types.go b/pkg/crd/testdata/featuregates/types.go new file mode 100644 index 000000000..052e54b1d --- /dev/null +++ b/pkg/crd/testdata/featuregates/types.go @@ -0,0 +1,74 @@ +/* +Copyright 2025. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +//go:generate ../../../../.run-controller-gen.sh crd:featureGates=alpha=true paths=. output:dir=./output_alpha +//go:generate ../../../../.run-controller-gen.sh crd:featureGates=beta=true paths=. output:dir=./output_beta +//go:generate ../../../../.run-controller-gen.sh crd paths=. output:dir=./output_none + +package featuregates + +import ( +metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// FeatureGateTestSpec defines the desired state with feature-gated fields +type FeatureGateTestSpec struct { + // Standard field - always included + Name string `json:"name"` + + // Alpha-gated field - only included when alpha gate is enabled + // +kubebuilder:feature-gate=alpha + AlphaFeature *string `json:"alphaFeature,omitempty"` + + // Beta-gated field - only included when beta gate is enabled + // +kubebuilder:feature-gate=beta + BetaFeature *string `json:"betaFeature,omitempty"` +} + +// FeatureGateTestStatus defines the observed state with feature-gated fields +type FeatureGateTestStatus struct { + // Standard status field + Ready bool `json:"ready"` + + // Alpha-gated status field + // +kubebuilder:feature-gate=alpha + AlphaStatus *string `json:"alphaStatus,omitempty"` + + // Beta-gated status field + // +kubebuilder:feature-gate=beta + BetaStatus *string `json:"betaStatus,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status + +// FeatureGateTest is the Schema for testing feature gates +type FeatureGateTest struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec FeatureGateTestSpec `json:"spec,omitempty"` + Status FeatureGateTestStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// FeatureGateTestList contains a list of FeatureGateTest +type FeatureGateTestList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []FeatureGateTest `json:"items"` +} diff --git a/pkg/rbac/advanced_feature_gates_test.go b/pkg/rbac/advanced_feature_gates_test.go new file mode 100644 index 000000000..ef7c7bab6 --- /dev/null +++ b/pkg/rbac/advanced_feature_gates_test.go @@ -0,0 +1,143 @@ +package rbac + +import ( + "strings" + "testing" + + "github.com/onsi/gomega" + rbacv1 "k8s.io/api/rbac/v1" + "sigs.k8s.io/controller-tools/pkg/genall" + "sigs.k8s.io/controller-tools/pkg/loader" + "sigs.k8s.io/controller-tools/pkg/markers" +) + +func TestAdvancedFeatureGates(t *testing.T) { + g := gomega.NewWithT(t) + + // Load test packages + pkgs, err := loader.LoadRoots("./testdata/advanced_feature_gates") + g.Expect(err).NotTo(gomega.HaveOccurred()) + + // Set up generation context + reg := &markers.Registry{} + g.Expect(reg.Register(RuleDefinition)).To(gomega.Succeed()) + + ctx := &genall.GenerationContext{ + Collector: &markers.Collector{Registry: reg}, + Roots: pkgs, + } + + tests := []struct { + name string + featureGates string + expectedRules int + shouldContain []string + shouldNotContain []string + }{ + { + name: "OR logic - alpha enabled", + featureGates: "alpha=true,beta=false", + expectedRules: 3, // always-on + OR rule (alpha|beta) + shouldContain: []string{"pods", "configmaps", "secrets"}, + shouldNotContain: []string{"services"}, + }, + { + name: "OR logic - beta enabled", + featureGates: "alpha=false,beta=true", + expectedRules: 3, // always-on + OR rule (alpha|beta) + shouldContain: []string{"pods", "configmaps", "secrets"}, + shouldNotContain: []string{"services"}, + }, + { + name: "OR logic - both enabled", + featureGates: "alpha=true,beta=true", + expectedRules: 4, // always-on + OR rule + AND rule + shouldContain: []string{"pods", "configmaps", "secrets", "services"}, + shouldNotContain: []string{}, + }, + { + name: "OR logic - neither enabled", + featureGates: "alpha=false,beta=false", + expectedRules: 2, // only always-on + shouldContain: []string{"pods", "configmaps"}, + shouldNotContain: []string{"secrets", "services"}, + }, + { + name: "AND logic - only alpha enabled", + featureGates: "alpha=true,beta=false", + expectedRules: 3, // always-on + OR rule (alpha|beta) + shouldContain: []string{"pods", "configmaps", "secrets"}, + shouldNotContain: []string{"services"}, + }, + { + name: "AND logic - both enabled", + featureGates: "alpha=true,beta=true", + expectedRules: 4, // always-on + OR rule + AND rule + shouldContain: []string{"pods", "configmaps", "secrets", "services"}, + shouldNotContain: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := gomega.NewWithT(t) + + objs, err := GenerateRoles(ctx, "test-role", tt.featureGates) + g.Expect(err).NotTo(gomega.HaveOccurred()) + g.Expect(objs).To(gomega.HaveLen(1)) + + role, ok := objs[0].(rbacv1.ClusterRole) + g.Expect(ok).To(gomega.BeTrue()) + g.Expect(role.Rules).To(gomega.HaveLen(tt.expectedRules)) + + // Convert rules to string for easier checking + rulesStr := "" + for _, rule := range role.Rules { + rulesStr += strings.Join(rule.Resources, ",") + " " + } + + for _, resource := range tt.shouldContain { + g.Expect(rulesStr).To(gomega.ContainSubstring(resource), + "Expected resource %s to be present", resource) + } + + for _, resource := range tt.shouldNotContain { + g.Expect(rulesStr).NotTo(gomega.ContainSubstring(resource), + "Expected resource %s to be absent", resource) + } + }) + } +} + +func TestFeatureGateValidation(t *testing.T) { + tests := []struct { + name string + expression string + shouldError bool + }{ + {name: "empty expression", expression: "", shouldError: false}, + {name: "single gate", expression: "alpha", shouldError: false}, + {name: "OR expression", expression: "alpha|beta", shouldError: false}, + {name: "AND expression", expression: "alpha&beta", shouldError: false}, + {name: "mixed operators", expression: "alpha&beta|gamma", shouldError: true}, + {name: "invalid character", expression: "alpha@beta", shouldError: true}, + {name: "hyphenated gate", expression: "feature-alpha", shouldError: false}, + {name: "underscore gate", expression: "feature_alpha", shouldError: false}, + {name: "numeric gate", expression: "v1beta1", shouldError: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateFeatureGateExpression(tt.expression) + if tt.shouldError { + if err == nil { + t.Errorf("Expected error for expression %s, but got none", tt.expression) + } + } else { + if err != nil { + t.Errorf("Expected no error for expression %s, but got: %v", tt.expression, err) + } + } + }) + } +} diff --git a/pkg/rbac/feature_gates_test.go b/pkg/rbac/feature_gates_test.go new file mode 100644 index 000000000..5efece0d0 --- /dev/null +++ b/pkg/rbac/feature_gates_test.go @@ -0,0 +1,103 @@ +package rbac + +import ( + "strings" + "testing" + + "github.com/onsi/gomega" + rbacv1 "k8s.io/api/rbac/v1" + "sigs.k8s.io/controller-tools/pkg/genall" + "sigs.k8s.io/controller-tools/pkg/loader" + "sigs.k8s.io/controller-tools/pkg/markers" +) + +func TestFeatureGates(t *testing.T) { + g := gomega.NewWithT(t) + + // Load test packages + pkgs, err := loader.LoadRoots("./testdata/feature_gates") + g.Expect(err).NotTo(gomega.HaveOccurred()) + + // Set up generation context + reg := &markers.Registry{} + g.Expect(reg.Register(RuleDefinition)).To(gomega.Succeed()) + + ctx := &genall.GenerationContext{ + Collector: &markers.Collector{Registry: reg}, + Roots: pkgs, + } + + tests := []struct { + name string + featureGates string + expectedRules int + shouldContain []string + shouldNotContain []string + }{ + { + name: "no feature gates", + featureGates: "", + expectedRules: 2, // only always-on rules + shouldContain: []string{"pods", "configmaps"}, + shouldNotContain: []string{"deployments", "ingresses"}, + }, + { + name: "alpha enabled", + featureGates: "alpha=true", + expectedRules: 3, // always-on + alpha + shouldContain: []string{"pods", "configmaps", "deployments"}, + shouldNotContain: []string{"ingresses"}, + }, + { + name: "beta enabled", + featureGates: "beta=true", + expectedRules: 3, // always-on + beta + shouldContain: []string{"pods", "configmaps", "ingresses"}, + shouldNotContain: []string{"deployments"}, + }, + { + name: "both enabled", + featureGates: "alpha=true,beta=true", + expectedRules: 4, // all rules + shouldContain: []string{"pods", "configmaps", "deployments", "ingresses"}, + shouldNotContain: []string{}, + }, + { + name: "alpha enabled beta disabled", + featureGates: "alpha=true,beta=false", + expectedRules: 3, // always-on + alpha + shouldContain: []string{"pods", "configmaps", "deployments"}, + shouldNotContain: []string{"ingresses"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := gomega.NewWithT(t) + + objs, err := GenerateRoles(ctx, "test-role", tt.featureGates) + g.Expect(err).NotTo(gomega.HaveOccurred()) + g.Expect(objs).To(gomega.HaveLen(1)) + + role, ok := objs[0].(rbacv1.ClusterRole) + g.Expect(ok).To(gomega.BeTrue()) + g.Expect(role.Rules).To(gomega.HaveLen(tt.expectedRules)) + + // Convert rules to string for easier checking + rulesStr := "" + for _, rule := range role.Rules { + rulesStr += strings.Join(rule.Resources, ",") + " " + } + + for _, resource := range tt.shouldContain { + g.Expect(rulesStr).To(gomega.ContainSubstring(resource), + "Expected resource %s to be present", resource) + } + + for _, resource := range tt.shouldNotContain { + g.Expect(rulesStr).NotTo(gomega.ContainSubstring(resource), + "Expected resource %s to be absent", resource) + } + }) + } +} diff --git a/pkg/rbac/parser.go b/pkg/rbac/parser.go index 6521d2658..af171591e 100644 --- a/pkg/rbac/parser.go +++ b/pkg/rbac/parser.go @@ -60,6 +60,12 @@ type Rule struct { // If not set, the Rule belongs to the generated ClusterRole. // If set, the Rule belongs to a Role, whose namespace is specified by this field. Namespace string `marker:",optional"` + // FeatureGate specifies the feature gate(s) that control this RBAC rule. + // If not set, the rule is always included. + // If set to a single gate (e.g., "alpha"), the rule is included when that gate is enabled. + // If set to multiple gates separated by "|" (e.g., "alpha|beta"), the rule is included when ANY of the gates are enabled (OR logic). + // If set to multiple gates separated by "&" (e.g., "alpha&beta"), the rule is included when ALL of the gates are enabled (AND logic). + FeatureGate string `marker:"featureGate,optional"` } // ruleKey represents the resources and non-resources a Rule applies. @@ -169,6 +175,12 @@ type Generator struct { // Year specifies the year to substitute for " YEAR" in the header file. Year string `marker:",optional"` + + // FeatureGates is a comma-separated list of feature gates to enable (e.g., "alpha=true,beta=false"). + // Only RBAC rules with matching feature gates will be included in the generated output. + // Feature gates not explicitly listed are treated as disabled. + // Usage: controller-gen 'rbac:roleName=manager,featureGates="alpha=true,beta=false"' paths=./... + FeatureGates string `marker:",optional"` } func (Generator) RegisterMarkers(into *markers.Registry) error { @@ -179,9 +191,102 @@ func (Generator) RegisterMarkers(into *markers.Registry) error { return nil } +// FeatureGateMap represents enabled feature gates as a map for efficient lookup +type FeatureGateMap map[string]bool + +// parseFeatureGates parses a comma-separated feature gate string into a map +// Format: "gate1=true,gate2=false,gate3=true" +func parseFeatureGates(featureGates string) FeatureGateMap { + gates := make(FeatureGateMap) + if featureGates == "" { + return gates + } + + pairs := strings.Split(featureGates, ",") + for _, pair := range pairs { + parts := strings.Split(strings.TrimSpace(pair), "=") + if len(parts) != 2 { + continue + } + gateName := strings.TrimSpace(parts[0]) + gateValue := strings.TrimSpace(parts[1]) + gates[gateName] = gateValue == "true" + } + return gates +} + +// validateFeatureGateExpression validates the syntax of a feature gate expression +func validateFeatureGateExpression(expr string) error { + if expr == "" { + return nil + } + + // Check for invalid characters (only allow alphanumeric, hyphens, underscores, &, |) + for _, char := range expr { + //nolint:staticcheck // De Morgan's law suggestion ignored for readability + if !((char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z') || + (char >= '0' && char <= '9') || char == '-' || char == '_' || + char == '&' || char == '|') { + return fmt.Errorf("invalid character '%c' in feature gate expression: %s", char, expr) + } + } + + // Check for mixing AND and OR operators + hasAnd := strings.Contains(expr, "&") + hasOr := strings.Contains(expr, "|") + if hasAnd && hasOr { + return fmt.Errorf("cannot mix '&' and '|' operators in feature gate expression: %s", expr) + } + + return nil +} + +// shouldIncludeRule determines if an RBAC rule should be included based on feature gates +// Supports multiple gate logic: +// - Single gate: "alpha" - included if alpha=true +// - OR logic: "alpha|beta" - included if either alpha=true OR beta=true +// - AND logic: "alpha&beta" - included if both alpha=true AND beta=true +func shouldIncludeRule(rule *Rule, enabledGates FeatureGateMap) bool { + if rule.FeatureGate == "" { + // No feature gate specified, always include + return true + } + + featureGateExpr := rule.FeatureGate + + // Handle AND logic (all gates must be enabled) + if strings.Contains(featureGateExpr, "&") { + gates := strings.Split(featureGateExpr, "&") + for _, gate := range gates { + gate = strings.TrimSpace(gate) + if enabled, exists := enabledGates[gate]; !exists || !enabled { + return false + } + } + return true + } + + // Handle OR logic (any gate can be enabled) + if strings.Contains(featureGateExpr, "|") { + gates := strings.Split(featureGateExpr, "|") + for _, gate := range gates { + gate = strings.TrimSpace(gate) + if enabled, exists := enabledGates[gate]; exists && enabled { + return true + } + } + return false + } + + // Single gate logic + enabled, exists := enabledGates[featureGateExpr] + return exists && enabled +} + // GenerateRoles generate a slice of objs representing either a ClusterRole or a Role object // The order of the objs in the returned slice is stable and determined by their namespaces. -func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{}, error) { +func GenerateRoles(ctx *genall.GenerationContext, roleName string, featureGates string) ([]interface{}, error) { + enabledGates := parseFeatureGates(featureGates) rulesByNSResource := make(map[string][]*Rule) for _, root := range ctx.Roots { markerSet, err := markers.PackageMarkers(ctx.Collector, root) @@ -192,6 +297,17 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{ // group RBAC markers by namespace and separate by resource for _, markerValue := range markerSet[RuleDefinition.Name] { rule := markerValue.(Rule) + + // Validate feature gate expression syntax + if err := validateFeatureGateExpression(rule.FeatureGate); err != nil { + return nil, fmt.Errorf("invalid feature gate expression in RBAC rule: %w", err) + } + + // Apply feature gate filtering + if !shouldIncludeRule(&rule, enabledGates) { + continue + } + if len(rule.Resources) == 0 { // Add a rule without any resource if Resources is empty. r := Rule{ @@ -201,6 +317,7 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{ URLs: rule.URLs, Namespace: rule.Namespace, Verbs: rule.Verbs, + FeatureGate: rule.FeatureGate, } namespace := r.Namespace rulesByNSResource[namespace] = append(rulesByNSResource[namespace], &r) @@ -214,6 +331,7 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{ URLs: rule.URLs, Namespace: rule.Namespace, Verbs: rule.Verbs, + FeatureGate: rule.FeatureGate, } namespace := r.Namespace rulesByNSResource[namespace] = append(rulesByNSResource[namespace], &r) @@ -367,7 +485,7 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{ } func (g Generator) Generate(ctx *genall.GenerationContext) error { - objs, err := GenerateRoles(ctx, g.RoleName) + objs, err := GenerateRoles(ctx, g.RoleName, g.FeatureGates) if err != nil { return err } diff --git a/pkg/rbac/parser_integration_test.go b/pkg/rbac/parser_integration_test.go index 6d877d966..63ec2235b 100644 --- a/pkg/rbac/parser_integration_test.go +++ b/pkg/rbac/parser_integration_test.go @@ -42,7 +42,7 @@ var _ = Describe("ClusterRole generated by the RBAC Generator", func() { } By("generating a ClusterRole") - objs, err := rbac.GenerateRoles(ctx, "manager-role") + objs, err := rbac.GenerateRoles(ctx, "manager-role", "") Expect(err).NotTo(HaveOccurred()) By("loading the desired YAML") diff --git a/pkg/rbac/testdata/advanced_feature_gates/controller.go b/pkg/rbac/testdata/advanced_feature_gates/controller.go new file mode 100644 index 000000000..d3c72d1c3 --- /dev/null +++ b/pkg/rbac/testdata/advanced_feature_gates/controller.go @@ -0,0 +1,17 @@ +package testdata + +// Always included RBAC rule +// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch + +// Another always included rule +// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list + +// OR logic: alpha OR beta feature gate RBAC rule +// +kubebuilder:rbac:featureGate=alpha|beta,groups="",resources=secrets,verbs=get;list;create + +// AND logic: alpha AND beta feature gate RBAC rule +// +kubebuilder:rbac:featureGate=alpha&beta,groups="",resources=services,verbs=get;list;create;update;delete + +func main() { + // Test file for advanced RBAC feature gates +} diff --git a/pkg/rbac/testdata/feature_gates/controller.go b/pkg/rbac/testdata/feature_gates/controller.go new file mode 100644 index 000000000..4a831b383 --- /dev/null +++ b/pkg/rbac/testdata/feature_gates/controller.go @@ -0,0 +1,17 @@ +package testdata + +// Always included RBAC rule +// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch + +// Another always included rule +// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list + +// Alpha feature gate RBAC rule +// +kubebuilder:rbac:featureGate=alpha,groups=apps,resources=deployments,verbs=get;list;create;update;delete + +// Beta feature gate RBAC rule +// +kubebuilder:rbac:featureGate=beta,groups=extensions,resources=ingresses,verbs=get;list;create;update;delete + +func main() { + // Test file for RBAC feature gates +} diff --git a/pkg/webhook/parser.go b/pkg/webhook/parser.go index 832edebb0..8da44c5c3 100644 --- a/pkg/webhook/parser.go +++ b/pkg/webhook/parser.go @@ -159,6 +159,13 @@ type Config struct { // The URL configuration should be between quotes. // `url` cannot be specified when `path` is specified. URL string `marker:"url,optional"` + + // FeatureGate specifies the feature gate(s) that control this webhook. + // If not set, the webhook is always included. + // If set to a single gate (e.g., "alpha"), the webhook is included when that gate is enabled. + // If set to multiple gates separated by "|" (e.g., "alpha|beta"), the webhook is included when ANY of the gates are enabled (OR logic). + // If set to multiple gates separated by "&" (e.g., "alpha&beta"), the webhook is included when ALL of the gates are enabled (AND logic). + FeatureGate string `marker:"featureGate,optional"` } // verbToAPIVariant converts a marker's verb to the proper value for the API. @@ -426,6 +433,12 @@ type Generator struct { // Year specifies the year to substitute for " YEAR" in the header file. Year string `marker:",optional"` + + // FeatureGates is a comma-separated list of feature gates to enable (e.g., "alpha=true,beta=false"). + // Only webhook configurations with matching feature gates will be included in the generated output. + // Feature gates not explicitly listed are treated as disabled. + // Usage: controller-gen 'webhook:featureGates="alpha=true,beta=false"' paths=./... + FeatureGates string `marker:",optional"` } func (Generator) RegisterMarkers(into *markers.Registry) error { @@ -440,6 +453,98 @@ func (Generator) RegisterMarkers(into *markers.Registry) error { return nil } +// FeatureGateMap represents enabled feature gates as a map for efficient lookup +type FeatureGateMap map[string]bool + +// parseFeatureGates parses a comma-separated feature gate string into a map +// Format: "gate1=true,gate2=false,gate3=true" +func parseFeatureGates(featureGates string) FeatureGateMap { + gates := make(FeatureGateMap) + if featureGates == "" { + return gates + } + + pairs := strings.Split(featureGates, ",") + for _, pair := range pairs { + parts := strings.Split(strings.TrimSpace(pair), "=") + if len(parts) != 2 { + continue + } + gateName := strings.TrimSpace(parts[0]) + gateValue := strings.TrimSpace(parts[1]) + gates[gateName] = gateValue == "true" + } + return gates +} + +// validateFeatureGateExpression validates the syntax of a feature gate expression +func validateFeatureGateExpression(expr string) error { + if expr == "" { + return nil + } + + // Check for invalid characters (only allow alphanumeric, hyphens, underscores, &, |) + for _, char := range expr { + //nolint:staticcheck // De Morgan's law suggestion ignored for readability + if !((char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z') || + (char >= '0' && char <= '9') || char == '-' || char == '_' || + char == '&' || char == '|') { + return fmt.Errorf("invalid character '%c' in feature gate expression: %s", char, expr) + } + } + + // Check for mixing AND and OR operators + hasAnd := strings.Contains(expr, "&") + hasOr := strings.Contains(expr, "|") + if hasAnd && hasOr { + return fmt.Errorf("cannot mix '&' and '|' operators in feature gate expression: %s", expr) + } + + return nil +} + +// shouldIncludeWebhook determines if a webhook should be included based on feature gates +// Supports multiple gate logic: +// - Single gate: "alpha" - included if alpha=true +// - OR logic: "alpha|beta" - included if either alpha=true OR beta=true +// - AND logic: "alpha&beta" - included if both alpha=true AND beta=true +func shouldIncludeWebhook(config *Config, enabledGates FeatureGateMap) bool { + if config.FeatureGate == "" { + // No feature gate specified, always include + return true + } + + featureGateExpr := config.FeatureGate + + // Handle AND logic (all gates must be enabled) + if strings.Contains(featureGateExpr, "&") { + gates := strings.Split(featureGateExpr, "&") + for _, gate := range gates { + gate = strings.TrimSpace(gate) + if enabled, exists := enabledGates[gate]; !exists || !enabled { + return false + } + } + return true + } + + // Handle OR logic (any gate can be enabled) + if strings.Contains(featureGateExpr, "|") { + gates := strings.Split(featureGateExpr, "|") + for _, gate := range gates { + gate = strings.TrimSpace(gate) + if enabled, exists := enabledGates[gate]; exists && enabled { + return true + } + } + return false + } + + // Single gate logic + enabled, exists := enabledGates[featureGateExpr] + return exists && enabled +} + //gocyclo:ignore func (g Generator) Generate(ctx *genall.GenerationContext) error { supportedWebhookVersions := supportedWebhookVersions() @@ -448,6 +553,9 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error { var mutatingWebhookCfgs admissionregv1.MutatingWebhookConfiguration var validatingWebhookCfgs admissionregv1.ValidatingWebhookConfiguration + // Parse feature gates from the CLI parameter + enabledGates := parseFeatureGates(g.FeatureGates) + for _, root := range ctx.Roots { markerSet, err := markers.PackageMarkers(ctx.Collector, root) if err != nil { @@ -489,6 +597,17 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error { for _, cfg := range cfgs { cfg := cfg.(Config) + + // Validate feature gate syntax if specified + if err := validateFeatureGateExpression(cfg.FeatureGate); err != nil { + return fmt.Errorf("invalid feature gate for webhook %s: %w", cfg.Name, err) + } + + // Check if this webhook should be included based on feature gates + if !shouldIncludeWebhook(&cfg, enabledGates) { + continue + } + webhookVersions, err := cfg.webhookVersions() if err != nil { return err diff --git a/pkg/webhook/parser_featuregate_test.go b/pkg/webhook/parser_featuregate_test.go new file mode 100644 index 000000000..1a2fddf03 --- /dev/null +++ b/pkg/webhook/parser_featuregate_test.go @@ -0,0 +1,490 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package webhook + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseFeatureGates(t *testing.T) { + tests := []struct { + name string + input string + expected FeatureGateMap + }{ + { + name: "empty string", + input: "", + expected: FeatureGateMap{}, + }, + { + name: "single gate enabled", + input: "alpha=true", + expected: FeatureGateMap{ + "alpha": true, + }, + }, + { + name: "single gate disabled", + input: "alpha=false", + expected: FeatureGateMap{ + "alpha": false, + }, + }, + { + name: "multiple gates", + input: "alpha=true,beta=false,gamma=true", + expected: FeatureGateMap{ + "alpha": true, + "beta": false, + "gamma": true, + }, + }, + { + name: "gates with spaces", + input: " alpha = true , beta = false ", + expected: FeatureGateMap{ + "alpha": true, + "beta": false, + }, + }, + { + name: "invalid format ignored", + input: "alpha=true,invalid,beta=false", + expected: FeatureGateMap{ + "alpha": true, + "beta": false, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseFeatureGates(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestValidateFeatureGateExpression(t *testing.T) { + tests := []struct { + name string + expr string + expectErr bool + errMsg string + }{ + { + name: "empty expression", + expr: "", + expectErr: false, + }, + { + name: "simple gate name", + expr: "alpha", + expectErr: false, + }, + { + name: "gate with hyphen", + expr: "alpha-feature", + expectErr: false, + }, + { + name: "gate with underscore", + expr: "alpha_feature", + expectErr: false, + }, + { + name: "OR expression", + expr: "alpha|beta", + expectErr: false, + }, + { + name: "AND expression", + expr: "alpha&beta", + expectErr: false, + }, + { + name: "mixed AND OR operators", + expr: "alpha&beta|gamma", + expectErr: true, + errMsg: "cannot mix '&' and '|' operators", + }, + { + name: "invalid character", + expr: "alpha@beta", + expectErr: true, + errMsg: "invalid character '@'", + }, + { + name: "invalid character space", + expr: "alpha beta", + expectErr: true, + errMsg: "invalid character ' '", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateFeatureGateExpression(tt.expr) + if tt.expectErr { + assert.Error(t, err) + if tt.errMsg != "" { + assert.Contains(t, err.Error(), tt.errMsg) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestShouldIncludeWebhook(t *testing.T) { + tests := []struct { + name string + config *Config + enabledGates FeatureGateMap + expected bool + }{ + { + name: "no feature gate always included", + config: &Config{ + Name: "test-webhook", + FeatureGate: "", + }, + enabledGates: FeatureGateMap{}, + expected: true, + }, + { + name: "single gate enabled", + config: &Config{ + Name: "alpha-webhook", + FeatureGate: "alpha", + }, + enabledGates: FeatureGateMap{ + "alpha": true, + }, + expected: true, + }, + { + name: "single gate disabled", + config: &Config{ + Name: "alpha-webhook", + FeatureGate: "alpha", + }, + enabledGates: FeatureGateMap{ + "alpha": false, + }, + expected: false, + }, + { + name: "single gate not present", + config: &Config{ + Name: "alpha-webhook", + FeatureGate: "alpha", + }, + enabledGates: FeatureGateMap{ + "beta": true, + }, + expected: false, + }, + { + name: "OR expression - first gate enabled", + config: &Config{ + Name: "alpha-beta-webhook", + FeatureGate: "alpha|beta", + }, + enabledGates: FeatureGateMap{ + "alpha": true, + "beta": false, + }, + expected: true, + }, + { + name: "OR expression - second gate enabled", + config: &Config{ + Name: "alpha-beta-webhook", + FeatureGate: "alpha|beta", + }, + enabledGates: FeatureGateMap{ + "alpha": false, + "beta": true, + }, + expected: true, + }, + { + name: "OR expression - both gates enabled", + config: &Config{ + Name: "alpha-beta-webhook", + FeatureGate: "alpha|beta", + }, + enabledGates: FeatureGateMap{ + "alpha": true, + "beta": true, + }, + expected: true, + }, + { + name: "OR expression - no gates enabled", + config: &Config{ + Name: "alpha-beta-webhook", + FeatureGate: "alpha|beta", + }, + enabledGates: FeatureGateMap{ + "alpha": false, + "beta": false, + }, + expected: false, + }, + { + name: "OR expression - gates not present", + config: &Config{ + Name: "alpha-beta-webhook", + FeatureGate: "alpha|beta", + }, + enabledGates: FeatureGateMap{ + "gamma": true, + }, + expected: false, + }, + { + name: "AND expression - both gates enabled", + config: &Config{ + Name: "alpha-beta-webhook", + FeatureGate: "alpha&beta", + }, + enabledGates: FeatureGateMap{ + "alpha": true, + "beta": true, + }, + expected: true, + }, + { + name: "AND expression - first gate disabled", + config: &Config{ + Name: "alpha-beta-webhook", + FeatureGate: "alpha&beta", + }, + enabledGates: FeatureGateMap{ + "alpha": false, + "beta": true, + }, + expected: false, + }, + { + name: "AND expression - second gate disabled", + config: &Config{ + Name: "alpha-beta-webhook", + FeatureGate: "alpha&beta", + }, + enabledGates: FeatureGateMap{ + "alpha": true, + "beta": false, + }, + expected: false, + }, + { + name: "AND expression - first gate not present", + config: &Config{ + Name: "alpha-beta-webhook", + FeatureGate: "alpha&beta", + }, + enabledGates: FeatureGateMap{ + "beta": true, + }, + expected: false, + }, + { + name: "complex OR expression with three gates", + config: &Config{ + Name: "multi-webhook", + FeatureGate: "alpha|beta|gamma", + }, + enabledGates: FeatureGateMap{ + "alpha": false, + "beta": false, + "gamma": true, + }, + expected: true, + }, + { + name: "complex AND expression with three gates", + config: &Config{ + Name: "multi-webhook", + FeatureGate: "alpha&beta&gamma", + }, + enabledGates: FeatureGateMap{ + "alpha": true, + "beta": true, + "gamma": false, + }, + expected: false, + }, + { + name: "complex AND expression with all gates enabled", + config: &Config{ + Name: "multi-webhook", + FeatureGate: "alpha&beta&gamma", + }, + enabledGates: FeatureGateMap{ + "alpha": true, + "beta": true, + "gamma": true, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := shouldIncludeWebhook(tt.config, tt.enabledGates) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestWebhookFeatureGateIntegration(t *testing.T) { + // Test the full workflow: parse -> validate -> filter + testCases := []struct { + name string + featureGates string + webhookConfigs []Config + expectedCount int + description string + }{ + { + name: "no feature gates - all webhooks included", + featureGates: "", + webhookConfigs: []Config{ + {Name: "webhook1", FeatureGate: ""}, + {Name: "webhook2", FeatureGate: "alpha"}, + {Name: "webhook3", FeatureGate: "beta"}, + }, + expectedCount: 1, // Only webhook1 (no feature gate) is included + description: "When no feature gates are enabled, only webhooks without feature gates are included", + }, + { + name: "alpha enabled - alpha webhooks included", + featureGates: "alpha=true", + webhookConfigs: []Config{ + {Name: "webhook1", FeatureGate: ""}, + {Name: "webhook2", FeatureGate: "alpha"}, + {Name: "webhook3", FeatureGate: "beta"}, + }, + expectedCount: 2, // webhook1 (no gate) and webhook2 (alpha) are included + description: "When alpha is enabled, webhooks without gates and alpha webhooks are included", + }, + { + name: "multiple gates with OR logic", + featureGates: "alpha=true,beta=false", + webhookConfigs: []Config{ + {Name: "webhook1", FeatureGate: ""}, + {Name: "webhook2", FeatureGate: "alpha"}, + {Name: "webhook3", FeatureGate: "beta"}, + {Name: "webhook4", FeatureGate: "alpha|beta"}, + }, + expectedCount: 3, // webhook1 (no gate), webhook2 (alpha), webhook4 (alpha|beta - alpha is true) + description: "OR logic includes webhook if any gate is enabled", + }, + { + name: "multiple gates with AND logic", + featureGates: "alpha=true,beta=true", + webhookConfigs: []Config{ + {Name: "webhook1", FeatureGate: ""}, + {Name: "webhook2", FeatureGate: "alpha"}, + {Name: "webhook3", FeatureGate: "beta"}, + {Name: "webhook4", FeatureGate: "alpha&beta"}, + }, + expectedCount: 4, // All webhooks included + description: "AND logic includes webhook if all gates are enabled", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Parse feature gates + enabledGates := parseFeatureGates(tc.featureGates) + + // Filter webhooks + includedCount := 0 + for _, config := range tc.webhookConfigs { + if shouldIncludeWebhook(&config, enabledGates) { + includedCount++ + } + } + + assert.Equal(t, tc.expectedCount, includedCount, tc.description) + }) + } +} + +func TestWebhookFeatureGateValidationInGenerate(t *testing.T) { + // Test validation errors that would occur in the Generate method + testCases := []struct { + name string + featureGate string + expectError bool + errorContains string + }{ + { + name: "valid single gate", + featureGate: "alpha", + expectError: false, + }, + { + name: "valid OR expression", + featureGate: "alpha|beta", + expectError: false, + }, + { + name: "valid AND expression", + featureGate: "alpha&beta", + expectError: false, + }, + { + name: "invalid mixed operators", + featureGate: "alpha&beta|gamma", + expectError: true, + errorContains: "cannot mix '&' and '|' operators", + }, + { + name: "invalid character", + featureGate: "alpha@beta", + expectError: true, + errorContains: "invalid character '@'", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + config := Config{ + FeatureGate: tc.featureGate, + } + + err := validateFeatureGateExpression(config.FeatureGate) + if tc.expectError { + assert.Error(t, err) + if tc.errorContains != "" { + assert.Contains(t, err.Error(), tc.errorContains) + } + } else { + assert.NoError(t, err) + } + }) + } +}