cleanup. doc strings.

agent/xdsv2/cluster_resources.go

@@ -348,31 +348,12 @@ func addEnvoyLBToCluster(dynamicConfig *pbproxystate.DynamicEndpointGroupConfig,
 func (pr *ProxyResources) makeEnvoyClusterFromL4Destination(l4 *pbproxystate.L4Destination) error {
 	switch l4.Destination.(type) {
 	case *pbproxystate.L4Destination_Cluster:
-		clusterName := l4.GetCluster().Name
-		clusters, _ := pr.makeClusters(clusterName)
-		for name, cluster := range clusters {
-			pr.envoyResources[xdscommon.ClusterType][name] = cluster
-		}
-
-		eps := pr.proxyState.GetEndpoints()[clusterName]
-		if eps != nil {
-			protoEndpoint := makeEnvoyClusterLoadAssignment(clusterName, eps.Endpoints)
-			pr.envoyResources[xdscommon.EndpointType][protoEndpoint.ClusterName] = protoEndpoint
-		}
+		pr.addEnvoyClustersAndEndpointsToEnvoyResources(l4.GetCluster().GetName())
 
 	case *pbproxystate.L4Destination_WeightedClusters:
 		psWeightedClusters := l4.GetWeightedClusters()
 		for _, psCluster := range psWeightedClusters.GetClusters() {
-			clusters, _ := pr.makeClusters(psCluster.Name)
-			for name, cluster := range clusters {
-				pr.envoyResources[xdscommon.ClusterType][name] = cluster
-			}
-
-			eps := pr.proxyState.GetEndpoints()[psCluster.Name]
-			if eps != nil {
-				protoEndpoint := makeEnvoyClusterLoadAssignment(psCluster.Name, eps.Endpoints)
-				pr.envoyResources[xdscommon.EndpointType][psCluster.Name] = protoEndpoint
-			}
+			pr.addEnvoyClustersAndEndpointsToEnvoyResources(psCluster.Name)
 		}
 	default:
 		return errors.New("cluster group type should be Endpoint Group or Failover Group")

agent/xdsv2/endpoint_resources.go

@@ -7,9 +7,7 @@ import (
 	envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	envoy_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
 	"github.com/hashicorp/consul/agent/xds/response"
-	"github.com/hashicorp/consul/envoyextensions/xdscommon"
 	"github.com/hashicorp/consul/proto-public/pbmesh/v2beta1/pbproxystate"
-	"google.golang.org/protobuf/proto"
 )
 
 func makeEnvoyLbEndpoint(endpoint *pbproxystate.Endpoint) *envoy_endpoint_v3.LbEndpoint {
@@ -46,16 +44,3 @@ func makeEnvoyClusterLoadAssignment(clusterName string, endpoints []*pbproxystat
 		Endpoints:   []*envoy_endpoint_v3.LocalityLbEndpoints{localityLbEndpoints},
 	}
 }
-
-func (pr *ProxyResources) makeXDSEndpoints() ([]proto.Message, error) {
-	endpoints := make([]proto.Message, 0)
-
-	for clusterName, eps := range pr.proxyState.GetEndpoints() {
-		if clusterName != xdscommon.LocalAppClusterName {
-			protoEndpoint := makeEnvoyClusterLoadAssignment(clusterName, eps.Endpoints)
-			endpoints = append(endpoints, protoEndpoint)
-		}
-	}
-
-	return endpoints, nil
-}

agent/xdsv2/listener_resources.go

@@ -42,7 +42,7 @@ const (
 	envoyHttpConnectionManagerFilterName       = "envoy.filters.network.http_connection_manager"
 )
 
-func (pr *ProxyResources) makeXDSListeners() error {
+func (pr *ProxyResources) makeEnvoyResourceGraphsStartingFromListeners() error {
 	for _, l := range pr.proxyState.Listeners {
 		protoListener, err := pr.makeListener(l)
 		// TODO: aggregate errors for listeners and still return any properly formed listeners.

agent/xdsv2/resources.go

@@ -20,6 +20,7 @@ type ResourceGenerator struct {
 	ProxyFeatures xdscommon.SupportedProxyFeatures
 }
 
+// NewResourceGenerator will create a new ResourceGenerator.
 func NewResourceGenerator(
 	logger hclog.Logger,
 ) *ResourceGenerator {
@@ -28,8 +29,18 @@ func NewResourceGenerator(
 	}
 }
 
+// ProxyResources is the main state used to convert proxyState resources to Envoy resources.
 type ProxyResources struct {
-	proxyState     *proxytracker.ProxyState
+	// proxyState is the final proxyState computed by Consul controllers.
+	proxyState *proxytracker.ProxyState
+	// envoyResources is a map of each resource type (listener, endpoint, route, cluster, etc.)
+	// with a corresponding map of k/v pairs of resource name to envoy proto message.
+	// map[string]map[string]proto.Message is used over map[string][]proto.Message because
+	// AllResourcesFromIR() will create envoy resource by walking the object graph from listener
+	// to endpoint.  In the process, the same resource might be referenced more than once,
+	// so the map is used to prevent duplicate resources being created and also will use
+	// an O(1) lookup to see if it exists (it actually will set the map key rather than
+	// checks everywhere) where as each lookup would be O(n) with a []proto structure.
 	envoyResources map[string]map[string]proto.Message
 }
 
@@ -43,28 +54,30 @@ func (g *ResourceGenerator) AllResourcesFromIR(proxyState *proxytracker.ProxySta
 	pr.envoyResources[xdscommon.ClusterType] = make(map[string]proto.Message)
 	pr.envoyResources[xdscommon.EndpointType] = make(map[string]proto.Message)
 
-	err := pr.generateXDSResources()
+	err := pr.makeEnvoyResourceGraphsStartingFromListeners()
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate xDS resources for ProxyState: %v", err)
 	}
-	envoyResources := make(map[string][]proto.Message)
-	envoyResources[xdscommon.ListenerType] = make([]proto.Message, 0)
-	envoyResources[xdscommon.RouteType] = make([]proto.Message, 0)
-	envoyResources[xdscommon.ClusterType] = make([]proto.Message, 0)
-	envoyResources[xdscommon.EndpointType] = make([]proto.Message, 0)
-	for resourceTypeName, resourceMap := range pr.envoyResources {
-		for _, resource := range resourceMap {
-			envoyResources[resourceTypeName] = append(envoyResources[resourceTypeName], resource)
-		}
-	}
+	envoyResources := convertResourceMapsToResourceArrays(pr.envoyResources)
 	return envoyResources, nil
 }
 
-func (pr *ProxyResources) generateXDSResources() error {
-	err := pr.makeXDSListeners()
-	if err != nil {
-		return err
-	}
+// convertResourceMapsToResourceArrays will convert map[string]map[string]proto.Message, which is used to
+// prevent duplicate resource being created, to map[string][]proto.Message which is used by Delta server.
+func convertResourceMapsToResourceArrays(resourceMap map[string]map[string]proto.Message) map[string][]proto.Message {
+	resources := make(map[string][]proto.Message)
+	resources[xdscommon.ListenerType] = make([]proto.Message, 0)
+	resources[xdscommon.RouteType] = make([]proto.Message, 0)
+	resources[xdscommon.ClusterType] = make([]proto.Message, 0)
+	resources[xdscommon.EndpointType] = make([]proto.Message, 0)
 
-	return nil
+	// This conversion incurs processing cost which is done once in the generating envoy resources.
+	// This tradeoff is preferable to doing array scan every time an envoy resource needs to be
+	// to pr.envoyResource to see if it already exists.
+	for resourceTypeName, resourceMap := range resourceMap {
+		for _, resource := range resourceMap {
+			resources[resourceTypeName] = append(resources[resourceTypeName], resource)
+		}
+	}
+	return resources
 }

agent/xdsv2/route_resources.go

@@ -16,21 +16,8 @@ import (
 	"github.com/hashicorp/consul/proto-public/pbmesh/v2beta1/pbproxystate"
 
 	envoy_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
-	"google.golang.org/protobuf/proto"
 )
 
-func (pr *ProxyResources) makeXDSRoutes() ([]proto.Message, error) {
-	routes := make([]proto.Message, 0)
-
-	for name, r := range pr.proxyState.Routes {
-		protoRoute := pr.makeEnvoyRouteConfigFromProxystateRoute(name, r)
-		// TODO: aggregate errors for routes and still return any properly formed routes.
-		routes = append(routes, protoRoute)
-	}
-
-	return routes, nil
-}
-
 func (pr *ProxyResources) makeEnvoyRoute(name string) (*envoy_route_v3.RouteConfiguration, error) {
 	var route *envoy_route_v3.RouteConfiguration
 	// TODO(proxystate): This will make routes in the future. This function should distinguish between static routes
@@ -247,6 +234,18 @@ func makeEnvoyQueryParamFromProxystateQueryMatch(psMatch *pbproxystate.QueryPara
 	return envoyQueryParamMatcher
 }
 
+func (pr *ProxyResources) addEnvoyClustersAndEndpointsToEnvoyResources(clusterName string) {
+	clusters, _ := pr.makeClusters(clusterName)
+	for name, cluster := range clusters {
+		pr.envoyResources[xdscommon.ClusterType][name] = cluster
+	}
+
+	if endpointList, ok := pr.proxyState.Endpoints[clusterName]; ok {
+		protoEndpoint := makeEnvoyClusterLoadAssignment(clusterName, endpointList.Endpoints)
+		pr.envoyResources[xdscommon.EndpointType][clusterName] = protoEndpoint
+	}
+}
+
 // TODO (dans): Will this always be envoy_route_v3.Route_Route?
 // Definitely for connect proxies this is the only option.
 func (pr *ProxyResources) makeEnvoyRouteActionFromProxystateRouteDestination(psRouteDestination *pbproxystate.RouteDestination) *envoy_route_v3.Route_Route {
@@ -260,32 +259,14 @@ func (pr *ProxyResources) makeEnvoyRouteActionFromProxystateRouteDestination(psR
 		envoyRouteRoute.Route.ClusterSpecifier = &envoy_route_v3.RouteAction_Cluster{
 			Cluster: psCluster.GetName(),
 		}
-		clusters, _ := pr.makeClusters(psCluster.Name)
-		for name, cluster := range clusters {
-			pr.envoyResources[xdscommon.ClusterType][name] = cluster
-		}
-
-		eps := pr.proxyState.GetEndpoints()[psCluster.Name]
-		if eps != nil {
-			protoEndpoint := makeEnvoyClusterLoadAssignment(psCluster.Name, eps.Endpoints)
-			pr.envoyResources[xdscommon.EndpointType][protoEndpoint.ClusterName] = protoEndpoint
-		}
+		pr.addEnvoyClustersAndEndpointsToEnvoyResources(psCluster.Name)
 
 	case *pbproxystate.RouteDestination_WeightedClusters:
 		psWeightedClusters := psRouteDestination.GetWeightedClusters()
 		envoyClusters := make([]*envoy_route_v3.WeightedCluster_ClusterWeight, 0, len(psWeightedClusters.GetClusters()))
 		totalWeight := 0
 		for _, psCluster := range psWeightedClusters.GetClusters() {
-			clusters, _ := pr.makeClusters(psCluster.Name)
-			for name, cluster := range clusters {
-				pr.envoyResources[xdscommon.ClusterType][name] = cluster
-			}
-
-			eps := pr.proxyState.GetEndpoints()[psCluster.Name]
-			if eps != nil {
-				protoEndpoint := makeEnvoyClusterLoadAssignment(psCluster.Name, eps.Endpoints)
-				pr.envoyResources[xdscommon.EndpointType][protoEndpoint.ClusterName] = protoEndpoint
-			}
+			pr.addEnvoyClustersAndEndpointsToEnvoyResources(psCluster.Name)
 
 			totalWeight += int(psCluster.Weight.GetValue())
 			envoyClusters = append(envoyClusters, makeEnvoyClusterWeightFromProxystateWeightedCluster(psCluster))

internal/mesh/internal/controllers/sidecarproxy/builder/testdata/source/multiple-workload-addresses-with-specific-ports.golden

@@ -108,11 +108,13 @@
                 }
               }
             },
-            "l4": {
-              "cluster": {
-                "name": "local_app:tcp"
+            "l7": {
+              "protocol": "L7_PROTOCOL_HTTP2",
+              "route": {
+                "name": "public_listener:http2"
               },
               "statPrefix": "public_listener",
+              "staticRoute": true,
               "trafficPermissions": {
                 "allowPermissions": [
                   {
@@ -129,7 +131,7 @@
             },
             "match": {
               "alpnProtocols": [
-                "consul~tcp"
+                "consul~http2"
               ]
             }
           },
@@ -145,8 +147,9 @@
               }
             },
             "l7": {
+              "protocol": "L7_PROTOCOL_GRPC",
               "route": {
-                "name": "public_listener:http"
+                "name": "public_listener:grpc"
               },
               "statPrefix": "public_listener",
               "staticRoute": true,
@@ -166,7 +169,7 @@
             },
             "match": {
               "alpnProtocols": [
-                "consul~http"
+                "consul~grpc"
               ]
             }
           },
@@ -181,13 +184,11 @@
                 }
               }
             },
-            "l7": {
-              "protocol": "L7_PROTOCOL_HTTP2",
-              "route": {
-                "name": "public_listener:http2"
+            "l4": {
+              "cluster": {
+                "name": "local_app:tcp"
               },
               "statPrefix": "public_listener",
-              "staticRoute": true,
               "trafficPermissions": {
                 "allowPermissions": [
                   {
@@ -204,7 +205,7 @@
             },
             "match": {
               "alpnProtocols": [
-                "consul~http2"
+                "consul~tcp"
               ]
             }
           },
@@ -220,9 +221,8 @@
               }
             },
             "l7": {
-              "protocol": "L7_PROTOCOL_GRPC",
               "route": {
-                "name": "public_listener:grpc"
+                "name": "public_listener:http"
               },
               "statPrefix": "public_listener",
               "staticRoute": true,
@@ -242,7 +242,7 @@
             },
             "match": {
               "alpnProtocols": [
-                "consul~grpc"
+                "consul~http"
               ]
             }
           }
@@ -336,4 +336,4 @@
       "peer": "local"
     }
   }
-}
+}