Skip to content

Commit e726354

Browse files
ciarams87ciarams87
committed
Add more validation; handle conflicts
1 parent a2ae465 commit e726354

File tree

12 files changed

+382
-36
lines changed

12 files changed

+382
-36
lines changed

‎internal/mode/static/manager.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,8 @@ func registerControllers(
390390
objectType: &gatewayv1alpha2.BackendTLSPolicy{},
391391
},
392392
{
393-
objectType: &apiv1.ConfigMap{}, // TODO(ciarams87): Use only metadata predicate
393+
// FIXME(ciarams87): If possible, use only metadata predicate
394+
objectType: &apiv1.ConfigMap{},
394395
},
395396
}
396397
controllerRegCfgs = append(controllerRegCfgs, backendTLSObjs...)

‎internal/mode/static/nginx/config/servers.go

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package config
33
import (
44
"encoding/json"
55
"fmt"
6-
"os"
76
"strings"
87
gotemplate "text/template"
98

@@ -275,8 +274,8 @@ func createProxyTLSFromBackends(backends []dataplane.Backend) *http.ProxySSLVeri
275274
proxyVerify := createProxySSLVerify(b.VerifyTLS)
276275
if proxyVerify != nil {
277276
// If any backend has a backend TLS policy defined, then we use that for the proxy SSL verification.
278-
// If multiple backends in the group have a backend TLS policy defined, then we use the first one we find.
279-
// TODO(ciarams87): Fix this
277+
// We require that all backends in a group have the same backend TLS policy.
278+
// Verification that all backends in a group have the same backend TLS policy is done in the graph package.
280279
return proxyVerify
281280
}
282281
}
@@ -291,32 +290,14 @@ func createProxySSLVerify(v *dataplane.VerifyTLS) *http.ProxySSLVerify {
291290
if v.CertBundleID != "" {
292291
trustedCert = generateCertBundleFileName(v.CertBundleID)
293292
} else {
294-
trustedCert = getRootCAPath()
293+
trustedCert = v.RootCAPath
295294
}
296295
return &http.ProxySSLVerify{
297296
TrustedCertificate: trustedCert,
298297
Name: v.Hostname,
299298
}
300299
}
301300

302-
// TODO(ciarams87): Move this logic earlier
303-
func getRootCAPath() string {
304-
certFiles := []string{
305-
"/etc/ssl/cert.pem", // Alpine Linux
306-
"/etc/ssl/certs/ca-certificates.crt", // Debian/Ubuntu/Gentoo etc.
307-
"/etc/pki/tls/certs/ca-bundle.crt", // Fedora/RHEL 6
308-
"/etc/ssl/ca-bundle.pem", // OpenSUSE
309-
"/etc/pki/tls/cacert.pem", // OpenELEC
310-
"/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", // CentOS/RHEL 7
311-
}
312-
for _, certFile := range certFiles {
313-
if _, err := os.Stat(certFile); err == nil {
314-
return certFile
315-
}
316-
}
317-
return ""
318-
}
319-
320301
func createReturnValForRedirectFilter(filter *dataplane.HTTPRequestRedirectFilter, listenerPort int32) *http.Return {
321302
if filter == nil {
322303
return nil

‎internal/mode/static/nginx/config/servers_template.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ server {
5454
proxy_pass {{ $l.ProxyPass }};
5555
{{- if $l.ProxySSLVerify }}
5656
proxy_ssl_verify on;
57-
proxy_ssl_name {{ $l.ProxySSLVerify.Hostname }};
57+
proxy_ssl_name {{ $l.ProxySSLVerify.Name }};
5858
proxy_ssl_trusted_certificate {{ $l.ProxySSLVerify.TrustedCertificate }};
5959
{{- end }}
6060
{{- end }}

‎internal/mode/static/state/dataplane/configuration.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package dataplane
33
import (
44
"context"
55
"fmt"
6+
"os"
67
"sort"
78

89
apiv1 "k8s.io/api/core/v1"
@@ -177,11 +178,31 @@ func convertBackendTLS(btp *graph.BackendTLSPolicy) *VerifyTLS {
177178
verify := &VerifyTLS{}
178179
if btp.CaCertRef.Name != "" {
179180
verify.CertBundleID = generateCertBundleID(btp.CaCertRef)
181+
} else {
182+
verify.RootCAPath = getRootCAPath()
180183
}
181184
verify.Hostname = string(btp.Source.Spec.TLS.Hostname)
182185
return verify
183186
}
184187

188+
// getRootCAPath returns the path to the root CA certificate bundle.
189+
func getRootCAPath() string {
190+
certFiles := []string{
191+
"/etc/ssl/cert.pem", // Alpine Linux
192+
"/etc/ssl/certs/ca-certificates.crt", // Debian/Ubuntu/Gentoo etc.
193+
"/etc/pki/tls/certs/ca-bundle.crt", // Fedora/RHEL 6
194+
"/etc/ssl/ca-bundle.pem", // OpenSUSE
195+
"/etc/pki/tls/cacert.pem", // OpenELEC
196+
"/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem", // CentOS/RHEL 7
197+
}
198+
for _, certFile := range certFiles {
199+
if _, err := os.Stat(certFile); err == nil {
200+
return certFile
201+
}
202+
}
203+
return ""
204+
}
205+
185206
func buildServers(listeners []*graph.Listener) (http, ssl []VirtualServer) {
186207
rulesForProtocol := map[v1.ProtocolType]portPathRules{
187208
v1.HTTPProtocolType: make(portPathRules),

‎internal/mode/static/state/dataplane/configuration_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2477,7 +2477,8 @@ func TestConvertBackendTLS(t *testing.T) {
24772477
}
24782478

24792479
expectedWithWellKnownCerts := &VerifyTLS{
2480-
Hostname: "example.com",
2480+
Hostname: "example.com",
2481+
RootCAPath: getRootCAPath(),
24812482
}
24822483

24832484
tests := []struct {

‎internal/mode/static/state/dataplane/types.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,4 +247,5 @@ type Backend struct {
247247
type VerifyTLS struct {
248248
CertBundleID CertBundleID
249249
Hostname string
250+
RootCAPath string
250251
}

‎internal/mode/static/state/graph/backend_refs.go

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
1010

1111
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/conditions"
12+
"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
1213
staticConds "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/conditions"
1314
)
1415

@@ -92,6 +93,13 @@ func addBackendRefsToRules(
9293
}
9394
}
9495

96+
if len(backendRefs) > 1 {
97+
cond := validateBackendTLSPolicyMatchingAllBackends(backendRefs)
98+
if cond != nil {
99+
route.Conditions = append(route.Conditions, *cond)
100+
}
101+
}
102+
95103
route.Rules[idx].BackendRefs = backendRefs
96104
}
97105
}
@@ -170,6 +178,40 @@ func createBackendRef(
170178
return backendRef, nil
171179
}
172180

181+
// validateBackendTLSPolicyMatchingAllBackends validates that all backends in a rule reference the same
182+
// BackendTLSPolicy. We require that all backends in a group have the same backend TLS policy configuration.
183+
// FIXME (ciarams87): This is a temporary solution until we can support multiple backend TLS policies per group.
184+
func validateBackendTLSPolicyMatchingAllBackends(backendRefs []BackendRef) *conditions.Condition {
185+
var mismatch bool
186+
var referencePolicy *BackendTLSPolicy
187+
188+
for _, backendRef := range backendRefs {
189+
if backendRef.BackendTLSPolicy == nil {
190+
if referencePolicy != nil {
191+
// There was a reference before, so they do not all match
192+
mismatch = true
193+
}
194+
continue
195+
}
196+
197+
if referencePolicy == nil {
198+
// First reference, store the policy as reference
199+
referencePolicy = backendRef.BackendTLSPolicy
200+
} else {
201+
// Check if the policies match
202+
if backendRef.BackendTLSPolicy.Source.Name != referencePolicy.Source.Name ||
203+
backendRef.BackendTLSPolicy.Source.Namespace != referencePolicy.Source.Namespace {
204+
mismatch = true
205+
}
206+
}
207+
}
208+
if mismatch {
209+
msg := "Backend TLS policies do not match for all backends"
210+
return helpers.GetPointer(staticConds.NewRouteBackendRefUnsupportedValue(msg))
211+
}
212+
return nil
213+
}
214+
173215
func findBackendTLSPolicyForService(
174216
backendTLSPolicies map[types.NamespacedName]*BackendTLSPolicy,
175217
ref gatewayv1.HTTPBackendRef,
@@ -189,9 +231,19 @@ func findBackendTLSPolicyForService(
189231
btpNs = string(*btp.Source.Spec.TargetRef.Namespace)
190232
}
191233
if btp.Source.Spec.TargetRef.Name == ref.Name && btpNs == refNs {
192-
// TODO: resolve conflicts between multiple backend TLS policies
193-
beTLSPolicy = btp
194-
break
234+
if beTLSPolicy != nil {
235+
if btp.Source.CreationTimestamp.Equal(&beTLSPolicy.Source.CreationTimestamp) {
236+
// if the policies have the same creation timestamp, the one that comes first alphabetically wins
237+
if btp.Source.Name < beTLSPolicy.Source.Name {
238+
beTLSPolicy = btp
239+
}
240+
} else if btp.Source.CreationTimestamp.Before(&beTLSPolicy.Source.CreationTimestamp) {
241+
// the oldest policy wins - see https://gateway-api.sigs.k8s.io/geps/gep-713/#conflict-resolution
242+
beTLSPolicy = btp
243+
}
244+
} else {
245+
beTLSPolicy = btp
246+
}
195247
}
196248
}
197249

0 commit comments

Comments
 (0)