xds/resolver: fix panic when two LDS updates are receives without RDS…

… in between (#4327)

Also confirmed that the LDS updates shouldn't trigger state update without the
RDS.

xds/internal/resolver/watch_service.go

@@ -132,7 +132,15 @@ func (w *serviceUpdateWatcher) handleLDSResp(update xdsclient.ListenerUpdate, er
 		//
 		// If the route name did change, then we must wait until the first RDS
 		// update before reporting this LDS config.
-		w.serviceCb(w.lastUpdate, nil)
+		if w.lastUpdate.virtualHost != nil {
+			// We want to send an update with the new fields from the new LDS
+			// (e.g. max stream duration), and old fields from the the previous
+			// RDS.
+			//
+			// But note that this should only happen when virtual host is set,
+			// which means an RDS was received.
+			w.serviceCb(w.lastUpdate, nil)
+		}
 		return
 	}
 	w.rdsName = update.RouteConfigName

xds/internal/resolver/xds_resolver_test.go

@@ -1030,6 +1030,48 @@ func (s) TestXDSResolverResourceNotFoundError(t *testing.T) {
 	}
 }
 
+// TestXDSResolverMultipleLDSUpdates tests the case where two LDS updates with
+// the same RDS name to watch are received without an RDS in between. Those LDS
+// updates shouldn't trigger service config update.
+//
+// This test case also makes sure the resolver doesn't panic.
+func (s) TestXDSResolverMultipleLDSUpdates(t *testing.T) {
+	xdsC := fakeclient.NewClient()
+	xdsR, tcc, cancel := testSetup(t, setupOpts{
+		xdsClientFunc: func() (xdsClientInterface, error) { return xdsC, nil },
+	})
+	defer func() {
+		cancel()
+		xdsR.Close()
+	}()
+
+	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
+	defer cancel()
+	waitForWatchListener(ctx, t, xdsC, targetStr)
+	xdsC.InvokeWatchListenerCallback(xdsclient.ListenerUpdate{RouteConfigName: routeStr, HTTPFilters: routerFilterList}, nil)
+	waitForWatchRouteConfig(ctx, t, xdsC, routeStr)
+	defer replaceRandNumGenerator(0)()
+
+	// Send a new LDS update, with the same fields.
+	xdsC.InvokeWatchListenerCallback(xdsclient.ListenerUpdate{RouteConfigName: routeStr, HTTPFilters: routerFilterList}, nil)
+	ctx, cancel = context.WithTimeout(context.Background(), defaultTestShortTimeout)
+	defer cancel()
+	// Should NOT trigger a state update.
+	gotState, err := tcc.stateCh.Receive(ctx)
+	if err == nil {
+		t.Fatalf("ClientConn.UpdateState received %v, want timeout error", gotState)
+	}
+
+	// Send a new LDS update, with the same RDS name, but different fields.
+	xdsC.InvokeWatchListenerCallback(xdsclient.ListenerUpdate{RouteConfigName: routeStr, MaxStreamDuration: time.Second, HTTPFilters: routerFilterList}, nil)
+	ctx, cancel = context.WithTimeout(context.Background(), defaultTestShortTimeout)
+	defer cancel()
+	gotState, err = tcc.stateCh.Receive(ctx)
+	if err == nil {
+		t.Fatalf("ClientConn.UpdateState received %v, want timeout error", gotState)
+	}
+}
+
 type filterBuilder struct {
 	httpfilter.Filter // embedded as we do not need to implement registry / parsing in this test.
 	path              *[]string