Skip to content

Commit

Permalink
Refactor internal/envoy/v3 package functions
Browse files Browse the repository at this point in the history 
Take two of #5523

That introduces a struct `NewEnvoysGen` that allows us to modify the
behaviour of Envoy config generation. This will help with

#6806

On the debate on `singleton` vs `struct` approach. I thought this again
and I realized that `struct` is probably worth the big diff here. For the
following reason:

* It allows us to test the behaviour much easier. With a singleton it would be
impossible for higher level tests like tests in `internal/featuretests` to modify
the behaviour of the Envoy config generation. With a struct, we can just pass the
config options to the struct and test the behaviour.

Signed-off-by: Sotiris Nanopoulos <[email protected]>
  • Loading branch information
@davinci26
davinci26 committed Jan 20, 2025
1 parent 2b05fca commit 928dcff
Show file tree
Hide file tree
Showing 30 changed files with 579 additions and 318 deletions.
5 changes: 4 additions & 1 deletion cmd/contour/contour.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ func main() {
if err := envoy.ValidAdminAddress(bootstrapCtx.AdminAddress); err != nil {
log.WithField("flag", "--admin-address").WithError(err).Fatal("failed to parse bootstrap args")
}
if err := envoy_v3.WriteBootstrap(bootstrapCtx); err != nil {
envoyGen := envoy_v3.NewEnvoysGen(envoy_v3.EnvoyGenOpt{
XDSClusterName: envoy_v3.DefaultXDSClusterName,
})
if err := envoyGen.WriteBootstrap(bootstrapCtx); err != nil {

Check warning on line 110 in cmd/contour/contour.go

View check run for this annotation

Codecov / codecov/patch

cmd/contour/contour.go#L107-L110 

Added lines #L107 - L110 were not covered by tests
log.WithError(err).Fatal("failed to write bootstrap configuration")
}
case certgenApp.FullCommand():
Expand Down
8 changes: 6 additions & 2 deletions cmd/contour/serve.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -493,11 +493,15 @@ func (s *Server) doServe() error {
endpointHandler = xdscache_v3.NewEndpointsTranslator(s.log.WithField("context", "endpointstranslator"))
}

envoyGen := envoy_v3.NewEnvoysGen(envoy_v3.EnvoyGenOpt{
XDSClusterName: envoy_v3.DefaultXDSClusterName,
})

Check warning on line 499 in cmd/contour/serve.go

View check run for this annotation

Codecov / codecov/patch

cmd/contour/serve.go#L496-L499 

Added lines #L496 - L499 were not covered by tests
resources := []xdscache.ResourceCache{
xdscache_v3.NewListenerCache(listenerConfig, *contourConfiguration.Envoy.Metrics, *contourConfiguration.Envoy.Health, *contourConfiguration.Envoy.Network.EnvoyAdminPort),
xdscache_v3.NewListenerCache(listenerConfig, *contourConfiguration.Envoy.Metrics, *contourConfiguration.Envoy.Health, *contourConfiguration.Envoy.Network.EnvoyAdminPort, envoyGen),

Check warning on line 501 in cmd/contour/serve.go

View check run for this annotation

Codecov / codecov/patch

cmd/contour/serve.go#L501 

Added line #L501 was not covered by tests
xdscache_v3.NewSecretsCache(envoy_v3.StatsSecrets(contourConfiguration.Envoy.Metrics.TLS)),
&xdscache_v3.RouteCache{},
&xdscache_v3.ClusterCache{},
xdscache_v3.NewClusterCache(envoyGen),

Check warning on line 504 in cmd/contour/serve.go

View check run for this annotation

Codecov / codecov/patch

cmd/contour/serve.go#L504 

Added line #L504 was not covered by tests
endpointHandler,
xdscache_v3.NewRuntimeCache(xdscache_v3.ConfigurableRuntimeSettings{
MaxRequestsPerIOCycle: contourConfiguration.Envoy.Listener.MaxRequestsPerIOCycle,
Expand Down
8 changes: 4 additions & 4 deletions internal/envoy/v3/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ import (
// UpstreamTLSContext creates an envoy_transport_socket_tls_v3.UpstreamTlsContext. By default
// UpstreamTLSContext returns a HTTP/1.1 TLS enabled context. A list of
// additional ALPN protocols can be provided.
func UpstreamTLSContext(peerValidationContext *dag.PeerValidationContext, sni string, clientSecret *dag.Secret, upstreamTLS *dag.UpstreamTLS, alpnProtocols ...string) *envoy_transport_socket_tls_v3.UpstreamTlsContext {
func (e *EnvoyGen) UpstreamTLSContext(peerValidationContext *dag.PeerValidationContext, sni string, clientSecret *dag.Secret, upstreamTLS *dag.UpstreamTLS, alpnProtocols ...string) *envoy_transport_socket_tls_v3.UpstreamTlsContext {
var clientSecretConfigs []*envoy_transport_socket_tls_v3.SdsSecretConfig
if clientSecret != nil {
clientSecretConfigs = []*envoy_transport_socket_tls_v3.SdsSecretConfig{{
Name: envoy.Secretname(clientSecret),
SdsConfig: ConfigSource("contour"),
SdsConfig: e.GetConfigSource(),
}}
}

Expand Down Expand Up @@ -115,7 +115,7 @@ func validationContext(ca []byte, subjectNames []string, skipVerifyPeerCert bool
}

// DownstreamTLSContext creates a new DownstreamTlsContext.
func DownstreamTLSContext(serverSecret *dag.Secret, tlsMinProtoVersion, tlsMaxProtoVersion envoy_transport_socket_tls_v3.TlsParameters_TlsProtocol, cipherSuites []string, peerValidationContext *dag.PeerValidationContext, alpnProtos ...string) *envoy_transport_socket_tls_v3.DownstreamTlsContext {
func (e *EnvoyGen) DownstreamTLSContext(serverSecret *dag.Secret, tlsMinProtoVersion, tlsMaxProtoVersion envoy_transport_socket_tls_v3.TlsParameters_TlsProtocol, cipherSuites []string, peerValidationContext *dag.PeerValidationContext, alpnProtos ...string) *envoy_transport_socket_tls_v3.DownstreamTlsContext {
context := &envoy_transport_socket_tls_v3.DownstreamTlsContext{
CommonTlsContext: &envoy_transport_socket_tls_v3.CommonTlsContext{
TlsParams: &envoy_transport_socket_tls_v3.TlsParameters{
Expand All @@ -125,7 +125,7 @@ func DownstreamTLSContext(serverSecret *dag.Secret, tlsMinProtoVersion, tlsMaxPr
},
TlsCertificateSdsSecretConfigs: []*envoy_transport_socket_tls_v3.SdsSecretConfig{{
Name: envoy.Secretname(serverSecret),
SdsConfig: ConfigSource("contour"),
SdsConfig: e.GetConfigSource(),
}},
AlpnProtocols: alpnProtos,
},
Expand Down
5 changes: 4 additions & 1 deletion internal/envoy/v3/auth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ func TestUpstreamTLSContext(t *testing.T) {

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
got := UpstreamTLSContext(tc.validation, tc.externalName, nil, tc.upstreamTLS, tc.alpnProtocols...)
e := NewEnvoysGen(EnvoyGenOpt{
XDSClusterName: DefaultXDSClusterName,
})
got := e.UpstreamTLSContext(tc.validation, tc.externalName, nil, tc.upstreamTLS, tc.alpnProtocols...)
protobuf.ExpectEqual(t, tc.want, got)
})
}
Expand Down
20 changes: 10 additions & 10 deletions internal/envoy/v3/bootstrap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ import (
)

// WriteBootstrap writes bootstrap configuration to files.
func WriteBootstrap(c *envoy.BootstrapConfig) error {
func (e *EnvoyGen) WriteBootstrap(c *envoy.BootstrapConfig) error {

Check warning on line 50 in internal/envoy/v3/bootstrap.go

View check run for this annotation

Codecov / codecov/patch

internal/envoy/v3/bootstrap.go#L50 

Added line #L50 was not covered by tests
// Create Envoy bootstrap config and associated resource files.
steps, err := bootstrap(c)
steps, err := e.bootstrap(c)

Check warning on line 52 in internal/envoy/v3/bootstrap.go

View check run for this annotation

Codecov / codecov/patch

internal/envoy/v3/bootstrap.go#L52 

Added line #L52 was not covered by tests
if err != nil {
return err
}
Expand Down Expand Up @@ -77,13 +77,13 @@ func WriteBootstrap(c *envoy.BootstrapConfig) error {
type bootstrapf func(*envoy.BootstrapConfig) (string, proto.Message)

// bootstrap creates a new v3 bootstrap configuration and associated resource files.
func bootstrap(c *envoy.BootstrapConfig) ([]bootstrapf, error) {
func (e *EnvoyGen) bootstrap(c *envoy.BootstrapConfig) ([]bootstrapf, error) {
var steps []bootstrapf

if c.GrpcClientCert == "" && c.GrpcClientKey == "" && c.GrpcCABundle == "" {
steps = append(steps,
func(*envoy.BootstrapConfig) (string, proto.Message) {
return c.Path, bootstrapConfig(c)
return c.Path, e.bootstrapConfig(c)
})

return steps, nil
Expand Down Expand Up @@ -122,7 +122,7 @@ func bootstrap(c *envoy.BootstrapConfig) ([]bootstrapf, error) {

steps = append(steps,
func(*envoy.BootstrapConfig) (string, proto.Message) {
b := bootstrapConfig(c)
b := e.bootstrapConfig(c)
b.StaticResources.Clusters[0].TransportSocket = UpstreamTLSTransportSocket(
upstreamFileTLSContext(c))
return c.Path, b
Expand Down Expand Up @@ -150,7 +150,7 @@ func bootstrap(c *envoy.BootstrapConfig) ([]bootstrapf, error) {
return sdsValidationContextPath, validationContextSdsSecretConfig(c)
},
func(*envoy.BootstrapConfig) (string, proto.Message) {
b := bootstrapConfig(c)
b := e.bootstrapConfig(c)
b.StaticResources.Clusters[0].TransportSocket = UpstreamTLSTransportSocket(
upstreamSdsTLSContext(sdsTLSCertificatePath, sdsValidationContextPath))
return c.Path, b
Expand All @@ -160,7 +160,7 @@ func bootstrap(c *envoy.BootstrapConfig) ([]bootstrapf, error) {
return steps, nil
}

func bootstrapConfig(c *envoy.BootstrapConfig) *envoy_config_bootstrap_v3.Bootstrap {
func (e *EnvoyGen) bootstrapConfig(c *envoy.BootstrapConfig) *envoy_config_bootstrap_v3.Bootstrap {
bootstrap := &envoy_config_bootstrap_v3.Bootstrap{
LayeredRuntime: &envoy_config_bootstrap_v3.LayeredRuntime{
Layers: []*envoy_config_bootstrap_v3.RuntimeLayer{
Expand All @@ -169,7 +169,7 @@ func bootstrapConfig(c *envoy.BootstrapConfig) *envoy_config_bootstrap_v3.Bootst
LayerSpecifier: &envoy_config_bootstrap_v3.RuntimeLayer_RtdsLayer_{
RtdsLayer: &envoy_config_bootstrap_v3.RuntimeLayer_RtdsLayer{
Name: DynamicRuntimeLayerName,
RtdsConfig: ConfigSource("contour"),
RtdsConfig: e.GetConfigSource(),
},
},
},
Expand All @@ -187,8 +187,8 @@ func bootstrapConfig(c *envoy.BootstrapConfig) *envoy_config_bootstrap_v3.Bootst
},
},
DynamicResources: &envoy_config_bootstrap_v3.Bootstrap_DynamicResources{
LdsConfig: ConfigSource("contour"),
CdsConfig: ConfigSource("contour"),
LdsConfig: e.GetConfigSource(),
CdsConfig: e.GetConfigSource(),
},
StaticResources: &envoy_config_bootstrap_v3.Bootstrap_StaticResources{
Clusters: []*envoy_config_cluster_v3.Cluster{{
Expand Down
5 changes: 4 additions & 1 deletion internal/envoy/v3/bootstrap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2010,7 +2010,10 @@ func TestBootstrap(t *testing.T) {
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
tc := tc
steps, gotError := bootstrap(&tc.config)
envoyGen := NewEnvoysGen(EnvoyGenOpt{
XDSClusterName: DefaultXDSClusterName,
})
steps, gotError := envoyGen.bootstrap(&tc.config)
assert.Equal(t, tc.wantedError, gotError != nil)

gotConfigs := map[string]proto.Message{}
Expand Down
16 changes: 8 additions & 8 deletions internal/envoy/v3/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func clusterDefaults() *envoy_config_cluster_v3.Cluster {
}

// Cluster creates new envoy_config_cluster_v3.Cluster from dag.Cluster.
func Cluster(c *dag.Cluster) *envoy_config_cluster_v3.Cluster {
func (e *EnvoyGen) Cluster(c *dag.Cluster) *envoy_config_cluster_v3.Cluster {
service := c.Upstream
cluster := clusterDefaults()

Expand Down Expand Up @@ -85,7 +85,7 @@ func Cluster(c *dag.Cluster) *envoy_config_cluster_v3.Cluster {
switch c.Protocol {
case "tls":
cluster.TransportSocket = UpstreamTLSTransportSocket(
UpstreamTLSContext(
e.UpstreamTLSContext(
c.UpstreamValidation,
c.SNI,
c.ClientCertificate,
Expand All @@ -95,7 +95,7 @@ func Cluster(c *dag.Cluster) *envoy_config_cluster_v3.Cluster {
case "h2":
httpVersion = HTTPVersion2
cluster.TransportSocket = UpstreamTLSTransportSocket(
UpstreamTLSContext(
e.UpstreamTLSContext(
c.UpstreamValidation,
c.SNI,
c.ClientCertificate,
Expand Down Expand Up @@ -136,7 +136,7 @@ func Cluster(c *dag.Cluster) *envoy_config_cluster_v3.Cluster {
}

// ExtensionCluster builds a envoy_config_cluster_v3.Cluster struct for the given extension service.
func ExtensionCluster(ext *dag.ExtensionCluster) *envoy_config_cluster_v3.Cluster {
func (e *EnvoyGen) ExtensionCluster(ext *dag.ExtensionCluster) *envoy_config_cluster_v3.Cluster {
cluster := clusterDefaults()

// The Envoy cluster name has already been set.
Expand All @@ -156,7 +156,7 @@ func ExtensionCluster(ext *dag.ExtensionCluster) *envoy_config_cluster_v3.Cluste
// Cluster will be discovered via EDS.
cluster.ClusterDiscoveryType = ClusterDiscoveryType(envoy_config_cluster_v3.Cluster_EDS)
cluster.EdsClusterConfig = &envoy_config_cluster_v3.Cluster_EdsClusterConfig{
EdsConfig: ConfigSource("contour"),
EdsConfig: e.GetConfigSource(),
ServiceName: ext.Upstream.ClusterName,
}

Expand All @@ -167,7 +167,7 @@ func ExtensionCluster(ext *dag.ExtensionCluster) *envoy_config_cluster_v3.Cluste
case "h2":
http2Version = HTTPVersion2
cluster.TransportSocket = UpstreamTLSTransportSocket(
UpstreamTLSContext(
e.UpstreamTLSContext(
ext.UpstreamValidation,
ext.SNI,
ext.ClientCertificate,
Expand Down Expand Up @@ -208,7 +208,7 @@ func applyCircuitBreakers(cluster *envoy_config_cluster_v3.Cluster, settings dag
}

// DNSNameCluster builds a envoy_config_cluster_v3.Cluster for the given *dag.DNSNameCluster.
func DNSNameCluster(c *dag.DNSNameCluster) *envoy_config_cluster_v3.Cluster {
func (e *EnvoyGen) DNSNameCluster(c *dag.DNSNameCluster) *envoy_config_cluster_v3.Cluster {
cluster := clusterDefaults()

cluster.Name = envoy.DNSNameClusterName(c)
Expand All @@ -222,7 +222,7 @@ func DNSNameCluster(c *dag.DNSNameCluster) *envoy_config_cluster_v3.Cluster {

var transportSocket *envoy_config_core_v3.TransportSocket
if c.Scheme == "https" {
transportSocket = UpstreamTLSTransportSocket(UpstreamTLSContext(c.UpstreamValidation, c.Address, nil, c.UpstreamTLS))
transportSocket = UpstreamTLSTransportSocket(e.UpstreamTLSContext(c.UpstreamValidation, c.Address, nil, c.UpstreamTLS))
}

cluster.LoadAssignment = ClusterLoadAssignment(envoy.DNSNameClusterName(c), SocketAddress(c.Address, c.Port))
Expand Down
Loading

0 comments on commit 928dcff

Please sign in to comment.