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 <arko@tetrate.io>

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.

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.

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.

kl52752 · 2025-06-25T13:03:15Z

apis/v1/gateway_types.go

@@ -289,6 +289,13 @@ type GatewaySpec struct {
 	//
 	// +optional
 	AllowedListeners *AllowedListeners `json:"allowedListeners,omitempty"`
+
+	// TLS is the TLS configuration for the Gateway.


This configuration currently supports only client certificate validation. Future releases may include extended functionalities.

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.

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

apis/v1/gateway_types.go

+	// Support: Core
+	//
+	// +optional
+	ClientCertificateOptional *bool `json:"clientCertificateOptional,omitempty"`


Can you explai why you need a separate fiel for that? ? What distinguishes a missing field from an invalid one? Can you combine them together into “mode” field with values:

AcceptValidTrustChain (or RejectInvalid)

AcceptMissingOrUntrusted

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.

howardjohn · 2025-06-25T15:04:07Z

apis/v1/gateway_types.go

+	//
+	// +optional
+	// +kubebuilder:default=ValidateTrustChain
+	Mode *FrontendValidationModeType `json:"mode,omitempty"`


IMO this Mode + ClientCertificateOptional config is confusing or at minimum missing validation to deny bad states.

The Go TLS API is the golden standard here for simplicity:

// ClientAuthType declares the policy the server will follow for // TLS Client Authentication. type ClientAuthType int const ( // NoClientCert indicates that no client certificate should be requested // during the handshake, and if any certificates are sent they will not // be verified. NoClientCert ClientAuthType = iota // RequestClientCert indicates that a client certificate should be requested // during the handshake, but does not require that the client send any // certificates. RequestClientCert // RequireAnyClientCert indicates that a client certificate should be requested // during the handshake, and that at least one certificate is required to be // sent by the client, but that certificate is not required to be valid. RequireAnyClientCert // VerifyClientCertIfGiven indicates that a client certificate should be requested // during the handshake, but does not require that the client sends a // certificate. If the client does send a certificate it is required to be // valid. VerifyClientCertIfGiven // RequireAndVerifyClientCert indicates that a client certificate should be requested // during the handshake, and that at least one valid certificate is required // to be sent by the client. RequireAndVerifyClientCert )

GEP 91: Update API

05ca32d

Relates to kubernetes-sigs#3760 (comment) Signed-off-by: Arko Dasgupta <arko@tetrate.io>

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

GEP 91: Update API #3881

GEP 91: Update API #3881

arkodg commented Jun 25, 2025

Uh oh!

k8s-ci-robot commented Jun 25, 2025

Uh oh!

kl52752 Jun 25, 2025

Uh oh!

kl52752 Jun 25, 2025

Uh oh!

kl52752 Jun 25, 2025

Uh oh!

kl52752 Jun 25, 2025

Uh oh!

kl52752 Jun 25, 2025

Uh oh!

kl52752 Jun 25, 2025

Uh oh!

kl52752 Jun 25, 2025

Uh oh!

howardjohn Jun 25, 2025

Uh oh!

howardjohn Jun 25, 2025

Uh oh!

kl52752 Jun 26, 2025

Uh oh!

howardjohn Jun 25, 2025

Uh oh!

Uh oh!

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!

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!

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!