Skip to content

Feat: detect and warn when OVN routingViaHost is not enabled#386

Merged
pdettori merged 1 commit into
rossoctl:mainfrom
Bobbins228:feat/rhaieng-5326-ovn-network-check
Jun 10, 2026
Merged

Feat: detect and warn when OVN routingViaHost is not enabled#386
pdettori merged 1 commit into
rossoctl:mainfrom
Bobbins228:feat/rhaieng-5326-ovn-network-check

Conversation

@Bobbins228

@Bobbins228 Bobbins228 commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a startup-time network configuration check that detects when OVN-Kubernetes routingViaHost is not enabled on OpenShift clusters. Without this setting, Istio ambient mode's ztunnel cannot intercept pod-to-pod traffic and mTLS/authorization policies are silently bypassed. The operator logs a warning at startup — no per-CR conditions, no per-reconcile overhead.

Changes

  • network_check.goNetworkOperatorCRDExists (CRD discovery at startup) and CheckOVNNetworkConfig (reads network.operator.openshift.io/cluster, returns a warning string if misconfigured)
  • cmd/main.go — runs the check once at startup after CRD discovery, logs warning if misconfigured
  • RBACget on operator.openshift.io/networks in config/rbac/role.yaml and Helm chart
  • network_check_test.go — 5 unit tests covering misconfigured OVN, correct config, non-OVN clusters, and missing resource

@ChristianZaccaria ChristianZaccaria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this, we may need to reconsider the approach, please see comments below for more info.

Comment thread kagenti-operator/internal/controller/agentruntime_controller.go Outdated
Comment thread kagenti-operator/internal/controller/network_check.go Outdated
@Bobbins228
Bobbins228 force-pushed the feat/rhaieng-5326-ovn-network-check branch from 4db79bb to 8d864c9 Compare May 29, 2026 13:32
@Bobbins228

Copy link
Copy Markdown
Contributor Author

Addressed both review comments:

  1. Moved check to startupCheckOVNNetworkConfig now runs once in main.go right after NetworkOperatorCRDExists. No per-reconcile overhead, no per-CR conditions to maintain. The reconciler is untouched.

  2. Removed NetworkCheckResult structCheckOVNNetworkConfig returns a plain warning string (empty = no issue). ~350 lines removed from the PR.

RBAC narrowed to just get (single read at startup, no list/watch needed).

@ChristianZaccaria ChristianZaccaria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm !!

@ChristianZaccaria ChristianZaccaria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase when you have a chance, then should be good to merge!

@cwiklik cwiklik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, low-risk, well-tested startup check: detects OVN-Kubernetes without routingViaHost (which silently breaks ambient mTLS) and logs an actionable warning with the exact remediation patch. Correctly OCP-gated via NetworkOperatorCRDExists, so it no-ops on non-OpenShift clusters. The read-only operator.openshift.io/networks get permission is added in all three places — the kubebuilder marker, config/rbac/role.yaml, and the chart role.yaml, kept in sync (the hygiene that avoids RBAC drift). Five unit tests cover OVN-unset/false/true, non-OVN, and missing-resource paths.

CI — E2E is red, but the failure is unrelated: Manager … should ensure the metrics endpoint is serving metrics timed out (300s) at test/e2e/e2e_test.go:249; 31/32 specs passed. The OVN code can't affect it — it no-ops on the Kind E2E cluster, and the only other changes are a gofmt whitespace fix and an RBAC marker. Recommend re-running E2E so branch protection goes green before merge; not a code blocker.

Security: read-only get; no secrets; the check hardens a silent mTLS-bypass condition.

Commits: signed-off. Nit: feat: subject lacks the emoji prefix Kagenti's convention favors (style only).

Comment thread kagenti-operator/cmd/main.go Outdated
@pdettori

pdettori commented Jun 8, 2026

Copy link
Copy Markdown
Member

@Bobbins228 need rebase

Add a reconcile-time check to the AgentRuntime controller that reads
network.operator.openshift.io/cluster on OpenShift clusters and surfaces
a NetworkReady status condition when OVN-Kubernetes is present but
routingViaHost is not configured. Without this setting, Istio ambient
mode's ztunnel cannot intercept pod-to-pod traffic, silently bypassing
mTLS and authorization policies.

The check is automatically enabled via CRD discovery at startup (same
pattern as TektonConfig) and only requires read-only RBAC on
operator.openshift.io/networks. No cluster infrastructure is mutated.

Closes: RHAIENG-5326
Spike: RHAIENG-4900

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Bobbins228 <mcampbel@redhat.com>
@Bobbins228
Bobbins228 force-pushed the feat/rhaieng-5326-ovn-network-check branch from 8d864c9 to 5103a61 Compare June 9, 2026 09:38
@pdettori pdettori changed the title feat: detect and warn when OVN routingViaHost is not enabled Feat: detect and warn when OVN routingViaHost is not enabled Jun 10, 2026
@pdettori
pdettori merged commit 6413ac1 into rossoctl:main Jun 10, 2026
15 of 17 checks passed
@github-project-automation github-project-automation Bot moved this from New /:ToDo to Done in Rossoctl Issue Prioritization Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants