From b2ad73a0f3244e80da7871a54809941d1b0dbf24 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 10 Aug 2023 13:54:43 -0700 Subject: [PATCH] grpc-js-xds: Add config parsing tests --- packages/grpc-js-xds/src/load-balancer-cds.ts | 7 +- packages/grpc-js-xds/src/load-balancer-lrs.ts | 5 +- .../grpc-js-xds/src/load-balancer-priority.ts | 3 +- packages/grpc-js-xds/src/xds-bootstrap.ts | 3 + .../grpc-js-xds/test/test-confg-parsing.ts | 370 ++++++++++++++++++ 5 files changed, 382 insertions(+), 6 deletions(-) create mode 100644 packages/grpc-js-xds/test/test-confg-parsing.ts diff --git a/packages/grpc-js-xds/src/load-balancer-cds.ts b/packages/grpc-js-xds/src/load-balancer-cds.ts index 3ebcbbdf..44de10ac 100644 --- a/packages/grpc-js-xds/src/load-balancer-cds.ts +++ b/packages/grpc-js-xds/src/load-balancer-cds.ts @@ -65,11 +65,10 @@ class CdsLoadBalancingConfig implements TypedLoadBalancingConfig { } static createFromJson(obj: any): CdsLoadBalancingConfig { - if ('cluster' in obj) { - return new CdsLoadBalancingConfig(obj.cluster); - } else { - throw new Error('Missing "cluster" in cds load balancing config'); + if (!('cluster' in obj && typeof obj.cluster === 'string')) { + throw new Error('cds config must have a string field cluster'); } + return new CdsLoadBalancingConfig(obj.cluster); } } diff --git a/packages/grpc-js-xds/src/load-balancer-lrs.ts b/packages/grpc-js-xds/src/load-balancer-lrs.ts index a0e568ea..40c653a0 100644 --- a/packages/grpc-js-xds/src/load-balancer-lrs.ts +++ b/packages/grpc-js-xds/src/load-balancer-lrs.ts @@ -46,7 +46,7 @@ class LrsLoadBalancingConfig implements TypedLoadBalancingConfig { [TYPE_NAME]: { cluster_name: this.clusterName, eds_service_name: this.edsServiceName, - lrs_load_reporting_server_name: this.lrsLoadReportingServer, + lrs_load_reporting_server: this.lrsLoadReportingServer, locality: this.locality, child_policy: [this.childPolicy.toJsonObject()] } @@ -97,6 +97,9 @@ 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'); } + if (!('lrs_load_reporting_server' in obj && obj.lrs_load_reporting_server !== null && typeof obj.lrs_load_reporting_server === 'object')) { + throw new Error('lrs config must have an object field lrs_load_reporting_server'); + } const childConfig = selectLbConfigFromList(obj.child_policy); if (!childConfig) { throw new Error('lrs config child_policy parsing failed'); diff --git a/packages/grpc-js-xds/src/load-balancer-priority.ts b/packages/grpc-js-xds/src/load-balancer-priority.ts index 4d26a0f4..4372b0ea 100644 --- a/packages/grpc-js-xds/src/load-balancer-priority.ts +++ b/packages/grpc-js-xds/src/load-balancer-priority.ts @@ -81,7 +81,8 @@ class PriorityLoadBalancingConfig implements TypedLoadBalancingConfig { const childrenField: {[key: string]: object} = {} for (const [childName, childValue] of this.children.entries()) { childrenField[childName] = { - config: [childValue.config.toJsonObject()] + config: [childValue.config.toJsonObject()], + ignore_reresolution_requests: childValue.ignore_reresolution_requests }; } return { diff --git a/packages/grpc-js-xds/src/xds-bootstrap.ts b/packages/grpc-js-xds/src/xds-bootstrap.ts index f5df10df..fccd3edc 100644 --- a/packages/grpc-js-xds/src/xds-bootstrap.ts +++ b/packages/grpc-js-xds/src/xds-bootstrap.ts @@ -112,6 +112,9 @@ const SUPPORTED_CHANNEL_CREDS_TYPES = [ ]; export function validateXdsServerConfig(obj: any): XdsServerConfig { + if (!(typeof obj === 'object' && obj !== null)) { + throw new Error('xDS server config must be an object'); + } if (!('server_uri' in obj)) { throw new Error('server_uri field missing in xds_servers element'); } diff --git a/packages/grpc-js-xds/test/test-confg-parsing.ts b/packages/grpc-js-xds/test/test-confg-parsing.ts new file mode 100644 index 00000000..ba91bb2e --- /dev/null +++ b/packages/grpc-js-xds/test/test-confg-parsing.ts @@ -0,0 +1,370 @@ +/* + * 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 { experimental, LoadBalancingConfig } from "@grpc/grpc-js"; +import { register } from "../src"; +import assert = require("assert"); +import parseLoadbalancingConfig = experimental.parseLoadBalancingConfig; + +register(); + +/** + * Describes a test case for config parsing. input is passed to + * parseLoadBalancingConfig. If error is set, the expectation is that that + * operation throws an error with a matching message. Otherwise, toJsonObject + * is called on the result, and it is expected to match output, or input if + * output is unset. + */ +interface TestCase { + name: string; + input: object, + output?: object; + error?: RegExp; +} + +/* The main purpose of these tests is to verify that configs that are expected + * to be valid parse successfully, and configs that are expected to be invalid + * throw errors. The specific output of this parsing is a lower priority + * concern. + * Note: some tests have an expected output that is different from the output, + * but all non-error tests additionally verify that parsing the output again + * produces the same output. */ +const allTestCases: {[lbPolicyName: string]: TestCase[]} = { + cds: [ + { + name: 'populated cluster field', + input: { + cluster: 'abc' + } + }, + { + name: 'empty', + input: {}, + error: /cluster/ + }, + { + name: 'non-string cluster', + input: { + cluster: 123 + }, + error: /string.*cluster/ + } + ], + xds_cluster_resolver: [ + { + name: 'empty fields', + input: { + discovery_mechanisms: [], + locality_picking_policy: [], + endpoint_picking_policy: [] + } + }, + { + name: 'missing discovery_mechanisms', + input: { + locality_picking_policy: [], + endpoint_picking_policy: [] + }, + error: /discovery_mechanisms/ + }, + { + name: 'missing locality_picking_policy', + input: { + discovery_mechanisms: [], + endpoint_picking_policy: [] + }, + error: /locality_picking_policy/ + }, + { + name: 'missing endpoint_picking_policy', + input: { + discovery_mechanisms: [], + locality_picking_policy: [] + }, + error: /endpoint_picking_policy/ + }, + { + name: 'discovery_mechanism: EDS', + input: { + discovery_mechanisms: [{ + cluster: 'abc', + type: 'EDS' + }], + locality_picking_policy: [], + endpoint_picking_policy: [] + }, + output: { + discovery_mechanisms: [{ + cluster: 'abc', + type: 'EDS', + lrs_load_reporting_server: undefined + }], + locality_picking_policy: [], + endpoint_picking_policy: [] + } + }, + { + name: 'discovery_mechanism: LOGICAL_DNS', + input: { + discovery_mechanisms: [{ + cluster: 'abc', + type: 'LOGICAL_DNS' + }], + locality_picking_policy: [], + endpoint_picking_policy: [] + }, + output: { + discovery_mechanisms: [{ + cluster: 'abc', + type: 'LOGICAL_DNS', + lrs_load_reporting_server: undefined + }], + locality_picking_policy: [], + endpoint_picking_policy: [] + } + }, + { + name: 'discovery_mechanism: undefined optional fields', + input: { + discovery_mechanisms: [{ + cluster: 'abc', + type: 'EDS', + max_concurrent_requests: undefined, + eds_service_name: undefined, + dns_hostname: undefined, + lrs_load_reporting_server: undefined + }], + locality_picking_policy: [], + endpoint_picking_policy: [] + } + }, + { + name: 'discovery_mechanism: populated optional fields', + input: { + discovery_mechanisms: [{ + cluster: 'abc', + type: 'EDS', + max_concurrent_requests: 100, + eds_service_name: 'def', + dns_hostname: 'localhost', + lrs_load_reporting_server: { + server_uri: 'localhost:12345', + channel_creds: [{ + type: 'google_default', + config: {} + }], + server_features: ['test'] + } + }], + locality_picking_policy: [], + endpoint_picking_policy: [] + } + } + ], + xds_cluster_impl: [ + { + name: 'only required fields', + input: { + cluster: 'abc', + drop_categories: [], + child_policy: [{round_robin: {}}] + }, + output: { + cluster: 'abc', + drop_categories: [], + child_policy: [{round_robin: {}}], + max_concurrent_requests: 1024 + } + }, + { + name: 'undefined optional fields', + input: { + cluster: 'abc', + drop_categories: [], + child_policy: [{round_robin: {}}], + eds_service_name: undefined, + max_concurrent_requests: undefined + }, + output: { + cluster: 'abc', + drop_categories: [], + child_policy: [{round_robin: {}}], + max_concurrent_requests: 1024 + } + }, + { + name: 'populated optional fields', + input: { + cluster: 'abc', + drop_categories: [{ + category: 'test', + requests_per_million: 100 + }], + child_policy: [{round_robin: {}}], + eds_service_name: 'def', + max_concurrent_requests: 123 + }, + } + ], + lrs: [ + { + name: 'only required fields', + input: { + cluster_name: 'abc', + eds_service_name: 'def', + locality: {}, + child_policy: [{round_robin: {}}], + lrs_load_reporting_server: { + server_uri: 'localhost:12345', + channel_creds: [{ + type: 'google_default', + config: {} + }], + server_features: ['test'] + } + }, + output: { + cluster_name: 'abc', + eds_service_name: 'def', + locality: { + region: '', + zone: '', + sub_zone: '' + }, + child_policy: [{round_robin: {}}], + lrs_load_reporting_server: { + server_uri: 'localhost:12345', + channel_creds: [{ + type: 'google_default', + config: {} + }], + server_features: ['test'] + } + } + }, + { + name: 'populated optional fields', + input: { + cluster_name: 'abc', + eds_service_name: 'def', + locality: { + region: 'a', + zone: 'b', + sub_zone: 'c' + }, + child_policy: [{round_robin: {}}], + lrs_load_reporting_server: { + server_uri: 'localhost:12345', + channel_creds: [{ + type: 'google_default', + config: {} + }], + server_features: ['test'] + } + } + } + ], + priority: [ + { + name: 'empty fields', + input: { + children: {}, + priorities: [] + } + }, + { + name: 'populated fields', + input: { + children: { + child0: { + config: [{round_robin: {}}], + ignore_reresolution_requests: true + }, + child1: { + config: [{round_robin: {}}], + ignore_reresolution_requests: false + } + }, + priorities: ['child0', 'child1'] + } + } + ], + weighted_target: [ + { + name: 'empty targets field', + input: { + targets: {} + } + }, + { + name: 'populated targets field', + input: { + targets: { + target0: { + weight: 1, + child_policy: [{round_robin: {}}] + }, + target1: { + weight: 2, + child_policy: [{round_robin: {}}] + } + } + } + } + ], + xds_cluster_manager: [ + { + name: 'empty children field', + input: { + children: {} + } + }, + { + name: 'populated children field', + input: { + children: { + child0: { + child_policy: [{round_robin: {}}] + } + } + } + } + ] +} + +describe('Load balancing policy config parsing', () => { + for (const [lbPolicyName, testCases] of Object.entries(allTestCases)) { + describe(lbPolicyName, () => { + for (const testCase of testCases) { + it(testCase.name, () => { + const lbConfigInput = {[lbPolicyName]: testCase.input}; + if (testCase.error) { + assert.throws(() => { + parseLoadbalancingConfig(lbConfigInput); + }, testCase.error); + } else { + const expectedOutput = testCase.output ?? testCase.input; + const parsedJson = parseLoadbalancingConfig(lbConfigInput).toJsonObject(); + assert.deepStrictEqual(parsedJson, {[lbPolicyName]: expectedOutput}); + // Test idempotency + assert.deepStrictEqual(parseLoadbalancingConfig(parsedJson).toJsonObject(), parsedJson); + } + }); + } + }); + } +});