Make feature flags and regions backwards compatible

satyakigh · Zee2413 · commit f59d1f1c9c40 · 2025-12-01T13:50:14.000-05:00
diff --git a/assets/featureFlag/alpha.json b/assets/featureFlag/alpha.json
@@ -41,7 +41,8 @@
         "me-south-1",
         "me-central-1",
         "af-south-1",
-        "il-central-1"
+        "il-central-1",
+        "test-region"
       ]
     }
   }
diff --git a/src/featureFlag/FeatureFlag.ts b/src/featureFlag/FeatureFlag.ts
@@ -1,4 +1,3 @@
-import { AwsRegion, getRegion } from '../utils/Region';
 import { FeatureFlag, TargetedFeatureFlag } from './FeatureFlagI';
 import { PartialFleetSelector } from './PartialFleetSelector';
 
@@ -37,18 +36,18 @@ export class FleetTargetedFeatureFlag implements TargetedFeatureFlag<string> {
 }
 
 export class RegionAllowlistFeatureFlag implements TargetedFeatureFlag<string> {
-    private readonly allowlist: Set<AwsRegion>;
+    private readonly allowlist: Set<string>;
 
     constructor(
         private readonly featureName: string,
-        allowedRegions: AwsRegion[],
+        allowedRegions: string[],
     ) {
         this.allowlist = new Set(allowedRegions);
     }
 
     isEnabled(region: string): boolean {
         try {
-            return this.allowlist.has(getRegion(region));
+            return this.allowlist.has(region);
         } catch {
             return false;
         }
diff --git a/src/featureFlag/FeatureFlagBuilder.ts b/src/featureFlag/FeatureFlagBuilder.ts
@@ -1,13 +1,12 @@
 import { z } from 'zod';
-import { AwsRegion } from '../utils/Region';
 import { AndFeatureFlag, CompoundFeatureFlag, LocalHostTargetedFeatureFlag } from './CombinedFeatureFlags';
 import { FleetTargetedFeatureFlag, RegionAllowlistFeatureFlag, StaticFeatureFlag } from './FeatureFlag';
 import { FeatureFlag, TargetedFeatureFlag } from './FeatureFlagI';
 
 const FeatureFlagSchema = z.object({
     enabled: z.boolean(),
     fleetPercentage: z.number().optional(),
-    allowlistedRegions: z.array(z.enum(Object.values(AwsRegion))).optional(),
+    allowlistedRegions: z.array(z.string()).optional(),
 });
 
 export const FeatureFlagConfigSchema = z.object({
@@ -47,7 +46,7 @@ export function buildLocalHost(name: string, config?: FeatureFlagConfigType) {
 }
 
 export function buildRegional(name: string, config?: FeatureFlagConfigType) {
-    let allowlist: AwsRegion[] = [];
+    let allowlist: string[] = [];
 
     if (config?.allowlistedRegions !== undefined) {
         allowlist = config.allowlistedRegions;
diff --git a/src/featureFlag/FeatureFlagProvider.ts b/src/featureFlag/FeatureFlagProvider.ts
@@ -21,11 +21,16 @@ export class FeatureFlagProvider implements Closeable {
         private readonly getLatestFeatureFlags: (env: string) => Promise<unknown>,
         private readonly localFile = join(__dirname, 'assets', 'featureFlag', `${AwsEnv.toLowerCase()}.json`),
     ) {
-        this.config = JSON.parse(readFileIfExists(localFile, 'utf8'));
+        this.config = defaultConfig(localFile);
 
-        this.supplier = new FeatureFlagSupplier(() => {
-            return this.config;
-        });
+        this.supplier = new FeatureFlagSupplier(
+            () => {
+                return this.config;
+            },
+            () => {
+                return defaultConfig(localFile);
+            },
+        );
 
         // https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#primary-rate-limit-for-unauthenticated-users
         // GitHub rate limits unauthenticated users to 60 requests per minute, so our refresh cycle has to be less than that
@@ -87,3 +92,7 @@ export function getFromGitHub(env: string): Promise<unknown> {
         `https://raw.githubusercontent.com/aws-cloudformation/cloudformation-languageserver/refs/heads/main/assets/featureFlag/${env.toLowerCase()}.json`,
     );
 }
+
+function defaultConfig(configFile: string): unknown {
+    return JSON.parse(readFileIfExists(configFile, 'utf8'));
+}
diff --git a/src/featureFlag/FeatureFlagSupplier.ts b/src/featureFlag/FeatureFlagSupplier.ts
@@ -23,14 +23,22 @@ export class FeatureFlagSupplier implements Closeable {
         TargetedFeatureFlag<unknown> | DynamicTargetedFeatureFlag<unknown>
     >();
 
-    constructor(configSupplier: () => unknown) {
+    constructor(configSupplier: () => unknown, defaultConfig: () => unknown) {
         for (const [key, builder] of Object.entries(FeatureBuilders)) {
-            const ff = new DynamicFeatureFlag(key, () => featureConfigSupplier(key, configSupplier), builder);
+            const ff = new DynamicFeatureFlag(
+                key,
+                () => featureConfigSupplier(key, configSupplier, defaultConfig),
+                builder,
+            );
             this._featureFlags.set(key, ff);
         }
 
         for (const [key, builder] of Object.entries(TargetedFeatureBuilders)) {
-            const ff = new DynamicTargetedFeatureFlag(key, () => featureConfigSupplier(key, configSupplier), builder);
+            const ff = new DynamicTargetedFeatureFlag(
+                key,
+                () => featureConfigSupplier(key, configSupplier, defaultConfig),
+                builder,
+            );
             this._targetedFeatureFlags.set(key, ff);
         }
     }
@@ -65,17 +73,16 @@ export class FeatureFlagSupplier implements Closeable {
     }
 }
 
-function featureConfigSupplier(key: string, configSupplier: () => unknown): FeatureFlagConfigType | undefined {
+function featureConfigSupplier(
+    key: string,
+    configSupplier: () => unknown,
+    defaultConfig: () => unknown,
+): FeatureFlagConfigType | undefined {
     try {
-        const config = configSupplier();
-        if (!config) {
-            return undefined;
-        }
-        const parsed = FeatureFlagConfigSchema.parse(config);
-        return parsed.features[key];
+        return FeatureFlagConfigSchema.parse(configSupplier()).features[key];
     } catch (err) {
-        log.error(err, `Failed to parse feature flag config\n${toString(configSupplier())}`);
-        return undefined;
+        log.warn(err, `Failed to parse feature flag config: \n${toString(configSupplier())}. Using defaults instead`);
+        return FeatureFlagConfigSchema.parse(defaultConfig()).features[key];
     }
 }
 
diff --git a/src/settings/SettingsParser.ts b/src/settings/SettingsParser.ts
@@ -1,13 +1,14 @@
 import { z } from 'zod';
-import { AwsRegion } from '../utils/Region';
+import { getRegion } from '../utils/Region';
 import { ProfileSettings, Settings } from './Settings';
 
-const AwsRegionSchema = z.enum(Object.values(AwsRegion));
-
 function createProfileSchema(defaults: Settings['profile']) {
     return z
         .object({
-            region: AwsRegionSchema.nullish().transform((val) => val ?? defaults.region),
+            region: z
+                .string()
+                .nullish()
+                .transform((val) => getRegion(val ?? defaults.region)),
             profile: z
                 .string()
                 .nullish()
diff --git a/src/utils/Region.ts b/src/utils/Region.ts
@@ -1,5 +1,6 @@
 // https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/resource-type-schemas.html
-import { dashesToUnderscores } from './String';
+import { LoggerFactory } from '../telemetry/LoggerFactory';
+import { dashesToUnderscores, toString } from './String';
 
 // Make sure keys and values are exactly the same (ignore casing, '-', '_')
 export enum AwsRegion {
@@ -65,7 +66,8 @@ export function getRegion(region: unknown): AwsRegion {
 
     const value = AwsRegion[enumKey as keyof typeof AwsRegion];
     if (!value) {
-        throw new Error(`Unknown region ${String(region)}`);
+        LoggerFactory.getLogger('Region').warn(`Unknown region ${toString(region)}`);
+        return enumKey.replaceAll('_', '-').toLowerCase() as AwsRegion;
     }
 
     return value;
diff --git a/tst/unit/featureFlag/FeatureFlagProvider.test.ts b/tst/unit/featureFlag/FeatureFlagProvider.test.ts
@@ -0,0 +1,18 @@
+import { readFileSync } from 'fs';
+import { join } from 'path';
+import { describe, it, expect } from 'vitest';
+import { FeatureFlagConfigSchema } from '../../../src/featureFlag/FeatureFlagBuilder';
+
+describe('FeatureFlagProvider', () => {
+    it('can parse feature flags', () => {
+        [
+            join(__dirname, '..', '..', '..', 'assets', 'featureFlag', 'alpha.json'),
+            join(__dirname, '..', '..', '..', 'assets', 'featureFlag', 'beta.json'),
+            join(__dirname, '..', '..', '..', 'assets', 'featureFlag', 'prod.json'),
+        ].map((path) => {
+            const file = readFileSync(path, 'utf8');
+            expect(file).toBeDefined();
+            expect(FeatureFlagConfigSchema.parse(JSON.parse(file))).toBeDefined();
+        });
+    });
+});
diff --git a/tst/unit/featureFlag/FeatureFlagSupplier.test.ts b/tst/unit/featureFlag/FeatureFlagSupplier.test.ts
@@ -3,57 +3,86 @@ import { DynamicTargetedFeatureFlag } from '../../../src/featureFlag/DynamicFeat
 import { FeatureFlagSupplier } from '../../../src/featureFlag/FeatureFlagSupplier';
 
 describe('FeatureFlagSupplier', () => {
-    afterEach(() => {
-        vi.restoreAllMocks();
-    });
-
-    it('should initialize with targeted feature flags', () => {
-        const configSupplier = vi.fn(() => ({
+    const configSupplier = () => {
+        return {
             version: 1,
             description: 'test',
             features: {
-                EnhancedDryRun: { enabled: true },
+                EnhancedDryRun: { enabled: true, fleetPercentage: 100, allowlistedRegions: ['us-east-1'] },
+                Constants: { enabled: false },
             },
-        }));
+        };
+    };
+
+    const throwError = () => {
+        throw new Error('Error fallback');
+    };
+
+    afterEach(() => {
+        vi.restoreAllMocks();
+    });
 
-        const supplier = new FeatureFlagSupplier(configSupplier);
+    it('should initialize with feature flags', () => {
+        const supplier = new FeatureFlagSupplier(configSupplier, throwError);
+
+        expect([...supplier.featureFlags.keys()]).toEqual(['Constants']);
+        expect(supplier.featureFlags.get('Constants')?.isEnabled()).toBe(false);
+
+        expect([...supplier.targetedFeatureFlags.keys()]).toEqual(['EnhancedDryRun']);
+        expect(supplier.targetedFeatureFlags.get('EnhancedDryRun')?.isEnabled('us-east-1')).toBe(true);
+        expect(supplier.targetedFeatureFlags.get('EnhancedDryRun')?.isEnabled('us-east-2')).toBe(false);
 
-        expect(supplier.featureFlags.size).toBe(1);
-        expect(supplier.targetedFeatureFlags.size).toBe(1);
-        expect(supplier.targetedFeatureFlags.has('EnhancedDryRun')).toBe(true);
         supplier.close();
     });
 
     it('should close all dynamic feature flags', () => {
-        const configSupplier = vi.fn(() => ({
-            version: 1,
-            description: 'test',
-            features: {},
-        }));
-
-        const supplier = new FeatureFlagSupplier(configSupplier);
+        const supplier = new FeatureFlagSupplier(
+            () => {
+                return { version: 1, description: 'test', features: {} };
+            },
+            () => {},
+        );
         const closeSpy = vi.spyOn(DynamicTargetedFeatureFlag.prototype, 'close');
 
         supplier.close();
 
         expect(closeSpy).toHaveBeenCalled();
     });
 
-    it('should handle invalid config gracefully', () => {
-        const configSupplier = vi.fn(() => 'invalid');
+    it('should handle invalid config and fallback to default', () => {
+        const supplier = new FeatureFlagSupplier(() => 'invalid', configSupplier);
 
-        const supplier = new FeatureFlagSupplier(configSupplier);
+        expect([...supplier.featureFlags.keys()]).toEqual(['Constants']);
+        expect([...supplier.targetedFeatureFlags.keys()]).toEqual(['EnhancedDryRun']);
 
-        expect(supplier.targetedFeatureFlags.size).toBe(1);
         supplier.close();
     });
 
     it('should handle undefined config', () => {
-        const configSupplier = vi.fn(() => undefined);
+        const supplier = new FeatureFlagSupplier(() => undefined, configSupplier);
 
-        const supplier = new FeatureFlagSupplier(configSupplier);
+        expect([...supplier.featureFlags.keys()]).toEqual(['Constants']);
+        expect([...supplier.targetedFeatureFlags.keys()]).toEqual(['EnhancedDryRun']);
 
-        expect(supplier.targetedFeatureFlags.size).toBe(1);
         supplier.close();
     });
+
+    it('throws if both suppliers fail', () => {
+        expect(
+            () =>
+                new FeatureFlagSupplier(
+                    () => undefined,
+                    () => undefined,
+                ),
+        ).toThrow(
+            '[\n' +
+                '  {\n' +
+                '    "expected": "object",\n' +
+                '    "code": "invalid_type",\n' +
+                '    "path": [],\n' +
+                '    "message": "Invalid input: expected object, received undefined"\n' +
+                '  }\n' +
+                ']',
+        );
+    });
 });
diff --git a/tst/unit/utils/Region.test.ts b/tst/unit/utils/Region.test.ts
@@ -26,9 +26,9 @@ describe('Region', () => {
             expect(getRegion('EU_WEST_1')).toBe(AwsRegion.EU_WEST_1);
         });
 
-        it('should throw an error for invalid region strings', () => {
-            expect(() => getRegion('invalid-region')).toThrow('Unknown region invalid-region');
-            expect(() => getRegion('us-central-1')).toThrow('Unknown region us-central-1');
+        it('should not throw when region is unknown or invalid', () => {
+            expect(getRegion('invalid_region')).toBe('invalid-region');
+            expect(getRegion('US_test-1')).toBe('us-test-1');
         });
     });
 });

Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,8 @@`
`41`	`41`	`"me-south-1",`
`42`	`42`	`"me-central-1",`
`43`	`43`	`"af-south-1",`
`44`		`- "il-central-1"`
	`44`	`+ "il-central-1",`
	`45`	`+ "test-region"`
`45`	`46`	`]`
`46`	`47`	`}`
`47`	`48`	`}`