Merge pull request #11163 from hashicorp/feature/ingress-tls-mixed

Add support for enabling connect-based ingress TLS per listener.

.changelog/11163.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+connect: ingress gateways may now enable built-in TLS for a subset of listeners.
+```

agent/proxycfg/ingress_gateway.go

@@ -196,10 +196,31 @@ func (s *handlerIngressGateway) watchIngressLeafCert(ctx context.Context, snap *
 	return nil
 }
 
+// connectTLSServingEnabled returns true if Connect TLS is enabled at either
+// gateway level or for at least one of the specific listeners.
+func connectTLSServingEnabled(snap *ConfigSnapshot) bool {
+	if snap.IngressGateway.TLSConfig.Enabled {
+		return true
+	}
+
+	for _, l := range snap.IngressGateway.Listeners {
+		if l.TLS != nil && l.TLS.Enabled {
+			return true
+		}
+	}
+	return false
+}
+
 func (s *handlerIngressGateway) generateIngressDNSSANs(snap *ConfigSnapshot) []string {
-	// Update our leaf cert watch with wildcard entries for our DNS domains as well as any
-	// configured custom hostnames from the service.
-	if !snap.IngressGateway.TLSConfig.Enabled {
+	// Update our leaf cert watch with wildcard entries for our DNS domains as
+	// well as any configured custom hostnames from the service. Note that in the
+	// case that only a subset of listeners are TLS-enabled, we still load DNS
+	// SANs for all upstreams. We could limit it to only those that are reachable
+	// from the enabled listeners but that adds a lot of complication and they are
+	// already wildcards anyway. It's simpler to have one certificate for the
+	// whole proxy that works for any possible upstream we might need than try to
+	// be more selective when we are already using wildcard DNS names!
+	if !connectTLSServingEnabled(snap) {
 		return nil
 	}
 

agent/proxycfg/state_test.go

@@ -349,15 +349,33 @@ func genVerifyConfigEntryWatch(expectedKind, expectedName, expectedDatacenter st
 	}
 }
 
-func ingressConfigWatchEvent(tlsEnabled bool) cache.UpdateEvent {
+func ingressConfigWatchEvent(gwTLS bool, mixedTLS bool) cache.UpdateEvent {
+	e := &structs.IngressGatewayConfigEntry{
+		TLS: structs.GatewayTLSConfig{
+			Enabled: gwTLS,
+		},
+	}
+
+	if mixedTLS {
+		// Add two listeners one with and one without connect TLS enabled
+		e.Listeners = []structs.IngressListener{
+			{
+				Port:     8080,
+				Protocol: "tcp",
+				TLS:      &structs.GatewayTLSConfig{Enabled: true},
+			},
+			{
+				Port:     9090,
+				Protocol: "tcp",
+				TLS:      nil,
+			},
+		}
+	}
+
 	return cache.UpdateEvent{
 		CorrelationID: gatewayConfigWatchID,
 		Result: &structs.ConfigEntryResponse{
-			Entry: &structs.IngressGatewayConfigEntry{
-				TLS: structs.GatewayTLSConfig{
-					Enabled: tlsEnabled,
-				},
-			},
+			Entry: e,
 		},
 		Err: nil,
 	}
@@ -938,7 +956,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 				},
 				{
 					events: []cache.UpdateEvent{
-						ingressConfigWatchEvent(false),
+						ingressConfigWatchEvent(false, false),
 					},
 					verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) {
 						require.False(t, snap.Valid(), "gateway without hosts set is not valid")
@@ -1088,7 +1106,7 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 					},
 					events: []cache.UpdateEvent{
 						rootWatchEvent(),
-						ingressConfigWatchEvent(true),
+						ingressConfigWatchEvent(true, false),
 						{
 							CorrelationID: gatewayServicesWatchID,
 							Result: &structs.IndexedGatewayServices{
@@ -1146,6 +1164,94 @@ func TestState_WatchesAndUpdates(t *testing.T) {
 				},
 			},
 		},
+		"ingress-gateway-with-mixed-tls": {
+			ns: structs.NodeService{
+				Kind:    structs.ServiceKindIngressGateway,
+				ID:      "ingress-gateway",
+				Service: "ingress-gateway",
+				Address: "10.0.1.1",
+			},
+			sourceDC: "dc1",
+			stages: []verificationStage{
+				{
+					requiredWatches: map[string]verifyWatchRequest{
+						rootsWatchID:           genVerifyRootsWatch("dc1"),
+						gatewayConfigWatchID:   genVerifyConfigEntryWatch(structs.IngressGateway, "ingress-gateway", "dc1"),
+						gatewayServicesWatchID: genVerifyGatewayServiceWatch("ingress-gateway", "dc1"),
+					},
+					events: []cache.UpdateEvent{
+						rootWatchEvent(),
+						ingressConfigWatchEvent(false, true),
+						{
+							CorrelationID: gatewayServicesWatchID,
+							Result: &structs.IndexedGatewayServices{
+								Services: structs.GatewayServices{
+									{
+										Gateway: structs.NewServiceName("ingress-gateway", nil),
+										Service: structs.NewServiceName("api", nil),
+										Hosts:   []string{"test.example.com"},
+										Port:    9999,
+									},
+								},
+							},
+							Err: nil,
+						},
+						{
+							CorrelationID: leafWatchID,
+							Result:        issuedCert,
+							Err:           nil,
+						},
+					},
+					verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) {
+						require.True(t, snap.Valid())
+						require.True(t, snap.IngressGateway.GatewayConfigLoaded)
+						// GW level TLS should be disabled
+						require.False(t, snap.IngressGateway.TLSConfig.Enabled)
+						// Mixed listener TLS
+						l, ok := snap.IngressGateway.Listeners[IngressListenerKey{"tcp", 8080}]
+						require.True(t, ok)
+						require.NotNil(t, l.TLS)
+						require.True(t, l.TLS.Enabled)
+						l, ok = snap.IngressGateway.Listeners[IngressListenerKey{"tcp", 9090}]
+						require.True(t, ok)
+						require.Nil(t, l.TLS)
+
+						require.True(t, snap.IngressGateway.HostsSet)
+						require.Len(t, snap.IngressGateway.Hosts, 1)
+						require.Len(t, snap.IngressGateway.Upstreams, 1)
+						require.Len(t, snap.IngressGateway.WatchedDiscoveryChains, 1)
+						require.Contains(t, snap.IngressGateway.WatchedDiscoveryChains, api.String())
+					},
+				},
+				{
+					requiredWatches: map[string]verifyWatchRequest{
+						// This is the real point of this test - ensure we still generate
+						// the right DNS SANs for the whole gateway even when only a subset
+						// of listeners have TLS enabled.
+						leafWatchID: genVerifyLeafWatchWithDNSSANs("ingress-gateway", "dc1", []string{
+							"test.example.com",
+							"*.ingress.consul.",
+							"*.ingress.dc1.consul.",
+							"*.ingress.alt.consul.",
+							"*.ingress.dc1.alt.consul.",
+						}),
+					},
+					events: []cache.UpdateEvent{
+						{
+							CorrelationID: gatewayServicesWatchID,
+							Result:        &structs.IndexedGatewayServices{},
+							Err:           nil,
+						},
+					},
+					verifySnapshot: func(t testing.TB, snap *ConfigSnapshot) {
+						require.True(t, snap.Valid())
+						require.Len(t, snap.IngressGateway.Upstreams, 0)
+						require.Len(t, snap.IngressGateway.WatchedDiscoveryChains, 0)
+						require.NotContains(t, snap.IngressGateway.WatchedDiscoveryChains, "api")
+					},
+				},
+			},
+		},
 		"terminating-gateway-initial": {
 			ns: structs.NodeService{
 				Kind:    structs.ServiceKindTerminatingGateway,

agent/xds/listeners.go

@@ -511,49 +511,6 @@ func (s *ResourceGenerator) listenersFromSnapshotGateway(cfgSnap *proxycfg.Confi
 	return resources, err
 }
 
-func resolveListenerSDSConfig(cfgSnap *proxycfg.ConfigSnapshot, listenerKey proxycfg.IngressListenerKey) (*structs.GatewayTLSSDSConfig, error) {
-	var mergedCfg structs.GatewayTLSSDSConfig
-
-	gwSDS := cfgSnap.IngressGateway.TLSConfig.SDS
-	if gwSDS != nil {
-		mergedCfg.ClusterName = gwSDS.ClusterName
-		mergedCfg.CertResource = gwSDS.CertResource
-	}
-
-	listenerCfg, ok := cfgSnap.IngressGateway.Listeners[listenerKey]
-	if !ok {
-		return nil, fmt.Errorf("no listener config found for listener on port %d", listenerKey.Port)
-	}
-
-	if listenerCfg.TLS != nil && listenerCfg.TLS.SDS != nil {
-		if listenerCfg.TLS.SDS.ClusterName != "" {
-			mergedCfg.ClusterName = listenerCfg.TLS.SDS.ClusterName
-		}
-		if listenerCfg.TLS.SDS.CertResource != "" {
-			mergedCfg.CertResource = listenerCfg.TLS.SDS.CertResource
-		}
-	}
-
-	// Validate. Either merged should have both fields empty or both set. Other
-	// cases shouldn't be possible as we validate them at input but be robust to
-	// bugs later.
-	switch {
-	case mergedCfg.ClusterName == "" && mergedCfg.CertResource == "":
-		return nil, nil
-
-	case mergedCfg.ClusterName != "" && mergedCfg.CertResource != "":
-		return &mergedCfg, nil
-
-	case mergedCfg.ClusterName == "" && mergedCfg.CertResource != "":
-		return nil, fmt.Errorf("missing SDS cluster name for listener on port %d", listenerKey.Port)
-
-	case mergedCfg.ClusterName != "" && mergedCfg.CertResource == "":
-		return nil, fmt.Errorf("missing SDS cert resource for listener on port %d", listenerKey.Port)
-	}
-
-	return &mergedCfg, nil
-}
-
 // makeListener returns a listener with name and bind details set. Filters must
 // be added before it's useful.
 //

agent/xds/listeners_ingress.go

@@ -19,7 +19,16 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap
 	for listenerKey, upstreams := range cfgSnap.IngressGateway.Upstreams {
 		var tlsContext *envoy_tls_v3.DownstreamTlsContext
 
-		sdsCfg, err := resolveListenerSDSConfig(cfgSnap, listenerKey)
+		listenerCfg, ok := cfgSnap.IngressGateway.Listeners[listenerKey]
+		if !ok {
+			return nil, fmt.Errorf("no listener config found for listener on port %d", listenerKey.Port)
+		}
+		// Enable connect TLS if it is enabled at the Gateway or specific listener
+		// level.
+		connectTLSEnabled := cfgSnap.IngressGateway.TLSConfig.Enabled ||
+			(listenerCfg.TLS != nil && listenerCfg.TLS.Enabled)
+
+		sdsCfg, err := resolveListenerSDSConfig(cfgSnap, listenerCfg)
 		if err != nil {
 			return nil, err
 		}
@@ -30,7 +39,7 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap
 				CommonTlsContext:         makeCommonTLSContextFromSDS(*sdsCfg),
 				RequireClientCertificate: &wrappers.BoolValue{Value: false},
 			}
-		} else if cfgSnap.IngressGateway.TLSConfig.Enabled {
+		} else if connectTLSEnabled {
 			tlsContext = &envoy_tls_v3.DownstreamTlsContext{
 				CommonTlsContext:         makeCommonTLSContextFromLeaf(cfgSnap, cfgSnap.Leaf()),
 				RequireClientCertificate: &wrappers.BoolValue{Value: false},
@@ -118,6 +127,44 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap
 	return resources, nil
 }
 
+func resolveListenerSDSConfig(cfgSnap *proxycfg.ConfigSnapshot, listenerCfg structs.IngressListener) (*structs.GatewayTLSSDSConfig, error) {
+	var mergedCfg structs.GatewayTLSSDSConfig
+
+	gwSDS := cfgSnap.IngressGateway.TLSConfig.SDS
+	if gwSDS != nil {
+		mergedCfg.ClusterName = gwSDS.ClusterName
+		mergedCfg.CertResource = gwSDS.CertResource
+	}
+
+	if listenerCfg.TLS != nil && listenerCfg.TLS.SDS != nil {
+		if listenerCfg.TLS.SDS.ClusterName != "" {
+			mergedCfg.ClusterName = listenerCfg.TLS.SDS.ClusterName
+		}
+		if listenerCfg.TLS.SDS.CertResource != "" {
+			mergedCfg.CertResource = listenerCfg.TLS.SDS.CertResource
+		}
+	}
+
+	// Validate. Either merged should have both fields empty or both set. Other
+	// cases shouldn't be possible as we validate them at input but be robust to
+	// bugs later.
+	switch {
+	case mergedCfg.ClusterName == "" && mergedCfg.CertResource == "":
+		return nil, nil
+
+	case mergedCfg.ClusterName != "" && mergedCfg.CertResource != "":
+		return &mergedCfg, nil
+
+	case mergedCfg.ClusterName == "" && mergedCfg.CertResource != "":
+		return nil, fmt.Errorf("missing SDS cluster name for listener on port %d", listenerCfg.Port)
+
+	case mergedCfg.ClusterName != "" && mergedCfg.CertResource == "":
+		return nil, fmt.Errorf("missing SDS cert resource for listener on port %d", listenerCfg.Port)
+	}
+
+	return &mergedCfg, nil
+}
+
 func routeNameForUpstream(l structs.IngressListener, s structs.IngressService) string {
 	key := proxycfg.IngressListenerKeyFromListener(l)
 

agent/xds/listeners_test.go

@@ -503,6 +503,59 @@ func TestListenersFromSnapshot(t *testing.T) {
 			create: proxycfg.TestConfigSnapshotIngressWithTLSListener,
 			setup:  nil,
 		},
+		{
+			name: "ingress-with-tls-mixed-listeners",
+			// Use SDS helper even though we aren't testing SDS since it already sets
+			// up most things we need.
+			create: proxycfg.TestConfigSnapshotIngressWithGatewaySDS,
+			setup: func(snap *proxycfg.ConfigSnapshot) {
+				// Undo gateway-level SDS
+				snap.IngressGateway.TLSConfig.SDS = nil
+
+				// No Gateway-level built-in TLS
+				snap.IngressGateway.TLSConfig.Enabled = false
+
+				// One listener has built-in TLS, one doesn't
+				snap.IngressGateway.Upstreams = map[proxycfg.IngressListenerKey]structs.Upstreams{
+					{Protocol: "http", Port: 8080}: {
+						{
+							DestinationName: "s1",
+							LocalBindPort:   8080,
+						},
+					},
+					{Protocol: "http", Port: 9090}: {
+						{
+							DestinationName: "s2",
+							LocalBindPort:   9090,
+						},
+					},
+				}
+				snap.IngressGateway.Listeners = map[proxycfg.IngressListenerKey]structs.IngressListener{
+					{Protocol: "http", Port: 8080}: {
+						Port: 8080,
+						Services: []structs.IngressService{
+							{
+								Name: "s1",
+							},
+						},
+						TLS: &structs.GatewayTLSConfig{
+							// built-in TLS enabled
+							Enabled: true,
+						},
+					},
+					{Protocol: "http", Port: 9090}: {
+						Port: 9090,
+						Services: []structs.IngressService{
+							{
+								Name: "s2",
+							},
+						},
+						// No TLS enabled
+						TLS: nil,
+					},
+				}
+			},
+		},
 		{
 			name:   "ingress-with-sds-listener-gw-level",
 			create: proxycfg.TestConfigSnapshotIngressWithGatewaySDS,
@@ -1119,7 +1172,7 @@ func TestResolveListenerSDSConfig(t *testing.T) {
 		snap := proxycfg.TestConfigSnapshotIngressWithGatewaySDS(t)
 		// Override TLS configs
 		snap.IngressGateway.TLSConfig.SDS = tc.gwSDS
-		var key proxycfg.IngressListenerKey
+		var listenerCfg structs.IngressListener
 		for k, lisCfg := range snap.IngressGateway.Listeners {
 			if tc.lisSDS == nil {
 				lisCfg.TLS = nil
@@ -1130,12 +1183,11 @@ func TestResolveListenerSDSConfig(t *testing.T) {
 			}
 			// Override listener cfg in map
 			snap.IngressGateway.Listeners[k] = lisCfg
-			// Save the last key doesn't matter which as we set same listener config
-			// for all.
-			key = k
+			// Save the last cfg doesn't matter which as we set same for all.
+			listenerCfg = lisCfg
 		}
 
-		got, err := resolveListenerSDSConfig(snap, key)
+		got, err := resolveListenerSDSConfig(snap, listenerCfg)
 		if tc.wantErr != "" {
 			require.Error(t, err)
 			require.Contains(t, err.Error(), tc.wantErr)