Skip to content

Commit

Permalink
Allow internal callers to inject RouteGenerator (#870)
Browse files Browse the repository at this point in the history 
Bubble up `routeGens` parameter to `MakeListener`, which we call internally.

Also allows us to remove `ServiceInfo` from `MakeListener` 💯  In follow-up CL, I will completely delete `ServiceInfo`.

Signed-off-by: Teju Nareddy <[email protected]>
  • Loading branch information
@nareddyt
nareddyt authored Nov 13, 2023
1 parent fc7dea4 commit a095b7b
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 63 deletions.
38 changes: 24 additions & 14 deletions src/go/configgenerator/listener_generator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import (
"fmt"

"github.com/GoogleCloudPlatform/esp-v2/src/go/configgenerator/filtergen"
"github.com/GoogleCloudPlatform/esp-v2/src/go/configgenerator/routegen"
sc "github.com/GoogleCloudPlatform/esp-v2/src/go/configinfo"
"github.com/GoogleCloudPlatform/esp-v2/src/go/options"
"github.com/GoogleCloudPlatform/esp-v2/src/go/util"
corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
Expand All @@ -40,7 +42,13 @@ func MakeListeners(serviceInfo *sc.ServiceInfo, scParams filtergen.ServiceContro
return nil, err
}

listener, err := MakeListener(serviceInfo, filterGens, connectionManager)
routeGenFactories := MakeRouteGenFactories()
routeGens, err := routegen.NewRouteGeneratorsFromOPConfig(serviceInfo.ServiceConfig(), serviceInfo.Options, routeGenFactories)
if err != nil {
return nil, err
}

listener, err := MakeListener(serviceInfo.Options, filterGens, connectionManager, routeGens)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -77,14 +85,16 @@ func MakeHttpFilterConfigs(filterGenerators []filtergen.FilterGenerator) ([]*hcm
return httpFilters, nil
}

// MakeListener provides a dynamic listener for Envoy
func MakeListener(serviceInfo *sc.ServiceInfo, httpFilterGenerators []filtergen.FilterGenerator, connectionManagerGen filtergen.FilterGenerator) (*listenerpb.Listener, error) {
// MakeListener provides a dynamic listener for Envoy.
// Allows dependency injection of FilterGenerator and RouteGenerator for
// internal use.
func MakeListener(opts options.ConfigGeneratorOptions, httpFilterGenerators []filtergen.FilterGenerator, connectionManagerGen filtergen.FilterGenerator, routeGenerators []routegen.RouteGenerator) (*listenerpb.Listener, error) {
httpFilterConfigs, err := MakeHttpFilterConfigs(httpFilterGenerators)
if err != nil {
return nil, err
}

routeConfig, err := MakeRouteConfig(serviceInfo, httpFilterGenerators)
routeConfig, err := MakeRouteConfig(opts, httpFilterGenerators, routeGenerators)
if err != nil {
return nil, fmt.Errorf("makeHttpConnectionManagerRouteConfig got err: %s", err)
}
Expand Down Expand Up @@ -116,13 +126,13 @@ func MakeListener(serviceInfo *sc.ServiceInfo, httpFilterGenerators []filtergen.
},
}

if serviceInfo.Options.SslServerCertPath != "" {
if opts.SslServerCertPath != "" {
transportSocket, err := util.CreateDownstreamTransportSocket(
serviceInfo.Options.SslServerCertPath,
serviceInfo.Options.SslServerRootCertPath,
serviceInfo.Options.SslMinimumProtocol,
serviceInfo.Options.SslMaximumProtocol,
serviceInfo.Options.SslServerCipherSuites,
opts.SslServerCertPath,
opts.SslServerRootCertPath,
opts.SslMinimumProtocol,
opts.SslMaximumProtocol,
opts.SslServerCipherSuites,
)
if err != nil {
return nil, err
Expand All @@ -135,19 +145,19 @@ func MakeListener(serviceInfo *sc.ServiceInfo, httpFilterGenerators []filtergen.
Address: &corepb.Address{
Address: &corepb.Address_SocketAddress{
SocketAddress: &corepb.SocketAddress{
Address: serviceInfo.Options.ListenerAddress,
Address: opts.ListenerAddress,
PortSpecifier: &corepb.SocketAddress_PortValue{
PortValue: uint32(serviceInfo.Options.ListenerPort),
PortValue: uint32(opts.ListenerPort),
},
},
},
},
FilterChains: []*listenerpb.FilterChain{filterChain},
}

if serviceInfo.Options.ConnectionBufferLimitBytes >= 0 {
if opts.ConnectionBufferLimitBytes >= 0 {
listener.PerConnectionBufferLimitBytes = &wrapperspb.UInt32Value{
Value: uint32(serviceInfo.Options.ConnectionBufferLimitBytes),
Value: uint32(opts.ConnectionBufferLimitBytes),
}
}

Expand Down
18 changes: 1 addition & 17 deletions src/go/configgenerator/route_generator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (

"github.com/GoogleCloudPlatform/esp-v2/src/go/configgenerator/filtergen"
"github.com/GoogleCloudPlatform/esp-v2/src/go/configgenerator/routegen"
"github.com/GoogleCloudPlatform/esp-v2/src/go/configinfo"
"github.com/GoogleCloudPlatform/esp-v2/src/go/options"
"github.com/GoogleCloudPlatform/esp-v2/src/go/util"
corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
Expand Down Expand Up @@ -55,22 +54,7 @@ func MakeRouteGenFactories() []routegen.RouteGeneratorOPFactory {

// MakeRouteConfig creates the virtual host and route table with the default
// route generators for ESPv2.
func MakeRouteConfig(serviceInfo *configinfo.ServiceInfo, filterGenerators []filtergen.FilterGenerator) (*routepb.RouteConfiguration, error) {
routeGenFactories := MakeRouteGenFactories()

routeGens, err := routegen.NewRouteGeneratorsFromOPConfig(serviceInfo.ServiceConfig(), serviceInfo.Options, routeGenFactories)
if err != nil {
return nil, err
}

return MakeRouteConfigWithGens(serviceInfo.Options, filterGenerators, routeGens)
}

// MakeRouteConfigWithGens is a version of MakeRouteConfig that allows injection
// of different route generators that are not the default.
//
// Useful when extending the config generator internally.
func MakeRouteConfigWithGens(opts options.ConfigGeneratorOptions, filterGenerators []filtergen.FilterGenerator, routeGenerators []routegen.RouteGenerator) (*routepb.RouteConfiguration, error) {
func MakeRouteConfig(opts options.ConfigGeneratorOptions, filterGenerators []filtergen.FilterGenerator, routeGenerators []routegen.RouteGenerator) (*routepb.RouteConfiguration, error) {
host := &routepb.VirtualHost{
Name: virtualHostName,
Domains: []string{"*"},
Expand Down
58 changes: 26 additions & 32 deletions src/go/configgenerator/route_generator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ import (
"time"

"github.com/GoogleCloudPlatform/esp-v2/src/go/configgenerator/filtergen"
"github.com/GoogleCloudPlatform/esp-v2/src/go/configinfo"
"github.com/GoogleCloudPlatform/esp-v2/src/go/configgenerator/routegen"
"github.com/GoogleCloudPlatform/esp-v2/src/go/options"
"github.com/GoogleCloudPlatform/esp-v2/src/go/util"
corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
corspb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/cors/v3"
matcher "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3"
"github.com/google/go-cmp/cmp"
Expand All @@ -37,6 +38,18 @@ import (
"google.golang.org/protobuf/types/known/wrapperspb"
)

// makeRouteConfigWithDefaults is a test helper to call MakeRouteConfig.
func makeRouteConfigWithDefaults(serviceConfig *servicepb.Service, opts options.ConfigGeneratorOptions, filterGens []filtergen.FilterGenerator) (*routepb.RouteConfiguration, error) {
routeGenFactories := MakeRouteGenFactories()

routeGens, err := routegen.NewRouteGeneratorsFromOPConfig(serviceConfig, opts, routeGenFactories)
if err != nil {
return nil, fmt.Errorf("fail to create route generators from factories: %v", err)
}

return MakeRouteConfig(opts, filterGens, routeGens)
}

func TestMakeRouteConfig(t *testing.T) {
testData := []struct {
desc string
Expand Down Expand Up @@ -2881,12 +2894,8 @@ func TestMakeRouteConfig(t *testing.T) {
opts.EnableOperationNameHeader = tc.enableOperationNameHeader
opts.DisallowColonInWildcardPathSegment = tc.disallowColonInWildcardPathSegment
opts.Healthz = tc.healthz
fakeServiceInfo, err := configinfo.NewServiceInfoFromServiceConfig(tc.fakeServiceConfig, opts)
if err != nil {
t.Fatal(err)
}

gotRoute, err := MakeRouteConfig(fakeServiceInfo, nil)
gotRoute, err := makeRouteConfigWithDefaults(tc.fakeServiceConfig, opts, nil)
if tc.wantedError != "" {
if err == nil || !strings.Contains(err.Error(), tc.wantedError) {
t.Fatalf("expected err: %v, got: %v", tc.wantedError, err)
Expand Down Expand Up @@ -3365,12 +3374,7 @@ func TestMakeRouteForBothGrpcAndHttpRoute_WithHttpBackend(t *testing.T) {
opts := options.DefaultConfigGeneratorOptions()
opts.BackendAddress = "grpc://grpc-backend:8080"

fakeServiceInfo, err := configinfo.NewServiceInfoFromServiceConfig(tc.serviceConfig, opts)
if err != nil {
t.Fatal(err)
}

gotRoute, err := MakeRouteConfig(fakeServiceInfo, nil)
gotRoute, err := makeRouteConfigWithDefaults(tc.serviceConfig, opts, nil)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -4732,18 +4736,13 @@ func TestMakeFallbackRoute(t *testing.T) {
opts.CorsExposeHeaders = tc.optsIn.CorsExposeHeaders
opts.CorsMaxAge = tc.optsIn.CorsMaxAge

fakeServiceInfo, err := configinfo.NewServiceInfoFromServiceConfig(tc.fakeServiceConfig, opts)
if err != nil {
t.Fatal(err)
}

filterGenFactories := MakeHTTPFilterGenFactories(filtergen.ServiceControlOPFactoryParams{})
filterGens, err := NewFilterGeneratorsFromOPConfig(tc.fakeServiceConfig, opts, filterGenFactories)
if err != nil {
t.Fatal(err)
}

gotRoute, err := MakeRouteConfig(fakeServiceInfo, filterGens)
gotRoute, err := makeRouteConfigWithDefaults(tc.fakeServiceConfig, opts, filterGens)
if err != nil {
t.Fatalf("got error: %v", err)
}
Expand Down Expand Up @@ -4862,18 +4861,13 @@ func TestMakeRouteConfigForCors(t *testing.T) {
opts.CorsAllowCredentials = tc.optsIn.CorsAllowCredentials

fakeServiceConfig := &servicepb.Service{}
fakeServiceInfo := &configinfo.ServiceInfo{
Name: "test-api",
Options: opts,
}

filterGenFactories := MakeHTTPFilterGenFactories(filtergen.ServiceControlOPFactoryParams{})
filterGens, err := NewFilterGeneratorsFromOPConfig(fakeServiceConfig, opts, filterGenFactories)
if err != nil {
t.Fatal(err)
}

gotRoute, err := MakeRouteConfig(fakeServiceInfo, filterGens)
gotRoute, err := makeRouteConfigWithDefaults(fakeServiceConfig, opts, filterGens)
if err != nil {
t.Fatalf("MakeRouteConfig(...) got err %v, want no err", err)
}
Expand Down Expand Up @@ -4955,12 +4949,11 @@ func TestMakeRouteConfigForCors_BadInput(t *testing.T) {
opts.CorsExposeHeaders = tc.optsIn.CorsExposeHeaders
opts.CorsMaxAge = tc.optsIn.CorsMaxAge

fakeServiceInfo := &configinfo.ServiceInfo{
Name: "test-api",
Options: opts,
fakeServiceConfig := &servicepb.Service{
Name: "test-api",
}

_, err := MakeRouteConfig(fakeServiceInfo, nil)
_, err := makeRouteConfigWithDefaults(fakeServiceConfig, opts, nil)
if err == nil || !strings.Contains(err.Error(), tc.wantedError) {
t.Errorf("Test (%s): expected err: %v, got: %v", tc.desc, tc.wantedError, err)
}
Expand Down Expand Up @@ -5077,10 +5070,11 @@ func TestHeadersToAdd(t *testing.T) {
opts.AddResponseHeaders = tc.addResponseHeaders
opts.AppendResponseHeaders = tc.appendResponseHeaders

gotRoute, err := MakeRouteConfig(&configinfo.ServiceInfo{
Name: "test-api",
Options: opts,
}, nil)
fakeServiceConfig := &servicepb.Service{
Name: "test-api",
}

gotRoute, err := makeRouteConfigWithDefaults(fakeServiceConfig, opts, nil)
if tc.wantedError != "" {
if err == nil || !strings.Contains(err.Error(), tc.wantedError) {
t.Errorf("Test (%s): expected err: %v, got: %v", tc.desc, tc.wantedError, err)
Expand Down

0 comments on commit a095b7b

Please sign in to comment.