Support for Gateway FrontendTLS#5127
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5127 +/- ##
==========================================
+ Coverage 86.43% 86.65% +0.21%
==========================================
Files 144 144
Lines 17801 18068 +267
Branches 35 35
==========================================
+ Hits 15386 15656 +270
+ Misses 2142 2141 -1
+ Partials 273 271 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds support for Gateway API FrontendTLS so operators can configure client certificate validation (mTLS-style) at the Gateway level, feeding the resulting CA bundles into the generated NGINX configuration.
Changes:
- Add graph-layer resolution/validation of
spec.tls.frontendCA certificate references (Secret/ConfigMap), including ReferenceGrant handling and listener/gateway conditions. - Extend dataplane representation + configuration build to generate frontend CA cert bundles and configure NGINX
ssl_client_certificate/ssl_verify_client. - Add/adjust unit tests across resolver, graph, conditions, dataplane, and NGINX config rendering.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/controller/status/prepare_requests_test.go | Updates expected Listener condition ordering/messages. |
| internal/controller/state/resolver/secrets_test.go | Adds a TLS Secret missing ca.crt test case for FrontendTLS expectations. |
| internal/controller/state/resolver/secrets.go | Enforces ca.crt presence for TLS Secrets when explicitly expected. |
| internal/controller/state/resolver/resolver.go | Tweaks not-found error message behavior for ConfigMaps. |
| internal/controller/state/resolver/configmaps_test.go | Updates resolver tests to assert specific error messages. |
| internal/controller/state/graph/reference_grant_test.go | Adds unit test for new ConfigMap ReferenceGrant helper. |
| internal/controller/state/graph/reference_grant.go | Adds toConfigMap helper for ReferenceGrant checks. |
| internal/controller/state/graph/gateway_test.go | Removes “TLS.Frontend unsupported” expectations; updates unsupported-field tests accordingly. |
| internal/controller/state/graph/gateway_listener_test.go | Adds extensive tests for FrontendTLS CA ref resolution/conditions. |
| internal/controller/state/graph/gateway_listener.go | Implements FrontendTLS CA ref resolution and listener fields for validation mode/CA refs. |
| internal/controller/state/graph/gateway.go | Wires listener building through *Gateway and removes FrontendTLS as “unsupported”. |
| internal/controller/state/dataplane/types.go | Adds frontend-TLS-related fields to SSL dataplane model. |
| internal/controller/state/dataplane/configuration_test.go | Adds tests for CA bundle extraction/building and SSL server client-auth configuration. |
| internal/controller/state/dataplane/configuration.go | Builds frontend CA bundles and mutates SSL servers to enable frontend client cert verification. |
| internal/controller/state/conditions/conditions_test.go | Updates default listener-condition expectations; adds tests for new CA-related listener conditions. |
| internal/controller/state/conditions/conditions.go | Adjusts default listener conditions and adds FrontendTLS-related condition constructors. |
| internal/controller/nginx/config/servers_test.go | Adds tests for propagating frontend TLS fields into rendered HTTP SSL config. |
| internal/controller/nginx/config/servers_template.go | Renders ssl_client_certificate, ssl_verify_client, and verification-failure handling. |
| internal/controller/nginx/config/servers.go | Maps new dataplane SSL fields into the HTTP template SSL struct. |
| internal/controller/nginx/config/http/config.go | Extends HTTP SSL template model with client-cert verification fields. |
Comments suppressed due to low confidence (1)
internal/controller/state/conditions/conditions.go:585
- Comment mismatch:
hasResolvedRefsConditionsonly checks forResolvedRefs=False, but the comment says “if there are no existing ResolvedRefs conditions”. Update the comment to reflect the actual behavior (suppresses default only when a failure ResolvedRefs condition already exists).
// Only add ResolvedRefs=true if there are no existing ResolvedRefs conditions
if !hasResolvedRefsConditions(existingConditions) {
defaultConds = append(defaultConds, NewListenerResolvedRefs())
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
internal/controller/state/conditions/conditions.go:596
- The comment for hasResolvedRefsConditions says it checks for ResolvedRefs=False, but the implementation returns true for any ResolvedRefs condition regardless of Status. Either update the comment to match the actual behavior (any ResolvedRefs condition) or update the code to only treat Status=False as suppressing the default ResolvedRefs=True condition.
// hasResolvedRefsConditions checks if the Listener has any ResolvedRefs=False conditions.
func hasResolvedRefsConditions(conditions []Condition) bool {
for _, cond := range conditions {
if cond.Type == string(v1.ListenerConditionResolvedRefs) {
return true
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
internal/controller/state/conditions/conditions.go:596
hasResolvedRefsConditionsis documented as checking for “ResolvedRefs=False conditions”, but the implementation returns true for anyResolvedRefscondition regardless of status. Either adjust the logic to match the comment (checkStatus == False) or update the comment to reflect the current behavior.
// hasResolvedRefsConditions checks if the Listener has any ResolvedRefs=False conditions.
func hasResolvedRefsConditions(conditions []Condition) bool {
for _, cond := range conditions {
if cond.Type == string(v1.ListenerConditionResolvedRefs) {
return true
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
internal/controller/state/conditions/conditions.go:595
- The comment for
hasResolvedRefsConditionssays it only checks forResolvedRefs=False, but the implementation returns true for anyResolvedRefscondition regardless of status. Either update the comment to match the actual behavior, or update the function to explicitly checkcond.Status == metav1.ConditionFalseif that’s what callers rely on.
// hasResolvedRefsConditions checks if the Listener has any ResolvedRefs=False conditions.
func hasResolvedRefsConditions(conditions []Condition) bool {
for _, cond := range conditions {
if cond.Type == string(v1.ListenerConditionResolvedRefs) {
return true
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
internal/controller/state/conditions/conditions.go:598
- The comment for hasResolvedRefsConditions says it checks for any ResolvedRefs=False conditions, but the implementation returns true for any ResolvedRefs condition regardless of Status. Either update the logic to match the comment (only treat ResolvedRefs=False as suppressing the default) or adjust the comment to reflect the actual behavior.
// hasResolvedRefsConditions checks if the Listener has any ResolvedRefs=False conditions.
func hasResolvedRefsConditions(conditions []Condition) bool {
for _, cond := range conditions {
if cond.Type == string(v1.ListenerConditionResolvedRefs) {
return true
}
}
return false
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proposed changes
Problem: System administrators want to ensure client requests to the gateway are secure and valid.
Solution: Provide a means to configure client certification validation (TLS) at the Gateway level
Testing:
Closes #3090
Checklist
Before creating a PR, run through this checklist and mark each as complete.
Release notes
If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.