From 2d9a3a91acfbab0b3f471bad3421113fec14385b Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Wed, 27 Sep 2023 11:10:27 -0300 Subject: [PATCH 1/4] [SDKS-7561] Input validation for evaluationcd --- src/logger/constants.ts | 1 + src/sdkClient/clientInputValidation.ts | 20 +++++---- .../__tests__/splitFilters.spec.ts | 44 ++++++++++++++++++- src/utils/settingsValidation/splitFilters.ts | 20 ++++++++- 4 files changed, 73 insertions(+), 12 deletions(-) diff --git a/src/logger/constants.ts b/src/logger/constants.ts index 5e0e5591..4ac3d64a 100644 --- a/src/logger/constants.ts +++ b/src/logger/constants.ts @@ -99,6 +99,7 @@ export const STREAMING_PARSING_MY_SEGMENTS_UPDATE_V2 = 223; export const STREAMING_PARSING_SPLIT_UPDATE = 224; export const WARN_SPLITS_FILTER_INVALID_SET = 225; export const WARN_SPLITS_FILTER_LOWERCASE_SET = 226; +export const WARN_FLAGSET_NOT_CONFIGURED = 227; export const ERROR_ENGINE_COMBINER_IFELSEIF = 300; export const ERROR_LOGLEVEL_INVALID = 301; diff --git a/src/sdkClient/clientInputValidation.ts b/src/sdkClient/clientInputValidation.ts index 94edfafb..28d4099c 100644 --- a/src/sdkClient/clientInputValidation.ts +++ b/src/sdkClient/clientInputValidation.ts @@ -17,6 +17,7 @@ import { IReadinessManager } from '../readiness/types'; import { MaybeThenable } from '../dtos/types'; import { ISettings, SplitIO } from '../types'; import { isStorageSync } from '../trackers/impressionObserver/utils'; +import { flagSetsAreValid } from '../utils/settingsValidation/splitFilters'; /** * Decorator that validates the input before actually executing the client methods. @@ -30,21 +31,22 @@ export function clientInputValidationDecorator 0) && attributes !== false; return { valid, @@ -126,20 +128,20 @@ export function clientInputValidationDecorator { @@ -128,4 +128,44 @@ describe('validateSplitFilters', () => { expect(loggerMock.warn.mock.calls.length).toEqual(12); expect(loggerMock.error.mock.calls.length).toEqual(3); }); + + test('flagSetsAreValid - Flag set validation for evaluations', () => { + + let flagSetsFilter = ['set_1', 'set_2']; + + // empty array + expect(flagSetsAreValid(loggerMock, 'test_method', [], flagSetsFilter)).toEqual([]); + + // must start with a letter or number + expect(flagSetsAreValid(loggerMock, 'test_method', ['_set_1'], flagSetsFilter)).toEqual([]); + expect(loggerMock.warn.mock.calls[0]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_1', regexp, '_set_1']]); + + // can contain _a-z0-9 + expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1'], flagSetsFilter)).toEqual([]); + expect(loggerMock.warn.mock.calls[1]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]); + + // have a max length of 50 characters + const longName = '1234567890_1234567890_1234567890_1234567890_1234567890'; + expect(flagSetsAreValid(loggerMock, 'test_method', [longName], flagSetsFilter)).toEqual([]); + expect(loggerMock.warn.mock.calls[2]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, [longName, regexp, longName]]); + + // both set names invalid -> empty list & warn + expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1','set*3'], flagSetsFilter)).toEqual([]); + expect(loggerMock.warn.mock.calls[3]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]); + expect(loggerMock.warn.mock.calls[4]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]); + + // only set_1 is valid => [set_1] & warn + expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1','set*3'], flagSetsFilter)).toEqual(['set_1']); + expect(loggerMock.warn.mock.calls[5]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]); + + // set_3 not included in configuration but set_1 included => [set_1] & warn + expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1','set_3'], flagSetsFilter)).toEqual(['set_1']); + expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['set_3']]); + + // set_3 not included in configuration => [] & warn + expect(flagSetsAreValid(loggerMock, 'test_method', ['set_3'], flagSetsFilter)).toEqual([]); + expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['set_3']]); + + }); + }); diff --git a/src/utils/settingsValidation/splitFilters.ts b/src/utils/settingsValidation/splitFilters.ts index 264c9975..4810ccb0 100644 --- a/src/utils/settingsValidation/splitFilters.ts +++ b/src/utils/settingsValidation/splitFilters.ts @@ -3,7 +3,7 @@ import { validateSplits } from '../inputValidation/splits'; import { ISplitFiltersValidation } from '../../dtos/types'; import { SplitIO } from '../../types'; import { ILogger } from '../../logger/types'; -import { WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SPLITS_FILTER_NAME_AND_SET, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET } from '../../logger/constants'; +import { WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SPLITS_FILTER_NAME_AND_SET, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET, ERROR_EMPTY_ARRAY, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants'; import { objectAssign } from '../lang/objectAssign'; import { find, uniq } from '../lang'; @@ -187,3 +187,21 @@ export function validateSplitFilters(log: ILogger, maybeSplitFilters: any, mode: return res; } + +export function flagSetsAreValid(log: ILogger, method: string, flagSets: string[], flagSetsInConfig: string[]): string[] { + let toReturn: string[] = []; + if (flagSets.length === 0) { + log.error(ERROR_EMPTY_ARRAY, [method, 'flagSets']); + return toReturn; + } + const sets = validateSplits(log, flagSets, method, 'flag sets', 'flag set'); + toReturn = sets ? sanitizeFlagSets(log, sets) : []; + return toReturn.filter(flagSet => { + if (flagSetsInConfig.indexOf(flagSet) > -1) { + return true; + } + log.warn(WARN_FLAGSET_NOT_CONFIGURED, [flagSet]); + return false; + }); + +} From 925d55288bd6f9f8f7484f1056fdb3a5020c36a9 Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Wed, 27 Sep 2023 11:47:44 -0300 Subject: [PATCH 2/4] Allow any set if flag sets in config is empty --- .../__tests__/splitFilters.spec.ts | 29 +++++++++++++++++++ src/utils/settingsValidation/splitFilters.ts | 17 ++++++----- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/utils/settingsValidation/__tests__/splitFilters.spec.ts b/src/utils/settingsValidation/__tests__/splitFilters.spec.ts index bca919ed..8d6b80ca 100644 --- a/src/utils/settingsValidation/__tests__/splitFilters.spec.ts +++ b/src/utils/settingsValidation/__tests__/splitFilters.spec.ts @@ -166,6 +166,35 @@ describe('validateSplitFilters', () => { expect(flagSetsAreValid(loggerMock, 'test_method', ['set_3'], flagSetsFilter)).toEqual([]); expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['set_3']]); + // empty config + + + // must start with a letter or number + expect(flagSetsAreValid(loggerMock, 'test_method', ['_set_1'], [])).toEqual([]); + expect(loggerMock.warn.mock.calls[8]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['_set_1', regexp, '_set_1']]); + + // can contain _a-z0-9 + expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1'], [])).toEqual([]); + expect(loggerMock.warn.mock.calls[9]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]); + + // have a max length of 50 characters + expect(flagSetsAreValid(loggerMock, 'test_method', [longName], [])).toEqual([]); + expect(loggerMock.warn.mock.calls[10]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, [longName, regexp, longName]]); + + // both set names invalid -> empty list & warn + expect(flagSetsAreValid(loggerMock, 'test_method', ['set*1','set*3'], [])).toEqual([]); + expect(loggerMock.warn.mock.calls[11]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*1', regexp, 'set*1']]); + expect(loggerMock.warn.mock.calls[12]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]); + + // only set_1 is valid => [set_1] & warn + expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1','set*3'], [])).toEqual(['set_1']); + expect(loggerMock.warn.mock.calls[13]).toEqual([WARN_SPLITS_FILTER_INVALID_SET, ['set*3', regexp, 'set*3']]); + + // any set should be returned if there isn't flag sets in filter + expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1'], [])).toEqual(['set_1']); + expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1','set_2'], [])).toEqual(['set_1','set_2']); + expect(flagSetsAreValid(loggerMock, 'test_method', ['set_3'], [])).toEqual(['set_3']); + }); }); diff --git a/src/utils/settingsValidation/splitFilters.ts b/src/utils/settingsValidation/splitFilters.ts index 4810ccb0..1df96f1b 100644 --- a/src/utils/settingsValidation/splitFilters.ts +++ b/src/utils/settingsValidation/splitFilters.ts @@ -196,12 +196,15 @@ export function flagSetsAreValid(log: ILogger, method: string, flagSets: string[ } const sets = validateSplits(log, flagSets, method, 'flag sets', 'flag set'); toReturn = sets ? sanitizeFlagSets(log, sets) : []; - return toReturn.filter(flagSet => { - if (flagSetsInConfig.indexOf(flagSet) > -1) { - return true; - } - log.warn(WARN_FLAGSET_NOT_CONFIGURED, [flagSet]); - return false; - }); + if (flagSetsInConfig.length > 0) { + toReturn = toReturn.filter(flagSet => { + if (flagSetsInConfig.indexOf(flagSet) > -1) { + return true; + } + log.warn(WARN_FLAGSET_NOT_CONFIGURED, [flagSet]); + return false; + }); + } + return toReturn; } From deaec3ff3b680440e1e16339a78bdee4a332537f Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Mon, 2 Oct 2023 11:02:04 -0300 Subject: [PATCH 3/4] Code optimization & warn declaration --- src/logger/messages/warn.ts | 1 + src/sdkClient/clientInputValidation.ts | 2 +- .../settingsValidation/__tests__/splitFilters.spec.ts | 4 ++-- src/utils/settingsValidation/splitFilters.ts | 9 ++------- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/logger/messages/warn.ts b/src/logger/messages/warn.ts index b576095a..3699ac52 100644 --- a/src/logger/messages/warn.ts +++ b/src/logger/messages/warn.ts @@ -24,6 +24,7 @@ export const codesWarn: [number, string][] = codesError.concat([ [c.WARN_NOT_EXISTENT_SPLIT, '%s: feature flag "%s" does not exist in this environment. Please double check what feature flags exist in the Split user interface.'], [c.WARN_LOWERCASE_TRAFFIC_TYPE, '%s: traffic_type_name should be all lowercase - converting string to lowercase.'], [c.WARN_NOT_EXISTENT_TT, '%s: traffic type "%s" does not have any corresponding feature flag in this environment, make sure you\'re tracking your events to a valid traffic type defined in the Split user interface.'], + [c.WARN_FLAGSET_NOT_CONFIGURED, '%s: : you passed %s wich is not part of the configured FlagSetsFilter, ignoring Flag Set.'], // initialization / settings validation [c.WARN_INTEGRATION_INVALID, c.LOG_PREFIX_SETTINGS+': %s integration item(s) at settings is invalid. %s'], [c.WARN_SPLITS_FILTER_IGNORED, c.LOG_PREFIX_SETTINGS+': feature flag filters have been configured but will have no effect if mode is not "%s", since synchronization is being deferred to an external tool.'], diff --git a/src/sdkClient/clientInputValidation.ts b/src/sdkClient/clientInputValidation.ts index 28d4099c..50860b9f 100644 --- a/src/sdkClient/clientInputValidation.ts +++ b/src/sdkClient/clientInputValidation.ts @@ -31,7 +31,7 @@ export function clientInputValidationDecorator { // set_3 not included in configuration but set_1 included => [set_1] & warn expect(flagSetsAreValid(loggerMock, 'test_method', ['set_1','set_3'], flagSetsFilter)).toEqual(['set_1']); - expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['set_3']]); + expect(loggerMock.warn.mock.calls[6]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method', 'set_3']]); // set_3 not included in configuration => [] & warn expect(flagSetsAreValid(loggerMock, 'test_method', ['set_3'], flagSetsFilter)).toEqual([]); - expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['set_3']]); + expect(loggerMock.warn.mock.calls[7]).toEqual([WARN_FLAGSET_NOT_CONFIGURED, ['test_method','set_3']]); // empty config diff --git a/src/utils/settingsValidation/splitFilters.ts b/src/utils/settingsValidation/splitFilters.ts index 1df96f1b..558a870c 100644 --- a/src/utils/settingsValidation/splitFilters.ts +++ b/src/utils/settingsValidation/splitFilters.ts @@ -189,19 +189,14 @@ export function validateSplitFilters(log: ILogger, maybeSplitFilters: any, mode: } export function flagSetsAreValid(log: ILogger, method: string, flagSets: string[], flagSetsInConfig: string[]): string[] { - let toReturn: string[] = []; - if (flagSets.length === 0) { - log.error(ERROR_EMPTY_ARRAY, [method, 'flagSets']); - return toReturn; - } const sets = validateSplits(log, flagSets, method, 'flag sets', 'flag set'); - toReturn = sets ? sanitizeFlagSets(log, sets) : []; + let toReturn = sets ? sanitizeFlagSets(log, sets) : []; if (flagSetsInConfig.length > 0) { toReturn = toReturn.filter(flagSet => { if (flagSetsInConfig.indexOf(flagSet) > -1) { return true; } - log.warn(WARN_FLAGSET_NOT_CONFIGURED, [flagSet]); + log.warn(WARN_FLAGSET_NOT_CONFIGURED, [method, flagSet]); return false; }); } From c9c89b81c52a77a71af75fa600a0b0364765077f Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Mon, 2 Oct 2023 11:03:21 -0300 Subject: [PATCH 4/4] Remove unused constant --- src/utils/settingsValidation/splitFilters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/settingsValidation/splitFilters.ts b/src/utils/settingsValidation/splitFilters.ts index 558a870c..7b6a7411 100644 --- a/src/utils/settingsValidation/splitFilters.ts +++ b/src/utils/settingsValidation/splitFilters.ts @@ -3,7 +3,7 @@ import { validateSplits } from '../inputValidation/splits'; import { ISplitFiltersValidation } from '../../dtos/types'; import { SplitIO } from '../../types'; import { ILogger } from '../../logger/types'; -import { WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SPLITS_FILTER_NAME_AND_SET, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET, ERROR_EMPTY_ARRAY, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants'; +import { WARN_SPLITS_FILTER_IGNORED, WARN_SPLITS_FILTER_EMPTY, WARN_SPLITS_FILTER_INVALID, SETTINGS_SPLITS_FILTER, LOG_PREFIX_SETTINGS, ERROR_SPLITS_FILTER_NAME_AND_SET, WARN_SPLITS_FILTER_LOWERCASE_SET, WARN_SPLITS_FILTER_INVALID_SET, WARN_FLAGSET_NOT_CONFIGURED } from '../../logger/constants'; import { objectAssign } from '../lang/objectAssign'; import { find, uniq } from '../lang';