Skip to content

Commit cc41ffa

Browse files
wazerywazery
committed
feat: implement webhook feature gates
Add feature gate support to webhook generation allowing conditional inclusion of webhooks based on enabled feature gates. Key features: - Single gate: featureGate=alpha - OR logic: featureGate=alpha|beta (include if ANY gate enabled) - AND logic: featureGate=alpha&beta (include if ALL gates enabled) - CLI parameter: controller-gen webhook featureGates="alpha=true,beta=false" - Expression validation with clear error messages - Comprehensive test coverage Maintains backward compatibility - webhooks without feature gates are always included. Follows same patterns as RBAC feature gates for API consistency.
1 parent 15beb5e commit cc41ffa

File tree

8 files changed

+697
-85
lines changed

8 files changed

+697
-85
lines changed

‎go.mod

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ 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
1314
golang.org/x/tools v0.36.0
1415
golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated
1516
gopkg.in/yaml.v2 v2.4.0
@@ -55,6 +56,7 @@ require (
5556
github.com/modern-go/reflect2 v1.0.3-0.20250322232337-35a7c28c31ee // indirect
5657
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
5758
github.com/nxadm/tail v1.4.8 // indirect
59+
github.com/pmezard/go-difflib v1.0.0 // indirect
5860
github.com/prometheus/client_golang v1.22.0 // indirect
5961
github.com/prometheus/client_model v0.6.1 // indirect
6062
github.com/prometheus/common v0.62.0 // indirect

‎pkg/crd/featuregate_integration_test.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,28 @@ limitations under the License.
1717
package crd_test
1818

1919
import (
20-
"bytes"
21-
"io"
22-
"os"
23-
"path/filepath"
24-
25-
"github.com/google/go-cmp/cmp"
26-
. "github.com/onsi/ginkgo"
27-
. "github.com/onsi/gomega"
28-
"sigs.k8s.io/controller-tools/pkg/crd"
29-
crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
30-
"sigs.k8s.io/controller-tools/pkg/genall"
31-
"sigs.k8s.io/controller-tools/pkg/loader"
32-
"sigs.k8s.io/controller-tools/pkg/markers"
20+
"bytes"
21+
"io"
22+
"os"
23+
"path/filepath"
24+
25+
"github.com/google/go-cmp/cmp"
26+
. "github.com/onsi/ginkgo"
27+
. "github.com/onsi/gomega"
28+
"sigs.k8s.io/controller-tools/pkg/crd"
29+
crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
30+
"sigs.k8s.io/controller-tools/pkg/genall"
31+
"sigs.k8s.io/controller-tools/pkg/loader"
32+
"sigs.k8s.io/controller-tools/pkg/markers"
3333
)
3434

3535
var _ = Describe("CRD Feature Gate Generation", func() {
3636
var (
37-
ctx *genall.GenerationContext
38-
out *featureGateOutputRule
39-
featureGateDir string
40-
originalWorkingDir string
41-
)
37+
ctx *genall.GenerationContext
38+
out *featureGateOutputRule
39+
featureGateDir string
40+
originalWorkingDir string
41+
)
4242

4343
BeforeEach(func() {
4444
var err error

‎pkg/crd/markers/featuregate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424

2525
// FeatureGate marks a field to be conditionally included based on feature gate enablement.
2626
// Fields marked with +kubebuilder:feature-gate will only be included in generated CRDs
27-
// when the specified feature gate is enabled via --feature-gates flag.
27+
// when the specified feature gate is enabled via the crd:featureGates parameter.
2828
type FeatureGate string
2929

3030
// ApplyToSchema does nothing for feature gates - they are processed by the generator

‎pkg/rbac/advanced_feature_gates_test.go

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ import (
55
"testing"
66

77
"github.com/onsi/gomega"
8+
rbacv1 "k8s.io/api/rbac/v1"
89
"sigs.k8s.io/controller-tools/pkg/genall"
910
"sigs.k8s.io/controller-tools/pkg/loader"
1011
"sigs.k8s.io/controller-tools/pkg/markers"
11-
rbacv1 "k8s.io/api/rbac/v1"
1212
)
1313

1414
func TestAdvancedFeatureGates(t *testing.T) {
@@ -21,67 +21,67 @@ func TestAdvancedFeatureGates(t *testing.T) {
2121
// Set up generation context
2222
reg := &markers.Registry{}
2323
g.Expect(reg.Register(RuleDefinition)).To(gomega.Succeed())
24-
24+
2525
ctx := &genall.GenerationContext{
2626
Collector: &markers.Collector{Registry: reg},
2727
Roots: pkgs,
2828
}
2929

3030
tests := []struct {
31-
name string
32-
featureGates string
33-
expectedRules int
34-
shouldContain []string
31+
name string
32+
featureGates string
33+
expectedRules int
34+
shouldContain []string
3535
shouldNotContain []string
3636
}{
3737
{
38-
name: "OR logic - alpha enabled",
39-
featureGates: "alpha=true,beta=false",
40-
expectedRules: 3, // always-on + OR rule (alpha|beta)
41-
shouldContain: []string{"pods", "configmaps", "secrets"},
38+
name: "OR logic - alpha enabled",
39+
featureGates: "alpha=true,beta=false",
40+
expectedRules: 3, // always-on + OR rule (alpha|beta)
41+
shouldContain: []string{"pods", "configmaps", "secrets"},
4242
shouldNotContain: []string{"services"},
4343
},
4444
{
45-
name: "OR logic - beta enabled",
46-
featureGates: "alpha=false,beta=true",
47-
expectedRules: 3, // always-on + OR rule (alpha|beta)
48-
shouldContain: []string{"pods", "configmaps", "secrets"},
45+
name: "OR logic - beta enabled",
46+
featureGates: "alpha=false,beta=true",
47+
expectedRules: 3, // always-on + OR rule (alpha|beta)
48+
shouldContain: []string{"pods", "configmaps", "secrets"},
4949
shouldNotContain: []string{"services"},
5050
},
5151
{
52-
name: "OR logic - both enabled",
53-
featureGates: "alpha=true,beta=true",
54-
expectedRules: 4, // always-on + OR rule + AND rule
55-
shouldContain: []string{"pods", "configmaps", "secrets", "services"},
52+
name: "OR logic - both enabled",
53+
featureGates: "alpha=true,beta=true",
54+
expectedRules: 4, // always-on + OR rule + AND rule
55+
shouldContain: []string{"pods", "configmaps", "secrets", "services"},
5656
shouldNotContain: []string{},
5757
},
5858
{
59-
name: "OR logic - neither enabled",
60-
featureGates: "alpha=false,beta=false",
61-
expectedRules: 2, // only always-on
62-
shouldContain: []string{"pods", "configmaps"},
59+
name: "OR logic - neither enabled",
60+
featureGates: "alpha=false,beta=false",
61+
expectedRules: 2, // only always-on
62+
shouldContain: []string{"pods", "configmaps"},
6363
shouldNotContain: []string{"secrets", "services"},
6464
},
6565
{
66-
name: "AND logic - only alpha enabled",
67-
featureGates: "alpha=true,beta=false",
68-
expectedRules: 3, // always-on + OR rule (alpha|beta)
69-
shouldContain: []string{"pods", "configmaps", "secrets"},
66+
name: "AND logic - only alpha enabled",
67+
featureGates: "alpha=true,beta=false",
68+
expectedRules: 3, // always-on + OR rule (alpha|beta)
69+
shouldContain: []string{"pods", "configmaps", "secrets"},
7070
shouldNotContain: []string{"services"},
7171
},
7272
{
73-
name: "AND logic - both enabled",
74-
featureGates: "alpha=true,beta=true",
75-
expectedRules: 4, // always-on + OR rule + AND rule
76-
shouldContain: []string{"pods", "configmaps", "secrets", "services"},
73+
name: "AND logic - both enabled",
74+
featureGates: "alpha=true,beta=true",
75+
expectedRules: 4, // always-on + OR rule + AND rule
76+
shouldContain: []string{"pods", "configmaps", "secrets", "services"},
7777
shouldNotContain: []string{},
7878
},
7979
}
8080

8181
for _, tt := range tests {
8282
t.Run(tt.name, func(t *testing.T) {
8383
g := gomega.NewWithT(t)
84-
84+
8585
objs, err := GenerateRoles(ctx, "test-role", tt.featureGates)
8686
g.Expect(err).NotTo(gomega.HaveOccurred())
8787
g.Expect(objs).To(gomega.HaveLen(1))
@@ -97,7 +97,7 @@ func TestAdvancedFeatureGates(t *testing.T) {
9797
}
9898

9999
for _, resource := range tt.shouldContain {
100-
g.Expect(rulesStr).To(gomega.ContainSubstring(resource),
100+
g.Expect(rulesStr).To(gomega.ContainSubstring(resource),
101101
"Expected resource %s to be present", resource)
102102
}
103103

‎pkg/rbac/feature_gates_test.go

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ import (
55
"testing"
66

77
"github.com/onsi/gomega"
8+
rbacv1 "k8s.io/api/rbac/v1"
89
"sigs.k8s.io/controller-tools/pkg/genall"
910
"sigs.k8s.io/controller-tools/pkg/loader"
1011
"sigs.k8s.io/controller-tools/pkg/markers"
11-
rbacv1 "k8s.io/api/rbac/v1"
1212
)
1313

1414
func TestFeatureGates(t *testing.T) {
@@ -21,60 +21,60 @@ func TestFeatureGates(t *testing.T) {
2121
// Set up generation context
2222
reg := &markers.Registry{}
2323
g.Expect(reg.Register(RuleDefinition)).To(gomega.Succeed())
24-
24+
2525
ctx := &genall.GenerationContext{
2626
Collector: &markers.Collector{Registry: reg},
2727
Roots: pkgs,
2828
}
2929

3030
tests := []struct {
31-
name string
32-
featureGates string
33-
expectedRules int
34-
shouldContain []string
31+
name string
32+
featureGates string
33+
expectedRules int
34+
shouldContain []string
3535
shouldNotContain []string
3636
}{
3737
{
38-
name: "no feature gates",
39-
featureGates: "",
40-
expectedRules: 2, // only always-on rules
41-
shouldContain: []string{"pods", "configmaps"},
38+
name: "no feature gates",
39+
featureGates: "",
40+
expectedRules: 2, // only always-on rules
41+
shouldContain: []string{"pods", "configmaps"},
4242
shouldNotContain: []string{"deployments", "ingresses"},
4343
},
4444
{
45-
name: "alpha enabled",
46-
featureGates: "alpha=true",
47-
expectedRules: 3, // always-on + alpha
48-
shouldContain: []string{"pods", "configmaps", "deployments"},
45+
name: "alpha enabled",
46+
featureGates: "alpha=true",
47+
expectedRules: 3, // always-on + alpha
48+
shouldContain: []string{"pods", "configmaps", "deployments"},
4949
shouldNotContain: []string{"ingresses"},
5050
},
5151
{
52-
name: "beta enabled",
53-
featureGates: "beta=true",
54-
expectedRules: 3, // always-on + beta
55-
shouldContain: []string{"pods", "configmaps", "ingresses"},
52+
name: "beta enabled",
53+
featureGates: "beta=true",
54+
expectedRules: 3, // always-on + beta
55+
shouldContain: []string{"pods", "configmaps", "ingresses"},
5656
shouldNotContain: []string{"deployments"},
5757
},
5858
{
59-
name: "both enabled",
60-
featureGates: "alpha=true,beta=true",
61-
expectedRules: 4, // all rules
62-
shouldContain: []string{"pods", "configmaps", "deployments", "ingresses"},
59+
name: "both enabled",
60+
featureGates: "alpha=true,beta=true",
61+
expectedRules: 4, // all rules
62+
shouldContain: []string{"pods", "configmaps", "deployments", "ingresses"},
6363
shouldNotContain: []string{},
6464
},
6565
{
66-
name: "alpha enabled beta disabled",
67-
featureGates: "alpha=true,beta=false",
68-
expectedRules: 3, // always-on + alpha
69-
shouldContain: []string{"pods", "configmaps", "deployments"},
66+
name: "alpha enabled beta disabled",
67+
featureGates: "alpha=true,beta=false",
68+
expectedRules: 3, // always-on + alpha
69+
shouldContain: []string{"pods", "configmaps", "deployments"},
7070
shouldNotContain: []string{"ingresses"},
7171
},
7272
}
7373

7474
for _, tt := range tests {
7575
t.Run(tt.name, func(t *testing.T) {
7676
g := gomega.NewWithT(t)
77-
77+
7878
objs, err := GenerateRoles(ctx, "test-role", tt.featureGates)
7979
g.Expect(err).NotTo(gomega.HaveOccurred())
8080
g.Expect(objs).To(gomega.HaveLen(1))
@@ -90,7 +90,7 @@ func TestFeatureGates(t *testing.T) {
9090
}
9191

9292
for _, resource := range tt.shouldContain {
93-
g.Expect(rulesStr).To(gomega.ContainSubstring(resource),
93+
g.Expect(rulesStr).To(gomega.ContainSubstring(resource),
9494
"Expected resource %s to be present", resource)
9595
}
9696

‎pkg/rbac/parser.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,10 @@ func validateFeatureGateExpression(expr string) error {
223223

224224
// Check for invalid characters (only allow alphanumeric, hyphens, underscores, &, |)
225225
for _, char := range expr {
226-
if !((char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z') ||
227-
(char >= '0' && char <= '9') || char == '-' || char == '_' ||
228-
char == '&' || char == '|') {
226+
//nolint:staticcheck // De Morgan's law suggestion ignored for readability
227+
if !((char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z') ||
228+
(char >= '0' && char <= '9') || char == '-' || char == '_' ||
229+
char == '&' || char == '|') {
229230
return fmt.Errorf("invalid character '%c' in feature gate expression: %s", char, expr)
230231
}
231232
}
@@ -296,17 +297,17 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string, featureGates
296297
// group RBAC markers by namespace and separate by resource
297298
for _, markerValue := range markerSet[RuleDefinition.Name] {
298299
rule := markerValue.(Rule)
299-
300+
300301
// Validate feature gate expression syntax
301302
if err := validateFeatureGateExpression(rule.FeatureGate); err != nil {
302303
return nil, fmt.Errorf("invalid feature gate expression in RBAC rule: %w", err)
303304
}
304-
305+
305306
// Apply feature gate filtering
306307
if !shouldIncludeRule(&rule, enabledGates) {
307308
continue
308309
}
309-
310+
310311
if len(rule.Resources) == 0 {
311312
// Add a rule without any resource if Resources is empty.
312313
r := Rule{

0 commit comments

Comments
 (0)