✨ WIP Add feature gate marker support for conditional CRD field inclusion #1259

wazery · 2025-08-11T15:18:02Z

Implements +kubebuilder:feature-gate=<gate-name> marker that allows fields to be conditionally included in generated CRDs based on enabled feature gates.

Add FeatureGate marker type in pkg/crd/markers/featuregate.go
Register feature gate markers in marker registry
Add CLI parameter crd:featureGates="gate1=true,gate2=false"
Implement conditional field inclusion in schema generation
Add feature gate context to Parser struct

Addresses #1238

linux-foundation-easycla · 2025-08-11T15:18:06Z

The committers listed above are authorized under a signed CLA.

✅ login: wazery / name: Wazery (ea9c147, 15beb5e, de2450b, cc41ffa, a4024f9)

k8s-ci-robot · 2025-08-11T15:18:09Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wazery
Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2025-08-11T15:18:11Z

Welcome @wazery!

It looks like this is your first PR to kubernetes-sigs/controller-tools 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/controller-tools has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2025-08-11T15:18:12Z

Hi @wazery. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

JoelSpeed

I'm keen to seen progress on this issue, thanks @wazery for starting the push forward.

Feature gating is complex when it comes to CRDs. I'd be keen to see if we can also work out a plan for how we will gate specific markers, is that something we can align on as a project before we move forward?

Specifically I've seen needs for feature gated different enum sets, different feature gated XValidations and different values for things like Min/Max items that are feature gated.

But perhaps we actually need to be able to support gating all of the various markers we support?

/ok-to-test

JoelSpeed · 2025-08-11T16:49:05Z

pkg/crd/gen.go

+// isFeatureGateEnabled checks if a feature gate is enabled.
+func (fg FeatureGateMap) isEnabled(gateName string) bool {
+	enabled, exists := fg[gateName]
+	return exists && enabled
+}


This means the absence of a feature gate in the list of provided gates automatically means its disabled, right?

Do we want to be more explicit about this behaviour and make it an error if the generator finds a gate for which it doesn't have an opinion?

yes you are right this is more like a silent disable, imho it's simple and predictable, yet it would allow typos in feature gate names to go unnoticed, I can switch that to explicitly error which will force decisions about every gate and would give user more clarity on possible issues, wdyt?

I think it would be safer to force a decision for each gate

My concern is that someone might accidentally miss one gate, generate an updated set of CRDs, and then publish a breaking change because they didn't review properly and notice that a gated field got removed

JoelSpeed · 2025-08-11T16:50:18Z

pkg/crd/markers/featuregate.go

+		Category: "CRD feature gates",
+		DetailedHelp: markers.DetailedHelp{
+			Summary: "marks a field to be conditionally included based on feature gate enablement",
+			Details: "Fields marked with +kubebuilder:feature-gate will only be included in generated CRDs when the specified feature gate is enabled via --feature-gates flag.",


Is the description here accurate, is it --feature-gates as a flag?

you are right, I just fixed it, thanks for spotting it.

JoelSpeed · 2025-08-11T16:52:39Z

pkg/crd/schema.go

+		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
+				}
+			}
+		}


I have examples of APIs where they are gated on multiple feature gates, is that something we would be able to support?

I have seen combinations in the past where both below cases are true:

enable this field if feature A OR feature B are enabled

enable this field if feature A AND feature B are enabled

Do we want to support those here?

yes this behaviour is supported by the current implementation.

Can you provide an example? Of how I might do both the OR and AND scenarios?

k8s-ci-robot · 2025-08-11T16:56:40Z

@wazery: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-controller-tools-test-main	`4d70735`	link	true	`/test pull-controller-tools-test-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Implements +kubebuilder:feature-gate=<gate-name> marker that allows fields to be conditionally included in generated CRDs based on enabled feature gates. - Add FeatureGate marker type in pkg/crd/markers/featuregate.go - Register feature gate markers in marker registry - Add CLI parameter crd:featureGates=gate1=true,gate2=false - Implement conditional field inclusion in schema generation - Add feature gate context to Parser struct Addresses kubernetes-sigs#1238

- Add FeatureGates field to Generator struct - Implement parseFeatureGates function for CLI integration - Add FeatureGate marker type for field-level gating - Update schema generation to conditionally include fields - Add comprehensive integration tests - Support single and multi-gate syntax: featureGates="alpha=true,beta=true"

wazery · 2025-08-12T12:54:03Z

I'm keen to seen progress on this issue, thanks @wazery for starting the push forward.

Feature gating is complex when it comes to CRDs. I'd be keen to see if we can also work out a plan for how we will gate specific markers, is that something we can align on as a project before we move forward?

Specifically I've seen needs for feature gated different enum sets, different feature gated XValidations and different values for things like Min/Max items that are feature gated.

But perhaps we actually need to be able to support gating all of the various markers we support?

/ok-to-test

Thanks a lot Joel for your review, I truly appreciate it and learn much from it.

As you can see in the current implementation it's very initial, and allows only binary field inclusion so either the entire field is in or out. Yet we can enhance the implementation further to address your cases:

Feature-gated enum values
Feature-gated validation rules (XValidations)
Feature-gated marker values (Min/MaxItems, etc.)
Conditional marker application

For example:

Option A: Inline Feature Gate Syntax

// Feature-gated enum values
// +kubebuilder:validation:Enum=stable;production
// +kubebuilder:validation:Enum=alpha;beta;experimental,featureGate=alpha

// Feature-gated validation rules  
// +kubebuilder:validation:XValidation=rule="self.size > 0"
// +kubebuilder:validation:XValidation=rule="self.advanced == true",featureGate=alpha

// Feature-gated min/max values
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MinItems=5,featureGate=strict

Option B: Dedicated Feature Gate Markers

// +kubebuilder:validation:Enum=stable;production
// +kubebuilder:validation:Enum:featureGate=alpha,value=alpha;beta;experimental

// +kubebuilder:validation:XValidation=rule="self.size > 0"  
// +kubebuilder:validation:XValidation:featureGate=alpha,rule="self.advanced == true"

Option C: Block-based Feature Gating

// +kubebuilder:feature-gate:alpha
// +kubebuilder:validation:Enum=alpha;beta;experimental  
// +kubebuilder:validation:XValidation=rule="self.advanced == true"
// +kubebuilder:feature-gate:end

// +kubebuilder:validation:Enum=stable;production  // always applied

I love to hear your thoughts on how do you think we have to move forward on this PR, and truly appreciated your help 🙇.

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.

- Add featureGate parameter to +kubebuilder:rbac marker - Enable conditional RBAC rule generation based on feature gates - Add FeatureGate field to Rule struct and Generator struct - Implement feature gate parsing and filtering logic - Update GenerateRoles function to accept feature gates parameter - Add comprehensive test suite for feature gate functionality - Maintain backward compatibility with existing RBAC generation Example usage: // +kubebuilder:rbac:featureGate=alpha,groups=apps,resources=deployments,verbs=get;list controller-gen 'rbac:roleName=manager,featureGates="alpha=true,beta=false"' paths=./...

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.

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.

JoelSpeed · 2025-08-12T16:04:17Z

From your suggestions, I'd be interested to see if we can move towards Option A, though I don't know how simple that is for markers where we currently don't have keyed values.

So for example, in XValidation, it's easy to add another value as it's already a multi-value marker. For the single value markers, adding an optional additional field isn't something I've looked into (not recently at least), but from the UX perspective sounds like the best IMO

Interested to see if other maintainers have opinions on this project @alvaroaleman @sbueringer (and @everettraven who is a colleague who helps me with API review at RH)

JoelSpeed · 2025-08-12T13:05:40Z

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


New dependency?

We do not use assert on the tests, give a look at the others.
You will see that the sintax is a little different but not hard to comply with.

JoelSpeed · 2025-08-12T15:49:54Z

pkg/crd/gen.go

+// isFeatureGateEnabled checks if a feature gate is enabled.
+func (fg FeatureGateMap) isEnabled(gateName string) bool {
+	enabled, exists := fg[gateName]
+	return exists && enabled
+}


I think it would be safer to force a decision for each gate

My concern is that someone might accidentally miss one gate, generate an updated set of CRDs, and then publish a breaking change because they didn't review properly and notice that a gated field got removed

JoelSpeed · 2025-08-12T15:54:54Z

pkg/crd/schema.go

+		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
+				}
+			}
+		}


Can you provide an example? Of how I might do both the OR and AND scenarios?

JoelSpeed · 2025-08-12T16:11:20Z

pkg/crd/testdata/featuregates/types.go

+	// 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"`


What would happen if I wanted one of these fields to be on when either gate was enabled, and then another field only when both were enabled?

I can see the | and & logic in the rbac, do we have that for the fields/markers as well?

JoelSpeed · 2025-08-12T16:14:03Z

pkg/rbac/advanced_feature_gates_test.go

+		{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},


What's wrong with mixed operators?

JoelSpeed · 2025-08-12T16:19:21Z

pkg/webhook/parser.go

@@ -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


The feature gate parsing logic appears to be needed in several places, should we create a centralised utility for this so that we only have to implement it once?

JoelSpeed · 2025-08-12T16:21:03Z

pkg/webhook/parser.go

+	// 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)
+	}


I don't know if I've seen a use case for this, but in most logic I'd expect to be able to do (A&B)|C which would be equivalent to omitting the () since OR takes precedence over AND in the ordering, right?

JoelSpeed · 2025-08-12T16:26:21Z

pkg/webhook/parser.go

+	// Handle OR logic (any gate can be enabled)
+	if strings.Contains(featureGateExpr, "|") {
+		gates := strings.Split(featureGateExpr, "|")
+		for _, gate := range gates {


Are you familiar with the K8s sets utility?

If you were to turn the list of enabled feature gates into a set, I believe the HasAny and HasAll would simplify this a lot

Take a look at https://pkg.go.dev/k8s.io/apimachinery/pkg/util/sets

In the implementation I have downstream, we keep sets of All, Enabled and Disabled, so that every gate we find in a marker can be checked if we first know the state (It exists in the All set), and then if it's Enabled or not. Disabled doesn't come into this part of the use case thinking about it.

Might want something similar here?

JoelSpeed · 2025-08-12T16:28:45Z

pkg/webhook/parser.go

+	}
+
+	// Single gate logic
+	enabled, exists := enabledGates[featureGateExpr]


Either the AND or the OR logic would actually pass this through as well 🤔

Another example of similar logic in https://github.com/openshift/kubernetes-sigs-controller-tools/blob/94812131024363d66cd1680f866bf694c560db39/pkg/crd/markers/patch_validation.go#L135-L147

In that example we have the OR options as m.FeatureGateNames and the AND options as m.RequiredFeatureGateNames

When there's only one gate to handle, it is a singleton in the OR set, but it could be in either set and still be functionally equivalent

JoelSpeed · 2025-08-12T16:34:01Z

pkg/webhook/parser.go

+	featureGateExpr := config.FeatureGate
+
+	// Handle AND logic (all gates must be enabled)
+	if strings.Contains(featureGateExpr, "&") {


This logic should also be centralised, perhaps we want the util to have centralised parsing and then a centralised IsEnabledByFeatureGates that would take whatever the featureGate value is from the marker/webhook/rbac and return a boolean to act upon?

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 11, 2025

k8s-ci-robot requested a review from joelanford August 11, 2025 15:18

k8s-ci-robot requested a review from vincepri August 11, 2025 15:18

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Aug 11, 2025

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 11, 2025

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 11, 2025

JoelSpeed reviewed Aug 11, 2025

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 11, 2025

wazery changed the title ~~✨ Add feature gate marker support for conditional CRD field inclusion~~ ✨ WIP Add feature gate marker support for conditional CRD field inclusion Aug 11, 2025

wazery force-pushed the w-feature-gate-support branch from 3001605 to b15c521 Compare August 11, 2025 18:30

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 11, 2025

wazery added 2 commits August 11, 2025 20:34

wazery force-pushed the w-feature-gate-support branch from b15c521 to de2450b Compare August 11, 2025 18:35

wazery added 3 commits August 12, 2025 14:58

wazery force-pushed the w-feature-gate-support branch from ff80782 to cc41ffa Compare August 12, 2025 12:58

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 12, 2025

JoelSpeed reviewed Aug 12, 2025

View reviewed changes

JoelSpeed mentioned this pull request Aug 13, 2025

Support for a feature gate flag/marker when generating CRDs #600

Open

✨ WIP Add feature gate marker support for conditional CRD field inclusion #1259

Are you sure you want to change the base?

✨ WIP Add feature gate marker support for conditional CRD field inclusion #1259

Conversation

wazery commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

linux-foundation-easycla bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Aug 11, 2025

Uh oh!

k8s-ci-robot commented Aug 11, 2025

Uh oh!

k8s-ci-robot commented Aug 11, 2025

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Aug 11, 2025

Uh oh!

wazery commented Aug 12, 2025

Option A: Inline Feature Gate Syntax

Option B: Dedicated Feature Gate Markers

Option C: Block-based Feature Gating

Uh oh!

JoelSpeed commented Aug 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wazery commented Aug 11, 2025 •

edited

Loading

linux-foundation-easycla bot commented Aug 11, 2025 •

edited

Loading