Merge pull request #2540 from murgatroid99/grpc-js-xds_lrs_config_handling_fix

grpc-js-xds: Fix handling of LRS server configs

Author: murgatroid99

packages/grpc-js-xds/src/load-balancer-lrs.ts

@@ -97,7 +97,7 @@ class LrsLoadBalancingConfig implements TypedLoadBalancingConfig {
     if (!('child_policy' in obj && Array.isArray(obj.child_policy))) {
       throw new Error('lrs config must have a child_policy array');
     }
-    const childConfig = selectLbConfigFromList(obj.config);
+    const childConfig = selectLbConfigFromList(obj.child_policy);
     if (!childConfig) {
       throw new Error('lrs config child_policy parsing failed');
     }

packages/grpc-js-xds/src/load-balancer-xds-cluster-resolver.ts

@@ -310,7 +310,8 @@ export class XdsClusterResolver implements LoadBalancer {
                 cluster_name: entry.discoveryMechanism.cluster,
                 eds_service_name: entry.discoveryMechanism.eds_service_name ?? '',
                 locality: {...localityObj.locality},
-                lrs_load_reporting_server: {...entry.discoveryMechanism.lrs_load_reporting_server}
+                lrs_load_reporting_server: {...entry.discoveryMechanism.lrs_load_reporting_server},
+                child_policy: endpointPickingPolicy
               }
             }];
           } else {

packages/grpc-js-xds/src/xds-bootstrap.ts

@@ -41,9 +41,9 @@ export interface ChannelCredsConfig {
 }
 
 export interface XdsServerConfig {
-  serverUri: string;
-  channelCreds: ChannelCredsConfig[];
-  serverFeatures: string[];
+  server_uri: string;
+  channel_creds: ChannelCredsConfig[];
+  server_features: string[];
 }
 
 export interface Authority {
@@ -61,19 +61,19 @@ export interface BootstrapInfo {
 const KNOWN_SERVER_FEATURES = ['ignore_resource_deletion'];
 
 export function serverConfigEqual(config1: XdsServerConfig, config2: XdsServerConfig): boolean {
-  if (config1.serverUri !== config2.serverUri) {
+  if (config1.server_uri !== config2.server_uri) {
     return false;
   }
   for (const feature of KNOWN_SERVER_FEATURES) {
-    if ((feature in config1.serverFeatures) !== (feature in config2.serverFeatures)) {
+    if ((feature in config1.server_features) !== (feature in config2.server_features)) {
       return false;
     }
   }
-  if (config1.channelCreds.length !== config2.channelCreds.length) {
+  if (config1.channel_creds.length !== config2.channel_creds.length) {
     return false;
   }
-  for (const [index, creds1] of config1.channelCreds.entries()) {
-    const creds2 = config2.channelCreds[index];
+  for (const [index, creds1] of config1.channel_creds.entries()) {
+    const creds2 = config2.channel_creds[index];
     if (creds1.type !== creds2.type) {
       return false;
     }
@@ -93,7 +93,7 @@ function validateChannelCredsConfig(obj: any): ChannelCredsConfig {
       `xds_servers.channel_creds.type field: expected string, got ${typeof obj.type}`
     );
   }
-  if ('config' in obj) {
+  if ('config' in obj && obj.config !== undefined) {
     if (typeof obj.config !== 'object' || obj.config === null) {
       throw new Error(
         'xds_servers.channel_creds config field must be an object if provided'
@@ -152,9 +152,9 @@ export function validateXdsServerConfig(obj: any): XdsServerConfig {
     }
   }
   return {
-    serverUri: obj.server_uri,
-    channelCreds: obj.channel_creds.map(validateChannelCredsConfig),
-    serverFeatures: obj.server_features ?? []
+    server_uri: obj.server_uri,
+    channel_creds: obj.channel_creds.map(validateChannelCredsConfig),
+    server_features: obj.server_features ?? []
   };
 }
 
@@ -387,7 +387,7 @@ export function loadBootstrapInfo(): BootstrapInfo {
     return loadedBootstrapInfo;
   }
 
-  
+
   throw new Error(
     'The GRPC_XDS_BOOTSTRAP or GRPC_XDS_BOOTSTRAP_CONFIG environment variables need to be set to the path to the bootstrap file to use xDS'
   );

packages/grpc-js-xds/src/xds-client.ts

@@ -602,9 +602,9 @@ class ClusterLoadReportMap {
    * Get the indicated map entry if it exists, or create a new one if it does
    * not. Increments the refcount of that entry, so a call to this method
    * should correspond to a later call to unref
-   * @param clusterName 
-   * @param edsServiceName 
-   * @returns 
+   * @param clusterName
+   * @param edsServiceName
+   * @returns
    */
   getOrCreate(clusterName: string, edsServiceName: string): ClusterLoadReport {
     for (const statsObj of this.statsMap) {
@@ -833,12 +833,12 @@ class XdsSingleServerClient {
       this.maybeStartLrsStream();
     });
     this.lrsBackoff.unref();
-    this.ignoreResourceDeletion = xdsServerConfig.serverFeatures.includes('ignore_resource_deletion');
+    this.ignoreResourceDeletion = xdsServerConfig.server_features.includes('ignore_resource_deletion');
     const channelArgs = {
       // 5 minutes
       'grpc.keepalive_time_ms': 5 * 60 * 1000
     }
-    const credentialsConfigs = xdsServerConfig.channelCreds;
+    const credentialsConfigs = xdsServerConfig.channel_creds;
     let channelCreds: ChannelCredentials | null = null;
     for (const config of credentialsConfigs) {
       if (config.type === 'google_default') {
@@ -849,8 +849,8 @@ class XdsSingleServerClient {
         break;
       }
     }
-    const serverUri = this.xdsServerConfig.serverUri
-    this.trace('Starting xDS client connected to server URI ' + this.xdsServerConfig.serverUri);
+    const serverUri = this.xdsServerConfig.server_uri
+    this.trace('Starting xDS client connected to server URI ' + this.xdsServerConfig.server_uri);
     /* Bootstrap validation rules guarantee that a matching channel credentials
      * config exists in the list. */
     const channel = new Channel(serverUri, channelCreds!, channelArgs);
@@ -949,7 +949,7 @@ class XdsSingleServerClient {
   }
 
   trace(text: string) {
-    trace(this.xdsServerConfig.serverUri + ' ' + text);
+    trace(this.xdsServerConfig.server_uri + ' ' + text);
   }
 
   subscribe(type: XdsResourceType, name: XdsResourceName) {

packages/grpc-js-xds/test/framework.ts

@@ -83,7 +83,8 @@ export class FakeEdsCluster implements FakeCluster {
       name: this.clusterName,
       type: 'EDS',
       eds_cluster_config: {eds_config: {ads: {}}, service_name: this.endpointName},
-      lb_policy: 'ROUND_ROBIN'
+      lb_policy: 'ROUND_ROBIN',
+      lrs_server: {self: {}}
     }
   }
 
@@ -156,7 +157,8 @@ export class FakeDnsCluster implements FakeCluster {
             }
           }]
         }]
-      }
+      },
+      lrs_server: {self: {}}
     };
   }
   getAllClusterConfigs(): Cluster[] {

packages/grpc-js-xds/test/test-bootstrap.ts

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2023 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+import * as assert from 'assert';
+import { validateXdsServerConfig } from "../src/xds-bootstrap";
+
+describe('bootstrap', () => {
+  /* validateXdsServerConfig is used when creating the cds config, and then
+   * the resulting value is validated again when creating the
+   * xds_cluster_resolver config. */
+  it('validateXdsServerConfig should be idempotent', () => {
+    const config = {
+      server_uri: 'localhost',
+      channel_creds: [{type: 'google_default'}],
+      server_features: ['test_feature']
+    };
+    assert.deepStrictEqual(validateXdsServerConfig(validateXdsServerConfig(config)), validateXdsServerConfig(config));
+  });
+});