GEP 91: Update API #3881

arkodg · 2025-06-25T03:55:51Z

What type of PR is this?

Does this PR introduce a user-facing change?:

NONE

Relates to kubernetes-sigs#3760 (comment) Signed-off-by: Arko Dasgupta <[email protected]>

k8s-ci-robot · 2025-06-25T03:55:57Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: arkodg
Once this PR has been reviewed and has the lgtm label, please assign thockin for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

kl52752 · 2025-06-25T12:01:44Z

apis/v1/gateway_types.go

@@ -570,6 +577,8 @@ type GatewayTLSConfig struct {
 	// that requests a user to specify the client certificate.
 	// The maximum depth of a certificate chain accepted in verification is Implementation specific.
 	//
+	// Setting any field will override the equivalent field setting that was applied at the Gateway level.


I have a couple of comments:

Regarding "Setting any fields": The current phrasing implies merging the Frontend Validation struct from both gateway and listener levels. Given that this struct contains fields whose values depend on each other, I propose allowing overrides at the struct level. Therefore, I would change the line to "Setting this field."

Regarding overriding Frontend validation config on the listener level: This does not resolve the HTTP Coalescing issue. To address this, we should restrict listener overrides. Two potential approaches are:
a) Restrict the use of the same TLS config for listeners sharing the same port.
b) Allow overrides only for listeners without a hostname set. We can extend this solution with a mechanism to inherit the config for all listeners with the same port from the parent (which is a listener without hostname set). This would provide clear signals to customers about the coalescing issue.

Let me know what you think about this proposal.

for 1. there's a case where you may want to configure the CACert at the Gateway level, but update the mode at the Listener level, which is why i phrased it this way.

https://gateway-api.sigs.k8s.io/geps/gep-3567/ takes a more lenient approach and proposes adding a OverlappingTLSConfig condition, imo we should do the same here, either piggyback off that condition or highlight via a new condition

@youngnick ccing you here for 1. remember you bringing up this use case for per field overrides during the meeting in kubecon

@robscott ccing you here for 2. and to get some guidance on whether we want to use the same status condition or add an extra one

rethinking for this case, this should affect the overlapping listeners and set Listener Accepted condition to False, with a ClientCertificateVerificationBypassed reason or something similar, wdyt ?

kl52752 · 2025-06-25T12:51:08Z

apis/v1/gateway_types.go

@@ -636,10 +662,47 @@ type FrontendTLSValidation struct {
 	// "RefNotPermitted" reason.
 	//
 	// +kubebuilder:validation:MaxItems=8
-	// +kubebuilder:validation:MinItems=1
+	// +kubebuilder:validation:MinItems=0


Can you elaborate why this change is needed? What is the use case not to pass any certificates?
From our customers we can see that there is a need to pass one or more certificates and to differentiate between trust anchors and allowlisted certificates. Since Allowlisted certificates are not technically CA certificates we can either rename existing field or introduce a new field for allowlisted certificates then sending an empty list of CAcertificates makes sense to me.
Also we should update the “Support: Core” line to reflect those changes.

this is for the case when the CA cert is applied at the gateway level, and a mode is applied at the listener level

relates to #3881 (comment)

kl52752 · 2025-06-25T13:00:46Z

geps/gep-91/index.md

@@ -27,7 +27,9 @@ This use case has been highlighted in the [TLS Configuration GEP][] under segmen
 * This new field is separate from the existing [BackendTLSPolicy][] configuration. [BackendTLSPolicy][] controls TLS certificate validation for connections *from* the
  Gateway to the backend service.  This proposal adds the ability to validate the TLS certificate presented by the *client* connecting to the Gateway (the
  frontend). These two validation mechanisms operate independently and can be used simultaneously.
-* Also introduce a `ObjectReference` structure that can be used to specify `caCertificateRefs` references.
+* Introduce a `ObjectReference` structure that can be used to specify `caCertificateRefs` references.
+* Add `frontendValidation` to 


this is not finished.

apis/v1/gateway_types.go

kl52752 · 2025-06-25T13:06:37Z

apis/v1/gateway_types.go

+// GatewayTLSConfig describes a TLS configuration that can be applied to a Gateway.
+type GatewayTLSConfig struct {
+	// FrontendValidation holds configuration information for validating the frontend (client).
+	// Setting this field will require clients to send a client certificate


This line is inconsistent with struct changes. Please see my comment about certificates arrey.

apis/v1/gateway_types.go

kl52752 · 2025-06-25T13:07:40Z

geps/gep-91/index.md

-* Also introduce a `ObjectReference` structure that can be used to specify `caCertificateRefs` references.
+* Introduce a `ObjectReference` structure that can be used to specify `caCertificateRefs` references.
+* Add `frontendValidation` to 
+* Introduce a `tls` field within the Gateway Spec to allow for a common TLS configuration to apply across all listeners.


Please include information that “the newly introduced struct currently contains only client certificate validation configuration, but may be extended in the future."

howardjohn · 2025-06-25T15:02:12Z

apis/v1/gateway_types.go

+	// that requests a user to specify the client certificate.
+	// The maximum depth of a certificate chain accepted in verification is Implementation specific.
+	//
+	// Each field may be overidden by an equivalent setting applied at the Listener level.


I don't get why we feel the need to have a Gateway level setting here.

Why not allow every setting in Listener at the top level? FrontendValidation doesn't seem unique at all. If anything, it seems less likely to be common across listeners.

Ah I see its for Add a top level field for frontendValidation to avoid the auth bypass that may occur with HTTP/2 connection coalescing highlighted in https://gateway-api.sigs.k8s.io/geps/gep-3567/#interaction-with-client-cert-validation'. Given you can override it at the listener level, we didn't actually solve this problem...

IMO this is better solved with status or external validation. We can ship a sample Validation Admission Policy to deny inconsistent config between listeners, for example

Yeah, Moving the frontend validation configuration to gateway was intended to resolve the HTTP coalescing issue. However, this may not be necessary if we can provide users with clear guidelines for configuring TLS settings for listeners.
What do you think about allowing overrides only for listeners without a hostname set? This would enable all listeners on the same port (regardless of hostname) to inherit the configuration, thus resolving the coalescing issue while keeping TLS settings at the listener level.

apis/v1/gateway_types.go

Signed-off-by: Arko Dasgupta <[email protected]>

k8s-ci-robot · 2025-07-09T01:48:56Z

@arkodg: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gateway-api-crds-validation-3	`64b4751`	link	true	`/test pull-gateway-api-crds-validation-3`
pull-gateway-api-test	`64b4751`	link	true	`/test pull-gateway-api-test`
pull-gateway-api-crds-validation-2	`64b4751`	link	true	`/test pull-gateway-api-crds-validation-2`
pull-gateway-api-crds-validation-5	`64b4751`	link	true	`/test pull-gateway-api-crds-validation-5`
pull-gateway-api-crds-validation-1	`64b4751`	link	true	`/test pull-gateway-api-crds-validation-1`
pull-gateway-api-crds-validation-4	`64b4751`	link	true	`/test pull-gateway-api-crds-validation-4`
pull-gateway-api-verify	`64b4751`	link	true	`/test pull-gateway-api-verify`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

kl52752 · 2025-07-09T13:44:23Z

apis/v1/gateway_types.go

+	//   presented during the handshake and validated
+	//   using CA certificates defined in CACertificateRefs.
+	//
+	// Defaults to RequireAndVerify.


In previous comments we discussed 3 options for cert validation:

AllowInvalid

AllowMissing

RequireValid
In the current proposal the validation settings are too granular, looking at the fact that they are in Core support.
IMO we could merge Missing and Invalid into 1 option because in both cases request validation is moved to backend.

this eliminates VerifyIfGiven

GEP 91: Update API

05ca32d

Relates to kubernetes-sigs#3760 (comment) Signed-off-by: Arko Dasgupta <[email protected]>

k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Jun 25, 2025

k8s-ci-robot requested review from candita and youngnick June 25, 2025 03:55

arkodg marked this pull request as draft June 25, 2025 03:56

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 25, 2025

kl52752 reviewed Jun 25, 2025

View reviewed changes

howardjohn reviewed Jun 25, 2025

View reviewed changes

address comments

2288894

Signed-off-by: Arko Dasgupta <[email protected]>

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 3, 2025

arkodg marked this pull request as ready for review July 3, 2025 18:56

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2025

k8s-ci-robot requested review from mlavacca and robscott July 3, 2025 18:56

arkodg requested a review from kl52752 July 3, 2025 18:56

add examples for connection coalescing

64b4751

Signed-off-by: Arko Dasgupta <[email protected]>

kl52752 reviewed Jul 9, 2025

View reviewed changes

GEP 91: Update API #3881

Are you sure you want to change the base?

GEP 91: Update API #3881

Conversation

arkodg commented Jun 25, 2025

Uh oh!

k8s-ci-robot commented Jun 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arkodg Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

k8s-ci-robot commented Jul 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arkodg Jul 3, 2025 •

edited

Loading