OCPSTRAT-3036: Promote MutatingAdmissionPolicy feature to Default#2832
OCPSTRAT-3036: Promote MutatingAdmissionPolicy feature to Default#2832jubittajohn wants to merge 1 commit intoopenshift:masterfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@jubittajohn: This pull request references OCPSTRAT-3036 which is a valid jira issue. DetailsIn response to this:
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. |
|
Hello @jubittajohn! Some important instructions when contributing to openshift/api: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe MutatingAdmissionPolicy feature gate was enabled by default by adding 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.12.1)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
features/features.go (1)
86-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep Default and OKD in sync for this gate.
Most gates promoted into the default matrix here add
inOKD()alongsideinDefault(). With Line 91 only addinginDefault(),MutatingAdmissionPolicystays disabled for OKD default clusters, which looks like an unintended divergence for a Kubernetes-scoped gate.Suggested fix
- enable(inDefault(), inTechPreviewNoUpgrade(), inDevPreviewNoUpgrade()). + enable(inDefault(), inOKD(), inTechPreviewNoUpgrade(), inDevPreviewNoUpgrade()).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/features.go` around lines 86 - 92, The FeatureGateMutatingAdmissionPolicy definition currently enables the gate with inDefault() but omits inOKD(), causing OKD default clusters to remain disabled; update the enable(...) call on FeatureGateMutatingAdmissionPolicy (the newFeatureGate(...) chain) to include inOKD() alongside inDefault() (i.e., enable(inDefault(), inOKD(), inTechPreviewNoUpgrade(), inDevPreviewNoUpgrade())) so the Kubernetes-scoped gate stays in sync for OKD.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@features/features.go`:
- Around line 86-92: The FeatureGateMutatingAdmissionPolicy definition currently
enables the gate with inDefault() but omits inOKD(), causing OKD default
clusters to remain disabled; update the enable(...) call on
FeatureGateMutatingAdmissionPolicy (the newFeatureGate(...) chain) to include
inOKD() alongside inDefault() (i.e., enable(inDefault(), inOKD(),
inTechPreviewNoUpgrade(), inDevPreviewNoUpgrade())) so the Kubernetes-scoped
gate stays in sync for OKD.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9f019e22-c89e-4a01-835b-ef5f050a9c63
📒 Files selected for processing (1)
features/features.go
f1683e9 to
83d1462
Compare
Signed-off-by: jubittajohn <jujohn@redhat.com>
83d1462 to
d03b52f
Compare
|
@jubittajohn: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Graduate
MutatingAdmissionPolicyto theDefaultfeature set.MutatingAdmissionPolicygraduated to GA in upstream Kubernetes1.36, and new conformance tests were added for it. This moves the feature to theDefaultfeature set to match. We need this before the1.36rebase because the new conformance tests poll indefinitely when the feature is disabled in OpenShift, causingk8s-e2e-conformance-awsto time out at 2 hours in openshift/kubernetes#2653 PR.Enabling it now on 1.35 is safe because the existing kas-operator version ranges(https://github.com/openshift/cluster-kube-apiserver-operator/blob/main/pkg/operator/configobservation/apienablement/observe_runtime_config.go#L[…]30) will serve the pre-GA APIs (v1alpha1/v1beta1). Once the rebase lands, only the native v1 API will be served, and we'll clean up the kas-operator runtime-config entries after that.