Fix topoloy intention with mixed connect-native/normal services.

If a service is registered twice, once with connect-native and once without, the topology views would prune the existing intentions. This change brings the code more in line with the transparent proxy behavior.
hashicorp · May 11, 2022 · 377860f · 377860f
1 parent 1bac154
commit 377860f
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 7 deletions.
diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go
@@ -4002,6 +4002,7 @@ func (s *Store) ServiceTopology(
 	}
 
 	idx, downstreamNames, err := s.downstreamsForServiceTxn(tx, ws, dc, sn)
+
 	if err != nil {
 		return 0, nil, err
 	}
@@ -4081,13 +4082,17 @@ func (s *Store) ServiceTopology(
 		maxIdx = idx
 	}
 
-	// Store downstreams with at least one instance in transparent proxy mode.
+	// Store downstreams with at least one instance in transparent proxy or connect native mode.
 	// This is to avoid returning downstreams from intentions when none of the downstreams are transparent proxies.
-	tproxyMap := make(map[structs.ServiceName]struct{})
+	proxyMap := make(map[structs.ServiceName]struct{})
 	for _, downstream := range unfilteredDownstreams {
 		if downstream.Service.Proxy.Mode == structs.ProxyModeTransparent {
 			sn := structs.NewServiceName(downstream.Service.Proxy.DestinationServiceName, &downstream.Service.EnterpriseMeta)
-			tproxyMap[sn] = struct{}{}
+			proxyMap[sn] = struct{}{}
+		}
+		if downstream.Service.Connect.Native {
+			sn := structs.NewServiceName(downstream.Service.Service, &downstream.Service.EnterpriseMeta)
+			proxyMap[sn] = struct{}{}
 		}
 	}
 
@@ -4097,7 +4102,7 @@ func (s *Store) ServiceTopology(
 		if downstream.Service.Kind == structs.ServiceKindConnectProxy {
 			sn = structs.NewServiceName(downstream.Service.Proxy.DestinationServiceName, &downstream.Service.EnterpriseMeta)
 		}
-		if _, ok := tproxyMap[sn]; !ok && !downstream.Service.Connect.Native && downstreamSources[sn.String()] != structs.TopologySourceRegistration {
+		if _, ok := proxyMap[sn]; !ok && downstreamSources[sn.String()] != structs.TopologySourceRegistration {
 			// If downstream is not a transparent proxy or connect native, remove references
 			delete(downstreamSources, sn.String())
 			delete(downstreamDecisions, sn.String())

diff --git a/agent/ui_endpoint_test.go b/agent/ui_endpoint_test.go
@@ -1400,19 +1400,43 @@ func TestUIServiceTopology(t *testing.T) {
 				SkipNodeUpdate: true,
 				Service: &structs.NodeService{
 					Kind:    structs.ServiceKindTypical,
-					ID:      "cproxy",
+					ID:      "cproxy-https",
 					Service: "cproxy",
 					Port:    1111,
 					Address: "198.18.1.70",
+					Tags:    []string{"https"},
 					Connect: structs.ServiceConnect{Native: true},
 				},
 				Checks: structs.HealthChecks{
 					&structs.HealthCheck{
 						Node:        "cnative",
-						CheckID:     "cnative:cproxy",
+						CheckID:     "cnative:cproxy-https",
 						Name:        "cproxy-liveness",
 						Status:      api.HealthPassing,
-						ServiceID:   "cproxy",
+						ServiceID:   "cproxy-https",
+						ServiceName: "cproxy",
+					},
+				},
+			},
+			"Service cproxy/http on cnative": {
+				Datacenter:     "dc1",
+				Node:           "cnative",
+				SkipNodeUpdate: true,
+				Service: &structs.NodeService{
+					Kind:    structs.ServiceKindTypical,
+					ID:      "cproxy-http",
+					Service: "cproxy",
+					Port:    1112,
+					Address: "198.18.1.70",
+					Tags:    []string{"http"},
+				},
+				Checks: structs.HealthChecks{
+					&structs.HealthCheck{
+						Node:        "cnative",
+						CheckID:     "cnative:cproxy-http",
+						Name:        "cproxy-liveness",
+						Status:      api.HealthPassing,
+						ServiceID:   "cproxy-http",
 						ServiceName: "cproxy",
 					},
 				},
@@ -1838,6 +1862,42 @@ func TestUIServiceTopology(t *testing.T) {
 				FilteredByACLs: false,
 			},
 		},
+		{
+			name: "cbackend",
+			httpReq: func() *http.Request {
+				req, _ := http.NewRequest("GET", "/v1/internal/ui/service-topology/cbackend?kind=", nil)
+				return req
+			}(),
+			want: &ServiceTopology{
+				Protocol:         "http",
+				TransparentProxy: false,
+				Upstreams:        []*ServiceTopologySummary{},
+				Downstreams: []*ServiceTopologySummary{
+					{
+						ServiceSummary: ServiceSummary{
+							Name:           "cproxy",
+							Datacenter:     "dc1",
+							Tags:           []string{"http", "https"},
+							Nodes:          []string{"cnative", "cnative", "cnative"},
+							InstanceCount:  3,
+							ChecksPassing:  3,
+							ChecksWarning:  0,
+							ChecksCritical: 0,
+							ConnectNative:  true,
+							EnterpriseMeta: *structs.DefaultEnterpriseMetaInDefaultPartition(),
+						},
+						Intention: structs.IntentionDecisionSummary{
+							DefaultAllow:   true,
+							Allowed:        true,
+							HasPermissions: false,
+							HasExact:       true,
+						},
+						Source: structs.TopologySourceSpecificIntention,
+					},
+				},
+				FilteredByACLs: false,
+			},
+		},
 	}
 
 	for _, tc := range tcs {