Skip to content

Target the backendRef port in HTTPRoute proxy services - #470

Merged
mlsmaycon merged 2 commits into
work/vibrant-goodall-ufrnk8-route-gateway-watchfrom
work/vibrant-goodall-ufrnk8-httproute-port
Oct 23, 2026
Merged

mlsmaycon merged 2 commits into
work/vibrant-goodall-ufrnk8-route-gateway-watchfrom
work/vibrant-goodall-ufrnk8-httproute-port

Conversation

@mlsmaycon

@mlsmaycon mlsmaycon commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #313. Stacked on #469; TestHTTPRouteBackendPort in #465 reproduces this.

The HTTPRoute controller built its reverse-proxy targets without a port, so the target came back with port 0 and the proxy only reached a Service that happened to listen on the default HTTP port. Each backendRef now becomes a target on the Service port it names. Targets also follow the route's backend order instead of map iteration order, so the request the controller sends no longer changes between reconciles of a route with several backends.

The controller spec moves its backend to port 8080 and checks the target's port; it fails with port 0 without this change. The e2e comment that explained port 80 as a workaround for this now says why the default backends keep it.

#335 attempted this together with path support and was closed unmerged; this change keeps to the port.

Summary by CodeRabbit

  • Bug Fixes
    • HTTP routes now send traffic to the port specified by each backend Service reference, including non-default ports.
    • Routes with multiple backends that use different ports are handled separately, while duplicate backend-and-port entries are included only once. This helps ensure traffic reaches the intended backend port without creating redundant proxy targets.

The HTTPRoute controller built proxy targets without a port, so NetBird
used port 0 and the proxy could only reach a Service by luck, when it
listened on the default HTTP port. Each backendRef now becomes a target on
the Service port it names, and targets follow the route's backend order
instead of map iteration order.
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 54245aa0-d587-445f-bbb7-c60e0014dcb2
📥 Commits

Reviewing files that changed from the base of the PR and between 4b3bc68 and e94a8a4.

📒 Files selected for processing: 3
  • internal/controller/httproute_controller.go
  • internal/controller/httproute_controller_test.go
  • test/e2e/workloads_test.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.

📝 Walkthrough

HTTPRoute reconciliation now records distinct backend Service-and-port pairs and uses each referenced port when creating proxy targets. The controller test verifies propagation of port 8080.

Changes: HTTPRoute backend port propagation

Layer / File(s) Summary
Collect backend references and set target ports
internal/controller/httproute_controller.go
The controller deduplicates backends by Service name and port. It builds proxy targets from those backends and sets each target port from the backend reference. A missing backend port maps to 0.
Verify backend port propagation
internal/controller/httproute_controller_test.go, test/e2e/workloads_test.go
The controller test verifies that a backend reference using port 8080 produces a target with port 8080. The end-to-end test comment describes the default backend port and points to coverage for another port.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: phillebaba

Merge Risk: ⚪ Minimal · up to e94a8

The requested backend-port behavior is implemented, with no current merge-blocking defect identified.

Security architecture risk: 🟡 Moderate · up to e94a8

Port propagation fixes nondefault-port routing, but it can also activate a backend port configured to receive no traffic. Namespace and Service identity remain unchanged. Actual downstream forwarding behavior has not been verified.

Retained concerns

  • Medium · security · inferred: A backend assigned weight 0 can now become an enabled proxy target on its nondefault port. Reconciliation ignores BackendRef.Weight and enables every retained Service-and-port pair. For a Service referenced on an active default port and a zero-weight second port, base emitted one target without a port; head emits enabled targets for both ports. This can activate a destination intentionally withheld during staged rollout. Weight handling was already missing, but activation of the additional port is new. Actual forwarding depends on downstream acceptance and selection of those targets.
Security review details

Security Blast Radius

  • inferred — The demonstrated reachability change concerns ports on same-namespace Service ClusterIPs exposed through the existing router and proxy. The source does not establish expansion to arbitrary hosts, other namespaces or additional credentials.

Security Findings and Attack Paths

  • inferred — For a persisted route containing a zero-weight nondefault-port backend, incoming proxy requests may reach a destination intended to receive no traffic after this change. Source proves submission of an enabled target, not observed exploitation or downstream traffic selection.

Trust Boundaries and Controls

  • observed — The route-supplied port is copied directly into the external target without comparison against Service.Spec.Ports. The external resource request carries the Service ClusterIP but no declared-port inventory. This establishes a validation gap in local evidence, not proof that undeclared ports are reachable through the production dataplane.
Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: using the HTTPRoute backendRef port for proxy service targets.
Linked Issues check Passed Issue #313 requires the HTTPRoute backendRef Service port to reach the proxy resource. The controller now records each backend reference and sets api.ServiceTarget.Port from ref.Port; the contro…
Out of Scope Changes check Passed The changes stay within the linked port objective. Backend-order preservation and duplicate backend handling support deterministic proxy targets for the same HTTPRoute. The controller test verifies th…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files.
Full details: Linked Issues check

Explanation

Issue #313 requires the HTTPRoute backendRef Service port to reach the proxy resource. The controller now records each backend reference and sets api.ServiceTarget.Port from ref.Port; the controller test uses port 8080 and asserts target port 8080. The implementation covers the reported non-default-port case.

Full details: Out of Scope Changes check

Explanation

The changes stay within the linked port objective. Backend-order preservation and duplicate backend handling support deterministic proxy targets for the same HTTPRoute. The controller test verifies the port, and the e2e comment documents the existing default-port fixture and its coverage. No unrelated product behavior is changed.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the route at night,
And finds the backend port set right.
Port eight-oh-eight-oh hops along,
Each target gets its matching song.
The rabbit thumps, then bounds away.

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

@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.01%. Comparing base (4b3bc68) to head (e94a8a4).

Files with missing lines Patch % Lines
internal/controller/httproute_controller.go 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                                 Coverage Diff                                 @@
##           work/vibrant-goodall-ufrnk8-route-gateway-watch     #470      +/-   ##
===================================================================================
+ Coverage                                            44.93%   45.01%   +0.07%     
===================================================================================
  Files                                                   28       28              
  Lines                                                 3843     3852       +9     
===================================================================================
+ Hits                                                  1727     1734       +7     
- Misses                                                1868     1869       +1     
- Partials                                               248      249       +1     
Flag Coverage Δ
unit 45.01% <83.33%> (+0.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…e-gateway-watch' into work/vibrant-goodall-ufrnk8-httproute-port
@mlsmaycon
mlsmaycon marked this pull request as ready for review October 23, 2026 21:41

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@mlsmaycon
mlsmaycon added this pull request to stack #472 October 23, 2026 21:43
@mlsmaycon
mlsmaycon merged commit 19fa613 into main Oct 23, 2026
8 checks passed
@mlsmaycon
mlsmaycon deleted the work/vibrant-goodall-ufrnk8-httproute-port branch October 23, 2026 22:46
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.

Gatewayapi httproute not setting correct port

2 participants