Skip to content

fix(gateway): tolerate optional SectionName in access-points status#1134

Merged
prometherion merged 3 commits intoclastix:masterfrom
criteo-forks:fix/gateway-access-points-optional-section-name
Apr 30, 2026
Merged

fix(gateway): tolerate optional SectionName in access-points status#1134
prometherion merged 3 commits intoclastix:masterfrom
criteo-forks:fix/gateway-access-points-optional-section-name

Conversation

@pierrecdn
Copy link
Copy Markdown
Contributor

Since cc8a8e1 [1], BuildGatewayAccessPointsStatus treats a nil ParentReference.SectionName as a fatal error via fetchGatewayByListener, which aborts the whole TCP status update.

Such situation leads to spamming of "missing sectionName" error lines on every reconcile. Even though nothing is functionally impacted the reporting of .status.kubernetesResources.gateway and status.addons.konnectivity.gateway isn't done, so any future logic relying on it would be broken.

The Gateway API spec makes SectionName optional [2] ("When unspecified (empty string), this will reference the entire resource"), and clients like the Kamaji CAPI control-plane provider do not currently emit it (at least until [3] is merged) so valid Route attachments end up blocking status progression.

Consequently relaxed the behavior by introducing
resolveMatchingListeners:

  • SectionName set -> existing listener-name indexer fast path;
  • SectionName nil/empty -> fetch the Gateway by namespace/name and emit one access point per listener, optionally filtered by ParentRef.Port;
  • missing or unprogrammed Gateways are skipped silently so a single unresolvable parentRef cannot abort the whole status update.

FindMatchingListener stays strict and keeps backing the indexer path.

Tests cover the unset, port-filter, missing-gateway, and not-programmed cases.

Since `cc8a8e1` [1], BuildGatewayAccessPointsStatus treats a nil
ParentReference.SectionName as a fatal error via
fetchGatewayByListener, which aborts the whole TCP status update.

Such situation leads to spamming of "missing sectionName" error lines
on every reconcile. Even though nothing is functionally impacted the
reporting of .status.kubernetesResources.gateway and
status.addons.konnectivity.gateway isn't done, so any future logic
relying on it would be broken.

The Gateway API spec makes SectionName optional [2] ("When unspecified
(empty string), this will reference the entire resource"), and clients
like the Kamaji CAPI control-plane provider do not currently emit it
(at least until [3] is merged) so valid Route attachments end up
blocking status progression.

Consequently relaxed the behavior by introducing
resolveMatchingListeners:
  - SectionName set -> existing listener-name indexer fast path;
  - SectionName nil/empty -> fetch the Gateway by namespace/name and
    emit one access point per listener, optionally filtered by
    ParentRef.Port;
  - missing or unprogrammed Gateways are skipped silently so a single
    unresolvable parentRef cannot abort the whole status update.

FindMatchingListener stays strict and keeps backing the indexer path.

Tests cover the unset, port-filter, missing-gateway, and not-programmed
cases.

[1] clastix#1074
[2] https://gateway-api.sigs.k8s.io/reference/spec/#parentreference
[3] clastix/cluster-api-control-plane-provider-kamaji#338

Signed-off-by: Pierre Cheynier <[email protected]>
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 24, 2026

Deploy Preview for kamaji-documentation canceled.

Name Link
🔨 Latest commit 9cb2b1e
🔍 Latest deploy log https://app.netlify.com/projects/kamaji-documentation/deploys/69f3090d3f8d980008036277

Comment thread internal/resources/k8s_gateway_resource_test.go
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates Gateway access-point status calculation so that TLSRoute parentRefs with an unset/empty SectionName (valid per Gateway API) no longer abort status updates, improving reconciliation stability and preventing repeated “missing sectionName” errors.

Changes:

  • Add resolveMatchingListeners to handle both strict (SectionName set) and tolerant (SectionName unset/empty) listener resolution.
  • Skip missing/unprogrammed Gateways during access-point status computation rather than failing the whole update.
  • Add Ginkgo tests covering unset SectionName, port filtering, missing Gateway, and unprogrammed Gateway scenarios.

Reviewed changes

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

File Description
internal/resources/k8s_gateway_utils.go Implements tolerant listener resolution and updates access-point status building logic.
internal/resources/k8s_gateway_resource_test.go Extends test suite to validate the new SectionName-optional behavior and edge cases.

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

Comment thread internal/resources/k8s_gateway_utils.go
Comment thread internal/resources/k8s_gateway_utils.go
Address review feedbacks on clastix#1134:

* discriminate "no gateway with that listener" from real failures.
  fetchGatewayByListener now returns a typed errGatewayListenerNotFound
  in that case and propagates every other error.

* in the "unset SectionName" branch, restrict listeners to those using
  Protocol == TLS via a new isEligibleListener helper, in which the port
  matching test is also moved.

Tests cover non-TLS listener filtering and error propagation from
BuildGatewayAccessPointsStatus.

Signed-off-by: Pierre Cheynier <[email protected]>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


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

Comment thread internal/resources/k8s_gateway_utils.go
Address new review feedback on clastix#1134:
* apply isEligibleListener on the SectionName fast path too, so a buggy
  Gateway controller cannot leave us emitting https://... access points
  for non-TLS listeners or for listeners using a port distinct from
  parentRef.Port.

Tests cover protocol-mismatch and port-mismatch fast-path cases.

Signed-off-by: Pierre Cheynier <[email protected]>
@prometherion prometherion merged commit acf121e into clastix:master Apr 30, 2026
12 checks passed
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.

3 participants