Skip to content

feat: support port-based routing for Gateway API routes#419

Open
AlinsRan wants to merge 4 commits into
masterfrom
feat/port-based-routing-gateway-api
Open

feat: support port-based routing for Gateway API routes#419
AlinsRan wants to merge 4 commits into
masterfrom
feat/port-based-routing-gateway-api

Conversation

@AlinsRan
Copy link
Copy Markdown
Contributor

@AlinsRan AlinsRan commented May 27, 2026

Type of change

  • New feature

What this PR does / why we need it

This change implements support for port-based routing in HTTPRoute and GRPCRoute by leveraging the server_port variable in APISIX. When a route targets a specific Gateway listener (via sectionName or port in parentRefs), the translator now adds a condition to ensure the route only matches traffic on that specific port.

Key changes:

  • Add listener_port_match_mode config option to control when server_port route vars are injected:
    • auto (default): inject when parentRefs explicitly target listeners via sectionName/port, or when multiple listener ports are matched
    • explicit: inject only when parentRefs explicitly target listeners
    • off: never inject server_port vars
  • Update ParseRouteParentRefs to track all matched listeners per parentRef (not just the first match), enabling accurate port collection for multi-port gateways
  • Add shouldInjectServerPortVars and addServerPortVars helpers in the translator
  • Inject server_port matching vars in HTTPRoute and GRPCRoute translation
  • Extract translateBackendsToUpstreams to reduce duplication in HTTPRoute translator
  • Wire ListenerPortMatchMode through provider options, manager, and both apisix and api7ee providers
  • Add comprehensive unit tests

Pre-submission checklist

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Is this PR backward compatible?

Summary by CodeRabbit

Release Notes

  • New Features

    • Added configurable listener_port_match_mode setting to control server port variable injection in routes (auto, explicit, or off).
    • Improved status conditions with observed generation tracking for Gateway API route types.
    • Enhanced support for multiple Gateway listeners in route processing.
  • Tests

    • Added comprehensive test coverage for listener port matching modes and server port variable injection.

Review Change Stack

AlinsRan and others added 3 commits May 26, 2026 10:32
Stamps condition.ObservedGeneration = cp.GetGeneration() inside the four
Gateway-API mutator closures (HTTPRoute, UDPRoute, TCPRoute, GRPCRoute)
before MergeCondition, mirroring what SetApisixCRDConditionWithGeneration
already does for the legacy CRDs.

Backport of apache/apisix-ingress-controller#2768

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a route targets a specific Gateway listener via sectionName/port in
parentRefs, or when multiple listener ports exist, the translator now
injects server_port matching vars into APISIX routes to ensure traffic
is only matched on the intended port.

Key changes:
- Add ListenerPortMatchMode config (auto/explicit/off, default: auto)
  - auto: inject when parentRefs explicitly target listeners or when
    multiple listener ports are matched
  - explicit: inject only when parentRefs explicitly target a listener
  - off: never inject server_port vars
- Update translator to track matched listeners per route (supporting
  multiple listener matches when sectionName is not specified)
- Extract translateBackendsToUpstreams helper in HTTPRoute translator
- Add addServerPortVars and shouldInjectServerPortVars helpers
- Inject server_port vars in HTTPRoute and GRPCRoute translators
- Wire ListenerPortMatchMode through provider options and manager
- Fix pre-existing compile error in apisixconsumer_test.go

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

This PR introduces server-port-based route disambiguation and improves status generation tracking. Routes can now inject server_port match variables when their Gateway listens on multiple ports, controlled by a configurable ListenerPortMatchMode. Additionally, route status conditions now carry ObservedGeneration for better state tracking.

Changes

Server Port-Based Route Matching

Layer / File(s) Summary
Config type system and validation
internal/controller/config/types.go, internal/controller/config/config.go, internal/controller/config/config_test.go, config/samples/config.yaml
New ListenerPortMatchMode enum with auto, explicit, and off modes is added to Config with JSON/YAML tags. NewDefaultConfig() initializes mode to auto, and Validate() enforces allowed values.
Translator mode storage and decision logic
internal/adc/translator/translator.go
Translator stores ListenerPortMatchMode, normalizes mode values via normalizeMode, and implements hasExplicitListenerTarget (detects explicit section/port targeting) and shouldInjectServerPortVars (decides injection based on mode, explicit targeting, and listener count).
Listener collection in translation context
internal/controller/context.go, internal/controller/utils.go, internal/controller/grpcroute_controller.go, internal/controller/httproute_controller.go
RouteParentRefContext adds Listeners slice. ParseRouteParentRefs collects all matching listeners when SectionName is unspecified, preserving backward compatibility with single Listener field. Controllers populate tctx.Listeners from Gateway listeners, preferring Listeners slice when available.
Route port variable injection
internal/adc/translator/httproute.go, internal/adc/translator/grpcroute.go, internal/adc/translator/grpcroute_test.go
Helper addServerPortVars injects server_port == <port> (single port) or server_port in [ports...] (multiple ports) into route vars. TranslateHTTPRoute and TranslateGRPCRoute collect listener ports and conditionally inject via decision logic. TestTranslateGRPCRouteServerPortVarsByMode covers injection behavior across all modes.
System wiring and test updates
internal/manager/run.go, internal/provider/options.go, internal/provider/api7ee/provider.go, internal/provider/apisix/provider.go, internal/webhook/v1/adc_validation.go, internal/adc/translator/*_test.go
Manager passes ListenerPortMatchMode to provider options. Both APISIX and API7ee providers initialize Translator with mode. Webhook validator uses mode in ADC translator. All translator tests updated to pass mode parameter (empty string normalizes to auto).

Route Status ObservedGeneration Tracking

Layer / File(s) Summary
Set ObservedGeneration in route conditions
internal/provider/apisix/status.go, internal/provider/api7ee/status.go, test/e2e/gatewayapi/status.go
For HTTPRoute, UDPRoute, TCPRoute, and GRPCRoute, the status mutators now set condition.ObservedGeneration from the route object's current GetGeneration() before merging parent ref conditions. E2E test verifies observedGeneration: 1 appears in scaled-up route YAML.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • nic-6443
  • shreemaan-abhishek
🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review ⚠️ Warning Critical issues: ignored backendErr at httproute.go:710, appendListeners dedup by name only (not name+port), listenersForGatewayContext unused Listeners field, weight index mismatch, no E2E tests. Handle httproute.go backendErr; fix appendListeners dedup key to (name,port); use RouteParentRefContext.Listeners; track validBackends for weight indexing; add E2E integration tests.
✅ 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 'feat: support port-based routing for Gateway API routes' directly and clearly summarizes the main change—adding port-based routing support for Gateway API routes via listener port injection.
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.
Security Check ✅ Passed Port-based routing PR contains no sensitive data exposure, auth bypass, unencrypted secrets, resource isolation issues, TLS errors, or secret reference problems across the seven security categories.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/port-based-routing-gateway-api

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
internal/provider/apisix/status.go (1)

268-289: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

TLSRoute should set ObservedGeneration for consistency.

The TLSRoute status mutator does not set condition.ObservedGeneration = cp.GetGeneration() before merging conditions, unlike HTTPRoute, UDPRoute, TCPRoute, and GRPCRoute (lines 126, 162, 198, 234). TLSRoute is also a Gateway API route type and should track ObservedGeneration in status conditions.

🔧 Proposed fix
 		Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
 			cp := obj.(*gatewayv1alpha2.TLSRoute).DeepCopy()
+			condition.ObservedGeneration = cp.GetGeneration()
 			gatewayNs := cp.GetNamespace()
🤖 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/provider/apisix/status.go` around lines 268 - 289, In the TLSRoute
status mutator (the status.MutatorFunc handling *gatewayv1alpha2.TLSRoute), set
condition.ObservedGeneration = cp.GetGeneration() on the local condition
variable before calling cutils.MergeCondition(ref.Conditions, condition) so the
TLSRoute status conditions record the observed generation consistently (match
the approach used in HTTPRoute/UDPRoute/TCPRoute/GRPCRoute).
internal/provider/api7ee/status.go (1)

267-289: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

TLSRoute should set ObservedGeneration for consistency.

The TLSRoute status mutator does not set condition.ObservedGeneration = cp.GetGeneration() before merging conditions, unlike HTTPRoute, UDPRoute, TCPRoute, and GRPCRoute (lines 125, 161, 197, 233). TLSRoute is also a Gateway API route type and should track ObservedGeneration in status conditions.

🔧 Proposed fix
 		Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
 			cp := obj.(*gatewayv1alpha2.TLSRoute).DeepCopy()
+			condition.ObservedGeneration = cp.GetGeneration()
 			gatewayNs := cp.GetNamespace()
🤖 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/provider/api7ee/status.go` around lines 267 - 289, The TLSRoute
status mutator is missing setting condition.ObservedGeneration before merging
conditions; update the MutatorFunc for TLSRoute (inside the anonymous func
handling cp := obj.(*gatewayv1alpha2.TLSRoute).DeepCopy()) to assign
condition.ObservedGeneration = cp.GetGeneration() immediately prior to calling
cutils.MergeCondition(ref.Conditions, condition) so TLSRoute matches the
HTTPRoute/UDPRoute/TCPRoute/GRPCRoute behavior and correctly records observed
generation in status conditions.
🤖 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 `@internal/adc/translator/httproute.go`:
- Line 710: The call to t.translateBackendsToUpstreams(tctx, rule, httpRoute,
service) currently ignores the returned error; change the assignment to capture
the error (e.g., enableWebsocket, backendErr :=
t.translateBackendsToUpstreams(...)) and handle backendErr instead of discarding
it—either return it up the call stack, set an appropriate status on the current
context, or log it via the function's logger (consistent with surrounding error
handling in this file). Ensure you reference the translateBackendsToUpstreams
call site and follow the existing error propagation pattern used by nearby
functions so failures are not silently swallowed.
- Around line 608-623: The traffic-split weight selection in httproute.go is
using positional indexing into rule.BackendRefs (lines around where
weightedUpstreams is built) which can desynchronize from the filtered upstreams
slice; instead, when building weightedUpstreams
(adctypes.TrafficSplitConfigRuleWeightedUpstream) look up the corresponding
BackendRef by a stable key (e.g., BackendRef.Name or
BackendRef.BackendRef.ServiceName/Identifier) rather than using i+1 positional
access, defaulting to apiv2.DefaultWeight when no matching BackendRef or Weight
is found; update the loop that iterates upstreams and the weight resolution
logic to perform this keyed lookup against rule.BackendRefs to ensure weights
bind to the correct upstream.

In `@internal/controller/utils.go`:
- Around line 1165-1174: appendListeners currently deduplicates only by
listener.Name which causes collisions for common names (e.g., "http"/"https")
across different ports; change the dedupe key in appendListeners to include both
Name and Port (or other port-identifying field) when building the seen map and
when checking toAdd entries (e.g., use a composite key like Name:Port), so
distinct listeners on different ports are not dropped; update the initial
population loop (for _, l := range existing) and the check/add loop (for _, l :=
range toAdd) to use that composite key (refer to function appendListeners and
variables seen, existing, toAdd).
- Around line 1135-1163: listenersForGatewayContext returns all gateway
listeners when RouteParentRefContext.ListenerName is empty, which incorrectly
includes non-matching listeners for parentRefs that target by port; update
listenersForGatewayContext (and use
RouteParentRefContext.Gateway.Port/ListenerName) so that if ListenerName is
empty but RouteParentRefContext.Port is set it filters
gateway.Gateway.Spec.Listeners to only those whose listener.Port equals that
Port, preserving the existing behavior that if ListenerName is set it returns
that single listener and otherwise returns all listeners only when no Port is
specified.

---

Outside diff comments:
In `@internal/provider/api7ee/status.go`:
- Around line 267-289: The TLSRoute status mutator is missing setting
condition.ObservedGeneration before merging conditions; update the MutatorFunc
for TLSRoute (inside the anonymous func handling cp :=
obj.(*gatewayv1alpha2.TLSRoute).DeepCopy()) to assign
condition.ObservedGeneration = cp.GetGeneration() immediately prior to calling
cutils.MergeCondition(ref.Conditions, condition) so TLSRoute matches the
HTTPRoute/UDPRoute/TCPRoute/GRPCRoute behavior and correctly records observed
generation in status conditions.

In `@internal/provider/apisix/status.go`:
- Around line 268-289: In the TLSRoute status mutator (the status.MutatorFunc
handling *gatewayv1alpha2.TLSRoute), set condition.ObservedGeneration =
cp.GetGeneration() on the local condition variable before calling
cutils.MergeCondition(ref.Conditions, condition) so the TLSRoute status
conditions record the observed generation consistently (match the approach used
in HTTPRoute/UDPRoute/TCPRoute/GRPCRoute).
🪄 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: CHILL

Plan: Pro

Run ID: cc0e5aef-565f-4f86-aa95-59ff8d33ca9c

📥 Commits

Reviewing files that changed from the base of the PR and between ecbb5d5 and 2701af6.

📒 Files selected for processing (25)
  • config/samples/config.yaml
  • internal/adc/translator/annotations_test.go
  • internal/adc/translator/apisixconsumer_test.go
  • internal/adc/translator/apisixroute_test.go
  • internal/adc/translator/consumer_test.go
  • internal/adc/translator/grpcroute.go
  • internal/adc/translator/grpcroute_test.go
  • internal/adc/translator/httproute.go
  • internal/adc/translator/httproute_test.go
  • internal/adc/translator/translator.go
  • internal/controller/config/config.go
  • internal/controller/config/config_test.go
  • internal/controller/config/types.go
  • internal/controller/context.go
  • internal/controller/grpcroute_controller.go
  • internal/controller/httproute_controller.go
  • internal/controller/utils.go
  • internal/manager/run.go
  • internal/provider/api7ee/provider.go
  • internal/provider/api7ee/status.go
  • internal/provider/apisix/provider.go
  • internal/provider/apisix/status.go
  • internal/provider/options.go
  • internal/webhook/v1/adc_validation.go
  • test/e2e/gatewayapi/status.go

Comment on lines +608 to +623
// Set weight in traffic-split for the default upstream
weight := apiv2.DefaultWeight
if rule.BackendRefs[0].Weight != nil {
weight = int(*rule.BackendRefs[0].Weight)
}
weightedUpstreams = append(weightedUpstreams, adctypes.TrafficSplitConfigRuleWeightedUpstream{
Weight: weight,
})

// Set other upstreams in traffic-split using upstream_id
for i, upstream := range upstreams {
weight := apiv2.DefaultWeight
// get weight from the backend refs starting from the second backend
if i+1 < len(rule.BackendRefs) && rule.BackendRefs[i+1].Weight != nil {
weight = int(*rule.BackendRefs[i+1].Weight)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Traffic-split weights can bind to the wrong backend after filtering.

If earlier BackendRefs are skipped (error or zero nodes), Line 610 / Line 621 still read weights from rule.BackendRefs by positional index, which can mismatch the upstreams slice and route traffic incorrectly.

💡 Suggested fix
 func (t *Translator) translateBackendsToUpstreams(
@@
-	upstreams := make([]*adctypes.Upstream, 0)
+	upstreams := make([]*adctypes.Upstream, 0)
+	validBackends := make([]gatewayv1.HTTPBackendRef, 0)
@@
 		upstream.ID = id.GenID(upstreamName)
 		upstreams = append(upstreams, upstream)
+		validBackends = append(validBackends, backend)
 	}
@@
-		if rule.BackendRefs[0].Weight != nil {
-			weight = int(*rule.BackendRefs[0].Weight)
+		if validBackends[0].Weight != nil {
+			weight = int(*validBackends[0].Weight)
 		}
@@
-			if i+1 < len(rule.BackendRefs) && rule.BackendRefs[i+1].Weight != nil {
-				weight = int(*rule.BackendRefs[i+1].Weight)
+			if i+1 < len(validBackends) && validBackends[i+1].Weight != nil {
+				weight = int(*validBackends[i+1].Weight)
 			}
🤖 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.go` around lines 608 - 623, The
traffic-split weight selection in httproute.go is using positional indexing into
rule.BackendRefs (lines around where weightedUpstreams is built) which can
desynchronize from the filtered upstreams slice; instead, when building
weightedUpstreams (adctypes.TrafficSplitConfigRuleWeightedUpstream) look up the
corresponding BackendRef by a stable key (e.g., BackendRef.Name or
BackendRef.BackendRef.ServiceName/Identifier) rather than using i+1 positional
access, defaulting to apiv2.DefaultWeight when no matching BackendRef or Weight
is found; update the loop that iterates upstreams and the weight resolution
logic to perform this keyed lookup against rule.BackendRefs to ensure weights
bind to the correct upstream.

},
}
}
enableWebsocket, _ := t.translateBackendsToUpstreams(tctx, rule, httpRoute, service)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not ignore backendErr from translateBackendsToUpstreams.

Line 710 drops the returned error (enableWebsocket, _ := ...). Handle it explicitly (log, status propagation, or return) so failures are not silently swallowed.

As per coding guidelines: Every function return value must be checked for errors; errors must be properly handled (not ignored, not silently swallowed).

🤖 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.go` at line 710, The call to
t.translateBackendsToUpstreams(tctx, rule, httpRoute, service) currently ignores
the returned error; change the assignment to capture the error (e.g.,
enableWebsocket, backendErr := t.translateBackendsToUpstreams(...)) and handle
backendErr instead of discarding it—either return it up the call stack, set an
appropriate status on the current context, or log it via the function's logger
(consistent with surrounding error handling in this file). Ensure you reference
the translateBackendsToUpstreams call site and follow the existing error
propagation pattern used by nearby functions so failures are not silently
swallowed.

Comment on lines +1135 to +1163
for _, listener := range listenersForGatewayContext(gateway) {
// Only consider listeners that can effectively configure hostnames (HTTP, HTTPS, or TLS).
if !isListenerHostnameEffective(listener) {
continue
}
} else {
// Otherwise, check all listeners
for _, listener := range gateway.Gateway.Spec.Listeners {
// Only consider listeners that can effectively configure hostnames (HTTP, HTTPS, or TLS)
if isListenerHostnameEffective(listener) {
if listener.Hostname == nil {
return nil, true
}
hostnames = append(hostnames, *listener.Hostname)
}
if listener.Hostname == nil {
return nil, true
}
hostnames = append(hostnames, *listener.Hostname)
}
}

return hostnames, false
}

// listenersForGatewayContext returns the listeners relevant for hostname resolution.
// When a specific listener was matched by sectionName, only that listener is returned.
// Otherwise, all gateway spec listeners are considered.
func listenersForGatewayContext(gateway RouteParentRefContext) []gatewayv1.Listener {
if gateway.ListenerName != "" {
for _, listener := range gateway.Gateway.Spec.Listeners {
if string(listener.Name) == gateway.ListenerName {
return []gatewayv1.Listener{listener}
}
}
return nil
}
return gateway.Gateway.Spec.Listeners
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use matched listeners for hostname resolution when available.

listenersForGatewayContext currently falls back to gateway.Gateway.Spec.Listeners whenever ListenerName is empty. For parentRefs that target by port (without sectionName), this can pull in non-matching listeners and produce incorrect hostname union/intersection behavior.

Suggested fix
 func listenersForGatewayContext(gateway RouteParentRefContext) []gatewayv1.Listener {
+	if len(gateway.Listeners) > 0 {
+		return gateway.Listeners
+	}
+	if gateway.Listener != nil {
+		return []gatewayv1.Listener{*gateway.Listener}
+	}
 	if gateway.ListenerName != "" {
 		for _, listener := range gateway.Gateway.Spec.Listeners {
 			if string(listener.Name) == gateway.ListenerName {
 				return []gatewayv1.Listener{listener}
 			}
 		}
 		return nil
 	}
 	return gateway.Gateway.Spec.Listeners
 }
🤖 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/controller/utils.go` around lines 1135 - 1163,
listenersForGatewayContext returns all gateway listeners when
RouteParentRefContext.ListenerName is empty, which incorrectly includes
non-matching listeners for parentRefs that target by port; update
listenersForGatewayContext (and use
RouteParentRefContext.Gateway.Port/ListenerName) so that if ListenerName is
empty but RouteParentRefContext.Port is set it filters
gateway.Gateway.Spec.Listeners to only those whose listener.Port equals that
Port, preserving the existing behavior that if ListenerName is set it returns
that single listener and otherwise returns all listeners only when no Port is
specified.

Comment on lines +1165 to +1174
// appendListeners appends listeners to the slice, avoiding duplicates by port+name.
func appendListeners(existing []gatewayv1.Listener, toAdd ...gatewayv1.Listener) []gatewayv1.Listener {
seen := make(map[string]struct{}, len(existing))
for _, l := range existing {
seen[string(l.Name)] = struct{}{}
}
for _, l := range toAdd {
if _, ok := seen[string(l.Name)]; !ok {
existing = append(existing, l)
seen[string(l.Name)] = struct{}{}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix listener dedup key to avoid dropping valid ports.

appendListeners deduplicates only by listener.Name. Since tctx.Listeners is aggregated across gateways, common names like http/https can collide and remove distinct listeners, which may drop required server-port matches.

Suggested fix
 func appendListeners(existing []gatewayv1.Listener, toAdd ...gatewayv1.Listener) []gatewayv1.Listener {
-	seen := make(map[string]struct{}, len(existing))
+	type listenerKey struct {
+		Name string
+		Port gatewayv1.PortNumber
+	}
+	seen := make(map[listenerKey]struct{}, len(existing))
 	for _, l := range existing {
-		seen[string(l.Name)] = struct{}{}
+		seen[listenerKey{Name: string(l.Name), Port: l.Port}] = struct{}{}
 	}
 	for _, l := range toAdd {
-		if _, ok := seen[string(l.Name)]; !ok {
+		key := listenerKey{Name: string(l.Name), Port: l.Port}
+		if _, ok := seen[key]; !ok {
 			existing = append(existing, l)
-			seen[string(l.Name)] = struct{}{}
+			seen[key] = struct{}{}
 		}
 	}
 	return existing
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// appendListeners appends listeners to the slice, avoiding duplicates by port+name.
func appendListeners(existing []gatewayv1.Listener, toAdd ...gatewayv1.Listener) []gatewayv1.Listener {
seen := make(map[string]struct{}, len(existing))
for _, l := range existing {
seen[string(l.Name)] = struct{}{}
}
for _, l := range toAdd {
if _, ok := seen[string(l.Name)]; !ok {
existing = append(existing, l)
seen[string(l.Name)] = struct{}{}
// appendListeners appends listeners to the slice, avoiding duplicates by port+name.
func appendListeners(existing []gatewayv1.Listener, toAdd ...gatewayv1.Listener) []gatewayv1.Listener {
type listenerKey struct {
Name string
Port gatewayv1.PortNumber
}
seen := make(map[listenerKey]struct{}, len(existing))
for _, l := range existing {
seen[listenerKey{Name: string(l.Name), Port: l.Port}] = struct{}{}
}
for _, l := range toAdd {
key := listenerKey{Name: string(l.Name), Port: l.Port}
if _, ok := seen[key]; !ok {
existing = append(existing, l)
seen[key] = struct{}{}
}
}
return existing
}
🤖 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/controller/utils.go` around lines 1165 - 1174, appendListeners
currently deduplicates only by listener.Name which causes collisions for common
names (e.g., "http"/"https") across different ports; change the dedupe key in
appendListeners to include both Name and Port (or other port-identifying field)
when building the seen map and when checking toAdd entries (e.g., use a
composite key like Name:Port), so distinct listeners on different ports are not
dropped; update the initial population loop (for _, l := range existing) and the
check/add loop (for _, l := range toAdd) to use that composite key (refer to
function appendListeners and variables seen, existing, toAdd).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

conformance test report - apisix-standalone mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-05-28T01:05:55Z"
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:
    - GRPCRouteListenerHostnameMatching
    result: failure
    statistics:
      Failed: 1
      Passed: 11
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests failed with 1 test failures.
- core:
    failedTests:
    - HTTPRouteListenerHostnameMatching
    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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-05-28T01:06:30Z"
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:
    - GRPCRouteListenerHostnameMatching
    result: failure
    statistics:
      Failed: 1
      Passed: 11
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests failed with 1 test failures.
- core:
    failedTests:
    - HTTPRouteInvalidBackendRefUnknownKind
    - HTTPRouteListenerHostnameMatching
    result: failure
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 2
      Passed: 30
      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 2 test failures. Extended tests failed with 1 test
    failures.
- core:
    result: partial
    skippedTests:
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 0
      Passed: 10
      Skipped: 1
  name: GATEWAY-TLS
  summary: Core tests partially succeeded with 1 test skips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2026-05-28T01:23:16Z"
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:
    - GRPCRouteListenerHostnameMatching
    - GatewayModifyListeners
    result: failure
    statistics:
      Failed: 2
      Passed: 10
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests failed with 2 test failures.
- core:
    failedTests:
    - GatewayModifyListeners
    - HTTPRouteListenerHostnameMatching
    result: failure
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 2
      Passed: 30
      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 2 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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant