Skip to content

Conversation

kl52752
Copy link
Contributor

@kl52752 kl52752 commented Jul 28, 2025

What type of PR is this?
/kind gep

What this PR does / why we need it:
Implements API changes for GEP-91

Which issue(s) this PR fixes:

Fixes 3567
Relates to: #3760 (comment)

Does this PR introduce a user-facing change?:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/gep PRs related to Gateway Enhancement Proposal(GEP) do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jul 28, 2025
@k8s-ci-robot k8s-ci-robot requested review from candita and robscott July 28, 2025 10:03
@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 28, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @kl52752. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 28, 2025
@kl52752
Copy link
Contributor Author

kl52752 commented Jul 28, 2025

/assign @robscott @arkodg @youngnick @shaneutt

Copy link
Contributor

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

Small clarification about pointer-to-string, but aside from that, LGTM.

@kl52752 kl52752 force-pushed the gep-91-api branch 2 times, most recently from 4c15adf to 1cde002 Compare July 30, 2025 07:26
@youngnick
Copy link
Contributor

/ok-to-test
/approve

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 6, 2025
@kl52752 kl52752 force-pushed the gep-91-api branch 2 times, most recently from 52f2fd5 to 6a5f47f Compare August 6, 2025 06:41
@k8s-ci-robot k8s-ci-robot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 6, 2025
@PT-GD
Copy link

PT-GD commented Aug 11, 2025

@kl52752 I am looking for a solution where the same certificate is on two distinct listeners on a gateway. This is necessary because the Gateway API allows a single listener per gateway and the hostname for a listener can be either an FQDN or a wildcard FQDN. The behavior of wildcard certificates in TLS is that they match the apex (FQDN) or one label deep of wildcarded names.

Will this GEP address coalescing connections to prevent HTTP/2 connection reuse issues when mutual TLS (client certificates) are not involved and two listeners attached to a gateway share the same server certificate? [Perhaps there should be a test for this, if there isn't one]?

See also https://istio.io/latest/docs/ops/common-problems/network-issues/#404-errors-occur-when-multiple-gateways-configured-with-same-tls-certificate for how this impact users of the Istio controller for the Gateway API.

@shaneutt shaneutt self-requested a review August 12, 2025 11:34
@shaneutt shaneutt dismissed their stale review August 12, 2025 11:35

I think we have consensus now on the approach, so removing my block.

@kl52752
Copy link
Contributor Author

kl52752 commented Aug 12, 2025

@kl52752 I am looking for a solution where the same certificate is on two distinct listeners on a gateway. This is necessary because the Gateway API allows a single listener per gateway and the hostname for a listener can be either an FQDN or a wildcard FQDN. The behavior of wildcard certificates in TLS is that they match the apex (FQDN) or one label deep of wildcarded names.

Will this GEP address coalescing connections to prevent HTTP/2 connection reuse issues when mutual TLS (client certificates) are not involved and two listeners attached to a gateway share the same server certificate? [Perhaps there should be a test for this, if there isn't one]?

See also https://istio.io/latest/docs/ops/common-problems/network-issues/#404-errors-occur-when-multiple-gateways-configured-with-same-tls-certificate for how this impact users of the Istio controller for the Gateway API.

hi @PT-GD in this GEP it is possible to attach the same FrontendValidation config (referencing the same ConfigMap) to 2 distinct Listeners in tlsPerPort field. The restriction here is that every port needs unique configuration but not the other way.
If they are listening on different port, you can do this by creating TLSConfig struct with the same FrontendTLSValidation but different port. If they are Listening on the same port only one FrontendTLSValidation is needed:
see the snippet:

apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
  name: client-validation-basic
spec:
  gatewayClassName: acme-lb
  tls:
    defaultTLS: 
      caCertificateRefs:
        - kind: ConfigMap
          group: ""
          name: default-cert
        mode: AllowInsecureFallback
    tlsPerPort:
        - port: 443 // this configuration will apply to all Listeners matching this port
          frontendValidation:
            caCertificateRefs:
            - kind: ConfigMap
            group: ""
            name: foo-example-com-ca-cert
        - port: 8443 // If needed you can create second entry referencing the same ConfigMap for another port
          frontendValidation:
            caCertificateRefs:
            - kind: ConfigMap
            group: ""
            name: foo-example-com-ca-cert
            

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 12, 2025
Copy link
Member

@robscott robscott left a comment

Choose a reason for hiding this comment

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

Thanks @kl52752!

@kl52752 kl52752 force-pushed the gep-91-api branch 3 times, most recently from d3abae8 to e003d67 Compare August 14, 2025 17:08
@kl52752
Copy link
Contributor Author

kl52752 commented Aug 14, 2025

/retest

Copy link
Member

@robscott robscott left a comment

Choose a reason for hiding this comment

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

Thanks @kl52752, a few more tiny nits but otherwise LGTM.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kl52752, robscott, youngnick

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kl52752 kl52752 force-pushed the gep-91-api branch 3 times, most recently from b09cb6d to 7b09bed Compare August 18, 2025 07:58
@kl52752
Copy link
Contributor Author

kl52752 commented Aug 18, 2025

/retest

Copy link
Member

@shaneutt shaneutt left a comment

Choose a reason for hiding this comment

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

/lgtm

/cc @youngnick

Nick do you want to give it the last pass before we unhold?

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 18, 2025
@youngnick
Copy link
Contributor

LGTM, thanks @kl52752

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 20, 2025
@k8s-ci-robot k8s-ci-robot merged commit 43b0cf7 into kubernetes-sigs:main Aug 20, 2025
19 checks passed
@github-project-automation github-project-automation bot moved this from Review to Done in Release v1.4.0 Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/gep PRs related to Gateway Enhancement Proposal(GEP) lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

GEP-3567: Gateway TLS Updates for HTTP Connection Coalescing
7 participants