[strausmann]

Fixes #113.

Problem

findMatchingRouter() matched routers only on Host(...) and returned the first router encountered while iterating an unordered Go map. When a host is served by more than one non-redirect router — e.g. Pangolin's multi-target path routing, where a single resource with several Targets produces multiple routers sharing an identical Host() but differing PathPrefix() — the function returned whichever router Go's randomized map iteration visited first.

Because the router map is freshly deserialized on every 5s cache refresh, the "winner" changed from call to call. The caller applyResourceOverrides mutates the returned router's priority field in place, so a resource's RouterPriority override could land on the wrong sibling router, corrupting its priority and creating a priority tie between routers whose relative order must stay fixed — causing sporadic misrouting on shared hosts.

Fix

• Sort the matched routers deterministically, removing the dependence on Go's randomized map iteration order.
• When more than one non-redirect router shares a host, treat it as ambiguous — host alone cannot identify the correct router — and report no match instead of guessing. The caller already skips the override entirely when no router is found, so this leaves Pangolin's own router configuration (including its priority) untouched in the ambiguous case, which is strictly safer than corrupting it.
• The single-candidate case is unchanged — no behavior change for normal single-target hosts.

Tests

Added TestConfigProxyFindMatchingRouter and TestApplyResourceOverridesSkipsAmbiguousMultiTargetHost in services/config_proxy_test.go, verified TDD-style: against the pre-fix code the new tests fail (e.g. the catch-all router's priority is mutated to a sibling's value); with the fix they pass. go build ./... is clean and the changed-code tests pass (go test -run 'ConfigProxy|ApplyResourceOverrides' ./services/).

Unrelated heads-up (not touched by this PR): go test ./... currently fails to compile the services test binary because services/resource_watcher_test.go compares watcher.isRunning as a bare bool, while the field is an atomic.Bool. This pre-exists this change; the fix here is confined to config_proxy.go.

Real-world validation

Built from this branch and run as an isolated, read-only instance against a production Pangolin deployment, sampled side-by-side with the current (unpatched) build over 6 minutes (5s interval):

• 17 hosts with multiple Targets: 16 showed 10–42 router-priority flips over the window with the current build; 0 flips with this fix.
• 101 single-target hosts: identical priorities before and after — no regression.

Possible follow-up

There is a related disambiguation in services/resource_watcher.go (updateOrCreateResource selects by host without path disambiguation). I kept it out of this PR to keep the change focused, and its existing tests are currently skipped upstream. Happy to address it separately if that would help.

Summary by CodeRabbit

• Bug Fixes

  • Improved router matching to select non-redirect routes consistently.
  • Prevented ambiguous host matches from applying resource overrides to an arbitrary route.
  • Preserved fallback behavior when only redirect routes are available.
  • Ensured unmatched or ambiguous hosts leave router priorities and middleware settings unchanged.

• Tests

  • Added coverage for deterministic matching, unmatched hosts, redirect-only fallbacks, and ambiguous host scenarios.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[vercel]

@strausmann is attempting to deploy a commit to the HHF Technologies' projects Team on Vercel.

A member of the Team first needs to authorize it.

[coderabbitai]

📝 Walkthrough

findMatchingRouter now selects host matches deterministically, rejects ambiguous non-redirect matches, and preserves redirect-only fallback behavior. Regression tests verify matching results and prevent ambiguous resource overrides from changing router state.

Changes: Router matching and override safety

Layer / File(s)	Summary
Deterministic host matching services/config_proxy.go	findMatchingRouter sorts host matches, prefers one non-redirect router, retains redirect-only fallback, and returns no match for multiple non-redirect routers.
Override safety regression coverage services/config_proxy_test.go	Tests cover unmatched hosts, redirect-only fallback, ambiguous matches, and unchanged priorities and middleware overrides.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• hhftechnology/middleware-manager#113 — The pull request implements deterministic host fallback and prevents ambiguous router overrides.

Pre-merge checks: 5

Passed checks: 5 passed

Check name	Status	Explanation
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: fixing non-deterministic router priority for hosts with multiple targets or paths.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit sorts routers by name,
Rejects two targets in the same lane.
Redirects may still provide a way,
While overrides safely stay.
No random hop, no muddled trail—
The matching rules now prevail.}

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

[Copilot]

Pull request overview

This PR fixes non-deterministic router selection in ConfigProxy.findMatchingRouter() when multiple Traefik routers share the same Host(...) rule (common with Pangolin multi-target path routing). The change makes router matching deterministic and avoids incorrectly applying RouterPriority overrides to the wrong sibling router, preventing sporadic priority ties and misrouting.

Changes:

• Deterministically sorts host-matched routers before selection to remove dependence on randomized Go map iteration order.
• Treats “multiple non-redirect routers share the same host” as ambiguous and returns no match (so overrides are skipped rather than applied incorrectly).
• Adds targeted tests covering both the router-matching behavior and the end-to-end override-skipping regression case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
services/config_proxy.go	Makes host-based router fallback deterministic and safely returns no match for ambiguous multi-router hosts.
services/config_proxy_test.go	Adds regression and behavior tests to ensure ambiguous multi-target hosts do not receive incorrect overrides.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@services/config_proxy_test.go`:
- Around line 232-259: The tests in services/config_proxy_test.go lack coverage
for findMatchingRouter’s redirect-only fallback (case 0). Add a test with only
host-matching router names ending in “-redirect”, call findMatchingRouter
repeatedly, and assert it consistently returns the same expected name and
router.

In `@services/config_proxy.go`:
- Around line 1064-1088: Remove the ineffective websecure entrypoint loop and
its misleading safety comment from the case 1 branch of the nonRedirect
selection switch. Return the sole non-redirect candidate directly, preserving
the existing behavior that does not require a websecure entrypoint.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: da2bb5a7-a408-47e1-8895-87accedf4a47

📥 Commits

Reviewing files that changed from the base of the PR and between 7eb1378 and cad4c55.

📒 Files selected for processing (2)

• services/config_proxy.go
• services/config_proxy_test.go

📜 Review details

⚠️ CI failures not shown inline (1)

Commit Status: Vercel: Vercel

Conclusion: failure

Authorization required to deploy.

🔇 Additional comments (4)

services/config_proxy.go (2)

989-1015: LGTM!

---

1045-1063: LGTM!

services/config_proxy_test.go (2)

171-230: LGTM!

---

261-332: LGTM!