Skip to content

Commit

Permalink
validate property types of feature flags (#17)
Browse files Browse the repository at this point in the history
* resovle conflicts

* update validation

* update

* update

* update

* update

* update

* update

---------

Co-authored-by: Lingling Ye (from Dev Box) <[email protected]>
  • Loading branch information
Eskibear and linglingye001 authored Nov 6, 2024
1 parent 5733f56 commit e7119e2
Show file tree
Hide file tree
Showing 7 changed files with 246 additions and 29 deletions.
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 7 additions & 10 deletions src/featureManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import { TimeWindowFilter } from "./filter/TimeWindowFilter.js";
import { IFeatureFilter } from "./filter/FeatureFilter.js";
import { RequirementType } from "./model.js";
import { RequirementType } from "./schema/model.js";
import { IFeatureFlagProvider } from "./featureProvider.js";
import { TargetingFilter } from "./filter/TargetingFilter.js";

Expand All @@ -30,15 +30,12 @@ export class FeatureManager {

// If multiple feature flags are found, the first one takes precedence.
async isEnabled(featureName: string, context?: unknown): Promise<boolean> {
const featureFlag = await this.#provider.getFeatureFlag(featureName);
const featureFlag = await this.#getFeatureFlag(featureName);
if (featureFlag === undefined) {
// If the feature is not found, then it is disabled.
return false;
}

// Ensure that the feature flag is in the correct format. Feature providers should validate the feature flags, but we do it here as a safeguard.
validateFeatureFlagFormat(featureFlag);

if (featureFlag.enabled !== true) {
// If the feature is not explicitly enabled, then it is disabled by default.
return false;
Expand Down Expand Up @@ -75,14 +72,14 @@ export class FeatureManager {
return !shortCircuitEvaluationResult;
}

async #getFeatureFlag(featureName: string): Promise<any> {
const featureFlag = await this.#provider.getFeatureFlag(featureName);
return featureFlag;
}

}

interface FeatureManagerOptions {
customFilters?: IFeatureFilter[];
}

function validateFeatureFlagFormat(featureFlag: any): void {
if (featureFlag.enabled !== undefined && typeof featureFlag.enabled !== "boolean") {
throw new Error(`Feature flag ${featureFlag.id} has an invalid 'enabled' value.`);
}
}
19 changes: 14 additions & 5 deletions src/featureProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
// Licensed under the MIT license.

import { IGettable } from "./gettable.js";
import { FeatureFlag, FeatureManagementConfiguration, FEATURE_MANAGEMENT_KEY, FEATURE_FLAGS_KEY } from "./model.js";
import { FeatureFlag, FeatureManagementConfiguration, FEATURE_MANAGEMENT_KEY, FEATURE_FLAGS_KEY } from "./schema/model.js";
import { validateFeatureFlag } from "./schema/validator.js";

export interface IFeatureFlagProvider {
/**
Expand All @@ -28,12 +29,16 @@ export class ConfigurationMapFeatureFlagProvider implements IFeatureFlagProvider
}
async getFeatureFlag(featureName: string): Promise<FeatureFlag | undefined> {
const featureConfig = this.#configuration.get<FeatureManagementConfiguration>(FEATURE_MANAGEMENT_KEY);
return featureConfig?.[FEATURE_FLAGS_KEY]?.findLast((feature) => feature.id === featureName);
const featureFlag = featureConfig?.[FEATURE_FLAGS_KEY]?.findLast((feature) => feature.id === featureName);
validateFeatureFlag(featureFlag);
return featureFlag;
}

async getFeatureFlags(): Promise<FeatureFlag[]> {
const featureConfig = this.#configuration.get<FeatureManagementConfiguration>(FEATURE_MANAGEMENT_KEY);
return featureConfig?.[FEATURE_FLAGS_KEY] ?? [];
const featureFlag = featureConfig?.[FEATURE_FLAGS_KEY] ?? [];
validateFeatureFlag(featureFlag);
return featureFlag;
}
}

Expand All @@ -49,10 +54,14 @@ export class ConfigurationObjectFeatureFlagProvider implements IFeatureFlagProvi

async getFeatureFlag(featureName: string): Promise<FeatureFlag | undefined> {
const featureFlags = this.#configuration[FEATURE_MANAGEMENT_KEY]?.[FEATURE_FLAGS_KEY];
return featureFlags?.findLast((feature: FeatureFlag) => feature.id === featureName);
const featureFlag = featureFlags?.findLast((feature: FeatureFlag) => feature.id === featureName);
validateFeatureFlag(featureFlag);
return featureFlag;
}

async getFeatureFlags(): Promise<FeatureFlag[]> {
return this.#configuration[FEATURE_MANAGEMENT_KEY]?.[FEATURE_FLAGS_KEY] ?? [];
const featureFlag = this.#configuration[FEATURE_MANAGEMENT_KEY]?.[FEATURE_FLAGS_KEY] ?? [];
validateFeatureFlag(featureFlag);
return featureFlag;
}
}
12 changes: 0 additions & 12 deletions src/model.ts → src/schema/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,6 @@ export interface FeatureFlag {
* An ID used to uniquely identify and reference the feature.
*/
id: string;
/**
* A description of the feature.
*/
description?: string;
/**
* A display name for the feature to use for display rather than the ID.
*/
display_name?: string;
/**
* A feature is OFF if enabled is false. If enabled is true, then the feature is ON if there are no conditions (null or empty) or if the conditions are satisfied.
*/
Expand Down Expand Up @@ -78,10 +70,6 @@ interface Variant {
* The configuration value for this feature variant.
*/
configuration_value?: unknown;
/**
* The path to a configuration section used as the configuration value for this feature variant.
*/
configuration_reference?: string;
/**
* Overrides the enabled state of the feature if the given variant is assigned. Does not override the state if value is None.
*/
Expand Down
186 changes: 186 additions & 0 deletions src/schema/validator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* Validates a feature flag object, checking if it conforms to the schema.
* @param featureFlag The feature flag object to validate.
*/
export function validateFeatureFlag(featureFlag: any): void {
if (featureFlag === undefined) {
return; // no-op if feature flag is undefined, indicating that the feature flag is not found
}
if (featureFlag === null || typeof featureFlag !== "object") { // Note: typeof null = "object"
throw new TypeError("Feature flag must be an object.");
}
if (typeof featureFlag.id !== "string") {
throw new TypeError("Feature flag 'id' must be a string.");
}
if (featureFlag.enabled !== undefined && typeof featureFlag.enabled !== "boolean") {
throw new TypeError("Feature flag 'enabled' must be a boolean.");
}
if (featureFlag.conditions !== undefined) {
validateFeatureEnablementConditions(featureFlag.conditions);
}
if (featureFlag.variants !== undefined) {
validateVariants(featureFlag.variants);
}
if (featureFlag.allocation !== undefined) {
validateVariantAllocation(featureFlag.allocation);
}
if (featureFlag.telemetry !== undefined) {
validateTelemetryOptions(featureFlag.telemetry);
}
}

function validateFeatureEnablementConditions(conditions: any) {
if (typeof conditions !== "object") {
throw new TypeError("Feature flag 'conditions' must be an object.");
}
if (conditions.requirement_type !== undefined && conditions.requirement_type !== "Any" && conditions.requirement_type !== "All") {
throw new TypeError("'requirement_type' must be 'Any' or 'All'.");
}
if (conditions.client_filters !== undefined) {
validateClientFilters(conditions.client_filters);
}
}

function validateClientFilters(client_filters: any) {
if (!Array.isArray(client_filters)) {
throw new TypeError("Feature flag conditions 'client_filters' must be an array.");
}

for (const filter of client_filters) {
if (typeof filter.name !== "string") {
throw new TypeError("Client filter 'name' must be a string.");
}
if (filter.parameters !== undefined && typeof filter.parameters !== "object") {
throw new TypeError("Client filter 'parameters' must be an object.");
}
}
}

function validateVariants(variants: any) {
if (!Array.isArray(variants)) {
throw new TypeError("Feature flag 'variants' must be an array.");
}

for (const variant of variants) {
if (typeof variant.name !== "string") {
throw new TypeError("Variant 'name' must be a string.");
}
// skip configuration_value validation as it accepts any type
if (variant.status_override !== undefined && typeof variant.status_override !== "string") {
throw new TypeError("Variant 'status_override' must be a string.");
}
if (variant.status_override !== undefined && variant.status_override !== "None" && variant.status_override !== "Enabled" && variant.status_override !== "Disabled") {
throw new TypeError("Variant 'status_override' must be 'None', 'Enabled', or 'Disabled'.");
}
}
}

function validateVariantAllocation(allocation: any) {
if (typeof allocation !== "object") {
throw new TypeError("Variant 'allocation' must be an object.");
}

if (allocation.default_when_disabled !== undefined && typeof allocation.default_when_disabled !== "string") {
throw new TypeError("Variant allocation 'default_when_disabled' must be a string.");
}
if (allocation.default_when_enabled !== undefined && typeof allocation.default_when_enabled !== "string") {
throw new TypeError("Variant allocation 'default_when_enabled' must be a string.");
}
if (allocation.user !== undefined) {
validateUserVariantAllocation(allocation.user);
}
if (allocation.group !== undefined) {
validateGroupVariantAllocation(allocation.group);
}
if (allocation.percentile !== undefined) {
validatePercentileVariantAllocation(allocation.percentile);
}
if (allocation.seed !== undefined && typeof allocation.seed !== "string") {
throw new TypeError("Variant allocation 'seed' must be a string.");
}
}

function validateUserVariantAllocation(UserAllocations: any) {
if (!Array.isArray(UserAllocations)) {
throw new TypeError("Variant 'user' allocation must be an array.");
}

for (const allocation of UserAllocations) {
if (typeof allocation !== "object") {
throw new TypeError("Elements in variant 'user' allocation must be an object.");
}
if (typeof allocation.variant !== "string") {
throw new TypeError("User allocation 'variant' must be a string.");
}
if (!Array.isArray(allocation.users)) {
throw new TypeError("User allocation 'users' must be an array.");
}
for (const user of allocation.users) {
if (typeof user !== "string") {
throw new TypeError("Elements in user allocation 'users' must be strings.");
}
}
}
}

function validateGroupVariantAllocation(groupAllocations: any) {
if (!Array.isArray(groupAllocations)) {
throw new TypeError("Variant 'group' allocation must be an array.");
}

for (const allocation of groupAllocations) {
if (typeof allocation !== "object") {
throw new TypeError("Elements in variant 'group' allocation must be an object.");
}
if (typeof allocation.variant !== "string") {
throw new TypeError("Group allocation 'variant' must be a string.");
}
if (!Array.isArray(allocation.groups)) {
throw new TypeError("Group allocation 'groups' must be an array.");
}
for (const group of allocation.groups) {
if (typeof group !== "string") {
throw new TypeError("Elements in group allocation 'groups' must be strings.");
}
}
}
}

function validatePercentileVariantAllocation(percentileAllocations: any) {
if (!Array.isArray(percentileAllocations)) {
throw new TypeError("Variant 'percentile' allocation must be an array.");
}

for (const allocation of percentileAllocations) {
if (typeof allocation !== "object") {
throw new TypeError("Elements in variant 'percentile' allocation must be an object.");
}
if (typeof allocation.variant !== "string") {
throw new TypeError("Percentile allocation 'variant' must be a string.");
}
if (typeof allocation.from !== "number" || allocation.from < 0 || allocation.from > 100) {
throw new TypeError("Percentile allocation 'from' must be a number between 0 and 100.");
}
if (typeof allocation.to !== "number" || allocation.to < 0 || allocation.to > 100) {
throw new TypeError("Percentile allocation 'to' must be a number between 0 and 100.");
}
}
}
// #endregion

// #region Telemetry
function validateTelemetryOptions(telemetry: any) {
if (typeof telemetry !== "object") {
throw new TypeError("Feature flag 'telemetry' must be an object.");
}
if (telemetry.enabled !== undefined && typeof telemetry.enabled !== "boolean") {
throw new TypeError("Telemetry 'enabled' must be a boolean.");
}
if (telemetry.metadata !== undefined && typeof telemetry.metadata !== "object") {
throw new TypeError("Telemetry 'metadata' must be an object.");
}
}
// #endregion
37 changes: 37 additions & 0 deletions test/featureManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,43 @@ describe("feature manager", () => {
]);
});

it("should evaluate features with conditions", () => {
const dataSource = new Map();
dataSource.set("feature_management", {
feature_flags: [
{
"id": "Gamma",
"description": "",
"enabled": true,
"conditions": {
"requirement_type": "invalid type",
"client_filters": [
{ "name": "Microsoft.Targeting", "parameters": { "Audience": { "DefaultRolloutPercentage": 50 } } }
]
}
},
{
"id": "Delta",
"description": "",
"enabled": true,
"conditions": {
"requirement_type": "Any",
"client_filters": [
{ "name": "Microsoft.Targeting", "parameters": "invalid parameter" }
]
}
}
],
});

const provider = new ConfigurationMapFeatureFlagProvider(dataSource);
const featureManager = new FeatureManager(provider);
return Promise.all([
expect(featureManager.isEnabled("Gamma")).eventually.rejectedWith("'requirement_type' must be 'Any' or 'All'."),
expect(featureManager.isEnabled("Delta")).eventually.rejectedWith("Client filter 'parameters' must be an object.")
]);
});

it("should let the last feature flag win", () => {
const jsonObject = {
"feature_management": {
Expand Down
2 changes: 1 addition & 1 deletion test/noFilters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe("feature flags with no filters", () => {
return Promise.all([
expect(featureManager.isEnabled("BooleanTrue")).eventually.eq(true),
expect(featureManager.isEnabled("BooleanFalse")).eventually.eq(false),
expect(featureManager.isEnabled("InvalidEnabled")).eventually.rejectedWith("Feature flag InvalidEnabled has an invalid 'enabled' value."),
expect(featureManager.isEnabled("InvalidEnabled")).eventually.rejectedWith("Feature flag 'enabled' must be a boolean."),
expect(featureManager.isEnabled("Minimal")).eventually.eq(true),
expect(featureManager.isEnabled("NoEnabled")).eventually.eq(false),
expect(featureManager.isEnabled("EmptyConditions")).eventually.eq(true)
Expand Down

0 comments on commit e7119e2

Please sign in to comment.