Skip to content

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

Merged
arkodg merged 1 commit into
envoyproxy:mainfrom
zhaohuabing:fix-8754
May 8, 2026
Merged

fix: do not downgrade ALPN for only hostnames-overlapping listeners#8934
arkodg merged 1 commit into
envoyproxy:mainfrom
zhaohuabing:fix-8754

Conversation

@zhaohuabing
Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing commented May 8, 2026

This PR preserves HTTP/2 for only hostname-overlapping listeners. Listeners with overlapping certificates are still downgraded to HTTP/1.1.

fix: #8754

…rlapping

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing zhaohuabing requested a review from a team as a code owner May 8, 2026 02:06
@netlify
Copy link
Copy Markdown

netlify Bot commented May 8, 2026

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit 087aab6
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/69fd451d9ee68c0008267a6f
😎 Deploy Preview https://deploy-preview-8934--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.

@zhaohuabing zhaohuabing changed the title fix: do not downgrade ALPN for overlapping hostnames withoug SANs ove… fix: do not downgrade ALPN for overlapping hostnames withoug SANs overlapping May 8, 2026
@zhaohuabing zhaohuabing changed the title fix: do not downgrade ALPN for overlapping hostnames withoug SANs overlapping fix: do not downgrade ALPN for overlapping hostnames without SANs overlapping May 8, 2026
@zhaohuabing zhaohuabing changed the title fix: do not downgrade ALPN for overlapping hostnames without SANs overlapping fix: do not downgrade ALPN for only hostnames-overlapping listeners May 8, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8934      +/-   ##
==========================================
- Coverage   74.74%   74.69%   -0.05%     
==========================================
  Files         251      251              
  Lines       40370    40368       -2     
==========================================
- Hits        30173    30154      -19     
- Misses       8131     8144      +13     
- Partials     2066     2070       +4     

☔ 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.

in listener https-bar. ALPN will default to HTTP/1.1 to prevent HTTP/2 connection
coalescing, unless explicitly configured via ClientTrafficPolicy
in listener https-bar.
reason: OverlappingHostnames
Copy link
Copy Markdown
Member Author

@zhaohuabing zhaohuabing May 8, 2026

Choose a reason for hiding this comment

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

This test file shouldn't have overlapping hostnames - we can clean it up later in a seperate PR.

tls:
mode: Terminate
certificateRefs:
- name: tls-secret-example-com
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Remove this listener to focus on overlapping hostnames only in this test yaml file.

@zhaohuabing
Copy link
Copy Markdown
Member Author

@codex review

@arkodg arkodg requested review from a team May 8, 2026 02:28
@zhaohuabing zhaohuabing added this to the v1.8.0 Release milestone May 8, 2026
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@arkodg arkodg merged commit 7afc3c6 into envoyproxy:main May 8, 2026
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.

enhancement: only downgrade ALPN for overlapping HTTPS listeners when certificate SANs overlap

3 participants