expose envoy cluster options in policy (#1804)

internal/cmd/pomerium/pomerium.go

@@ -64,8 +64,15 @@ func Run(ctx context.Context, configFile string) error {
 	if err != nil {
 		return fmt.Errorf("error creating control plane: %w", err)
 	}
-	src.OnConfigChange(controlPlane.OnConfigChange)
-	controlPlane.OnConfigChange(src.GetConfig())
+	src.OnConfigChange(func(cfg *config.Config) {
+		if err := controlPlane.OnConfigChange(cfg); err != nil {
+			log.Error().Err(err).Msg("config change")
+		}
+	})
+
+	if err = controlPlane.OnConfigChange(src.GetConfig()); err != nil {
+		return fmt.Errorf("applying config: %w", err)
+	}
 
 	_, grpcPort, _ := net.SplitHostPort(controlPlane.GRPCListener.Addr().String())
 	_, httpPort, _ := net.SplitHostPort(controlPlane.HTTPListener.Addr().String())

internal/controlplane/constants.go

@@ -0,0 +1,23 @@
+package controlplane
+
+import (
+	"errors"
+	"time"
+
+	envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
+	"github.com/golang/protobuf/ptypes"
+)
+
+var (
+	errNoEndpoints           = errors.New("cluster must have endpoints")
+	defaultConnectionTimeout = ptypes.DurationProto(time.Second * 10)
+)
+
+// newDefaultEnvoyClusterConfig creates envoy cluster with certain default values
+func newDefaultEnvoyClusterConfig() *envoy_config_cluster_v3.Cluster {
+	return &envoy_config_cluster_v3.Cluster{
+		ConnectTimeout:  defaultConnectionTimeout,
+		RespectDnsTtl:   true,
+		DnsLookupFamily: envoy_config_cluster_v3.Cluster_AUTO,
+	}
+}

internal/controlplane/server.go

@@ -88,7 +88,12 @@ func NewServer(name string) (*Server, error) {
 	srv.HTTPRouter = mux.NewRouter()
 	srv.addHTTPMiddleware()
 
-	srv.xdsmgr = xdsmgr.NewManager(srv.buildDiscoveryResources())
+	res, err := srv.buildDiscoveryResources()
+	if err != nil {
+		return nil, err
+	}
+
+	srv.xdsmgr = xdsmgr.NewManager(res)
 	envoy_service_discovery_v3.RegisterAggregatedDiscoveryServiceServer(srv.GRPCServer, srv.xdsmgr)
 
 	srv.filemgr = filemgr.NewManager()
@@ -158,11 +163,16 @@ func (srv *Server) Run(ctx context.Context) error {
 }
 
 // OnConfigChange updates the pomerium config options.
-func (srv *Server) OnConfigChange(cfg *config.Config) {
+func (srv *Server) OnConfigChange(cfg *config.Config) error {
 	prev := srv.currentConfig.Load()
 	srv.currentConfig.Store(versionedConfig{
 		Config:  cfg,
 		version: prev.version + 1,
 	})
-	srv.xdsmgr.Update(srv.buildDiscoveryResources())
+	res, err := srv.buildDiscoveryResources()
+	if err != nil {
+		return err
+	}
+	srv.xdsmgr.Update(res)
+	return nil
 }

internal/controlplane/xds.go

@@ -31,10 +31,15 @@ const (
 	listenerTypeURL = "type.googleapis.com/envoy.config.listener.v3.Listener"
 )
 
-func (srv *Server) buildDiscoveryResources() map[string][]*envoy_service_discovery_v3.Resource {
+func (srv *Server) buildDiscoveryResources() (map[string][]*envoy_service_discovery_v3.Resource, error) {
 	resources := map[string][]*envoy_service_discovery_v3.Resource{}
 	cfg := srv.currentConfig.Load()
-	for _, cluster := range srv.buildClusters(cfg.Options) {
+
+	clusters, err := srv.buildClusters(cfg.Options)
+	if err != nil {
+		return nil, err
+	}
+	for _, cluster := range clusters {
 		any, _ := anypb.New(cluster)
 		resources[clusterTypeURL] = append(resources[clusterTypeURL], &envoy_service_discovery_v3.Resource{
 			Name:     cluster.Name,
@@ -50,7 +55,7 @@ func (srv *Server) buildDiscoveryResources() map[string][]*envoy_service_discove
 			Resource: any,
 		})
 	}
-	return resources
+	return resources, nil
 }
 
 func buildAccessLogs(options *config.Options) []*envoy_config_accesslog_v3.AccessLog {

internal/controlplane/xds_cluster_test.go

@@ -8,6 +8,7 @@ import (
 
 	envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/types/known/wrapperspb"
 
 	"github.com/pomerium/pomerium/config"
@@ -212,9 +213,10 @@ func Test_buildCluster(t *testing.T) {
 		endpoints := srv.buildPolicyEndpoints(&config.Policy{
 			Destinations: mustParseURLs("http://example.com"),
 		})
-		cluster := buildCluster("example", endpoints, true,
-			config.GetEnvoyDNSLookupFamily(config.DNSLookupFamilyV4Only),
-			nil, nil)
+		cluster := newDefaultEnvoyClusterConfig()
+		cluster.DnsLookupFamily = envoy_config_cluster_v3.Cluster_V4_ONLY
+		err := buildCluster(cluster, "example", endpoints, true)
+		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
 				"name": "example",
@@ -251,9 +253,9 @@ func Test_buildCluster(t *testing.T) {
 				"https://example.com",
 			),
 		})
-		cluster := buildCluster("example", endpoints, true,
-			config.GetEnvoyDNSLookupFamily(config.DNSLookupFamilyAuto),
-			nil, nil)
+		cluster := newDefaultEnvoyClusterConfig()
+		err := buildCluster(cluster, "example", endpoints, true)
+		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
 				"name": "example",
@@ -342,9 +344,9 @@ func Test_buildCluster(t *testing.T) {
 		endpoints := srv.buildPolicyEndpoints(&config.Policy{
 			Destinations: mustParseURLs("http://127.0.0.1"),
 		})
-		cluster := buildCluster("example", endpoints, true,
-			config.GetEnvoyDNSLookupFamily(config.DNSLookupFamilyAuto),
-			nil, nil)
+		cluster := newDefaultEnvoyClusterConfig()
+		err := buildCluster(cluster, "example", endpoints, true)
+		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
 				"name": "example",
@@ -377,9 +379,9 @@ func Test_buildCluster(t *testing.T) {
 		endpoints := srv.buildPolicyEndpoints(&config.Policy{
 			Destinations: mustParseURLs("http://localhost"),
 		})
-		cluster := buildCluster("example", endpoints, true,
-			config.GetEnvoyDNSLookupFamily(config.DNSLookupFamilyAuto),
-			nil, nil)
+		cluster := newDefaultEnvoyClusterConfig()
+		err := buildCluster(cluster, "example", endpoints, true)
+		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
 				"name": "example",
@@ -412,12 +414,14 @@ func Test_buildCluster(t *testing.T) {
 		endpoints := srv.buildPolicyEndpoints(&config.Policy{
 			Destinations: mustParseURLs("http://example.com"),
 		})
-		cluster := buildCluster("example", endpoints, true,
-			config.GetEnvoyDNSLookupFamily(config.DNSLookupFamilyV4Only),
-			&envoy_config_cluster_v3.OutlierDetection{
-				EnforcingConsecutive_5Xx:       wrapperspb.UInt32(17),
-				SplitExternalLocalOriginErrors: true,
-			}, nil)
+		cluster := newDefaultEnvoyClusterConfig()
+		cluster.DnsLookupFamily = envoy_config_cluster_v3.Cluster_V4_ONLY
+		cluster.OutlierDetection = &envoy_config_cluster_v3.OutlierDetection{
+			EnforcingConsecutive_5Xx:       wrapperspb.UInt32(17),
+			SplitExternalLocalOriginErrors: true,
+		}
+		err := buildCluster(cluster, "example", endpoints, true)
+		require.NoErrorf(t, err, "cluster %+v", cluster)
 		testutil.AssertProtoJSONEqual(t, `
 			{
 				"name": "example",

internal/controlplane/xds_clusters.go

@@ -2,17 +2,16 @@ package controlplane
 
 import (
 	"encoding/base64"
+	"fmt"
 	"net"
 	"net/url"
 	"strings"
-	"time"
 
 	envoy_config_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
 	envoy_config_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
 	envoy_config_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
 	envoy_extensions_transport_sockets_tls_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
 	envoy_type_matcher_v3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
-	"github.com/golang/protobuf/ptypes"
 	"github.com/martinlindhe/base36"
 	"google.golang.org/protobuf/types/known/structpb"
 
@@ -42,7 +41,7 @@ func (e Endpoint) TransportSocketName() string {
 	return "ts-" + base36.EncodeBytes(h)
 }
 
-func (srv *Server) buildClusters(options *config.Options) []*envoy_config_cluster_v3.Cluster {
+func (srv *Server) buildClusters(options *config.Options) ([]*envoy_config_cluster_v3.Cluster, error) {
 	grpcURL := &url.URL{
 		Scheme: "http",
 		Host:   srv.GRPCListener.Addr().String(),
@@ -56,40 +55,73 @@ func (srv *Server) buildClusters(options *config.Options) []*envoy_config_cluste
 		Host:   options.GetAuthorizeURL().Host,
 	}
 
-	clusters := []*envoy_config_cluster_v3.Cluster{
-		srv.buildInternalCluster(options, "pomerium-control-plane-grpc", grpcURL, true),
-		srv.buildInternalCluster(options, "pomerium-control-plane-http", httpURL, false),
+	controlGRPC, err := srv.buildInternalCluster(options, "pomerium-control-plane-grpc", grpcURL, true)
+	if err != nil {
+		return nil, err
+	}
+	controlHTTP, err := srv.buildInternalCluster(options, "pomerium-control-plane-http", httpURL, false)
+	if err != nil {
+		return nil, err
+	}
+	authZ, err := srv.buildInternalCluster(options, authzURL.Host, authzURL, true)
+	if err != nil {
+		return nil, err
 	}
 
-	clusters = append(clusters, srv.buildInternalCluster(options, authzURL.Host, authzURL, true))
+	clusters := []*envoy_config_cluster_v3.Cluster{
+		controlGRPC,
+		controlHTTP,
+		authZ,
+	}
 
 	if config.IsProxy(options.Services) {
 		for i := range options.Policies {
 			policy := options.Policies[i]
+			if policy.EnvoyOpts == nil {
+				policy.EnvoyOpts = newDefaultEnvoyClusterConfig()
+			}
 			if len(policy.Destinations) > 0 {
-				clusters = append(clusters, srv.buildPolicyCluster(options, &policy))
+				cluster, err := srv.buildPolicyCluster(options, &policy)
+				if err != nil {
+					return nil, fmt.Errorf("policy #%d: %w", i, err)
+				}
+				clusters = append(clusters, cluster)
 			}
 		}
 	}
 
-	return clusters
+	return clusters, nil
 }
 
-func (srv *Server) buildInternalCluster(options *config.Options, name string, dst *url.URL, forceHTTP2 bool) *envoy_config_cluster_v3.Cluster {
+func (srv *Server) buildInternalCluster(options *config.Options, name string, dst *url.URL, forceHTTP2 bool) (*envoy_config_cluster_v3.Cluster, error) {
+	cluster := newDefaultEnvoyClusterConfig()
+	cluster.DnsLookupFamily = config.GetEnvoyDNSLookupFamily(options.DNSLookupFamily)
 	endpoints := []Endpoint{NewEndpoint(dst, srv.buildInternalTransportSocket(options, dst))}
-	dnsLookupFamily := config.GetEnvoyDNSLookupFamily(options.DNSLookupFamily)
-	return buildCluster(name, endpoints, forceHTTP2, dnsLookupFamily, nil, nil)
+	if err := buildCluster(cluster, name, endpoints, forceHTTP2); err != nil {
+		return nil, err
+	}
+	return cluster, nil
 }
 
-func (srv *Server) buildPolicyCluster(options *config.Options, policy *config.Policy) *envoy_config_cluster_v3.Cluster {
+func (srv *Server) buildPolicyCluster(options *config.Options, policy *config.Policy) (*envoy_config_cluster_v3.Cluster, error) {
+	cluster := policy.EnvoyOpts
+
 	name := getPolicyName(policy)
 	endpoints := srv.buildPolicyEndpoints(policy)
-	dnsLookupFamily := config.GetEnvoyDNSLookupFamily(options.DNSLookupFamily)
-	if policy.EnableGoogleCloudServerlessAuthentication {
-		dnsLookupFamily = envoy_config_cluster_v3.Cluster_V4_ONLY
+
+	if cluster.DnsLookupFamily == envoy_config_cluster_v3.Cluster_AUTO {
+		cluster.DnsLookupFamily = config.GetEnvoyDNSLookupFamily(options.DNSLookupFamily)
 	}
-	outlierDetection := (*envoy_config_cluster_v3.OutlierDetection)(policy.OutlierDetection)
-	return buildCluster(name, endpoints, false, dnsLookupFamily, outlierDetection, policy.HealthCheck)
+
+	if policy.EnableGoogleCloudServerlessAuthentication {
+		cluster.DnsLookupFamily = envoy_config_cluster_v3.Cluster_V4_ONLY
+	}
+
+	if err := buildCluster(cluster, name, endpoints, false); err != nil {
+		return nil, err
+	}
+
+	return cluster, nil
 }
 
 func (srv *Server) buildPolicyEndpoints(policy *config.Policy) []Endpoint {
@@ -230,36 +262,28 @@ func (srv *Server) buildPolicyValidationContext(policy *config.Policy, dst *url.
 }
 
 func buildCluster(
+	cluster *envoy_config_cluster_v3.Cluster,
 	name string,
 	endpoints []Endpoint,
 	forceHTTP2 bool,
-	dnsLookupFamily envoy_config_cluster_v3.Cluster_DnsLookupFamily,
-	outlierDetection *envoy_config_cluster_v3.OutlierDetection,
-	healthCheck *envoy_config_core_v3.HealthCheck,
-) *envoy_config_cluster_v3.Cluster {
+) error {
 	if len(endpoints) == 0 {
-		return nil
+		return errNoEndpoints
 	}
 
+	if cluster.ConnectTimeout == nil {
+		cluster.ConnectTimeout = defaultConnectionTimeout
+	}
+	cluster.RespectDnsTtl = true
 	lbEndpoints := buildLbEndpoints(endpoints)
-	cluster := &envoy_config_cluster_v3.Cluster{
-		Name:           name,
-		ConnectTimeout: ptypes.DurationProto(time.Second * 10),
-		LoadAssignment: &envoy_config_endpoint_v3.ClusterLoadAssignment{
-			ClusterName: name,
-			Endpoints: []*envoy_config_endpoint_v3.LocalityLbEndpoints{{
-				LbEndpoints: lbEndpoints,
-			}},
-		},
-		RespectDnsTtl:          true,
-		TransportSocketMatches: buildTransportSocketMatches(endpoints),
-		DnsLookupFamily:        dnsLookupFamily,
-		OutlierDetection:       outlierDetection,
-	}
-
-	if healthCheck != nil {
-		cluster.HealthChecks = append(cluster.HealthChecks, healthCheck)
+	cluster.Name = name
+	cluster.LoadAssignment = &envoy_config_endpoint_v3.ClusterLoadAssignment{
+		ClusterName: name,
+		Endpoints: []*envoy_config_endpoint_v3.LocalityLbEndpoints{{
+			LbEndpoints: lbEndpoints,
+		}},
 	}
+	cluster.TransportSocketMatches = buildTransportSocketMatches(endpoints)
 
 	if forceHTTP2 {
 		cluster.Http2ProtocolOptions = &envoy_config_core_v3.Http2ProtocolOptions{
@@ -280,7 +304,7 @@ func buildCluster(
 		cluster.ClusterDiscoveryType = &envoy_config_cluster_v3.Cluster_Type{Type: envoy_config_cluster_v3.Cluster_STRICT_DNS}
 	}
 
-	return cluster
+	return cluster.Validate()
 }
 
 func buildLbEndpoints(endpoints []Endpoint) []*envoy_config_endpoint_v3.LbEndpoint {