Merge pull request #2277 from kleinsch/add_service_config_disable_res…

…olution grpc-js: Add support for grpc.service_config_disable_resolution
grpc · Jan 3, 2023 · 08cc571 · 08cc571
2 parents fbfa73c + 677c009
commit 08cc571
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 1 deletion.
diff --git a/packages/grpc-js/README.md b/packages/grpc-js/README.md
@@ -62,6 +62,7 @@ Many channel arguments supported in `grpc` are not supported in `@grpc/grpc-js`.
   - `grpc.enable_retries`
   - `grpc.per_rpc_retry_buffer_size`
   - `grpc.retry_buffer_size`
+  - `grpc.service_config_disable_resolution`
   - `grpc-node.max_session_memory`
   - `channelOverride`
   - `channelFactoryOverride`

diff --git a/packages/grpc-js/src/channel-options.ts b/packages/grpc-js/src/channel-options.ts
@@ -55,6 +55,7 @@ export interface ChannelOptions {
   'grpc.max_connection_age_ms'?: number;
   'grpc.max_connection_age_grace_ms'?: number;
   'grpc-node.max_session_memory'?: number;
+  'grpc.service_config_disable_resolution'?: number;
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   [key: string]: any;
 }
@@ -87,6 +88,7 @@ export const recognizedOptions = {
   'grpc.max_connection_age_ms': true,
   'grpc.max_connection_age_grace_ms': true,
   'grpc-node.max_session_memory': true,
+  'grpc.service_config_disable_resolution': true,
 };
 
 export function channelOptionsEqual(

diff --git a/packages/grpc-js/src/resolver-dns.ts b/packages/grpc-js/src/resolver-dns.ts
@@ -98,6 +98,7 @@ class DnsResolver implements Resolver {
   private continueResolving = false;
   private nextResolutionTimer: NodeJS.Timer;
   private isNextResolutionTimerRunning = false;
+  private isServiceConfigEnabled = true;
   constructor(
     private target: GrpcUri,
     private listener: ResolverListener,
@@ -127,6 +128,10 @@ class DnsResolver implements Resolver {
     }
     this.percentage = Math.random() * 100;
 
+    if (channelOptions['grpc.service_config_disable_resolution'] === 1) {
+      this.isServiceConfigEnabled = false;
+    }
+
     this.defaultResolutionError = {
       code: Status.UNAVAILABLE,
       details: `Name resolution failed for target ${uriToString(this.target)}`,
@@ -255,7 +260,7 @@ class DnsResolver implements Resolver {
       );
       /* If there already is a still-pending TXT resolution, we can just use
        * that result when it comes in */
-      if (this.pendingTxtPromise === null) {
+      if (this.isServiceConfigEnabled && this.pendingTxtPromise === null) {
         /* We handle the TXT query promise differently than the others because
          * the name resolution attempt as a whole is a success even if the TXT
          * lookup fails */

diff --git a/packages/grpc-js/test/test-resolver.ts b/packages/grpc-js/test/test-resolver.ts
@@ -207,6 +207,64 @@ describe('Name Resolver', () => {
       const resolver = resolverManager.createResolver(target, listener, {});
       resolver.updateResolution();
     });
+    // Created DNS TXT record using TXT sample from https://github.com/grpc/proposal/blob/master/A2-service-configs-in-dns.md
+    // "grpc_config=[{\"serviceConfig\":{\"loadBalancingPolicy\":\"round_robin\",\"methodConfig\":[{\"name\":[{\"service\":\"MyService\",\"method\":\"Foo\"}],\"waitForReady\":true}]}}]"
+    it.skip('Should resolve a name with TXT service config', done => {
+      const target = resolverManager.mapUriDefaultScheme(parseUri('grpctest.kleinsch.com')!)!;
+      const listener: resolverManager.ResolverListener = {
+        onSuccessfulResolution: (
+          addressList: SubchannelAddress[],
+          serviceConfig: ServiceConfig | null,
+          serviceConfigError: StatusObject | null
+        ) => {
+          if (serviceConfig !== null) {
+            assert(
+              serviceConfig.loadBalancingPolicy === 'round_robin',
+              'Should have found round robin LB policy'
+            );
+            done();
+          }
+        },
+        onError: (error: StatusObject) => {
+          done(new Error(`Failed with status ${error.details}`));
+        },
+      };
+      const resolver = resolverManager.createResolver(target, listener, {});
+      resolver.updateResolution();
+    });
+    it.skip(
+      'Should not resolve TXT service config if we disabled service config',
+      (done) => {
+        const target = resolverManager.mapUriDefaultScheme(
+          parseUri('grpctest.kleinsch.com')!
+        )!;
+        let count = 0;
+        const listener: resolverManager.ResolverListener = {
+          onSuccessfulResolution: (
+            addressList: SubchannelAddress[],
+            serviceConfig: ServiceConfig | null,
+            serviceConfigError: StatusObject | null
+          ) => {
+            assert(
+              serviceConfig === null,
+              'Should not have found service config'
+            );
+            count++;
+          },
+          onError: (error: StatusObject) => {
+            done(new Error(`Failed with status ${error.details}`));
+          },
+        };
+        const resolver = resolverManager.createResolver(target, listener, {
+          'grpc.service_config_disable_resolution': 1,
+        });
+        resolver.updateResolution();
+        setTimeout(() => {
+          assert(count === 1, 'Should have only resolved once');
+          done();
+        }, 2_000);
+      }
+    );
     /* The DNS entry for loopback4.unittest.grpc.io only has a single A record
      * with the address 127.0.0.1, but the Mac DNS resolver appears to use
      * NAT64 to create an IPv6 address in that case, so it instead returns