Refactor Envoy bootstrap from BuildFromConfig() to Builder{}.Build() + health probe tests (#4858)

* leverage builder pattern for building the bootstrap config

Signed-off-by: Sean Teeling <[email protected]>

* address comments

Signed-off-by: Sean Teeling <[email protected]>

Author: steeling

pkg/envoy/bootstrap/config.go

@@ -16,7 +16,6 @@ import (
 	"github.com/golang/protobuf/ptypes/any"
 	"google.golang.org/protobuf/types/known/anypb"
 
-	"github.com/openservicemesh/osm/pkg/configurator"
 	"github.com/openservicemesh/osm/pkg/constants"
 	"github.com/openservicemesh/osm/pkg/envoy"
 	"github.com/openservicemesh/osm/pkg/errcode"
@@ -115,8 +114,8 @@ func BuildValidationSecret() (*xds_discovery.DiscoveryResponse, error) {
 	}, nil
 }
 
-// BuildFromConfig builds and returns an Envoy Bootstrap object from the given config
-func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
+// Build builds and returns an Envoy Bootstrap object from the given config
+func (b *Builder) Build() (*xds_bootstrap.Bootstrap, error) {
 	httpProtocolOptions := &xds_upstream_http.HttpProtocolOptions{
 		UpstreamProtocolOptions: &xds_upstream_http.HttpProtocolOptions_ExplicitHttpConfig_{
 			ExplicitHttpConfig: &xds_upstream_http.HttpProtocolOptions_ExplicitHttpConfig{
@@ -139,8 +138,8 @@ func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
 		return nil, err
 	}
 
-	minVersionInt := xds_transport_sockets.TlsParameters_TlsProtocol_value[config.TLSMinProtocolVersion]
-	maxVersionInt := xds_transport_sockets.TlsParameters_TlsProtocol_value[config.TLSMaxProtocolVersion]
+	minVersionInt := xds_transport_sockets.TlsParameters_TlsProtocol_value[b.TLSMinProtocolVersion]
+	maxVersionInt := xds_transport_sockets.TlsParameters_TlsProtocol_value[b.TLSMaxProtocolVersion]
 	tlsMinVersion := xds_transport_sockets.TlsParameters_TlsProtocol(minVersionInt)
 	tlsMaxVersion := xds_transport_sockets.TlsParameters_TlsProtocol(maxVersionInt)
 
@@ -162,8 +161,8 @@ func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
 			TlsParams: &xds_transport_sockets.TlsParameters{
 				TlsMinimumProtocolVersion: tlsMinVersion,
 				TlsMaximumProtocolVersion: tlsMaxVersion,
-				CipherSuites:              config.CipherSuites,
-				EcdhCurves:                config.ECDHCurves,
+				CipherSuites:              b.CipherSuites,
+				EcdhCurves:                b.ECDHCurves,
 			},
 			TlsCertificateSdsSecretConfigs: []*xds_transport_sockets.SdsSecretConfig{
 				{
@@ -186,7 +185,7 @@ func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
 
 	bootstrap := &xds_bootstrap.Bootstrap{
 		Node: &xds_core.Node{
-			Id: config.NodeID,
+			Id: b.NodeID,
 		},
 		Admin: &xds_bootstrap.Admin{
 			AccessLog: []*xds_accesslog_config.AccessLog{
@@ -202,7 +201,7 @@ func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
 					SocketAddress: &xds_core.SocketAddress{
 						Address: constants.LocalhostIPAddress,
 						PortSpecifier: &xds_core.SocketAddress_PortValue{
-							PortValue: config.AdminPort,
+							PortValue: constants.EnvoyAdminPort,
 						},
 					},
 				},
@@ -216,7 +215,7 @@ func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
 					{
 						TargetSpecifier: &xds_core.GrpcService_EnvoyGrpc_{
 							EnvoyGrpc: &xds_core.GrpcService_EnvoyGrpc{
-								ClusterName: config.XDSClusterName,
+								ClusterName: constants.OSMControllerName,
 							},
 						},
 					},
@@ -239,7 +238,7 @@ func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
 		StaticResources: &xds_bootstrap.Bootstrap_StaticResources{
 			Clusters: []*xds_cluster.Cluster{
 				{
-					Name: config.XDSClusterName,
+					Name: constants.OSMControllerName,
 					ClusterDiscoveryType: &xds_cluster.Cluster_Type{
 						Type: xds_cluster.Cluster_LOGICAL_DNS,
 					},
@@ -254,7 +253,7 @@ func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
 					},
 					LbPolicy: xds_cluster.Cluster_ROUND_ROBIN,
 					LoadAssignment: &xds_endpoint.ClusterLoadAssignment{
-						ClusterName: config.XDSClusterName,
+						ClusterName: constants.OSMControllerName,
 						Endpoints: []*xds_endpoint.LocalityLbEndpoints{
 							{
 								LbEndpoints: []*xds_endpoint.LbEndpoint{
@@ -264,9 +263,9 @@ func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
 												Address: &xds_core.Address{
 													Address: &xds_core.Address_SocketAddress{
 														SocketAddress: &xds_core.SocketAddress{
-															Address: config.XDSHost,
+															Address: b.XDSHost,
 															PortSpecifier: &xds_core.SocketAddress_PortValue{
-																PortValue: config.XDSPort,
+																PortValue: constants.ADSServerPort,
 															},
 														},
 													},
@@ -283,35 +282,14 @@ func BuildFromConfig(config Config) (*xds_bootstrap.Bootstrap, error) {
 		},
 	}
 
-	return bootstrap, nil
-}
-
-// GenerateEnvoyConfig generates an envoy bootstrap configuration from the given metadata.
-func GenerateEnvoyConfig(config EnvoyBootstrapConfigMeta, cfg configurator.Configurator) (*xds_bootstrap.Bootstrap, error) {
-	bootstrapConfig, err := BuildFromConfig(Config{
-		NodeID:                config.NodeID,
-		AdminPort:             constants.EnvoyAdminPort,
-		XDSClusterName:        constants.OSMControllerName,
-		XDSHost:               config.XDSHost,
-		XDSPort:               config.XDSPort,
-		TLSMinProtocolVersion: config.TLSMinProtocolVersion,
-		TLSMaxProtocolVersion: config.TLSMaxProtocolVersion,
-		CipherSuites:          config.CipherSuites,
-		ECDHCurves:            config.ECDHCurves,
-	})
-	if err != nil {
-		log.Error().Err(err).Msgf("Error building Envoy boostrap config")
-		return nil, err
-	}
-
-	probeListeners, probeClusters, err := getProbeResources(config)
+	probeListeners, probeClusters, err := b.getProbeResources()
 	if err != nil {
 		return nil, err
 	}
-	bootstrapConfig.StaticResources.Listeners = append(bootstrapConfig.StaticResources.Listeners, probeListeners...)
-	bootstrapConfig.StaticResources.Clusters = append(bootstrapConfig.StaticResources.Clusters, probeClusters...)
+	bootstrap.StaticResources.Listeners = append(bootstrap.StaticResources.Listeners, probeListeners...)
+	bootstrap.StaticResources.Clusters = append(bootstrap.StaticResources.Clusters, probeClusters...)
 
-	return bootstrapConfig, nil
+	return bootstrap, nil
 }
 
 // GetTLSSDSConfigYAML returns the statically used TLS SDS config YAML.
@@ -353,42 +331,42 @@ func GetValidationContextSDSConfigYAML() ([]byte, error) {
 // These will not change during the lifetime of the Pod.
 // If the original probe defined a TCPSocket action, listener and cluster objects are not configured
 // to serve that probe.
-func getProbeResources(config EnvoyBootstrapConfigMeta) ([]*xds_listener.Listener, []*xds_cluster.Cluster, error) {
+func (b *Builder) getProbeResources() ([]*xds_listener.Listener, []*xds_cluster.Cluster, error) {
 	// This slice is the list of listeners for liveness, readiness, startup IF these have been configured in the Pod Spec
 	var listeners []*xds_listener.Listener
 	var clusters []*xds_cluster.Cluster
 
 	// Is there a liveness probe in the Pod Spec?
-	if config.OriginalHealthProbes.Liveness != nil && !config.OriginalHealthProbes.Liveness.IsTCPSocket {
-		listener, err := getLivenessListener(config.OriginalHealthProbes.Liveness)
+	if b.OriginalHealthProbes.Liveness != nil && !b.OriginalHealthProbes.Liveness.IsTCPSocket {
+		listener, err := getLivenessListener(b.OriginalHealthProbes.Liveness)
 		if err != nil {
 			log.Error().Err(err).Msgf("Error getting liveness listener")
 			return nil, nil, err
 		}
 		listeners = append(listeners, listener)
-		clusters = append(clusters, getLivenessCluster(config.OriginalHealthProbes.Liveness))
+		clusters = append(clusters, getLivenessCluster(b.OriginalHealthProbes.Liveness))
 	}
 
 	// Is there a readiness probe in the Pod Spec?
-	if config.OriginalHealthProbes.Readiness != nil && !config.OriginalHealthProbes.Readiness.IsTCPSocket {
-		listener, err := getReadinessListener(config.OriginalHealthProbes.Readiness)
+	if b.OriginalHealthProbes.Readiness != nil && !b.OriginalHealthProbes.Readiness.IsTCPSocket {
+		listener, err := getReadinessListener(b.OriginalHealthProbes.Readiness)
 		if err != nil {
 			log.Error().Err(err).Msgf("Error getting readiness listener")
 			return nil, nil, err
 		}
 		listeners = append(listeners, listener)
-		clusters = append(clusters, getReadinessCluster(config.OriginalHealthProbes.Readiness))
+		clusters = append(clusters, getReadinessCluster(b.OriginalHealthProbes.Readiness))
 	}
 
 	// Is there a startup probe in the Pod Spec?
-	if config.OriginalHealthProbes.Startup != nil && !config.OriginalHealthProbes.Startup.IsTCPSocket {
-		listener, err := getStartupListener(config.OriginalHealthProbes.Startup)
+	if b.OriginalHealthProbes.Startup != nil && !b.OriginalHealthProbes.Startup.IsTCPSocket {
+		listener, err := getStartupListener(b.OriginalHealthProbes.Startup)
 		if err != nil {
 			log.Error().Err(err).Msgf("Error getting startup listener")
 			return nil, nil, err
 		}
 		listeners = append(listeners, listener)
-		clusters = append(clusters, getStartupCluster(config.OriginalHealthProbes.Startup))
+		clusters = append(clusters, getStartupCluster(b.OriginalHealthProbes.Startup))
 	}
 
 	return listeners, clusters, nil