Skip to content

fix(translator): set ListenerSet and listener Accepted:True for InvalidCertificateRef#8871

Merged
arkodg merged 5 commits into
envoyproxy:mainfrom
apkatsikas:fix/listenerset-accepted-invalid-cert
May 8, 2026
Merged

fix(translator): set ListenerSet and listener Accepted:True for InvalidCertificateRef#8871
arkodg merged 5 commits into
envoyproxy:mainfrom
apkatsikas:fix/listenerset-accepted-invalid-cert

Conversation

@apkatsikas
Copy link
Copy Markdown
Contributor

@apkatsikas apkatsikas commented Apr 27, 2026

What type of PR is this?
fix

What this PR does / why we need it:

When a ListenerSet listener has an InvalidCertificateRef, Envoy Gateway was incorrectly setting Accepted: False on both the listener and the ListenerSet object. A Gateway in the identical situation correctly remains Accepted: True.

The Gateway API spec places InvalidCertificateRef exclusively under ResolvedRefs: False for both Listeners and ListenerEntries — it is not listed as a valid reason for Accepted: False on either resource type. A missing or malformed certificate is a reference resolution concern, not a structural validity concern; the listener configuration itself is valid.

This can create a bootstrapping deadlock for users of tools such as external-dns that gate DNS record provisioning on Accepted: True — without the DNS record, ACME HTTP-01 challenges cannot complete, so the certificate is never issued and the ListenerSet remains Accepted: False.

This change:

  • Sets Accepted: True on listeners with InvalidCertificateRef, consistent with Gateway behavior. Programmed: False is still set correctly since the listener cannot serve traffic without a valid certificate. Note: RefNotPermitted is intentionally excluded — a missing ReferenceGrant is a security policy violation and the conformance suite requires Accepted: False in that case.
  • Consolidates the two separate ResolvedRefs-only condition blocks in validateListenerConditions into a single readable block with a named boolean for clarity.
  • Gives the Accepted and Programmed rollup conditions on the ListenerSet object distinct, accurate messages rather than sharing the same string.
  • Adds test coverage for the missing cert and mixed valid/invalid cert ListenerSet scenarios.

Which issue(s) this PR fixes:

Fixes #8870

Release Notes: Yes

@apkatsikas apkatsikas requested a review from a team as a code owner April 27, 2026 19:44
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 27, 2026

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit 5225068
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/69fd3b9a5a53af00088816ba
😎 Deploy Preview https://deploy-preview-8871--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.73%. Comparing base (9236db0) to head (5225068).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8871      +/-   ##
==========================================
- Coverage   74.74%   74.73%   -0.01%     
==========================================
  Files         251      251              
  Lines       40370    40396      +26     
==========================================
+ Hits        30173    30189      +16     
- Misses       8131     8139       +8     
- Partials     2066     2068       +2     

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@apkatsikas
Copy link
Copy Markdown
Contributor Author

Hi @zhaohuabing , any thoughts on this implementation?

@apkatsikas apkatsikas force-pushed the fix/listenerset-accepted-invalid-cert branch 2 times, most recently from 770e339 to 7289c3d Compare May 1, 2026 15:27
@zhaohuabing
Copy link
Copy Markdown
Member

/retest

@apkatsikas
Copy link
Copy Markdown
Contributor Author

/retest

Hi @zhaohuabing - for both run #3 and run #4 the e2e-test (v1.32.11, ipv4, default job failed, but for different tests. For #4 it was TestMultipleGC/Internet_GC_Test/PolicyStatusAggregatesAcrossGatewayClasses/backendtrafficpolicy_status_aggregates_across_gateway_classes and #3 it was TestE2E/WasmHTTPCodeSource/http_route_with_http_wasm_source - so it does not look like anything consistent to me, perhaps a test flake?

@zhaohuabing
Copy link
Copy Markdown
Member

/retest

@zhaohuabing
Copy link
Copy Markdown
Member

/retest

Hi @zhaohuabing - for both run #3 and run #4 the e2e-test (v1.32.11, ipv4, default job failed, but for different tests. For #4 it was TestMultipleGC/Internet_GC_Test/PolicyStatusAggregatesAcrossGatewayClasses/backendtrafficpolicy_status_aggregates_across_gateway_classes and #3 it was TestE2E/WasmHTTPCodeSource/http_route_with_http_wasm_source - so it does not look like anything consistent to me, perhaps a test flake?

Yes, these tests are flaky. You can rerun these tests with /retest.

@apkatsikas
Copy link
Copy Markdown
Contributor Author

/retest

Hi @zhaohuabing - for both run #3 and run #4 the e2e-test (v1.32.11, ipv4, default job failed, but for different tests. For #4 it was TestMultipleGC/Internet_GC_Test/PolicyStatusAggregatesAcrossGatewayClasses/backendtrafficpolicy_status_aggregates_across_gateway_classes and #3 it was TestE2E/WasmHTTPCodeSource/http_route_with_http_wasm_source - so it does not look like anything consistent to me, perhaps a test flake?

Yes, these tests are flaky. You can rerun these tests with /retest.

Cool, looks like the last run passed.

@zirain zirain force-pushed the fix/listenerset-accepted-invalid-cert branch from a670446 to 775136b Compare May 8, 2026 00:02
zirain
zirain previously approved these changes May 8, 2026
Copy link
Copy Markdown
Member

@zirain zirain left a comment

Choose a reason for hiding this comment

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

Thanks!

apkatsikas and others added 4 commits May 8, 2026 08:18
…idCertificateRef

When a ListenerSet listener has an unresolvable TLS certificate reference
(InvalidCertificateRef or RefNotPermitted), Accepted: False was incorrectly
set on both the listener and ListenerSet object. The Gateway API spec places
InvalidCertificateRef exclusively under ResolvedRefs, not Accepted — a missing
certificate is a reference resolution concern, not a structural one.

Fixes envoyproxy#8870

Signed-off-by: apkatsikas <apkatsikas@gmail.com>
Signed-off-by: apkatsikas <apkatsikas@gmail.com>
…handling

Unlike InvalidCertificateRef, RefNotPermitted should not set Accepted:True.
Update unit test fixtures to match.

Signed-off-by: apkatsikas <apkatsikas@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
@zirain zirain force-pushed the fix/listenerset-accepted-invalid-cert branch from 775136b to c607a5d Compare May 8, 2026 00:19
@apkatsikas apkatsikas requested a review from zirain May 8, 2026 00:36
zirain
zirain previously approved these changes May 8, 2026
lsProgrammedReason = gwapiv1.ListenerSetReasonProgrammed
lsMsg = "Some listeners are invalid"
lsAcceptedReason = gwapiv1.ListenerSetReasonListenersNotValid
lsAcceptedMsg = "Some listeners are invalid"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Non-blocking nit: should the reason and message be positive since accepted = true.

zhaohuabing
zhaohuabing previously approved these changes May 8, 2026
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

Signed-off-by: zirain <zirain2009@gmail.com>
@zirain zirain dismissed stale reviews from zhaohuabing and themself via 5225068 May 8, 2026 01:25
@zirain zirain requested a review from zhaohuabing May 8, 2026 01:25
@zirain zirain self-requested a review May 8, 2026 01:25
Copy link
Copy Markdown

@arko-oai arko-oai left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@arkodg arkodg merged commit 09b544f into envoyproxy:main May 8, 2026
37 of 39 checks passed
jukie added a commit that referenced this pull request May 13, 2026
* fix(api): increase RateLimitSelectCondition.headers MaxItems from 16 to 64 (#8906)

* fix(api): increase RateLimitSelectCondition.headers MaxItems from 16 to 64

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* feat: policy field owner (#8538)

* feat: policy field owner

Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* skip invalid listener first in IR (#8577)

* skip invalid listener

Signed-off-by: zirain <zirain2009@gmail.com>

* fix specValid

Signed-off-by: zirain <zirain2009@gmail.com>

* nit

Signed-off-by: zirain <zirain2009@gmail.com>

* fix

Signed-off-by: zirain <zirain2009@gmail.com>

* MUST NOT pick one conflicting Listener as the winner

Signed-off-by: zirain <zirain2009@gmail.com>

* update

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: zirain <zirain2009@gmail.com>
Co-authored-by: Isaac Wilson <isaac.wilson514@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix: remove cross ns policy attachment status (#8901)

* Revert "add warning for partially accepted targets"

This reverts commit 5d88fbb.

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

* remove warning condition for cross-ns policy attachments without referenceGrants

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* feat: add runner event metrics (#8802)

* add metrics for runner

Signed-off-by: zirain <zirain2009@gmail.com>

* rename

Signed-off-by: zirain <zirain2009@gmail.com>

* rename

Signed-off-by: zirain <zirain2009@gmail.com>

* reuse

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix: respect backend endpoint hostname for health checks (#8929)

* fix: respect backend endpoint hostname for health checks

### Summary
- Keep BackendTrafficPolicy HTTP health check hostnames as explicit cluster-level hosts, and leave route-derived host fallback to xDS cluster translation.
- Preserve Backend endpoint hostnames as per-endpoint overrides via Endpoint.HealthCheckConfig.hostname, ahead of the route fallback.
- Update gatewayapi/xDS fixtures, release notes, and generated API docs/CRDs for the host selection order.

### Test plan
- go test ./internal/ir
- go test ./internal/xds/translator
- go test ./internal/gatewayapi -run TestTranslate/backendtrafficpolicy
- go test ./internal/gatewayapi -run TestTranslate/(clienttrafficpolicy-http-health-check|envoyextensionpolicy-with-extproc-with-retries|envoyextensionpolicy-with-extproc-with-traffic-features|envoyproxy-accesslog-with-traffic|envoyproxy-tracing-backend-uds|envoyproxy-tracing-backend|securitypolicy-with-jwt-backendcluster|securitypolicy-with-jwt-backendsettings)
- make generate
- make manifests
- git diff --check

Signed-off-by: Arko Dasgupta <arkodg@gmail.com>
Co-authored-by: Codex <noreply@openai.com>

* fix gen

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: Arko Dasgupta <arkodg@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: zirain <zirain2009@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix(helm): propagate commonLabels to RBAC resources (#8818)

* feat(helm): propagate commonLabels to RBAC resources

Issue #8817 reported that 'helm template ... --set
commonLabels.custom-label=custom-value' left ClusterRole,
ClusterRoleBinding, Role, and RoleBinding resources unlabelled.
The other resources in the chart already include 'eg.labels' in
their metadata - which picks up 'commonLabels' via the helper
at _helpers.tpl:43 - but envoy-gateway-rbac.yaml didn't set any
labels block.

Add 'labels: {{- include "eg.labels" . | nindent 4 }}' on every
Role / RoleBinding / ClusterRole / ClusterRoleBinding declared in
envoy-gateway-rbac.yaml. Matches the existing labels pattern used
in certgen-rbac.yaml and envoy-gateway-deployment.yaml. Scopes are
'$' inside the watched-namespaces 'range' and '.' at the template
root, same rule the helper block inside the file already used.

Verified locally with:
  helm dependency update charts/gateway-helm
  envsubst < charts/gateway-helm/values.tmpl.yaml > \
    charts/gateway-helm/values.yaml
  helm template eg charts/gateway-helm \
    --set commonLabels.custom-label=custom-value | yq ...

All four RBAC resources now emit 'custom-label: custom-value' in
their metadata.labels, matching the issue's repro steps. Cert-gen
RBAC resources already carried it; this PR brings the core
envoy-gateway RBAC set into parity.

Fixes #8817

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

* chore: regenerate helm-template snapshots for RBAC labels

Run 'make helm-template.gateway-helm' to regenerate the snapshot fixtures
after the envoy-gateway-rbac.yaml labels change. Adds the 'labels:' block
to the RBAC resources in all 27 test cases.

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>

* fix gen

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

* add release note

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix(translator): set ListenerSet and listener Accepted:True for InvalidCertificateRef (#8871)

* fix(translator): set ListenerSet and listener Accepted:True for InvalidCertificateRef

When a ListenerSet listener has an unresolvable TLS certificate reference
(InvalidCertificateRef or RefNotPermitted), Accepted: False was incorrectly
set on both the listener and ListenerSet object. The Gateway API spec places
InvalidCertificateRef exclusively under ResolvedRefs, not Accepted — a missing
certificate is a reference resolution concern, not a structural one.

Fixes #8870

Signed-off-by: apkatsikas <apkatsikas@gmail.com>

* chore: fix gofumpt formatting in validateListenerConditions

Signed-off-by: apkatsikas <apkatsikas@gmail.com>

* fix(translator): separate RefNotPermitted from InvalidCertificateRef handling

Unlike InvalidCertificateRef, RefNotPermitted should not set Accepted:True.
Update unit test fixtures to match.

Signed-off-by: apkatsikas <apkatsikas@gmail.com>

* fix gen

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: apkatsikas <apkatsikas@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix: do not downgrade ALPN for only hostnames-overlapping listeners (#8934)

fix: do not downgrade ALPN for overlapping hostnames withoug SANs overlapping

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* feat: enableDeferredCreationStats by default (#8937)

* feat: enableDeferredCreationStats by default

Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* fix: restore last transition time in merge status conditions (#8962)

* fix: restore last transition time in merge status conditions

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

* add release note

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>

---------

Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

* [release/v1.8] v1.8.0 release notes

Cherry-picked release-notes/v1.8.0.yaml and VERSION bump from #8942.

Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>

---------

Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
Signed-off-by: jukie <10012479+jukie@users.noreply.github.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Signed-off-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Signed-off-by: Arko Dasgupta <arkodg@gmail.com>
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Signed-off-by: apkatsikas <apkatsikas@gmail.com>
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
Co-authored-by: wucm667 <109257021+wucm667@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Kota Kimura <86363983+kkk777-7@users.noreply.github.com>
Co-authored-by: zirain <zirain2009@gmail.com>
Co-authored-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
Co-authored-by: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Matt Van Horn <mvanhorn@users.noreply.github.com>
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Andrew Katsikas <apkatsikas@gmail.com>
Co-authored-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
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.

ListenerSet sets Accepted: False for InvalidCertificateRef, inconsistent with spec and Gateway behavior

5 participants