From d7c27fb3aa329fc924d7ae35a4334849898ff18e Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 11 Aug 2023 11:09:55 -0700 Subject: [PATCH] grpc-js: Add config parsing tests and fix outlier detection config parsing --- .../src/load-balancer-outlier-detection.ts | 26 ++- packages/grpc-js/test/test-confg-parsing.ts | 212 ++++++++++++++++++ 2 files changed, 226 insertions(+), 12 deletions(-) create mode 100644 packages/grpc-js/test/test-confg-parsing.ts diff --git a/packages/grpc-js/src/load-balancer-outlier-detection.ts b/packages/grpc-js/src/load-balancer-outlier-detection.ts index 00d0e0a53..be62b4c36 100644 --- a/packages/grpc-js/src/load-balancer-outlier-detection.ts +++ b/packages/grpc-js/src/load-balancer-outlier-detection.ts @@ -107,7 +107,7 @@ function validateFieldType( expectedType: TypeofValues, objectName?: string ) { - if (fieldName in obj && typeof obj[fieldName] !== expectedType) { + if (fieldName in obj && obj[fieldName] !== undefined && typeof obj[fieldName] !== expectedType) { const fullFieldName = objectName ? `${objectName}.${fieldName}` : fieldName; throw new Error( `outlier detection config ${fullFieldName} parse error: expected ${expectedType}, got ${typeof obj[ @@ -123,7 +123,7 @@ function validatePositiveDuration( objectName?: string ) { const fullFieldName = objectName ? `${objectName}.${fieldName}` : fieldName; - if (fieldName in obj) { + if (fieldName in obj && obj[fieldName] !== undefined) { if (!isDuration(obj[fieldName])) { throw new Error( `outlier detection config ${fullFieldName} parse error: expected Duration, got ${typeof obj[ @@ -149,7 +149,7 @@ function validatePositiveDuration( function validatePercentage(obj: any, fieldName: string, objectName?: string) { const fullFieldName = objectName ? `${objectName}.${fieldName}` : fieldName; validateFieldType(obj, fieldName, 'number', objectName); - if (fieldName in obj && !(obj[fieldName] >= 0 && obj[fieldName] <= 100)) { + if (fieldName in obj && obj[fieldName] !== undefined && !(obj[fieldName] >= 0 && obj[fieldName] <= 100)) { throw new Error( `outlier detection config ${fullFieldName} parse error: value out of range for percentage (0-100)` ); @@ -201,13 +201,15 @@ export class OutlierDetectionLoadBalancingConfig } toJsonObject(): object { return { - interval: msToDuration(this.intervalMs), - base_ejection_time: msToDuration(this.baseEjectionTimeMs), - max_ejection_time: msToDuration(this.maxEjectionTimeMs), - max_ejection_percent: this.maxEjectionPercent, - success_rate_ejection: this.successRateEjection, - failure_percentage_ejection: this.failurePercentageEjection, - child_policy: [this.childPolicy.toJsonObject()] + outlier_detection: { + interval: msToDuration(this.intervalMs), + base_ejection_time: msToDuration(this.baseEjectionTimeMs), + max_ejection_time: msToDuration(this.maxEjectionTimeMs), + max_ejection_percent: this.maxEjectionPercent, + success_rate_ejection: this.successRateEjection ?? undefined, + failure_percentage_ejection: this.failurePercentageEjection ?? undefined, + child_policy: [this.childPolicy.toJsonObject()] + } }; } @@ -238,7 +240,7 @@ export class OutlierDetectionLoadBalancingConfig validatePositiveDuration(obj, 'base_ejection_time'); validatePositiveDuration(obj, 'max_ejection_time'); validatePercentage(obj, 'max_ejection_percent'); - if ('success_rate_ejection' in obj) { + if ('success_rate_ejection' in obj && obj.success_rate_ejection !== undefined) { if (typeof obj.success_rate_ejection !== 'object') { throw new Error( 'outlier detection config success_rate_ejection must be an object' @@ -268,7 +270,7 @@ export class OutlierDetectionLoadBalancingConfig 'success_rate_ejection' ); } - if ('failure_percentage_ejection' in obj) { + if ('failure_percentage_ejection' in obj && obj.failure_percentage_ejection !== undefined) { if (typeof obj.failure_percentage_ejection !== 'object') { throw new Error( 'outlier detection config failure_percentage_ejection must be an object' diff --git a/packages/grpc-js/test/test-confg-parsing.ts b/packages/grpc-js/test/test-confg-parsing.ts new file mode 100644 index 000000000..569b83f5e --- /dev/null +++ b/packages/grpc-js/test/test-confg-parsing.ts @@ -0,0 +1,212 @@ +/* + * 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 } from '../src'; +import * as assert from 'assert'; +import parseLoadBalancingConfig = experimental.parseLoadBalancingConfig; + +/** + * 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[]} = { + pick_first: [ + { + name: 'no fields set', + input: {}, + output: { + shuffleAddressList: false + } + }, + { + name: 'shuffleAddressList set', + input: { + shuffleAddressList: true + } + } + ], + round_robin: [ + { + name: 'no fields set', + input: {} + } + ], + outlier_detection: [ + { + name: 'only required fields set', + input: { + child_policy: [{round_robin: {}}] + }, + output: { + interval: { + seconds: 10, + nanos: 0 + }, + base_ejection_time: { + seconds: 30, + nanos: 0 + }, + max_ejection_time: { + seconds: 300, + nanos: 0 + }, + max_ejection_percent: 10, + success_rate_ejection: undefined, + failure_percentage_ejection: undefined, + child_policy: [{round_robin: {}}] + } + }, + { + name: 'all optional fields undefined', + input: { + interval: undefined, + base_ejection_time: undefined, + max_ejection_time: undefined, + max_ejection_percent: undefined, + success_rate_ejection: undefined, + failure_percentage_ejection: undefined, + child_policy: [{round_robin: {}}] + }, + output: { + interval: { + seconds: 10, + nanos: 0 + }, + base_ejection_time: { + seconds: 30, + nanos: 0 + }, + max_ejection_time: { + seconds: 300, + nanos: 0 + }, + max_ejection_percent: 10, + success_rate_ejection: undefined, + failure_percentage_ejection: undefined, + child_policy: [{round_robin: {}}] + } + }, + { + name: 'empty ejection configs', + input: { + success_rate_ejection: {}, + failure_percentage_ejection: {}, + child_policy: [{round_robin: {}}] + }, + output: { + interval: { + seconds: 10, + nanos: 0 + }, + base_ejection_time: { + seconds: 30, + nanos: 0 + }, + max_ejection_time: { + seconds: 300, + nanos: 0 + }, + max_ejection_percent: 10, + success_rate_ejection: { + stdev_factor: 1900, + enforcement_percentage: 100, + minimum_hosts: 5, + request_volume: 100 + }, + failure_percentage_ejection: { + threshold: 85, + enforcement_percentage: 100, + minimum_hosts: 5, + request_volume: 50, + }, + child_policy: [{round_robin: {}}] + } + }, + { + name: 'all fields populated', + input: { + interval: { + seconds: 20, + nanos: 0 + }, + base_ejection_time: { + seconds: 40, + nanos: 0 + }, + max_ejection_time: { + seconds: 400, + nanos: 0 + }, + max_ejection_percent: 20, + success_rate_ejection: { + stdev_factor: 1800, + enforcement_percentage: 90, + minimum_hosts: 4, + request_volume: 200 + }, + failure_percentage_ejection: { + threshold: 95, + enforcement_percentage: 90, + minimum_hosts: 4, + request_volume: 60, + }, + 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); + } + }); + } + }); + } +});