Feat/exp flag loader (#1961)

* fix: remove unused exp flag

* fix: remove unused flag

* fix: add support for external flag resolver

* fix: rename flagsresolver to flagresolver

* fix: disable external flag resolver

* fix: refactor a bit

* fix: stop using unleash in server-dev

* fix: remove userGroups flag

* fix: revert bumping frontend

src/lib/__snapshots__/create-config.test.ts.snap

@@ -62,8 +62,26 @@ Object {
   },
   "eventHook": undefined,
   "experimental": Object {
-    "batchMetrics": false,
-    "embedProxy": false,
+    "externalResolver": Object {
+      "isEnabled": [Function],
+    },
+    "flags": Object {
+      "ENABLE_DARK_MODE_SUPPORT": false,
+      "anonymiseEventLog": false,
+      "batchMetrics": false,
+      "embedProxy": false,
+    },
+  },
+  "flagResolver": FlagResolver {
+    "experiments": Object {
+      "ENABLE_DARK_MODE_SUPPORT": false,
+      "anonymiseEventLog": false,
+      "batchMetrics": false,
+      "embedProxy": false,
+    },
+    "externalResolver": Object {
+      "isEnabled": [Function],
+    },
   },
   "frontendApiOrigins": Array [],
   "getLogger": [Function],
@@ -106,6 +124,7 @@ Object {
   "ui": Object {
     "flags": Object {
       "E": true,
+      "ENABLE_DARK_MODE_SUPPORT": false,
     },
   },
   "versionCheck": Object {

src/lib/app.ts

@@ -70,7 +70,7 @@ export default async function getApp(
     }
 
     if (
-        config.experimental.embedProxy &&
+        config.experimental.flags.embedProxy &&
         config.frontendApiOrigins.length > 0
     ) {
         // Support CORS preflight requests for the frontend endpoints.

src/lib/create-config.ts

@@ -34,11 +34,15 @@ import {
     parseEnvVarNumber,
     parseEnvVarStrings,
 } from './util/parseEnvVar';
-import { IExperimentalOptions } from './experimental';
+import {
+    defaultExperimentalOptions,
+    IExperimentalOptions,
+} from './types/experimental';
 import {
     DEFAULT_SEGMENT_VALUES_LIMIT,
     DEFAULT_STRATEGY_SEGMENTS_LIMIT,
 } from './util/segments';
+import FlagResolver from './util/flag-resolver';
 
 const safeToUpper = (s: string) => (s ? s.toUpperCase() : s);
 
@@ -55,15 +59,12 @@ function mergeAll<T>(objects: Partial<T>[]): T {
 
 function loadExperimental(options: IUnleashOptions): IExperimentalOptions {
     return {
+        ...defaultExperimentalOptions,
         ...options.experimental,
-        embedProxy: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_EMBED_PROXY,
-            Boolean(options.experimental?.embedProxy),
-        ),
-        batchMetrics: parseEnvVarBoolean(
-            process.env.UNLEASH_EXPERIMENTAL_BATCH_METRICS,
-            Boolean(options.experimental?.batchMetrics),
-        ),
+        flags: {
+            ...defaultExperimentalOptions.flags,
+            ...options.experimental?.flags,
+        },
     };
 }
 
@@ -102,6 +103,7 @@ function loadUI(options: IUnleashOptions): IUIConfig {
 
     ui.flags = {
         E: true,
+        ENABLE_DARK_MODE_SUPPORT: false,
     };
     return mergeAll([uiO, ui]);
 }
@@ -375,6 +377,7 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig {
     ]);
 
     const experimental = loadExperimental(options);
+    const flagResolver = new FlagResolver(experimental);
 
     const ui = loadUI(options);
 
@@ -434,6 +437,7 @@ export function createConfig(options: IUnleashOptions): IUnleashConfig {
         ui,
         import: importSetting,
         experimental,
+        flagResolver,
         email,
         secureHeaders,
         enableOAS,

src/lib/db/access-store.ts

@@ -45,16 +45,8 @@ export class AccessStore implements IAccessStore {
 
     private db: Knex;
 
-    private enableUserGroupPermissions: boolean;
-
-    constructor(
-        db: Knex,
-        eventBus: EventEmitter,
-        getLogger: Function,
-        enableUserGroupPermissions: boolean,
-    ) {
+    constructor(db: Knex, eventBus: EventEmitter, getLogger: Function) {
         this.db = db;
-        this.enableUserGroupPermissions = enableUserGroupPermissions;
         this.logger = getLogger('access-store.ts');
         this.timer = (action: string) =>
             metricsHelper.wrapTimer(eventBus, DB_TIME, {
@@ -133,27 +125,21 @@ export class AccessStore implements IAccessStore {
             .join(`${T.PERMISSIONS} AS p`, 'p.id', 'rp.permission_id')
             .where('ur.user_id', '=', userId);
 
-        if (this.enableUserGroupPermissions) {
-            userPermissionQuery = userPermissionQuery.union((db) => {
-                db.select(
-                    'project',
-                    'permission',
-                    'environment',
-                    'p.type',
-                    'gr.role_id',
-                )
-                    .from<IPermissionRow>(`${T.GROUP_USER} AS gu`)
-                    .join(`${T.GROUPS} AS g`, 'g.id', 'gu.group_id')
-                    .join(`${T.GROUP_ROLE} AS gr`, 'gu.group_id', 'gr.group_id')
-                    .join(
-                        `${T.ROLE_PERMISSION} AS rp`,
-                        'rp.role_id',
-                        'gr.role_id',
-                    )
-                    .join(`${T.PERMISSIONS} AS p`, 'p.id', 'rp.permission_id')
-                    .where('gu.user_id', '=', userId);
-            });
-        }
+        userPermissionQuery = userPermissionQuery.union((db) => {
+            db.select(
+                'project',
+                'permission',
+                'environment',
+                'p.type',
+                'gr.role_id',
+            )
+                .from<IPermissionRow>(`${T.GROUP_USER} AS gu`)
+                .join(`${T.GROUPS} AS g`, 'g.id', 'gu.group_id')
+                .join(`${T.GROUP_ROLE} AS gr`, 'gu.group_id', 'gr.group_id')
+                .join(`${T.ROLE_PERMISSION} AS rp`, 'rp.role_id', 'gr.role_id')
+                .join(`${T.PERMISSIONS} AS p`, 'p.id', 'rp.permission_id')
+                .where('gu.user_id', '=', userId);
+        });
         const rows = await userPermissionQuery;
         stopTimer();
         return rows.map(this.mapUserPermission);

src/lib/db/index.ts

@@ -57,12 +57,7 @@ export const createStores = (
         tagStore: new TagStore(db, eventBus, getLogger),
         tagTypeStore: new TagTypeStore(db, eventBus, getLogger),
         addonStore: new AddonStore(db, eventBus, getLogger),
-        accessStore: new AccessStore(
-            db,
-            eventBus,
-            getLogger,
-            config?.experimental?.userGroups,
-        ),
+        accessStore: new AccessStore(db, eventBus, getLogger),
         apiTokenStore: new ApiTokenStore(db, eventBus, getLogger),
         resetTokenStore: new ResetTokenStore(db, eventBus, getLogger),
         sessionStore: new SessionStore(db, eventBus, getLogger),

src/lib/experimental.ts

@@ -1,12 +0,0 @@
-export interface IExperimentalOptions {
-    metricsV2?: IExperimentalToggle;
-    clientFeatureMemoize?: IExperimentalToggle;
-    userGroups?: boolean;
-    anonymiseEventLog?: boolean;
-    embedProxy?: boolean;
-    batchMetrics?: boolean;
-}
-
-export interface IExperimentalToggle {
-    enabled: boolean;
-}

src/lib/middleware/api-token-middleware.ts

@@ -42,7 +42,8 @@ const apiAccessMiddleware = (
                 if (
                     (apiUser.type === CLIENT && !isClientApi(req)) ||
                     (apiUser.type === FRONTEND && !isProxyApi(req)) ||
-                    (apiUser.type === FRONTEND && !experimental.embedProxy)
+                    (apiUser.type === FRONTEND &&
+                        !experimental.flags.embedProxy)
                 ) {
                     res.status(403).send({ message: TOKEN_TYPE_ERROR_MESSAGE });
                     return;

src/lib/routes/admin-api/config.ts

@@ -1,4 +1,5 @@
-import { Request, Response } from 'express';
+import { Response } from 'express';
+import { AuthedRequest } from '../../types/core';
 import { IUnleashServices } from '../../types/services';
 import { IAuthType, IUnleashConfig } from '../../types/option';
 import version from '../../util/version';
@@ -66,7 +67,7 @@ class ConfigController extends Controller {
     }
 
     async getUIConfig(
-        req: Request,
+        req: AuthedRequest,
         res: Response<UiConfigSchema>,
     ): Promise<void> {
         const simpleAuthSettings =
@@ -76,8 +77,14 @@ class ConfigController extends Controller {
             simpleAuthSettings?.disabled ||
             this.config.authentication.type == IAuthType.NONE;
 
+        const expFlags = this.config.flagResolver.getAll({
+            email: req.user.email,
+        });
+        const flags = { ...this.config.ui.flags, ...expFlags };
+
         const response: UiConfigSchema = {
             ...this.config.ui,
+            flags,
             version,
             emailEnabled: this.emailService.isEnabled(),
             unleashUrl: this.config.server.unleashUrl,
@@ -87,7 +94,7 @@ class ConfigController extends Controller {
             strategySegmentsLimit: this.config.strategySegmentsLimit,
             versionInfo: this.versionService.getVersionInfo(),
             disablePasswordAuth,
-            embedProxy: this.config.experimental.embedProxy,
+            embedProxy: this.config.experimental.flags.embedProxy,
         };
 
         this.openApiService.respondWithValidation(

src/lib/routes/admin-api/event.ts

@@ -21,12 +21,13 @@ import {
 import { getStandardResponses } from '../../../lib/openapi/util/standard-responses';
 import { createRequestSchema } from '../../openapi/util/create-request-schema';
 import { SearchEventsSchema } from '../../openapi/spec/search-events-schema';
+import { IFlagResolver } from '../../types/experimental';
 
 const version = 1;
 export default class EventController extends Controller {
     private eventService: EventService;
 
-    private anonymise: boolean = false;
+    private flagResolver: IFlagResolver;
 
     private openApiService: OpenApiService;
 
@@ -39,7 +40,7 @@ export default class EventController extends Controller {
     ) {
         super(config);
         this.eventService = eventService;
-        this.anonymise = config.experimental?.anonymiseEventLog;
+        this.flagResolver = config.flagResolver;
         this.openApiService = openApiService;
 
         this.route({
@@ -106,7 +107,7 @@ export default class EventController extends Controller {
     }
 
     maybeAnonymiseEvents(events: IEvent[]): IEvent[] {
-        if (this.anonymise) {
+        if (this.flagResolver.isEnabled('anonymiseEventLog')) {
             return events.map((e: IEvent) => ({
                 ...e,
                 createdBy: anonymise(e.createdBy),

src/lib/routes/admin-api/events.test.ts

@@ -12,7 +12,7 @@ async function getSetup(anonymise: boolean = false) {
     const stores = createStores();
     const config = createTestConfig({
         server: { baseUriPath: base },
-        experimental: { anonymiseEventLog: anonymise },
+        experimental: { flags: { anonymiseEventLog: anonymise } },
     });
     const services = createServices(stores, config);
     const app = await getApp(config, stores, services);

src/lib/routes/admin-api/user-admin.ts

@@ -37,9 +37,10 @@ import {
     usersGroupsBaseSchema,
 } from '../../openapi/spec/users-groups-base-schema';
 import { IGroup } from '../../types/group';
+import { IFlagResolver } from '../../types/experimental';
 
 export default class UserAdminController extends Controller {
-    private anonymise: boolean = false;
+    private flagResolver: IFlagResolver;
 
     private userService: UserService;
 
@@ -90,7 +91,7 @@ export default class UserAdminController extends Controller {
         this.groupService = groupService;
         this.logger = config.getLogger('routes/user-controller.ts');
         this.unleashUrl = config.server.unleashUrl;
-        this.anonymise = config.experimental?.anonymiseEventLog;
+        this.flagResolver = config.flagResolver;
 
         this.route({
             method: 'post',
@@ -294,7 +295,7 @@ export default class UserAdminController extends Controller {
             typeof q === 'string' && q.length > 1
                 ? await this.userService.search(q)
                 : [];
-        if (this.anonymise) {
+        if (this.flagResolver.isEnabled('anonymiseEventLog')) {
             users = this.anonymiseUsers(users);
         }
         this.openApiService.respondWithValidation(

src/lib/routes/client-api/metrics.test.ts

@@ -82,9 +82,7 @@ test('should accept client metrics with yes/no', () => {
 });
 
 test('should accept client metrics with yes/no with metricsV2', async () => {
-    const testRunner = await getSetup({
-        experimental: { metricsV2: { enabled: true } },
-    });
+    const testRunner = await getSetup();
     await testRunner.request
         .post('/api/client/metrics')
         .send({

src/lib/routes/index.ts

@@ -28,7 +28,7 @@ class IndexRouter extends Controller {
         this.use('/api/admin', new AdminApi(config, services).router);
         this.use('/api/client', new ClientApi(config, services).router);
 
-        if (config.experimental.embedProxy) {
+        if (config.experimental.flags.embedProxy) {
             this.use(
                 '/api/frontend',
                 new ProxyController(config, services).router,

src/lib/services/client-metrics/metrics-service-v2.ts

@@ -16,7 +16,6 @@ import ApiUser from '../../types/api-user';
 import { ALL } from '../../types/models/api-token';
 import User from '../../types/user';
 import { collapseHourlyMetrics } from '../../util/collapseHourlyMetrics';
-import { IExperimentalOptions } from '../../experimental';
 
 export default class ClientMetricsServiceV2 {
     private timers: NodeJS.Timeout[] = [];
@@ -27,7 +26,7 @@ export default class ClientMetricsServiceV2 {
 
     private featureToggleStore: IFeatureToggleStore;
 
-    private experimental: IExperimentalOptions;
+    private batchMetricsEnabled: boolean;
 
     private eventBus: EventEmitter;
 
@@ -47,13 +46,13 @@ export default class ClientMetricsServiceV2 {
     ) {
         this.featureToggleStore = featureToggleStore;
         this.clientMetricsStoreV2 = clientMetricsStoreV2;
-        this.experimental = experimental;
+        this.batchMetricsEnabled = experimental.flags.batchMetrics;
         this.eventBus = eventBus;
         this.logger = getLogger(
             '/services/client-metrics/client-metrics-service-v2.ts',
         );
 
-        if (this.experimental.batchMetrics) {
+        if (this.batchMetricsEnabled) {
             this.timers.push(
                 setInterval(() => {
                     this.bulkAdd().catch(console.error);
@@ -91,7 +90,7 @@ export default class ClientMetricsServiceV2 {
             }))
             .filter((item) => !(item.yes === 0 && item.no === 0));
 
-        if (this.experimental.batchMetrics) {
+        if (this.batchMetricsEnabled) {
             this.unsavedMetrics = collapseHourlyMetrics([
                 ...this.unsavedMetrics,
                 ...clientMetrics,
@@ -104,7 +103,7 @@ export default class ClientMetricsServiceV2 {
     }
 
     async bulkAdd(): Promise<void> {
-        if (this.experimental.batchMetrics && this.unsavedMetrics.length > 0) {
+        if (this.batchMetricsEnabled && this.unsavedMetrics.length > 0) {
             // Make a copy of `unsavedMetrics` in case new metrics
             // arrive while awaiting `batchInsertMetrics`.
             const copy = [...this.unsavedMetrics];

src/lib/types/experimental.ts

@@ -0,0 +1,43 @@
+import { parseEnvVarBoolean } from '../util/parseEnvVar';
+
+export type IFlags = Partial<Record<string, boolean>>;
+
+export const defaultExperimentalOptions = {
+    flags: {
+        ENABLE_DARK_MODE_SUPPORT: false,
+        anonymiseEventLog: false,
+        embedProxy: parseEnvVarBoolean(
+            process.env.UNLEASH_EXPERIMENTAL_EMBED_PROXY,
+            false,
+        ),
+        batchMetrics: parseEnvVarBoolean(
+            process.env.UNLEASH_EXPERIMENTAL_BATCH_METRICS,
+            false,
+        ),
+    },
+    externalResolver: { isEnabled: (): boolean => false },
+};
+
+export interface IExperimentalOptions {
+    flags: {
+        [key: string]: boolean;
+        ENABLE_DARK_MODE_SUPPORT?: boolean;
+        embedProxy?: boolean;
+        batchMetrics?: boolean;
+        anonymiseEventLog?: boolean;
+    };
+    externalResolver: IExternalFlagResolver;
+}
+
+export interface IFlagContext {
+    [key: string]: string;
+}
+
+export interface IFlagResolver {
+    getAll: (context?: IFlagContext) => IFlags;
+    isEnabled: (expName: string, context?: IFlagContext) => boolean;
+}
+
+export interface IExternalFlagResolver {
+    isEnabled: (flagName: string, context?: IFlagContext) => boolean;
+}

src/lib/types/option.ts

@@ -1,7 +1,7 @@
 import EventEmitter from 'events';
 import { LogLevel, LogProvider } from '../logger';
 import { ILegacyApiTokenCreate } from './models/api-token';
-import { IExperimentalOptions } from '../experimental';
+import { IFlagResolver, IExperimentalOptions, IFlags } from './experimental';
 import SMTPTransport from 'nodemailer/lib/smtp-transport';
 
 export type EventHook = (eventName: string, data: object) => void;
@@ -102,7 +102,7 @@ export interface IUnleashOptions {
     authentication?: Partial<IAuthOption>;
     ui?: object;
     import?: Partial<IImportOption>;
-    experimental?: IExperimentalOptions;
+    experimental?: Partial<IExperimentalOptions>;
     email?: Partial<IEmailOption>;
     secureHeaders?: boolean;
     additionalCspAllowedDomains?: ICspDomainOptions;
@@ -139,7 +139,6 @@ export interface IListeningHost {
 export interface IUIConfig {
     slogan?: string;
     name?: string;
-    flags?: { [key: string]: boolean };
     links?: [
         {
             value: string;
@@ -148,6 +147,7 @@ export interface IUIConfig {
             title: string;
         },
     ];
+    flags?: IFlags;
 }
 
 export interface ICspDomainOptions {
@@ -177,6 +177,7 @@ export interface IUnleashConfig {
     ui: IUIConfig;
     import: IImportOption;
     experimental?: IExperimentalOptions;
+    flagResolver: IFlagResolver;
     email: IEmailOption;
     secureHeaders: boolean;
     additionalCspAllowedDomains: ICspDomainConfig;

src/lib/util/flag-resolver.test.ts

@@ -0,0 +1,101 @@
+import { defaultExperimentalOptions } from '../types/experimental';
+import FlagResolver from './flag-resolver';
+
+test('should produce empty exposed flags', () => {
+    const resolver = new FlagResolver(defaultExperimentalOptions);
+
+    const result = resolver.getAll();
+
+    expect(result.ENABLE_DARK_MODE_SUPPORT).toBe(false);
+});
+
+test('should produce UI flags with extra dynamic flags', () => {
+    const config = {
+        ...defaultExperimentalOptions,
+        flags: { extraFlag: false },
+    };
+    const resolver = new FlagResolver(config);
+    const result = resolver.getAll();
+
+    expect(result.extraFlag).toBe(false);
+});
+
+test('should use external resolver for dynamic flags', () => {
+    const externalResolver = {
+        isEnabled: (name: string) => {
+            if (name === 'extraFlag') {
+                return true;
+            }
+        },
+    };
+
+    const resolver = new FlagResolver({
+        flags: {
+            extraFlag: false,
+        },
+        externalResolver,
+    });
+
+    const result = resolver.getAll();
+
+    expect(result.extraFlag).toBe(true);
+});
+
+test('should not use external resolver for enabled experiments', () => {
+    const externalResolver = {
+        isEnabled: () => {
+            return false;
+        },
+    };
+
+    const resolver = new FlagResolver({
+        flags: {
+            should_be_enabled: true,
+            extraFlag: false,
+        },
+        externalResolver,
+    });
+
+    const result = resolver.getAll();
+
+    expect(result.should_be_enabled).toBe(true);
+});
+
+test('should load experimental flags', () => {
+    const externalResolver = {
+        isEnabled: () => {
+            return false;
+        },
+    };
+    const resolver = new FlagResolver({
+        flags: {
+            extraFlag: false,
+            someFlag: true,
+        },
+        externalResolver,
+    });
+
+    expect(resolver.isEnabled('someFlag')).toBe(true);
+    expect(resolver.isEnabled('extraFlag')).toBe(false);
+});
+
+test('should load experimental flags from external provider', () => {
+    const externalResolver = {
+        isEnabled: (name: string) => {
+            if (name === 'extraFlag') {
+                return true;
+            }
+        },
+    };
+
+    const resolver = new FlagResolver({
+        flags: {
+            extraFlag: false,
+            someFlag: true,
+        },
+        externalResolver,
+    });
+
+    expect(resolver.isEnabled('someFlag')).toBe(true);
+    expect(resolver.isEnabled('extraFlag')).toBe(true);
+});

src/lib/util/flag-resolver.ts

@@ -0,0 +1,38 @@
+import {
+    IExperimentalOptions,
+    IExternalFlagResolver,
+    IFlagContext,
+    IFlags,
+    IFlagResolver,
+} from '../types/experimental';
+export default class FlagResolver implements IFlagResolver {
+    private experiments: IFlags;
+
+    private externalResolver: IExternalFlagResolver;
+
+    constructor(expOpt: IExperimentalOptions) {
+        this.experiments = expOpt.flags;
+        this.externalResolver = expOpt.externalResolver;
+    }
+
+    getAll(context?: IFlagContext): IFlags {
+        const flags: IFlags = { ...this.experiments };
+
+        Object.keys(flags).forEach((flagName) => {
+            if (!this.experiments[flagName])
+                flags[flagName] = this.externalResolver.isEnabled(
+                    flagName,
+                    context,
+                );
+        });
+
+        return flags;
+    }
+
+    isEnabled(expName: string, context?: IFlagContext): boolean {
+        if (this.experiments[expName]) {
+            return true;
+        }
+        return this.externalResolver.isEnabled(expName, context);
+    }
+}

src/server-dev.ts

@@ -31,11 +31,12 @@ process.nextTick(async () => {
                     enable: false,
                 },
                 experimental: {
-                    metricsV2: { enabled: true },
-                    anonymiseEventLog: false,
-                    userGroups: true,
-                    embedProxy: true,
-                    batchMetrics: true,
+                    // externalResolver: unleash,
+                    flags: {
+                        embedProxy: true,
+                        batchMetrics: true,
+                        anonymiseEventLog: false,
+                    },
                 },
                 authentication: {
                     initApiTokens: [

src/test/config/test-config.ts

@@ -23,9 +23,10 @@ export function createTestConfig(config?: IUnleashOptions): IUnleashConfig {
             enabled: false,
         },
         experimental: {
-            userGroups: true,
-            embedProxy: true,
-            batchMetrics: true,
+            flags: {
+                embedProxy: true,
+                batchMetrics: true,
+            },
         },
     };
     const options = mergeAll<IUnleashOptions>([testConfig, config]);

src/test/e2e/api/admin/client-metrics.e2e.test.ts

@@ -9,9 +9,7 @@ let db: ITestDb;
 
 beforeAll(async () => {
     db = await dbInit('client_metrics_serial', getLogger);
-    app = await setupAppWithCustomConfig(db.stores, {
-        experimental: { metricsV2: { enabled: true } },
-    });
+    app = await setupAppWithCustomConfig(db.stores, {});
 });
 
 afterAll(async () => {

src/test/e2e/api/client/metricsV2.e2e.test.ts

@@ -11,9 +11,7 @@ let defaultToken;
 
 beforeAll(async () => {
     db = await dbInit('metrics_two_api_client', getLogger);
-    app = await setupAppWithAuth(db.stores, {
-        experimental: { metricsV2: { enabled: true } },
-    });
+    app = await setupAppWithAuth(db.stores, {});
     defaultToken = await app.services.apiTokenService.createApiToken({
         type: ApiTokenType.CLIENT,
         project: 'default',