Merge pull request #2550 from murgatroid99/grpc-js_service_config_par…

…sing grpc-js: Fix method config name handling in service configs
grpc · Aug 22, 2023 · 7ca0af6 · 7ca0af6
2 parents cd25bad + 69257a7
commit 7ca0af6
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 27 deletions.
diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@grpc/grpc-js",
-  "version": "1.9.0",
+  "version": "1.9.1",
   "description": "gRPC Library for Node - pure JS implementation",
   "homepage": "https://grpc.io/",
   "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",

diff --git a/packages/grpc-js/src/resolving-load-balancer.ts b/packages/grpc-js/src/resolving-load-balancer.ts
@@ -21,7 +21,11 @@ import {
   LoadBalancingConfig,
   getFirstUsableConfig,
 } from './load-balancer';
-import { ServiceConfig, validateServiceConfig } from './service-config';
+import {
+  MethodConfig,
+  ServiceConfig,
+  validateServiceConfig,
+} from './service-config';
 import { ConnectivityState } from './connectivity-state';
 import { ConfigSelector, createResolver, Resolver } from './resolver';
 import { ServiceError } from './call';
@@ -43,6 +47,59 @@ function trace(text: string): void {
   logging.trace(LogVerbosity.DEBUG, TRACER_NAME, text);
 }
 
+type NameMatchLevel = 'EMPTY' | 'SERVICE' | 'SERVICE_AND_METHOD';
+
+/**
+ * Name match levels in order from most to least specific. This is the order in
+ * which searches will be performed.
+ */
+const NAME_MATCH_LEVEL_ORDER: NameMatchLevel[] = [
+  'SERVICE_AND_METHOD',
+  'SERVICE',
+  'EMPTY',
+];
+
+function hasMatchingName(
+  service: string,
+  method: string,
+  methodConfig: MethodConfig,
+  matchLevel: NameMatchLevel
+): boolean {
+  for (const name of methodConfig.name) {
+    switch (matchLevel) {
+      case 'EMPTY':
+        if (!name.service && !name.method) {
+          return true;
+        }
+        break;
+      case 'SERVICE':
+        if (name.service === service && !name.method) {
+          return true;
+        }
+        break;
+      case 'SERVICE_AND_METHOD':
+        if (name.service === service && name.method === method) {
+          return true;
+        }
+    }
+  }
+  return false;
+}
+
+function findMatchingConfig(
+  service: string,
+  method: string,
+  methodConfigs: MethodConfig[],
+  matchLevel: NameMatchLevel
+): MethodConfig | null {
+  for (const config of methodConfigs) {
+    if (hasMatchingName(service, method, config, matchLevel)) {
+      return config;
+    }
+  }
+  return null;
+}
+
 function getDefaultConfigSelector(
   serviceConfig: ServiceConfig | null
 ): ConfigSelector {
@@ -54,19 +111,26 @@ function getDefaultConfigSelector(
     const service = splitName[0] ?? '';
     const method = splitName[1] ?? '';
     if (serviceConfig && serviceConfig.methodConfig) {
-      for (const methodConfig of serviceConfig.methodConfig) {
-        for (const name of methodConfig.name) {
-          if (
-            name.service === service &&
-            (name.method === undefined || name.method === method)
-          ) {
-            return {
-              methodConfig: methodConfig,
-              pickInformation: {},
-              status: Status.OK,
-              dynamicFilterFactories: [],
-            };
-          }
+      /* Check for the following in order, and return the first method
+       * config that matches:
+       * 1. A name that exactly matches the service and method
+       * 2. A name with no method set that matches the service
+       * 3. An empty name
+       */
+      for (const matchLevel of NAME_MATCH_LEVEL_ORDER) {
+        const matchingConfig = findMatchingConfig(
+          service,
+          method,
+          serviceConfig.methodConfig,
+          matchLevel
+        );
+        if (matchingConfig) {
+          return {
+            methodConfig: matchingConfig,
+            pickInformation: {},
+            status: Status.OK,
+            dynamicFilterFactories: [],
+          };
         }
       }
     }

diff --git a/packages/grpc-js/src/service-config.ts b/packages/grpc-js/src/service-config.ts
@@ -35,7 +35,7 @@ import {
 } from './load-balancer';
 
 export interface MethodConfigName {
-  service: string;
+  service?: string;
   method?: string;
 }
 
@@ -95,20 +95,36 @@ const DURATION_REGEX = /^\d+(\.\d{1,9})?s$/;
 const CLIENT_LANGUAGE_STRING = 'node';
 
 function validateName(obj: any): MethodConfigName {
-  if (!('service' in obj) || typeof obj.service !== 'string') {
-    throw new Error('Invalid method config name: invalid service');
-  }
-  const result: MethodConfigName = {
-    service: obj.service,
-  };
-  if ('method' in obj) {
-    if (typeof obj.method === 'string') {
-      result.method = obj.method;
+  // In this context, and unset field and '' are considered the same
+  if ('service' in obj && obj.service !== '') {
+    if (typeof obj.service !== 'string') {
+      throw new Error(
+        `Invalid method config name: invalid service: expected type string, got ${typeof obj.service}`
+      );
+    }
+    if ('method' in obj && obj.method !== '') {
+      if (typeof obj.method !== 'string') {
+        throw new Error(
+          `Invalid method config name: invalid method: expected type string, got ${typeof obj.service}`
+        );
+      }
+      return {
+        service: obj.service,
+        method: obj.method,
+      };
     } else {
-      throw new Error('Invalid method config name: invalid method');
+      return {
+        service: obj.service,
+      };
     }
+  } else {
+    if ('method' in obj && obj.method !== undefined) {
+      throw new Error(
+        `Invalid method config name: method set with empty or unset service`
+      );
+    }
+    return {};
   }
-  return result;
 }
 
 function validateRetryPolicy(obj: any): RetryPolicy {