Fix bug where non-typical services are associated with gateways (#7662)

On every service registration, we check to see if a service should be
assassociated to a wildcard gateway-service. This fixes an issue where
we did not correctly check to see if the service being registered was a
"typical" service or not.

agent/consul/state/catalog.go

@@ -776,21 +776,9 @@ func (s *Store) ensureServiceTxn(tx *memdb.Txn, idx uint64, node string, svc *st
 	}
 
 	// Check if this service is covered by a gateway's wildcard specifier
-	svcGateways, err := s.serviceGateways(tx, structs.WildcardSpecifier, &svc.EnterpriseMeta)
+	err = s.checkGatewayWildcardsAndUpdate(tx, idx, svc)
 	if err != nil {
-		return fmt.Errorf("failed gateway lookup for %q: %s", svc.Service, err)
+		return fmt.Errorf("failed updating gateway mapping: %s", err)
-	}
-	for service := svcGateways.Next(); service != nil; service = svcGateways.Next() {
-		if wildcardSvc, ok := service.(*structs.GatewayService); ok && wildcardSvc != nil {
-
-			// Copy the wildcard mapping and modify it
-			gatewaySvc := wildcardSvc.Clone()
-			gatewaySvc.Service = structs.NewServiceID(svc.Service, &svc.EnterpriseMeta)
-
-			if err = s.updateGatewayService(tx, idx, gatewaySvc); err != nil {
-				return fmt.Errorf("Failed to associate service %q with gateway %q", gatewaySvc.Service.String(), gatewaySvc.Gateway.String())
-			}
-		}
 	}
 
 	// Create the service node entry and populate the indexes. Note that
@@ -2606,6 +2594,34 @@ func (s *Store) updateGatewayService(tx *memdb.Txn, idx uint64, mapping *structs
 	return nil
 }
 
+// checkWildcardForGatewaysAndUpdate checks whether a service matches a
+// wildcard definition in gateway config entries and if so adds it the the
+// gateway-services table.
+func (s *Store) checkGatewayWildcardsAndUpdate(tx *memdb.Txn, idx uint64, svc *structs.NodeService) error {
+	// Do not associate non-typical services with gateways or consul services
+	if svc.Kind != structs.ServiceKindTypical || svc.Service == "consul" {
+		return nil
+	}
+
+	svcGateways, err := s.serviceGateways(tx, structs.WildcardSpecifier, &svc.EnterpriseMeta)
+	if err != nil {
+		return fmt.Errorf("failed gateway lookup for %q: %s", svc.Service, err)
+	}
+	for service := svcGateways.Next(); service != nil; service = svcGateways.Next() {
+		if wildcardSvc, ok := service.(*structs.GatewayService); ok && wildcardSvc != nil {
+
+			// Copy the wildcard mapping and modify it
+			gatewaySvc := wildcardSvc.Clone()
+			gatewaySvc.Service = structs.NewServiceID(svc.Service, &svc.EnterpriseMeta)
+
+			if err = s.updateGatewayService(tx, idx, gatewaySvc); err != nil {
+				return fmt.Errorf("Failed to associate service %q with gateway %q", gatewaySvc.Service.String(), gatewaySvc.Gateway.String())
+			}
+		}
+	}
+	return nil
+}
+
 // serviceGateways returns all GatewayService entries with the given service name. This effectively looks up
 // all the gateways mapped to this service.
 func (s *Store) serviceGateways(tx *memdb.Txn, name string, entMeta *structs.EnterpriseMeta) (memdb.ResultIterator, error) {

agent/consul/state/catalog_test.go

@@ -4854,6 +4854,62 @@ func TestStateStore_GatewayServices_Ingress(t *testing.T) {
 	})
 }
 
+func TestStateStore_GatewayServices_WildcardAssociation(t *testing.T) {
+	s := testStateStore(t)
+	setupIngressState(t, s)
+	require := require.New(t)
+	ws := memdb.NewWatchSet()
+
+	t.Run("base case for wildcard", func(t *testing.T) {
+		idx, results, err := s.GatewayServices(ws, "wildcardIngress", nil)
+		require.NoError(err)
+		require.Equal(uint64(14), idx)
+		require.Len(results, 3)
+	})
+
+	t.Run("do not associate ingress services with gateway", func(t *testing.T) {
+		testRegisterIngressService(t, s, 15, "node1", "testIngress")
+		require.False(watchFired(ws))
+		idx, results, err := s.GatewayServices(ws, "wildcardIngress", nil)
+		require.NoError(err)
+		require.Equal(uint64(14), idx)
+		require.Len(results, 3)
+	})
+
+	t.Run("do not associate terminating-gateway services with gateway", func(t *testing.T) {
+		require.Nil(s.EnsureService(16, "node1",
+			&structs.NodeService{
+				Kind: structs.ServiceKindTerminatingGateway, ID: "gateway", Service: "gateway", Port: 443,
+			},
+		))
+		require.False(watchFired(ws))
+		idx, results, err := s.GatewayServices(ws, "wildcardIngress", nil)
+		require.NoError(err)
+		require.Equal(uint64(14), idx)
+		require.Len(results, 3)
+	})
+
+	t.Run("do not associate connect-proxy services with gateway", func(t *testing.T) {
+		testRegisterSidecarProxy(t, s, 17, "node1", "web")
+		require.False(watchFired(ws))
+		idx, results, err := s.GatewayServices(ws, "wildcardIngress", nil)
+		require.NoError(err)
+		require.Equal(uint64(14), idx)
+		require.Len(results, 3)
+	})
+
+	t.Run("do not associate consul services with gateway", func(t *testing.T) {
+		require.Nil(s.EnsureService(18, "node1",
+			&structs.NodeService{ID: "consul", Service: "consul", Tags: nil},
+		))
+		require.False(watchFired(ws))
+		idx, results, err := s.GatewayServices(ws, "wildcardIngress", nil)
+		require.NoError(err)
+		require.Equal(uint64(14), idx)
+		require.Len(results, 3)
+	})
+}
+
 func setupIngressState(t *testing.T, s *Store) memdb.WatchSet {
 	// Querying with no matches gives an empty response
 	ws := memdb.NewWatchSet()