Skip to content

Commit eb3e7c2

Browse files
shraddhabangshraddhabang
authored
Fix the auto cert discovery logic for secure listeners on gateways (#4214)
1 parent 4c5857e commit eb3e7c2

File tree

2 files changed

+132
-22
lines changed

2 files changed

+132
-22
lines changed

‎pkg/gateway/model/model_build_listener.go‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func (l listenerBuilderImpl) buildListenerSpec(ctx context.Context, stack core.S
103103
if sslPolicyErr != nil {
104104
return &elbv2model.ListenerSpec{}, sslPolicyErr
105105
}
106-
certificates, certsErr := l.buildCertificates(ctx, gwLsCfg, lbLsCfg)
106+
certificates, certsErr := l.buildCertificates(ctx, gw, port, gwLsCfg, lbLsCfg)
107107
if certsErr != nil {
108108
return &elbv2model.ListenerSpec{}, certsErr
109109
}
@@ -253,21 +253,23 @@ func buildListenerAttributes(lsCfg *elbv2gw.ListenerConfiguration) ([]elbv2model
253253
return attributes, nil
254254
}
255255

256-
func (l listenerBuilderImpl) buildCertificates(ctx context.Context, gwLsCfg *gwListenerConfig, lbLsCfg *elbv2gw.ListenerConfiguration) ([]elbv2model.Certificate, error) {
257-
// TODO for cert discovery and secure listeners during L7 and L4 gateways implementations
258-
256+
func (l listenerBuilderImpl) buildCertificates(ctx context.Context, gw *gwv1.Gateway, port int32, gwLsCfg *gwListenerConfig, lbLsCfg *elbv2gw.ListenerConfiguration) ([]elbv2model.Certificate, error) {
257+
if !isHTTPSOrTLSProtocol(gwLsCfg.protocol) {
258+
return []elbv2model.Certificate{}, nil
259+
}
259260
certs := make([]elbv2model.Certificate, 0)
260-
261261
// Build explict certs
262262
if lbLsCfg != nil {
263263
certs = append(certs, l.buildExplicitTLSCertARNs(ctx, *lbLsCfg)...)
264264
}
265-
266-
// Build inferred certs
267-
if len(certs) == 0 && lbLsCfg != nil {
268-
discoveredCerts, err := l.buildInferredTLSCertARNs(ctx, lbLsCfg.ProtocolPort, gwLsCfg.hostnames)
265+
// If any explicit certs are not found then build inferred certs using cert discovery
266+
if len(certs) == 0 {
267+
if len(gwLsCfg.hostnames) == 0 {
268+
return []elbv2model.Certificate{}, errors.Errorf("No hostnames found for TLS cert discovery for listener on gateway %s with protocol:port %s:%v", k8s.NamespacedName(gw), gwLsCfg.protocol, port)
269+
}
270+
discoveredCerts, err := l.buildInferredTLSCertARNs(ctx, gwLsCfg.hostnames)
269271
if err != nil {
270-
l.logger.Error(err, "Unable to discover certs for listener")
272+
l.logger.Error(err, "Unable to discover certs for listener on gateway %s with protocol:port %s:%v\", k8s.NamespacedName(gw), gwLsCfg.protocol, port")
271273
return []elbv2model.Certificate{}, err
272274
}
273275
for _, cert := range discoveredCerts {
@@ -298,12 +300,7 @@ func (l listenerBuilderImpl) buildExplicitTLSCertARNs(ctx context.Context, liste
298300
return certs
299301
}
300302

301-
func (l listenerBuilderImpl) buildInferredTLSCertARNs(ctx context.Context, protocolPort elbv2gw.ProtocolPort, hostnames []string) ([]string, error) {
302-
if len(hostnames) == 0 {
303-
l.logger.Info("No hostnames found for TLS cert discovery", "protocolPort", protocolPort)
304-
return nil, nil
305-
}
306-
303+
func (l listenerBuilderImpl) buildInferredTLSCertARNs(ctx context.Context, hostnames []string) ([]string, error) {
307304
hosts := sets.NewString()
308305
for _, hostname := range hostnames {
309306
hosts.Insert(hostname)

‎pkg/gateway/model/model_build_listener_test.go‎

Lines changed: 119 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,8 @@ func Test_buildListenerALPNPolicy(t *testing.T) {
357357
func TestBuildCertificates(t *testing.T) {
358358
tests := []struct {
359359
name string
360+
gateway *gwv1.Gateway
361+
port int32
360362
gwLsCfg *gwListenerConfig
361363
lbLsCfg *elbv2gw.ListenerConfiguration
362364
setupMocks func(mockCertDiscovery *certs.MockCertDiscovery)
@@ -365,7 +367,20 @@ func TestBuildCertificates(t *testing.T) {
365367
}{
366368
{
367369
name: "default certificate only - explicit config",
370+
gateway: &gwv1.Gateway{
371+
Spec: gwv1.GatewaySpec{
372+
Listeners: []gwv1.Listener{
373+
{
374+
Name: "https",
375+
Port: 443,
376+
Protocol: gwv1.HTTPSProtocolType,
377+
},
378+
},
379+
},
380+
},
381+
port: 443,
368382
gwLsCfg: &gwListenerConfig{
383+
protocol: elbv2model.ProtocolHTTPS,
369384
hostnames: []string{"my-host-1", "my-host-2"},
370385
},
371386
lbLsCfg: &elbv2gw.ListenerConfiguration{
@@ -379,7 +394,20 @@ func TestBuildCertificates(t *testing.T) {
379394
},
380395
{
381396
name: "multiple certificates without default - explicit config",
397+
gateway: &gwv1.Gateway{
398+
Spec: gwv1.GatewaySpec{
399+
Listeners: []gwv1.Listener{
400+
{
401+
Name: "tls",
402+
Port: 443,
403+
Protocol: gwv1.TLSProtocolType,
404+
},
405+
},
406+
},
407+
},
408+
port: 443,
382409
gwLsCfg: &gwListenerConfig{
410+
protocol: elbv2model.ProtocolTLS,
383411
hostnames: []string{"my-host-1", "my-host-2"},
384412
},
385413
lbLsCfg: &elbv2gw.ListenerConfiguration{
@@ -399,7 +427,20 @@ func TestBuildCertificates(t *testing.T) {
399427
},
400428
{
401429
name: "multiple certificates with default - explicit config",
430+
gateway: &gwv1.Gateway{
431+
Spec: gwv1.GatewaySpec{
432+
Listeners: []gwv1.Listener{
433+
{
434+
Name: "https",
435+
Port: 443,
436+
Protocol: gwv1.HTTPSProtocolType,
437+
},
438+
},
439+
},
440+
},
441+
port: 443,
402442
gwLsCfg: &gwListenerConfig{
443+
protocol: elbv2model.ProtocolHTTPS,
403444
hostnames: []string{"my-host-1", "my-host-2"},
404445
},
405446
lbLsCfg: &elbv2gw.ListenerConfiguration{
@@ -423,7 +464,20 @@ func TestBuildCertificates(t *testing.T) {
423464
},
424465
{
425466
name: "auto-discover certificates for one hosts",
467+
gateway: &gwv1.Gateway{
468+
Spec: gwv1.GatewaySpec{
469+
Listeners: []gwv1.Listener{
470+
{
471+
Name: "tls",
472+
Port: 443,
473+
Protocol: gwv1.TLSProtocolType,
474+
},
475+
},
476+
},
477+
},
478+
port: 443,
426479
gwLsCfg: &gwListenerConfig{
480+
protocol: elbv2model.ProtocolTLS,
427481
hostnames: []string{"example.com"},
428482
},
429483
lbLsCfg: &elbv2gw.ListenerConfiguration{
@@ -444,12 +498,23 @@ func TestBuildCertificates(t *testing.T) {
444498
},
445499
{
446500
name: "auto-discover certificates for hosts",
501+
gateway: &gwv1.Gateway{
502+
Spec: gwv1.GatewaySpec{
503+
Listeners: []gwv1.Listener{
504+
{
505+
Name: "tls",
506+
Port: 443,
507+
Protocol: gwv1.TLSProtocolType,
508+
},
509+
},
510+
},
511+
},
512+
port: 443,
447513
gwLsCfg: &gwListenerConfig{
514+
protocol: elbv2model.ProtocolTLS,
448515
hostnames: []string{"example.com", "*.example.org"},
449516
},
450-
lbLsCfg: &elbv2gw.ListenerConfiguration{
451-
ProtocolPort: "TLS:443",
452-
},
517+
lbLsCfg: nil,
453518
setupMocks: func(mockCertDiscovery *certs.MockCertDiscovery) {
454519
// The hostnames will be sorted alphabetically by sets.NewString().List()
455520
mockCertDiscovery.EXPECT().
@@ -470,7 +535,20 @@ func TestBuildCertificates(t *testing.T) {
470535
},
471536
{
472537
name: "certificate discovery fails",
538+
gateway: &gwv1.Gateway{
539+
Spec: gwv1.GatewaySpec{
540+
Listeners: []gwv1.Listener{
541+
{
542+
Name: "https",
543+
Port: 443,
544+
Protocol: gwv1.HTTPSProtocolType,
545+
},
546+
},
547+
},
548+
},
549+
port: 443,
473550
gwLsCfg: &gwListenerConfig{
551+
protocol: elbv2model.ProtocolHTTPS,
474552
hostnames: []string{"example.com"},
475553
},
476554
lbLsCfg: &elbv2gw.ListenerConfiguration{
@@ -485,15 +563,50 @@ func TestBuildCertificates(t *testing.T) {
485563
wantErr: true,
486564
},
487565
{
488-
name: "no hostname for discovery",
566+
name: "no hostname for discovery : no secure listeners",
567+
gateway: &gwv1.Gateway{
568+
Spec: gwv1.GatewaySpec{
569+
Listeners: []gwv1.Listener{
570+
{
571+
Name: "http",
572+
Port: 80,
573+
Protocol: gwv1.HTTPProtocolType,
574+
},
575+
},
576+
},
577+
},
578+
port: 443,
489579
gwLsCfg: &gwListenerConfig{
580+
protocol: elbv2model.ProtocolHTTP,
490581
hostnames: []string{},
491582
},
492583
lbLsCfg: &elbv2gw.ListenerConfiguration{
493-
ProtocolPort: "HTTPS:443",
584+
ProtocolPort: "HTTP:80",
494585
},
495586
want: []elbv2model.Certificate{},
496587
},
588+
{
589+
name: "no hostname for discovery : secure listeners",
590+
gateway: &gwv1.Gateway{
591+
Spec: gwv1.GatewaySpec{
592+
Listeners: []gwv1.Listener{
593+
{
594+
Name: "https",
595+
Port: 443,
596+
Protocol: gwv1.HTTPSProtocolType,
597+
},
598+
},
599+
},
600+
},
601+
port: 443,
602+
gwLsCfg: &gwListenerConfig{
603+
protocol: elbv2model.ProtocolHTTPS,
604+
hostnames: []string{},
605+
},
606+
lbLsCfg: nil,
607+
want: []elbv2model.Certificate{},
608+
wantErr: true,
609+
},
497610
}
498611

499612
for _, tt := range tests {
@@ -510,7 +623,7 @@ func TestBuildCertificates(t *testing.T) {
510623
certDiscovery: mockCertDiscovery,
511624
}
512625

513-
got, err := builder.buildCertificates(context.Background(), tt.gwLsCfg, tt.lbLsCfg)
626+
got, err := builder.buildCertificates(context.Background(), tt.gateway, tt.port, tt.gwLsCfg, tt.lbLsCfg)
514627
if (err != nil) != tt.wantErr {
515628
t.Errorf("buildCertificates() error = %v, wantErr %v", err, tt.wantErr)
516629
return

0 commit comments

Comments
 (0)