Skip to content

fix(omniroute): fix ndots DNS collision hijacking opencode.ai lookups - #4324

Merged
Tanguille merged 1 commit into
mainfrom
fix/omniroute-dns-ndots
Aug 3, 2026
Merged

fix(omniroute): fix ndots DNS collision hijacking opencode.ai lookups#4324
Tanguille merged 1 commit into
mainfrom
fix/omniroute-dns-ndots

Conversation

@Tanguille

@Tanguille Tanguille commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Summary

  • Root cause of the persistent opencode provider failures in omniroute (previously attributed to the upstream opencode.ai service in fix(ai): resolve omniroute reliability issues (DNS collision, timeout race) #4323): default ndots:5 search-expands the external domain opencode.ai (1 dot, < ndots) to opencode.ai.svc.cluster.local before trying it as an absolute name. That search-expanded name exactly matches our own in-namespace opencode Service (ai namespace, unrelated self-hosted app), so every DNS lookup of the real provider was silently hijacked to our internal app instead.
  • Fixed with dnsConfig.options: ndots=1, the same pattern already used by seerr and the *-exporter apps for this exact class of bug — absolute-looking names try direct resolution first.
  • This was meant to ship together with fix(ai): resolve omniroute reliability issues (DNS collision, timeout race) #4323 but the commit landed 5 minutes after that PR was merged, so it never made it into main. Standalone now.

Test plan

  • kustomize build kubernetes/apps/ai/omniroute/app clean, dnsConfig present in rendered Deployment
  • After rollout, confirm dig opencode.ai / omniroute logs show the real provider resolving instead of 10.43.254.109 (our internal opencode Service ClusterIP)
  • omniroute's opencode provider connection stops erroring

Summary by CodeRabbit

  • Bug Fixes
    • Improved provider hostname resolution for the Omniroute service.
    • Absolute hostnames are now checked before Kubernetes search-domain expansion, helping prevent connection and lookup issues.

Default ndots:5 search-expands "opencode.ai" (1 dot) to
"opencode.ai.svc.cluster.local" before trying it as absolute, which
exactly matches our own in-namespace opencode Service — every lookup
of the real opencode.ai provider was silently resolving to our
internal opencode app instead. Explains the persistent opencode
502s/timeouts blamed on the upstream provider. ndots:1 (same pattern
as seerr, prowlarr-exporter, etc.) tries the absolute name first.

This commit was cut into PR #4323 too late to be included in the
merge (pushed 5min after merge); reopening standalone.
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9142e1c7-343e-43dc-bc14-980ce1985e36

📥 Commits

Reviewing files that changed from the base of the PR and between 1ab49d5 and c4454b1.

📒 Files selected for processing (1)
  • kubernetes/apps/ai/omniroute/app/helmrelease.yaml
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • Tanguille/LLMKube (auto-detected)
  • Tanguille/toolhive (auto-detected)
  • Tanguille/pr-reviewer-action (auto-detected)
  • Tanguille/renovate-presets (auto-detected)
  • Tanguille/litellm-operator (auto-detected)
  • Tanguille/2x-R9700-RDNA4-GFX1201-sglang-inference (auto-detected)

📝 Walkthrough

Walkthrough

The Omniroute HelmRelease adds pod DNS configuration with ndots: "1" to prioritize absolute provider hostname lookups before Kubernetes search-domain expansion.

Changes

Omniroute DNS configuration

Layer / File(s) Summary
Configure pod DNS lookup order
kubernetes/apps/ai/omniroute/app/helmrelease.yaml
Adds defaultPodOptions.dnsConfig with ndots: "1" for Omniroute pods.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the DNS collision fix for omniroute and matches the primary change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/omniroute-dns-ndots

Comment @coderabbitai help to get the list of available commands.

@deepsource-io

deepsource-io Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

DeepSource Code Review

We reviewed changes in 1ab49d5...c4454b1 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Aug 3, 2026 8:49p.m. Review ↗
Shell Aug 3, 2026 8:49p.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@tanguille-cluster

Copy link
Copy Markdown
@@ spec.template.spec @@
# apps/v1/Deployment/ai/omniroute
! + one map entry added:
+ dnsConfig:
+   options:
+   - name: ndots
+     value: "1"

@tanguille-cluster

Copy link
Copy Markdown
@@ spec.values.defaultPodOptions @@
# helm.toolkit.fluxcd.io/v2/HelmRelease/ai/omniroute
! + one map entry added:
+ dnsConfig:
+   options:
+   - name: ndots
+     value: "1"

@Tanguille
Tanguille merged commit aa31986 into main Aug 3, 2026
15 checks passed
@Tanguille
Tanguille deleted the fix/omniroute-dns-ndots branch August 3, 2026 21:07
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

AI Automated Review

Analysis engine: omniroute@http://litellm.ai.svc.cluster.local/v1 (openai)

Recommendation

Approve. The PR correctly resolves a DNS search-domain collision by setting ndots: 1 in the pod's dnsConfig. This is a well-known Kubernetes DNS behavior fix and follows an established pattern already used elsewhere in this repository.

Change-by-Change Findings

kubernetes/apps/ai/omniroute/app/helmrelease.yaml

  • Adds dnsConfig.options.ndots: "1" under defaultPodOptions (lines 50–57).
  • Root cause is accurately described: with the default ndots: 5, the external domain opencode.ai (which has only 1 dot) triggers the cluster DNS search suffix expansion, resolving to opencode.ai.svc.cluster.local — which collides with an internal Service named opencode in the same namespace. Setting ndots: 1 forces direct resolution of names containing any dots before trying search suffixes.
  • The YAML structure is correct for the app-template chart's defaultPodOptions.dnsConfig field.
  • No resource quota or limit changes are introduced; existing CPU/memory requests and limits remain untouched.

Standards Compliance

  • Follows Conventional Commit title (fix(omniroute): ...).
  • Uses the same ndots: 1 pattern documented as already in use by seerr and *-exporter apps in this repo for identical DNS collision issues.
  • No secrets, credentials, or SOPS-encrypted content modified.
  • Manifest is scoped to a single HelmRelease; no cross-cutting changes.

Must-Check Items

  1. Validate manifest against target cluster version: The dnsConfig field is part of the standard Kubernetes PodSpec (v1) and has been stable since early 1.x releases. It is fully compatible with any Talos Linux / Kubernetes version in active use. No version-specific concerns.
  2. Check for resource quota / limit changes: No resource requests or limits were modified. Only DNS configuration was added. This check does not apply.

Unknowns or Needs Verification

  • Post-rollout validation (confirming dig opencode.ai resolves externally rather than to the internal ClusterIP) remains a runtime verification step noted in the PR test plan. This cannot be confirmed from the diff alone but is outside the scope of a static review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant