Skip to content

Commit 124ddba

Browse files
author
sami-wazery
committed
feat: Enhance RBAC feature gates with multiple gate support
Based on feedback from PR kubernetes-sigs#1259 review comments, enhance RBAC feature gates with: - Support for complex feature gate expressions using OR (|) and AND (&) logic - Enhanced documentation with clear usage examples - Validation for feature gate expressions with proper error handling - Comprehensive test coverage for multiple gate scenarios New feature gate syntax: - Single gate: featureGate=alpha - OR logic: featureGate=alpha|beta (enabled if ANY gate is true) - AND logic: featureGate=alpha&beta (enabled if ALL gates are true) Examples: // +kubebuilder:rbac:featureGate=alpha|beta,groups=apps,resources=deployments,verbs=get;list // +kubebuilder:rbac:featureGate=alpha&beta,groups="",resources=services,verbs=get;list Addresses maintainer feedback on expression validation, multiple gate support, and improved documentation clarity.
1 parent 756a2f3 commit 124ddba

File tree

3 files changed

+228
-4
lines changed

3 files changed

+228
-4
lines changed
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
package rbac
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/onsi/gomega"
8+
"sigs.k8s.io/controller-tools/pkg/genall"
9+
"sigs.k8s.io/controller-tools/pkg/loader"
10+
"sigs.k8s.io/controller-tools/pkg/markers"
11+
rbacv1 "k8s.io/api/rbac/v1"
12+
)
13+
14+
func TestAdvancedFeatureGates(t *testing.T) {
15+
g := gomega.NewWithT(t)
16+
17+
// Load test packages
18+
pkgs, err := loader.LoadRoots("./testdata/advanced_feature_gates")
19+
g.Expect(err).NotTo(gomega.HaveOccurred())
20+
21+
// Set up generation context
22+
reg := &markers.Registry{}
23+
g.Expect(reg.Register(RuleDefinition)).To(gomega.Succeed())
24+
25+
ctx := &genall.GenerationContext{
26+
Collector: &markers.Collector{Registry: reg},
27+
Roots: pkgs,
28+
}
29+
30+
tests := []struct {
31+
name string
32+
featureGates string
33+
expectedRules int
34+
shouldContain []string
35+
shouldNotContain []string
36+
}{
37+
{
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"},
42+
shouldNotContain: []string{"services"},
43+
},
44+
{
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"},
49+
shouldNotContain: []string{"services"},
50+
},
51+
{
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"},
56+
shouldNotContain: []string{},
57+
},
58+
{
59+
name: "OR logic - neither enabled",
60+
featureGates: "alpha=false,beta=false",
61+
expectedRules: 2, // only always-on
62+
shouldContain: []string{"pods", "configmaps"},
63+
shouldNotContain: []string{"secrets", "services"},
64+
},
65+
{
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"},
70+
shouldNotContain: []string{"services"},
71+
},
72+
{
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"},
77+
shouldNotContain: []string{},
78+
},
79+
}
80+
81+
for _, tt := range tests {
82+
t.Run(tt.name, func(t *testing.T) {
83+
g := gomega.NewWithT(t)
84+
85+
objs, err := GenerateRoles(ctx, "test-role", tt.featureGates)
86+
g.Expect(err).NotTo(gomega.HaveOccurred())
87+
g.Expect(objs).To(gomega.HaveLen(1))
88+
89+
role, ok := objs[0].(rbacv1.ClusterRole)
90+
g.Expect(ok).To(gomega.BeTrue())
91+
g.Expect(role.Rules).To(gomega.HaveLen(tt.expectedRules))
92+
93+
// Convert rules to string for easier checking
94+
rulesStr := ""
95+
for _, rule := range role.Rules {
96+
rulesStr += strings.Join(rule.Resources, ",") + " "
97+
}
98+
99+
for _, resource := range tt.shouldContain {
100+
g.Expect(rulesStr).To(gomega.ContainSubstring(resource),
101+
"Expected resource %s to be present", resource)
102+
}
103+
104+
for _, resource := range tt.shouldNotContain {
105+
g.Expect(rulesStr).NotTo(gomega.ContainSubstring(resource),
106+
"Expected resource %s to be absent", resource)
107+
}
108+
})
109+
}
110+
}
111+
112+
func TestFeatureGateValidation(t *testing.T) {
113+
tests := []struct {
114+
name string
115+
expression string
116+
shouldError bool
117+
}{
118+
{name: "empty expression", expression: "", shouldError: false},
119+
{name: "single gate", expression: "alpha", shouldError: false},
120+
{name: "OR expression", expression: "alpha|beta", shouldError: false},
121+
{name: "AND expression", expression: "alpha&beta", shouldError: false},
122+
{name: "mixed operators", expression: "alpha&beta|gamma", shouldError: true},
123+
{name: "invalid character", expression: "alpha@beta", shouldError: true},
124+
{name: "hyphenated gate", expression: "feature-alpha", shouldError: false},
125+
{name: "underscore gate", expression: "feature_alpha", shouldError: false},
126+
{name: "numeric gate", expression: "v1beta1", shouldError: false},
127+
}
128+
129+
for _, tt := range tests {
130+
t.Run(tt.name, func(t *testing.T) {
131+
err := validateFeatureGateExpression(tt.expression)
132+
if tt.shouldError {
133+
if err == nil {
134+
t.Errorf("Expected error for expression %s, but got none", tt.expression)
135+
}
136+
} else {
137+
if err != nil {
138+
t.Errorf("Expected no error for expression %s, but got: %v", tt.expression, err)
139+
}
140+
}
141+
})
142+
}
143+
}

pkg/rbac/parser.go

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,11 @@ type Rule struct {
6060
// If not set, the Rule belongs to the generated ClusterRole.
6161
// If set, the Rule belongs to a Role, whose namespace is specified by this field.
6262
Namespace string `marker:",optional"`
63-
// FeatureGate specifies the feature gate that controls this RBAC rule.
63+
// FeatureGate specifies the feature gate(s) that control this RBAC rule.
6464
// If not set, the rule is always included.
65-
// If set, the rule is only included when the specified feature gate is enabled.
65+
// If set to a single gate (e.g., "alpha"), the rule is included when that gate is enabled.
66+
// If set to multiple gates separated by "|" (e.g., "alpha|beta"), the rule is included when ANY of the gates are enabled (OR logic).
67+
// If set to multiple gates separated by "&" (e.g., "alpha&beta"), the rule is included when ALL of the gates are enabled (AND logic).
6668
FeatureGate string `marker:"featureGate,optional"`
6769
}
6870

@@ -176,6 +178,8 @@ type Generator struct {
176178

177179
// FeatureGates is a comma-separated list of feature gates to enable (e.g., "alpha=true,beta=false").
178180
// Only RBAC rules with matching feature gates will be included in the generated output.
181+
// Feature gates not explicitly listed are treated as disabled.
182+
// Usage: controller-gen 'rbac:roleName=manager,featureGates="alpha=true,beta=false"' paths=./...
179183
FeatureGates string `marker:",optional"`
180184
}
181185

@@ -211,15 +215,70 @@ func parseFeatureGates(featureGates string) FeatureGateMap {
211215
return gates
212216
}
213217

218+
// validateFeatureGateExpression validates the syntax of a feature gate expression
219+
func validateFeatureGateExpression(expr string) error {
220+
if expr == "" {
221+
return nil
222+
}
223+
224+
// Check for invalid characters (only allow alphanumeric, hyphens, underscores, &, |)
225+
for _, char := range expr {
226+
if !((char >= 'a' && char <= 'z') || (char >= 'A' && char <= 'Z') ||
227+
(char >= '0' && char <= '9') || char == '-' || char == '_' ||
228+
char == '&' || char == '|') {
229+
return fmt.Errorf("invalid character '%c' in feature gate expression: %s", char, expr)
230+
}
231+
}
232+
233+
// Check for mixing AND and OR operators
234+
hasAnd := strings.Contains(expr, "&")
235+
hasOr := strings.Contains(expr, "|")
236+
if hasAnd && hasOr {
237+
return fmt.Errorf("cannot mix '&' and '|' operators in feature gate expression: %s", expr)
238+
}
239+
240+
return nil
241+
}
242+
214243
// shouldIncludeRule determines if an RBAC rule should be included based on feature gates
244+
// Supports multiple gate logic:
245+
// - Single gate: "alpha" - included if alpha=true
246+
// - OR logic: "alpha|beta" - included if either alpha=true OR beta=true
247+
// - AND logic: "alpha&beta" - included if both alpha=true AND beta=true
215248
func shouldIncludeRule(rule *Rule, enabledGates FeatureGateMap) bool {
216249
if rule.FeatureGate == "" {
217250
// No feature gate specified, always include
218251
return true
219252
}
220253

221-
// Check if the feature gate is enabled
222-
enabled, exists := enabledGates[rule.FeatureGate]
254+
featureGateExpr := rule.FeatureGate
255+
256+
// Handle AND logic (all gates must be enabled)
257+
if strings.Contains(featureGateExpr, "&") {
258+
gates := strings.Split(featureGateExpr, "&")
259+
for _, gate := range gates {
260+
gate = strings.TrimSpace(gate)
261+
if enabled, exists := enabledGates[gate]; !exists || !enabled {
262+
return false
263+
}
264+
}
265+
return true
266+
}
267+
268+
// Handle OR logic (any gate can be enabled)
269+
if strings.Contains(featureGateExpr, "|") {
270+
gates := strings.Split(featureGateExpr, "|")
271+
for _, gate := range gates {
272+
gate = strings.TrimSpace(gate)
273+
if enabled, exists := enabledGates[gate]; exists && enabled {
274+
return true
275+
}
276+
}
277+
return false
278+
}
279+
280+
// Single gate logic
281+
enabled, exists := enabledGates[featureGateExpr]
223282
return exists && enabled
224283
}
225284

@@ -238,6 +297,11 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string, featureGates
238297
for _, markerValue := range markerSet[RuleDefinition.Name] {
239298
rule := markerValue.(Rule)
240299

300+
// Validate feature gate expression syntax
301+
if err := validateFeatureGateExpression(rule.FeatureGate); err != nil {
302+
return nil, fmt.Errorf("invalid feature gate expression in RBAC rule: %w", err)
303+
}
304+
241305
// Apply feature gate filtering
242306
if !shouldIncludeRule(&rule, enabledGates) {
243307
continue
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package testdata
2+
3+
// Always included RBAC rule
4+
// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch
5+
6+
// Another always included rule
7+
// +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list
8+
9+
// OR logic: alpha OR beta feature gate RBAC rule
10+
// +kubebuilder:rbac:featureGate=alpha|beta,groups="",resources=secrets,verbs=get;list;create
11+
12+
// AND logic: alpha AND beta feature gate RBAC rule
13+
// +kubebuilder:rbac:featureGate=alpha&beta,groups="",resources=services,verbs=get;list;create;update;delete
14+
15+
func main() {
16+
// Test file for advanced RBAC feature gates
17+
}

0 commit comments

Comments
 (0)