review comments #2

Author: easwars

xds/internal/client/xds.go

@@ -269,7 +269,11 @@ func processServerSideListener(lis *v3listenerpb.Listener) (*ListenerUpdate, err
 			Port:    strconv.Itoa(int(sockAddr.GetPortValue())),
 		},
 	}
-	if err := validateNetworkFilterChains(append(lis.GetFilterChains(), lis.GetDefaultFilterChain())); err != nil {
+	chains := lis.GetFilterChains()
+	if def := lis.GetDefaultFilterChain(); def != nil {
+		chains = append(chains, def)
+	}
+	if err := validateNetworkFilterChains(chains); err != nil {
 		return nil, err
 	}
 
@@ -283,9 +287,6 @@ func processServerSideListener(lis *v3listenerpb.Listener) (*ListenerUpdate, err
 
 func validateNetworkFilterChains(filterChains []*v3listenerpb.FilterChain) error {
 	for _, filterChain := range filterChains {
-		if filterChain == nil {
-			continue
-		}
 		seenNames := make(map[string]bool, len(filterChain.GetFilters()))
 		seenHCM := false
 		for _, filter := range filterChain.GetFilters() {
@@ -315,11 +316,11 @@ func validateNetworkFilterChains(filterChains []*v3listenerpb.FilterChain) error
 				// we have for HTTP filters), when we have to support network
 				// filters other than HttpConnectionManager.
 				if tc.GetTypeUrl() != version.V3HTTPConnManagerURL {
-					return fmt.Errorf("filter chain {%+v} has unsupported network filter: %s", filterChain, tc.GetTypeUrl())
+					return fmt.Errorf("filter chain {%+v} has unsupported network filter %q in filter {%+v}", filterChain, tc.GetTypeUrl(), filter)
 				}
 				hcm := &v3httppb.HttpConnectionManager{}
 				if err := ptypes.UnmarshalAny(tc, hcm); err != nil {
-					return fmt.Errorf("filter chain {%+v} failed unmarshaling of network filter: %v", filterChain, err)
+					return fmt.Errorf("filter chain {%+v} failed unmarshaling of network filter {%+v}: %v", filterChain, filter, err)
 				}
 				// We currently don't support HTTP filters on the server-side.
 				// We will be adding support for it in the future. So, we want

xds/internal/test/xds_server_integration_test.go

@@ -33,30 +33,29 @@ import (
 	"strconv"
 	"testing"
 
-	v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
-	v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
-	v3httppb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
-	v3tlspb "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
-	wrapperspb "github.com/golang/protobuf/ptypes/wrappers"
 	"github.com/google/uuid"
-	"google.golang.org/protobuf/proto"
-	"google.golang.org/protobuf/types/known/anypb"
-
-	"google.golang.org/grpc/internal/testutils"
-	xds2 "google.golang.org/grpc/internal/xds"
-
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials"
 	"google.golang.org/grpc/credentials/insecure"
-	xdscreds "google.golang.org/grpc/credentials/xds"
+	"google.golang.org/grpc/internal/testutils"
 	"google.golang.org/grpc/status"
-	testpb "google.golang.org/grpc/test/grpc_testing"
 	"google.golang.org/grpc/testdata"
 	"google.golang.org/grpc/xds"
-	xdstestutils "google.golang.org/grpc/xds/internal/testutils"
 	"google.golang.org/grpc/xds/internal/testutils/e2e"
 	"google.golang.org/grpc/xds/internal/version"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/known/anypb"
+
+	v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
+	v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
+	v3httppb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
+	v3tlspb "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3"
+	wrapperspb "github.com/golang/protobuf/ptypes/wrappers"
+	xdscreds "google.golang.org/grpc/credentials/xds"
+	xdsinternal "google.golang.org/grpc/internal/xds"
+	testpb "google.golang.org/grpc/test/grpc_testing"
+	xdstestutils "google.golang.org/grpc/xds/internal/testutils"
 )
 
 const (
@@ -154,8 +153,8 @@ func commonSetup(t *testing.T) (*e2e.ManagementServer, string, net.Listener, fun
 	cpc := e2e.DefaultFileWatcherConfig(path.Join(tmpdir, certFile), path.Join(tmpdir, keyFile), path.Join(tmpdir, rootFile))
 
 	// Create a bootstrap file in a temporary directory.
-	bootstrapCleanup, err := xds2.SetupBootstrapFile(xds2.BootstrapOptions{
-		Version:                            xds2.TransportV3,
+	bootstrapCleanup, err := xdsinternal.SetupBootstrapFile(xdsinternal.BootstrapOptions{
+		Version:                            xdsinternal.TransportV3,
 		NodeID:                             nodeID,
 		ServerURI:                          fs.Address,
 		CertificateProviders:               cpc,