[ACM-42853] Add missing webhook containerPort declaration - #2337
Conversation
The discovery-operator's validating webhook was added without a corresponding containerPort declaration in the deployment manifest. This went unnoticed until a NetworkPolicy enforcing port-based ingress rules was introduced, at which point the webhook became unreachable and all DiscoveredCluster create/update operations began failing with context deadline exceeded timeouts. - Add containerPort: 9443 (name: webhook-server) to the discovery-operator container spec in config/manager/manager.yaml - Regenerate bundle/manifests/discovery.clusterserviceversion.yaml via make manifests && make bundle to reflect the new port declaration Signed-off-by: dislbenn <dbennett@redhat.com>
|
Warning Review limit reachedNext included review available in 32 minutes. View limit detailsLimit details: You’ve used the included review currently available. This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe deployment manifests add a ChangesWebhook server port exposure
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This localized manifest change declares the webhook container port needed for network policy routing, with no actionable merge-blocking risk remaining beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.) Full details: Description checkExplanation The description is complete and relevant. It includes the purpose, related issue, detailed changes, testing evidence, checklist status, additional notes, reviewers, and definition of done. It also explains the NetworkPolicy context and the required downstream propagation. ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
The discovery-operator-allow-webhook NetworkPolicy combined a
namespaceSelector (matching the special openshift-host-network virtual
namespace) with a podSelector: {} in the same 'from' entry.
Verified on a live OVN-Kubernetes cluster that this combination causes
ingress traffic from host-network pods (i.e. kube-apiserver, which
performs admission webhook calls for the DiscoveredCluster CRD) to be
silently dropped, even though the namespaceSelector alone matches
correctly. Removing the redundant podSelector restores connectivity.
Without this fix, DiscoveredCluster create/update/delete operations
fail with 'context deadline exceeded' calling the webhook, even when
containerPort: 9443 is correctly declared on the deployment.
Signed-off-by: dislbenn <dbennett@redhat.com>
6773bed to
f822e70
Compare
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dislbenn, msmigiel-rh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherry-pick backplane-5.0 |
|
@dislbenn: new pull request created: #2338 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 kubernetes-sigs/prow repository. |
3 participants
Description
The discovery-operator's validating webhook was affected by two independent
bugs that combined to cause all DiscoveredCluster create/update/delete
operations to fail with
context deadline exceeded:containerPortdeclaration in the deployment manifest.discovery-operator-allow-webhookNetworkPolicy combined anamespaceSelector(matching the specialopenshift-host-networkvirtual namespace used by OVN-Kubernetes) with an empty
podSelector: {}in the same
fromentry. This combination causes OVN-Kubernetes tosilently drop ingress traffic from host-network pods — including
kube-apiserver, which performs the admission webhook calls for the
DiscoveredCluster CRD — even though the
namespaceSelectoralonematches correctly.
Both issues were verified independently on a live OVN-Kubernetes cluster
using isolated test namespaces/pods to rule out other variables.
Related Issue
ACM-42853 - Discovery operator
validating webhook configuration exists but webhook server is not running,
causing all DiscoveredCluster creation to fail with timeout errors
Changes Made
containerPort: 9443(name: webhook-server) to the discovery-operatorcontainer spec in
config/manager/manager.yamlpodSelector: {}from thediscovery-operator-allow-webhookNetworkPolicy's ingress
fromentry inbundle/manifests/discovery-operator-allow-webhook_networking.k8s.io_v1_networkpolicy.yamlbundle/manifests/discovery.clusterserviceversion.yamlviamake manifests && make bundleto reflect the new port declarationScreenshots (if applicable)
N/A
Checklist
Additional Notes
Verified on a live cluster in multiple stages:
containerPort: 9443fixed but the NetworkPolicy stillcombining
namespaceSelector+podSelector: {}, webhook calls fromkube-apiserver continued to time out.
namespaceSelector-onlypolicy against the
openshift-host-networkvirtual namespace, ahost-network test pod could reach a test service instantly (
200,~25ms).namespaceSelector-only fix to the realdiscovery-operator-allow-webhookpolicy immediately restoredconnectivity from a host-network pod to the real webhook service, and
the discovery-operator's reconcile loop began completing successfully
with no webhook timeouts.
Note:
stolostron/backplane-operatorvendors a copy of these manifests(
pkg/templates/charts/toggle/discovery-operator/) via an automated syncprocess. Both fixes will need to propagate there as well (either via the
automated sync after this merges and releases, or via a follow-up manual
PR) before they take effect on MCE-managed clusters.
Reviewers
/cc @cameronmwall @ngraham20
Definition of Done