From 9d2a9169fd7aedb863bb9419d9ef9278cabb288b Mon Sep 17 00:00:00 2001 From: freddygv Date: Fri, 11 Sep 2020 10:49:26 -0600 Subject: [PATCH] PR comments --- agent/xds/clusters.go | 84 ++++++++++++++++++++-------------------- agent/xds/routes.go | 90 ++++++++++++++++++++++--------------------- 2 files changed, 88 insertions(+), 86 deletions(-) diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 9f2831b64f..aade207009 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -33,7 +33,7 @@ func (s *Server) clustersFromSnapshot(_ connectionInfo, cfgSnap *proxycfg.Config case structs.ServiceKindConnectProxy: return s.clustersFromSnapshotConnectProxy(cfgSnap) case structs.ServiceKindTerminatingGateway: - return s.makeGatewayServiceClusters(cfgSnap) + return s.makeGatewayServiceClusters(cfgSnap, cfgSnap.TerminatingGateway.ServiceGroups, cfgSnap.TerminatingGateway.ServiceResolvers) case structs.ServiceKindMeshGateway: return s.clustersFromSnapshotMeshGateway(cfgSnap) case structs.ServiceKindIngressGateway: @@ -175,7 +175,7 @@ func (s *Server) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsho } // generate the per-service/subset clusters - c, err := s.makeGatewayServiceClusters(cfgSnap) + c, err := s.makeGatewayServiceClusters(cfgSnap, cfgSnap.MeshGateway.ServiceGroups, cfgSnap.MeshGateway.ServiceResolvers) if err != nil { return nil, err } @@ -184,18 +184,16 @@ func (s *Server) clustersFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsho return clusters, nil } -func (s *Server) makeGatewayServiceClusters(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { - var services map[structs.ServiceName]structs.CheckServiceNodes - var resolvers map[structs.ServiceName]*structs.ServiceResolverConfigEntry +func (s *Server) makeGatewayServiceClusters( + cfgSnap *proxycfg.ConfigSnapshot, + services map[structs.ServiceName]structs.CheckServiceNodes, + resolvers map[structs.ServiceName]*structs.ServiceResolverConfigEntry, +) ([]proto.Message, error) { + var hostnameEndpoints structs.CheckServiceNodes switch cfgSnap.Kind { - case structs.ServiceKindTerminatingGateway: - services = cfgSnap.TerminatingGateway.ServiceGroups - resolvers = cfgSnap.TerminatingGateway.ServiceResolvers - case structs.ServiceKindMeshGateway: - services = cfgSnap.MeshGateway.ServiceGroups - resolvers = cfgSnap.MeshGateway.ServiceResolvers + case structs.ServiceKindTerminatingGateway, structs.ServiceKindMeshGateway: default: return nil, fmt.Errorf("unsupported gateway kind %q", cfgSnap.Kind) } @@ -229,21 +227,8 @@ func (s *Server) makeGatewayServiceClusters(cfgSnap *proxycfg.ConfigSnapshot) ([ } cluster := s.makeGatewayCluster(cfgSnap, opts) - switch cfgSnap.Kind { - case structs.ServiceKindTerminatingGateway: - injectTerminatingGatewayTLSContext(cfgSnap, cluster, svc) - - if err := injectLBToCluster(loadBalancer, cluster); err != nil { - return nil, fmt.Errorf("failed to apply load balancer configuration to cluster %q: %v", clusterName, err) - } - case structs.ServiceKindMeshGateway: - // We can't apply hash based LB config to mesh gateways because they rely on inspecting HTTP attributes - // and mesh gateways do not decrypt traffic - if !loadBalancer.IsHashBased() { - if err := injectLBToCluster(loadBalancer, cluster); err != nil { - return nil, fmt.Errorf("failed to apply load balancer configuration to cluster %q: %v", clusterName, err) - } - } + if err := s.injectGatewayServiceAddons(cfgSnap, cluster, svc, loadBalancer); err != nil { + return nil, err } clusters = append(clusters, cluster) @@ -262,22 +247,8 @@ func (s *Server) makeGatewayServiceClusters(cfgSnap *proxycfg.ConfigSnapshot) ([ } cluster := s.makeGatewayCluster(cfgSnap, opts) - switch cfgSnap.Kind { - case structs.ServiceKindTerminatingGateway: - injectTerminatingGatewayTLSContext(cfgSnap, cluster, svc) - - if err := injectLBToCluster(loadBalancer, cluster); err != nil { - return nil, fmt.Errorf("failed to apply load balancer configuration to cluster %q: %v", clusterName, err) - } - - case structs.ServiceKindMeshGateway: - // We can't apply hash based LB config to mesh gateways because they rely on inspecting HTTP attributes - // and mesh gateways do not decrypt traffic - if !loadBalancer.IsHashBased() { - if err := injectLBToCluster(loadBalancer, cluster); err != nil { - return nil, fmt.Errorf("failed to apply load balancer configuration to cluster %q: %v", clusterName, err) - } - } + if err := s.injectGatewayServiceAddons(cfgSnap, cluster, svc, loadBalancer); err != nil { + return nil, err } clusters = append(clusters, cluster) } @@ -286,6 +257,35 @@ func (s *Server) makeGatewayServiceClusters(cfgSnap *proxycfg.ConfigSnapshot) ([ return clusters, nil } +func (s *Server) injectGatewayServiceAddons(cfgSnap *proxycfg.ConfigSnapshot, c *envoy.Cluster, svc structs.ServiceName, lb *structs.LoadBalancer) error { + switch cfgSnap.Kind { + case structs.ServiceKindMeshGateway: + // We can't apply hash based LB config to mesh gateways because they rely on inspecting HTTP attributes + // and mesh gateways do not decrypt traffic + if !lb.IsHashBased() { + if err := injectLBToCluster(lb, c); err != nil { + return fmt.Errorf("failed to apply load balancer configuration to cluster %q: %v", c.Name, err) + } + } + case structs.ServiceKindTerminatingGateway: + // Context used for TLS origination to the cluster + if mapping, ok := cfgSnap.TerminatingGateway.GatewayServices[svc]; ok && mapping.CAFile != "" { + context := envoyauth.UpstreamTlsContext{ + CommonTlsContext: makeCommonTLSContextFromFiles(mapping.CAFile, mapping.CertFile, mapping.KeyFile), + } + if mapping.SNI != "" { + context.Sni = mapping.SNI + } + c.TlsContext = &context + } + if err := injectLBToCluster(lb, c); err != nil { + return fmt.Errorf("failed to apply load balancer configuration to cluster %q: %v", c.Name, err) + } + + } + return nil +} + func (s *Server) clustersFromSnapshotIngressGateway(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { var clusters []proto.Message createdClusters := make(map[string]bool) diff --git a/agent/xds/routes.go b/agent/xds/routes.go index b31e1fcfc8..ad20f5f189 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -3,7 +3,6 @@ package xds import ( "errors" "fmt" - "github.com/hashicorp/consul/logging" "net" "strings" @@ -15,6 +14,7 @@ import ( "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/proxycfg" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/logging" ) // routesFromSnapshot returns the xDS API representation of the "routes" in the @@ -36,6 +36,47 @@ func (s *Server) routesFromSnapshot(cInfo connectionInfo, cfgSnap *proxycfg.Conf } } +// routesFromSnapshotConnectProxy returns the xDS API representation of the +// "routes" in the snapshot. +func routesForConnectProxy( + cInfo connectionInfo, + upstreams structs.Upstreams, + chains map[string]*structs.CompiledDiscoveryChain, +) ([]proto.Message, error) { + + var resources []proto.Message + for _, u := range upstreams { + upstreamID := u.Identifier() + + var chain *structs.CompiledDiscoveryChain + if u.DestinationType != structs.UpstreamDestTypePreparedQuery { + chain = chains[upstreamID] + } + + if chain == nil || chain.IsDefault() { + // TODO(rb): make this do the old school stuff too + } else { + virtualHost, err := makeUpstreamRouteForDiscoveryChain(cInfo, upstreamID, chain, []string{"*"}) + if err != nil { + return nil, err + } + + route := &envoy.RouteConfiguration{ + Name: upstreamID, + VirtualHosts: []*envoyroute.VirtualHost{virtualHost}, + // ValidateClusters defaults to true when defined statically and false + // when done via RDS. Re-set the sane value of true to prevent + // null-routing traffic. + ValidateClusters: makeBoolValue(true), + } + resources = append(resources, route) + } + } + + // TODO(rb): make sure we don't generate an empty result + return resources, nil +} + // routesFromSnapshotTerminatingGateway returns the xDS API representation of the "routes" in the snapshot. // For any HTTP service we will return a default route. func (s *Server) routesFromSnapshotTerminatingGateway(_ connectionInfo, cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) { @@ -119,47 +160,6 @@ func makeNamedDefaultRouteWithLB(clusterName string, lb *structs.LoadBalancer) ( }, nil } -// routesFromSnapshotConnectProxy returns the xDS API representation of the -// "routes" in the snapshot. -func routesForConnectProxy( - cInfo connectionInfo, - upstreams structs.Upstreams, - chains map[string]*structs.CompiledDiscoveryChain, -) ([]proto.Message, error) { - - var resources []proto.Message - for _, u := range upstreams { - upstreamID := u.Identifier() - - var chain *structs.CompiledDiscoveryChain - if u.DestinationType != structs.UpstreamDestTypePreparedQuery { - chain = chains[upstreamID] - } - - if chain == nil || chain.IsDefault() { - // TODO(rb): make this do the old school stuff too - } else { - virtualHost, err := makeUpstreamRouteForDiscoveryChain(cInfo, upstreamID, chain, []string{"*"}) - if err != nil { - return nil, err - } - - route := &envoy.RouteConfiguration{ - Name: upstreamID, - VirtualHosts: []*envoyroute.VirtualHost{virtualHost}, - // ValidateClusters defaults to true when defined statically and false - // when done via RDS. Re-set the sane value of true to prevent - // null-routing traffic. - ValidateClusters: makeBoolValue(true), - } - resources = append(resources, route) - } - } - - // TODO(rb): make sure we don't generate an empty result - return resources, nil -} - // routesForIngressGateway returns the xDS API representation of the // "routes" in the snapshot. func routesForIngressGateway( @@ -262,8 +262,6 @@ func makeUpstreamRouteForDiscoveryChain( return nil, fmt.Errorf("missing first node in compiled discovery chain for: %s", chain.ServiceName) } - var lb *structs.LoadBalancer - switch startNode.Type { case structs.DiscoveryGraphNodeTypeRouter: routes = make([]*envoyroute.Route, 0, len(startNode.Routes)) @@ -277,6 +275,8 @@ func makeUpstreamRouteForDiscoveryChain( ) nextNode := chain.Nodes[discoveryRoute.NextNode] + + var lb *structs.LoadBalancer if nextNode.LoadBalancer != nil { lb = nextNode.LoadBalancer } @@ -350,6 +350,7 @@ func makeUpstreamRouteForDiscoveryChain( return nil, err } + var lb *structs.LoadBalancer if startNode.LoadBalancer != nil { lb = startNode.LoadBalancer } @@ -367,6 +368,7 @@ func makeUpstreamRouteForDiscoveryChain( case structs.DiscoveryGraphNodeTypeResolver: routeAction := makeRouteActionForChainCluster(startNode.Resolver.Target, chain) + var lb *structs.LoadBalancer if startNode.LoadBalancer != nil { lb = startNode.LoadBalancer }