OCPBUGS-78594: reject feature gate lists when FeatureSet is empty

[agullon]

Summary

• When featureSet is empty but customNoUpgrade or specialHandlingSupportExceptionRequired enabled/disabled lists are populated, ToApiserverArgs() applies the feature gates to kube-apiserver without creating an upgrade lock file. This means custom feature gates get applied without upgrade protection.
• Added a validation check in validateFeatureGates() that rejects non-empty customNoUpgrade and specialHandlingSupportExceptionRequired lists when FeatureSet is empty, preventing users from accidentally running unprotected custom feature gates.
• Updated the corresponding unit test expectation and the vendored copy of apiserver.go.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 955715cd-a36c-4244-9fac-7a198499527d

📥 Commits

Reviewing files that changed from the base of the PR and between 57d7055 and cc1a7cc.

📒 Files selected for processing (1)

• pkg/config/config_test.go

---

Walkthrough

Adds a validation in FeatureGates.validateFeatureGates that requires CustomNoUpgrade and SpecialHandlingSupportExceptionRequired lists to be empty when FeatureSet is empty; updates tests to expect errors for those cases.

Changes

Cohort / File(s)	Summary
FeatureGates validation & tests pkg/config/apiserver.go, pkg/config/config_test.go	Adds a new check in FeatureGates.validateFeatureGates that errors if FeatureSet is empty while either CustomNoUpgrade or SpecialHandlingSupportExceptionRequired contains entries; updates/adds tests to expect these errors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.3)

level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: err: exit status 1: stderr: go: inconsistent vendoring in :\n\tgithub.com/apparentlymart/go-cidr@v1.1.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/coreos/go-systemd@v0.0.0-20190321100706-95778dfbb74e: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/google/go-cmp@v0.7.0: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/miekg/dns@v1.1.63: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/openshift/api@v0.0.0-20260309155933-45fd88d185dd: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt\n\tgithub.com/openshift/build-machinery-go@v0.0.0-20251023084048-5d77c1a5e5af: is explicitly required in go.mod, but not marked as explicit

... [truncated 29518 characters] ...

belet: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/metrics: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/mount-utils: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/pod-security-admission: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-apiserver: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-cli-plugin: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\tk8s.io/sample-controller: is replaced in go.mod, but not marked as replaced in vendor/modules.txt\n\n\tTo ignore the vendor directory, use -mod=readonly or -mod=mod.\n\tTo sync the vendor directory, run:\n\t\tgo mod vendor\n"

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

pkg/config/config_test.go (1)

822-831: Add a sibling test for specialHandlingSupportExceptionRequired with empty FeatureSet.

This case now validates CustomNoUpgrade, but the new rule also covers SpecialHandlingSupportExceptionRequired; adding that explicit case will better protect against regressions in that branch.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/config/config_test.go` around lines 822 - 831, Add a sibling unit test
alongside the existing "feature-gates-custom-no-upgrade-with-feature-set-empty"
case that exercises the SpecialHandlingSupportExceptionRequired rule when
ApiServer.FeatureGates.FeatureSet is empty: create a case named like
"feature-gates-special-handling-support-exception-required-with-feature-set-empty",
build the config via mkDefaultConfig(), set c.ApiServer.FeatureGates.FeatureSet
= "", populate
c.ApiServer.FeatureGates.SpecialHandlingSupportExceptionRequired.Enabled =
[]string{"feature1"} and Disabled = []string{"feature2"} (or similar values),
and set expectErr: true so the test asserts the same error behavior as the
CustomNoUpgrade case; place it in the same table of test cases in
pkg/config/config_test.go.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/config/config_test.go`:
- Around line 822-831: Add a sibling unit test alongside the existing
"feature-gates-custom-no-upgrade-with-feature-set-empty" case that exercises the
SpecialHandlingSupportExceptionRequired rule when
ApiServer.FeatureGates.FeatureSet is empty: create a case named like
"feature-gates-special-handling-support-exception-required-with-feature-set-empty",
build the config via mkDefaultConfig(), set c.ApiServer.FeatureGates.FeatureSet
= "", populate
c.ApiServer.FeatureGates.SpecialHandlingSupportExceptionRequired.Enabled =
[]string{"feature1"} and Disabled = []string{"feature2"} (or similar values),
and set expectErr: true so the test asserts the same error behavior as the
CustomNoUpgrade case; place it in the same table of test cases in
pkg/config/config_test.go.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 6d4cfacb-7083-4be3-94e2-c11618fec24b

📥 Commits

Reviewing files that changed from the base of the PR and between c46ae08 and 57d7055.

⛔ Files ignored due to path filters (1)

• etcd/vendor/github.com/openshift/microshift/pkg/config/apiserver.go is excluded by !**/vendor/**

📒 Files selected for processing (2)

• pkg/config/apiserver.go
• pkg/config/config_test.go

[openshift-ci-robot]

@agullon: This pull request references Jira Issue OCPBUGS-78594, which is invalid:

• expected the bug to target the "4.22.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Summary

• When featureSet is empty but customNoUpgrade or specialHandlingSupportExceptionRequired enabled/disabled lists are populated, ToApiserverArgs() applies the feature gates to kube-apiserver without creating an upgrade lock file. This means custom feature gates get applied without upgrade protection.
• Added a validation check in validateFeatureGates() that rejects non-empty customNoUpgrade and specialHandlingSupportExceptionRequired lists when FeatureSet is empty, preventing users from accidentally running unprotected custom feature gates.
• Updated the corresponding unit test expectation and the vendored copy of apiserver.go.

Test plan

• Verified GOOS=linux go vet ./pkg/config/ passes
• Verified GOOS=linux go test -c ./pkg/config/ compiles successfully
• Run go test ./pkg/config/... -run TestValidateConfig -count=1 on a Linux host to confirm all tests pass
• Verify that configuring feature gate lists with an empty FeatureSet is properly rejected at startup

🤖 Generated with Claude Code

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 openshift-eng/jira-lifecycle-plugin repository.

[agullon]

/jira refresh

[openshift-ci-robot]

@agullon: This pull request references Jira Issue OCPBUGS-78594, which is invalid:

• expected the bug to target the "4.22.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

[agullon]

/jira refresh

[openshift-ci-robot]

@agullon: This pull request references Jira Issue OCPBUGS-78594, which is invalid:

• expected the bug to target the "4.22.0" version, but no target version was set
• expected the bug to be in one of the following states: NEW, ASSIGNED, POST, but it is MODIFIED instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

[agullon]

/jira refresh

[openshift-ci-robot]

@agullon: This pull request references Jira Issue OCPBUGS-78594, which is invalid:

• expected the bug to target the "4.22.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

[agullon]

/jira refresh

[openshift-ci-robot]

@agullon: This pull request references Jira Issue OCPBUGS-78594, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.22.0) matches configured target version for branch (4.22.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

[agullon]

/retest

[agullon]

/cherry-pick release-4.21

[openshift-cherrypick-robot]

@agullon: once the present PR merges, I will cherry-pick it on top of release-4.21 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.21

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.

[openshift-ci]

@agullon: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

[copejon]

/verified by @copejon,ci
/lgtm

[openshift-ci-robot]

@copejon: This PR has been marked as verified by @copejon,ci.

Details

In response to this:

/verified by @copejon,ci
/lgtm

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 openshift-eng/jira-lifecycle-plugin repository.

[copejon]

/lgtm

[openshift-ci-robot]

@agullon: This pull request references Jira Issue OCPBUGS-78594, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.22.0) matches configured target version for branch (4.22.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

Summary

• When featureSet is empty but customNoUpgrade or specialHandlingSupportExceptionRequired enabled/disabled lists are populated, ToApiserverArgs() applies the feature gates to kube-apiserver without creating an upgrade lock file. This means custom feature gates get applied without upgrade protection.
• Added a validation check in validateFeatureGates() that rejects non-empty customNoUpgrade and specialHandlingSupportExceptionRequired lists when FeatureSet is empty, preventing users from accidentally running unprotected custom feature gates.
• Updated the corresponding unit test expectation and the vendored copy of apiserver.go.

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: agullon, copejon

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [agullon,copejon]

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

[openshift-ci-robot]

@agullon: Jira Issue OCPBUGS-78594: All pull requests linked via external trackers have merged:

• openshift/microshift#6367

All linked pull requests have the verified tag. Jira Issue OCPBUGS-78594 has been moved to the VERIFIED state.

Details

In response to this:

Summary

• When featureSet is empty but customNoUpgrade or specialHandlingSupportExceptionRequired enabled/disabled lists are populated, ToApiserverArgs() applies the feature gates to kube-apiserver without creating an upgrade lock file. This means custom feature gates get applied without upgrade protection.
• Added a validation check in validateFeatureGates() that rejects non-empty customNoUpgrade and specialHandlingSupportExceptionRequired lists when FeatureSet is empty, preventing users from accidentally running unprotected custom feature gates.
• Updated the corresponding unit test expectation and the vendored copy of apiserver.go.

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-cherrypick-robot]

@agullon: #6367 failed to apply on top of branch "release-4.21":

Applying: fix: reject feature gate lists when FeatureSet is empty
Using index info to reconstruct a base tree...
M	pkg/config/config_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/config/config_test.go
CONFLICT (content): Merge conflict in pkg/config/config_test.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 fix: reject feature gate lists when FeatureSet is empty

Details

In response to this:

/cherry-pick release-4.21

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.