Skip to content

Commit

Permalink
Fixed race in Filter Chain (#4728)
Browse files Browse the repository at this point in the history
  • Loading branch information
zasweq committed Sep 2, 2021
1 parent b189f5e commit c93e472
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 13 deletions.
5 changes: 3 additions & 2 deletions xds/internal/server/listener_wrapper.go
Expand Up @@ -324,12 +324,13 @@ func (l *listenerWrapper) Accept() (net.Conn, error) {
// can come it at any time), and connections aren't accepted too often,
// so this reinstantation of the Route Configuration is an acceptable
// tradeoff for simplicity.
if err := fc.ConstructUsableRouteConfiguration(rc); err != nil {
vhswi, err := fc.ConstructUsableRouteConfiguration(rc)
if err != nil {
l.logger.Warningf("route configuration construction: %v", err)
conn.Close()
continue
}
return &connWrapper{Conn: conn, filterChain: fc, parent: l, virtualHosts: fc.VirtualHosts}, nil
return &connWrapper{Conn: conn, filterChain: fc, parent: l, virtualHosts: vhswi}, nil
}
}

Expand Down
12 changes: 3 additions & 9 deletions xds/internal/xdsclient/filter_chain.go
Expand Up @@ -67,11 +67,6 @@ type FilterChain struct {
//
// Only one of RouteConfigName and InlineRouteConfig is set.
InlineRouteConfig *RouteConfigUpdate

// VirtualHosts are the virtual hosts ready to be used in the xds interceptors.
// It contains a way to match routes using a matcher and also instantiates
// HTTPFilter overrides to simply run incoming RPC's through if they are selected.
VirtualHosts []VirtualHostWithInterceptors
}

// VirtualHostWithInterceptors captures information present in a VirtualHost
Expand All @@ -98,17 +93,16 @@ type RouteWithInterceptors struct {

// ConstructUsableRouteConfiguration takes Route Configuration and converts it
// into matchable route configuration, with instantiated HTTP Filters per route.
func (f *FilterChain) ConstructUsableRouteConfiguration(config RouteConfigUpdate) error {
func (f *FilterChain) ConstructUsableRouteConfiguration(config RouteConfigUpdate) ([]VirtualHostWithInterceptors, error) {
vhs := make([]VirtualHostWithInterceptors, len(config.VirtualHosts))
for _, vh := range config.VirtualHosts {
vhwi, err := f.convertVirtualHost(vh)
if err != nil {
return fmt.Errorf("virtual host construction: %v", err)
return nil, fmt.Errorf("virtual host construction: %v", err)
}
vhs = append(vhs, vhwi)
}
f.VirtualHosts = vhs
return nil
return vhs, nil
}

func (f *FilterChain) convertVirtualHost(virtualHost *VirtualHost) (VirtualHostWithInterceptors, error) {
Expand Down
7 changes: 5 additions & 2 deletions xds/internal/xdsclient/filter_chain_test.go
Expand Up @@ -2629,11 +2629,14 @@ func TestHTTPFilterInstantiation(t *testing.T) {
fc := FilterChain{
HTTPFilters: test.filters,
}
fc.ConstructUsableRouteConfiguration(test.routeConfig)
vhswi, err := fc.ConstructUsableRouteConfiguration(test.routeConfig)
if err != nil {
t.Fatalf("Error constructing usable route configuration: %v", err)
}
// Build out list of errors by iterating through the virtual hosts and routes,
// and running the filters in route configurations.
var errs []string
for _, vh := range fc.VirtualHosts {
for _, vh := range vhswi {
for _, r := range vh.Routes {
for _, int := range r.Interceptors {
errs = append(errs, int.AllowRPC(context.Background()).Error())
Expand Down

0 comments on commit c93e472

Please sign in to comment.