From d57931124f9c57cd0b575a4a9cf1eda6b332986c Mon Sep 17 00:00:00 2001
From: Paul Banks <pbanks@hashicorp.com>
Date: Thu, 23 Sep 2021 10:05:42 +0100
Subject: [PATCH] Final readability tweaks from review

---
 agent/structs/config_entry_gateways.go |  4 +-
 agent/xds/listeners_ingress.go         | 78 +++++++++++++-------------
 api/config_entry_gateways.go           |  4 +-
 3 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/agent/structs/config_entry_gateways.go b/agent/structs/config_entry_gateways.go
index 470149ce2..aebb0a0f3 100644
--- a/agent/structs/config_entry_gateways.go
+++ b/agent/structs/config_entry_gateways.go
@@ -212,19 +212,19 @@ func (e *IngressGatewayConfigEntry) validateServiceSDS(lis IngressListener, svc
 	}
 
 	// Validate service-level SDS config
-	sid := NewServiceID(svc.Name, &svc.EnterpriseMeta)
-
 	svcSDSSet := (svc.TLS != nil && svc.TLS.SDS != nil && svc.TLS.SDS.CertResource != "")
 
 	// Service SDS is only supported with Host names because we need to bind
 	// specific service certs to one or more SNI hostnames.
 	if svcSDSSet && len(svc.Hosts) < 1 {
+		sid := NewServiceID(svc.Name, &svc.EnterpriseMeta)
 		return fmt.Errorf("A service specifying TLS.SDS.CertResource must have at least one item in Hosts (service %q on listener on port %d)",
 			sid.String(), lis.Port)
 	}
 	// If this service specified a certificate, there must be an SDS cluster set
 	// at one of the three levels.
 	if svcSDSSet && svc.TLS.SDS.ClusterName == "" && !lisSDSClusterSet && !gwSDSClusterSet {
+		sid := NewServiceID(svc.Name, &svc.EnterpriseMeta)
 		return fmt.Errorf("TLS.SDS.ClusterName is required if CertResource is set (service %q on listener on port %d)",
 			sid.String(), lis.Port)
 	}
diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go
index 3d86a329e..df9010a3b 100644
--- a/agent/xds/listeners_ingress.go
+++ b/agent/xds/listeners_ingress.go
@@ -164,45 +164,47 @@ func makeSDSOverrideFilterChains(cfgSnap *proxycfg.ConfigSnapshot,
 	var chains []*envoy_listener_v3.FilterChain
 
 	for _, svc := range listenerCfg.Services {
-		if ingressServiceHasSDSOverrides(svc) {
-			if len(svc.Hosts) < 1 {
-				// Shouldn't be possible with validation but be careful
-				return nil, fmt.Errorf("no hosts specified with SDS certificate (service %q on listener on port %d)",
-					svc.ToServiceName().ToServiceID().String(), listenerKey.Port)
-			}
-
-			// Service has a certificate resource override. Return a new filter chain
-			// with the right TLS cert and a filter that will load only the routes for
-			// this service.
-			routeName := routeNameForUpstream(listenerCfg, svc)
-			filterOpts.filterName = routeName
-			filterOpts.routeName = routeName
-			filter, err := makeListenerFilter(filterOpts)
-			if err != nil {
-				return nil, err
-			}
-
-			tlsContext := &envoy_tls_v3.DownstreamTlsContext{
-				CommonTlsContext:         makeCommonTLSContextFromSDS(*svc.TLS.SDS),
-				RequireClientCertificate: &wrappers.BoolValue{Value: false},
-			}
-
-			transportSocket, err := makeDownstreamTLSTransportSocket(tlsContext)
-			if err != nil {
-				return nil, err
-			}
-
-			chain := &envoy_listener_v3.FilterChain{
-				// Only match traffic for this service's hosts.
-				FilterChainMatch: makeSNIFilterChainMatch(svc.Hosts...),
-				Filters: []*envoy_listener_v3.Filter{
-					filter,
-				},
-				TransportSocket: transportSocket,
-			}
-
-			chains = append(chains, chain)
+		if !ingressServiceHasSDSOverrides(svc) {
+			continue
 		}
+
+		if len(svc.Hosts) < 1 {
+			// Shouldn't be possible with validation but be careful
+			return nil, fmt.Errorf("no hosts specified with SDS certificate (service %q on listener on port %d)",
+				svc.ToServiceName().ToServiceID().String(), listenerKey.Port)
+		}
+
+		// Service has a certificate resource override. Return a new filter chain
+		// with the right TLS cert and a filter that will load only the routes for
+		// this service.
+		routeName := routeNameForUpstream(listenerCfg, svc)
+		filterOpts.filterName = routeName
+		filterOpts.routeName = routeName
+		filter, err := makeListenerFilter(filterOpts)
+		if err != nil {
+			return nil, err
+		}
+
+		tlsContext := &envoy_tls_v3.DownstreamTlsContext{
+			CommonTlsContext:         makeCommonTLSContextFromSDS(*svc.TLS.SDS),
+			RequireClientCertificate: &wrappers.BoolValue{Value: false},
+		}
+
+		transportSocket, err := makeDownstreamTLSTransportSocket(tlsContext)
+		if err != nil {
+			return nil, err
+		}
+
+		chain := &envoy_listener_v3.FilterChain{
+			// Only match traffic for this service's hosts.
+			FilterChainMatch: makeSNIFilterChainMatch(svc.Hosts...),
+			Filters: []*envoy_listener_v3.Filter{
+				filter,
+			},
+			TransportSocket: transportSocket,
+		}
+
+		chains = append(chains, chain)
 	}
 
 	return chains, nil
diff --git a/api/config_entry_gateways.go b/api/config_entry_gateways.go
index e1364b45e..c3eb07f12 100644
--- a/api/config_entry_gateways.go
+++ b/api/config_entry_gateways.go
@@ -74,7 +74,7 @@ type IngressListener struct {
 	Services []IngressService
 
 	// TLS allows specifying some TLS configuration per listener.
-	TLS *GatewayTLSConfig
+	TLS *GatewayTLSConfig `json:",omitempty"`
 }
 
 // IngressService manages configuration for services that are exposed to
@@ -110,7 +110,7 @@ type IngressService struct {
 	Namespace string `json:",omitempty"`
 
 	// TLS allows specifying some TLS configuration per listener.
-	TLS *GatewayServiceTLSConfig
+	TLS *GatewayServiceTLSConfig `json:",omitempty"`
 
 	// Allow HTTP header manipulation to be configured.
 	RequestHeaders  *HTTPHeaderModifiers `json:",omitempty" alias:"request_headers"`