fix(backport): preserve HTTPRoute upstream scheme from backend ref#395
Merged
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughTranslateHTTPRoute now preserves any pre-set ChangesScheme assignment and tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR backports a fix to the HTTPRoute translator so it no longer overwrites an upstream scheme that has already been set earlier in backend processing (e.g., via BackendTrafficPolicy), preserving the expected https/wss behavior when applicable.
Changes:
- Only set
upstream.Schemefrom the resolved backend appProtocol whenupstream.Schemeis currently empty. - Align HTTPRoute scheme precedence behavior with other translators (e.g., ApisixRoute/Ingress) that avoid clobbering an already-populated scheme.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
conformance test report - apisix-standalone modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-05-06T02:29:58Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
result: partial
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 0
Passed: 32
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests partially succeeded with 1 test skips. Extended tests partially
succeeded with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
Contributor
conformance test report - apisix modeapiVersion: gateway.networking.k8s.io/v1
date: "2026-05-06T02:30:28Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
result: success
statistics:
Failed: 0
Passed: 12
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests succeeded.
- core:
failedTests:
- HTTPRouteInvalidBackendRefUnknownKind
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
result: partial
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 0
Passed: 11
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests partially succeeded
with 1 test skips.
- core:
result: partial
skippedTests:
- TLSRouteSimpleSameNamespace
statistics:
Failed: 0
Passed: 10
Skipped: 1
name: GATEWAY-TLS
summary: Core tests partially succeeded with 1 test skips. |
Contributor
conformance test reportapiVersion: gateway.networking.k8s.io/v1
date: "2026-05-06T02:51:47Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
contact: null
organization: APISIX
project: apisix-ingress-controller
url: https://github.com/apache/apisix-ingress-controller.git
version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
failedTests:
- GRPCExactMethodMatching
- GRPCRouteHeaderMatching
- GRPCRouteListenerHostnameMatching
- GatewayModifyListeners
result: failure
statistics:
Failed: 4
Passed: 8
Skipped: 0
name: GATEWAY-GRPC
summary: Core tests failed with 4 test failures.
- core:
failedTests:
- GatewayModifyListeners
result: failure
skippedTests:
- HTTPRouteHTTPSListener
statistics:
Failed: 1
Passed: 31
Skipped: 1
extended:
failedTests:
- HTTPRouteBackendProtocolWebSocket
result: failure
skippedTests:
- HTTPRouteRedirectPortAndScheme
statistics:
Failed: 1
Passed: 10
Skipped: 1
supportedFeatures:
- GatewayAddressEmpty
- GatewayPort8080
- HTTPRouteBackendProtocolWebSocket
- HTTPRouteDestinationPortMatching
- HTTPRouteHostRewrite
- HTTPRouteMethodMatching
- HTTPRoutePathRewrite
- HTTPRoutePortRedirect
- HTTPRouteQueryParamMatching
- HTTPRouteRequestMirror
- HTTPRouteResponseHeaderModification
- HTTPRouteSchemeRedirect
unsupportedFeatures:
- GatewayHTTPListenerIsolation
- GatewayInfrastructurePropagation
- GatewayStaticAddresses
- HTTPRouteBackendProtocolH2C
- HTTPRouteBackendRequestHeaderModification
- HTTPRouteBackendTimeout
- HTTPRouteParentRefPort
- HTTPRoutePathRedirect
- HTTPRouteRequestMultipleMirrors
- HTTPRouteRequestPercentageMirror
- HTTPRouteRequestTimeout
name: GATEWAY-HTTP
summary: Core tests failed with 1 test failures. Extended tests failed with 1 test
failures.
- core:
failedTests:
- GatewayModifyListeners
- TLSRouteSimpleSameNamespace
result: failure
statistics:
Failed: 2
Passed: 9
Skipped: 0
name: GATEWAY-TLS
summary: Core tests failed with 2 test failures. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/adc/translator/httproute_test.go (1)
146-147: ⚡ Quick winGuard against empty upstream nodes before indexing.
On Line 147,
Nodes[0]can panic if translation returns an empty node list, which makes failures less actionable. Add arequirelength/non-empty check first.Suggested patch
require.NoError(t, err) require.Len(t, result.Services, 1) require.NotNil(t, result.Services[0].Upstream) + require.NotEmpty(t, result.Services[0].Upstream.Nodes) assert.Equal(t, tt.wantScheme, result.Services[0].Upstream.Scheme) assert.Equal(t, "10.0.0.1", result.Services[0].Upstream.Nodes[0].Host)🤖 Prompt for 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. In `@internal/adc/translator/httproute_test.go` around lines 146 - 147, Add a guard that asserts the upstream nodes slice is non-empty before indexing into Nodes[0]; specifically, before the line that asserts result.Services[0].Upstream.Nodes[0].Host, use require.NotEmpty or require.Greater to check result.Services[0].Upstream.Nodes (or result.Services[0].Upstream.Nodes.Len()) so the test fails with a clear message rather than panicking when Nodes is empty.
🤖 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.
Nitpick comments:
In `@internal/adc/translator/httproute_test.go`:
- Around line 146-147: Add a guard that asserts the upstream nodes slice is
non-empty before indexing into Nodes[0]; specifically, before the line that
asserts result.Services[0].Upstream.Nodes[0].Host, use require.NotEmpty or
require.Greater to check result.Services[0].Upstream.Nodes (or
result.Services[0].Upstream.Nodes.Len()) so the test fails with a clear message
rather than panicking when Nodes is empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e2ffef3a-ef12-4a9a-a6ab-1a6d917c8dbb
📒 Files selected for processing (1)
internal/adc/translator/httproute_test.go
AlinsRan
changed the title
nic-6443
approved these changes
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When translating an
HTTPRoute, the translator calledAttachBackendTrafficPolicyToUpstreamto apply anyBackendTrafficPolicy(including an explicitschemesuch ashttpsorwss) to the upstream object. Immediately after, it unconditionally overwroteupstream.Schemewith the value derived from the service port'sappProtocol:This meant that even when a
BackendTrafficPolicyexplicitly setscheme: https, the appProtocol-based derivation silently overwrote it — causing backends configured for TLS or WebSocket-Secure to be contacted over the wrong scheme.Fix
Add a guard so the appProtocol-derived scheme is only applied when the upstream scheme has not already been set:
This preserves any scheme set by
BackendTrafficPolicywhile still falling back to the appProtocol-derived value when none is specified.Tests
Added
TestTranslateHTTPRouteUpstreamSchemecovering:BackendTrafficPolicyscheme is preserved overappProtocol-derived schemeappProtocol(e.g.kubernetes.io/wss) correctly deriveshttpswhen no policy scheme is set