diff --git a/packages/backend-core/src/features/index.ts b/packages/backend-core/src/features/index.ts index ad517082de..d7f7c76436 100644 --- a/packages/backend-core/src/features/index.ts +++ b/packages/backend-core/src/features/index.ts @@ -1,79 +1,108 @@ import env from "../environment" import * as context from "../context" +import { cloneDeep } from "lodash" -export * from "./installation" - -/** - * Read the TENANT_FEATURE_FLAGS env var and return an array of features flags for each tenant. - * The env var is formatted as: - * tenant1:feature1:feature2,tenant2:feature1 - */ -export function buildFeatureFlags() { - if (!env.TENANT_FEATURE_FLAGS) { - return +class Flag { + static withDefault(value: T) { + return new Flag(value) } - const tenantFeatureFlags: Record = {} + private constructor(public defaultValue: T) {} +} - env.TENANT_FEATURE_FLAGS.split(",").forEach(tenantToFeatures => { - const [tenantId, ...features] = tenantToFeatures.split(":") +// This is the primary source of truth for feature flags. If you want to add a +// new flag, add it here and use the `fetch` and `get` functions to access it. +// All of the machinery in this file is to make sure that flags have their +// default values set correctly and their types flow through the system. +const FLAGS = { + LICENSING: Flag.withDefault(false), + GOOGLE_SHEETS: Flag.withDefault(false), + USER_GROUPS: Flag.withDefault(false), + ONBOARDING_TOUR: Flag.withDefault(false), +} - features.forEach(feature => { - if (!tenantFeatureFlags[tenantId]) { - tenantFeatureFlags[tenantId] = [] +const DEFAULTS = Object.keys(FLAGS).reduce((acc, key) => { + const typedKey = key as keyof typeof FLAGS + // @ts-ignore + acc[typedKey] = FLAGS[typedKey].defaultValue + return acc +}, {} as Flags) + +type UnwrapFlag = F extends Flag ? U : never +export type Flags = { + [K in keyof typeof FLAGS]: UnwrapFlag<(typeof FLAGS)[K]> +} + +// Exported for use in tests, should not be used outside of this file. +export function defaultFlags(): Flags { + return cloneDeep(DEFAULTS) +} + +function isFlagName(name: string): name is keyof Flags { + return FLAGS[name as keyof typeof FLAGS] !== undefined +} + +/** + * Reads the TENANT_FEATURE_FLAGS environment variable and returns a Flags object + * populated with the flags for the current tenant, filling in the default values + * if the flag is not set. + * + * Check the tests for examples of how TENANT_FEATURE_FLAGS should be formatted. + * + * In future we plan to add more ways of setting feature flags, e.g. PostHog, and + * they will be accessed through this function as well. + */ +export async function fetch(): Promise { + const currentTenantId = context.getTenantId() + const flags = defaultFlags() + + const split = (env.TENANT_FEATURE_FLAGS || "") + .split(",") + .map(x => x.split(":")) + for (const [tenantId, ...features] of split) { + if (!tenantId || (tenantId !== "*" && tenantId !== currentTenantId)) { + continue + } + + for (let feature of features) { + let value = true + if (feature.startsWith("!")) { + feature = feature.slice(1) + value = false } - tenantFeatureFlags[tenantId].push(feature) - }) - }) - return tenantFeatureFlags -} + if (!isFlagName(feature)) { + throw new Error(`Feature: ${feature} is not an allowed option`) + } -export function isEnabled(featureFlag: string) { - const tenantId = context.getTenantId() - const flags = getTenantFeatureFlags(tenantId) - return flags.includes(featureFlag) -} + if (typeof flags[feature] !== "boolean") { + throw new Error(`Feature: ${feature} is not a boolean`) + } -export function getTenantFeatureFlags(tenantId: string) { - let flags: string[] = [] - const envFlags = buildFeatureFlags() - if (envFlags) { - const globalFlags = envFlags["*"] - const tenantFlags = envFlags[tenantId] || [] - - // Explicitly exclude tenants from global features if required. - // Prefix the tenant flag with '!' - const tenantOverrides = tenantFlags.reduce( - (acc: string[], flag: string) => { - if (flag.startsWith("!")) { - let stripped = flag.substring(1) - acc.push(stripped) - } - return acc - }, - [] - ) - - if (globalFlags) { - flags.push(...globalFlags) + // @ts-ignore + flags[feature] = value } - if (tenantFlags.length) { - flags.push(...tenantFlags) - } - - // Purge any tenant specific overrides - flags = flags.filter(flag => { - return tenantOverrides.indexOf(flag) == -1 && !flag.startsWith("!") - }) } return flags } -export enum TenantFeatureFlag { - LICENSING = "LICENSING", - GOOGLE_SHEETS = "GOOGLE_SHEETS", - USER_GROUPS = "USER_GROUPS", - ONBOARDING_TOUR = "ONBOARDING_TOUR", +// Gets a single feature flag value. This is a convenience function for +// `fetch().then(flags => flags[name])`. +export async function get(name: K): Promise { + const flags = await fetch() + return flags[name] +} + +type BooleanFlags = { + [K in keyof typeof FLAGS]: (typeof FLAGS)[K] extends Flag ? K : never +}[keyof typeof FLAGS] + +// Convenience function for boolean flag values. This makes callsites more +// readable for boolean flags. +export async function isEnabled( + name: K +): Promise { + const flags = await fetch() + return flags[name] } diff --git a/packages/backend-core/src/features/installation.ts b/packages/backend-core/src/features/installation.ts deleted file mode 100644 index defc8bf987..0000000000 --- a/packages/backend-core/src/features/installation.ts +++ /dev/null @@ -1,17 +0,0 @@ -export function processFeatureEnvVar( - fullList: string[], - featureList?: string -) { - let list - if (!featureList) { - list = fullList - } else { - list = featureList.split(",") - } - for (let feature of list) { - if (!fullList.includes(feature)) { - throw new Error(`Feature: ${feature} is not an allowed option`) - } - } - return list as unknown as T[] -} diff --git a/packages/backend-core/src/features/tests/featureFlags.spec.ts b/packages/backend-core/src/features/tests/featureFlags.spec.ts deleted file mode 100644 index 1b68959329..0000000000 --- a/packages/backend-core/src/features/tests/featureFlags.spec.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { - TenantFeatureFlag, - buildFeatureFlags, - getTenantFeatureFlags, -} from "../" -import env from "../../environment" - -const { ONBOARDING_TOUR, LICENSING, USER_GROUPS } = TenantFeatureFlag - -describe("featureFlags", () => { - beforeEach(() => { - env._set("TENANT_FEATURE_FLAGS", "") - }) - - it("Should return no flags when the TENANT_FEATURE_FLAG is empty", async () => { - let features = buildFeatureFlags() - expect(features).toBeUndefined() - }) - - it("Should generate a map of global and named tenant feature flags from the env value", async () => { - env._set( - "TENANT_FEATURE_FLAGS", - `*:${ONBOARDING_TOUR},tenant1:!${ONBOARDING_TOUR},tenant2:${USER_GROUPS},tenant1:${LICENSING}` - ) - - const parsedFlags: Record = { - "*": [ONBOARDING_TOUR], - tenant1: [`!${ONBOARDING_TOUR}`, LICENSING], - tenant2: [USER_GROUPS], - } - - let features = buildFeatureFlags() - - expect(features).toBeDefined() - expect(features).toEqual(parsedFlags) - }) - - it("Should add feature flag flag only to explicitly configured tenant", async () => { - env._set( - "TENANT_FEATURE_FLAGS", - `*:${LICENSING},*:${USER_GROUPS},tenant1:${ONBOARDING_TOUR}` - ) - - let tenant1Flags = getTenantFeatureFlags("tenant1") - let tenant2Flags = getTenantFeatureFlags("tenant2") - - expect(tenant1Flags).toBeDefined() - expect(tenant1Flags).toEqual([LICENSING, USER_GROUPS, ONBOARDING_TOUR]) - - expect(tenant2Flags).toBeDefined() - expect(tenant2Flags).toEqual([LICENSING, USER_GROUPS]) - }) -}) - -it("Should exclude tenant1 from global feature flag", async () => { - env._set( - "TENANT_FEATURE_FLAGS", - `*:${LICENSING},*:${ONBOARDING_TOUR},tenant1:!${ONBOARDING_TOUR}` - ) - - let tenant1Flags = getTenantFeatureFlags("tenant1") - let tenant2Flags = getTenantFeatureFlags("tenant2") - - expect(tenant1Flags).toBeDefined() - expect(tenant1Flags).toEqual([LICENSING]) - - expect(tenant2Flags).toBeDefined() - expect(tenant2Flags).toEqual([LICENSING, ONBOARDING_TOUR]) -}) - -it("Should explicitly add flags to configured tenants only", async () => { - env._set( - "TENANT_FEATURE_FLAGS", - `tenant1:${ONBOARDING_TOUR},tenant1:${LICENSING},tenant2:${LICENSING}` - ) - - let tenant1Flags = getTenantFeatureFlags("tenant1") - let tenant2Flags = getTenantFeatureFlags("tenant2") - - expect(tenant1Flags).toBeDefined() - expect(tenant1Flags).toEqual([ONBOARDING_TOUR, LICENSING]) - - expect(tenant2Flags).toBeDefined() - expect(tenant2Flags).toEqual([LICENSING]) -}) diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts new file mode 100644 index 0000000000..83a89940b8 --- /dev/null +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -0,0 +1,86 @@ +import { defaultFlags, fetch, get, Flags } from "../" +import { context } from "../.." +import env from "../../environment" + +async function withFlags(flags: string, f: () => T): Promise { + const oldFlags = env.TENANT_FEATURE_FLAGS + env._set("TENANT_FEATURE_FLAGS", flags) + try { + return await f() + } finally { + env._set("TENANT_FEATURE_FLAGS", oldFlags) + } +} + +describe("feature flags", () => { + interface TestCase { + tenant: string + flags: string + expected: Partial + } + + it.each([ + { + tenant: "tenant1", + flags: "tenant1:ONBOARDING_TOUR", + expected: { ONBOARDING_TOUR: true }, + }, + { + tenant: "tenant1", + flags: "tenant1:!ONBOARDING_TOUR", + expected: { ONBOARDING_TOUR: false }, + }, + { + tenant: "tenant1", + flags: "*:ONBOARDING_TOUR", + expected: { ONBOARDING_TOUR: true }, + }, + { + tenant: "tenant1", + flags: "tenant2:ONBOARDING_TOUR", + expected: { ONBOARDING_TOUR: false }, + }, + { + tenant: "tenant1", + flags: "", + expected: defaultFlags(), + }, + ])( + 'should find flags $expected for $tenant with string "$flags"', + ({ tenant, flags, expected }) => + context.doInTenant(tenant, () => + withFlags(flags, async () => { + const flags = await fetch() + expect(flags).toMatchObject(expected) + + for (const [key, expectedValue] of Object.entries(expected)) { + const value = await get(key as keyof Flags) + expect(value).toBe(expectedValue) + } + }) + ) + ) + + interface FailedTestCase { + tenant: string + flags: string + expected: string | RegExp + } + + it.each([ + { + tenant: "tenant1", + flags: "tenant1:ONBOARDING_TOUR,tenant1:FOO", + expected: "Feature: FOO is not an allowed option", + }, + ])( + "should fail with message \"$expected\" for $tenant with string '$flags'", + async ({ tenant, flags, expected }) => { + context.doInTenant(tenant, () => + withFlags(flags, async () => { + await expect(fetch()).rejects.toThrow(expected) + }) + ) + } + ) +}) diff --git a/packages/backend-core/src/index.ts b/packages/backend-core/src/index.ts index 30c5fbdd7a..a14a344655 100644 --- a/packages/backend-core/src/index.ts +++ b/packages/backend-core/src/index.ts @@ -7,8 +7,7 @@ export * as roles from "./security/roles" export * as permissions from "./security/permissions" export * as accounts from "./accounts" export * as installation from "./installation" -export * as featureFlags from "./features" -export * as features from "./features/installation" +export * as features from "./features" export * as sessions from "./security/sessions" export * as platform from "./platform" export * as auth from "./auth" diff --git a/packages/builder/src/helpers/featureFlags.js b/packages/builder/src/helpers/featureFlags.js index 462dae8c54..fe30fb9980 100644 --- a/packages/builder/src/helpers/featureFlags.js +++ b/packages/builder/src/helpers/featureFlags.js @@ -5,9 +5,10 @@ export const TENANT_FEATURE_FLAGS = { LICENSING: "LICENSING", USER_GROUPS: "USER_GROUPS", ONBOARDING_TOUR: "ONBOARDING_TOUR", + GOOGLE_SHEETS: "GOOGLE_SHEETS", } export const isEnabled = featureFlag => { const user = get(auth).user - return !!user?.featureFlags?.includes(featureFlag) + return !!user?.flags?.[featureFlag] } diff --git a/packages/pro b/packages/pro index 7dbe323aec..62ef0e2d6e 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 7dbe323aec724ae6336b13c06aaefa4a89837edf +Subproject commit 62ef0e2d6e83522b6732fb3c61338de303f06ff0 diff --git a/packages/server/src/features.ts b/packages/server/src/features.ts index f040cf82a2..bf92ede18e 100644 --- a/packages/server/src/features.ts +++ b/packages/server/src/features.ts @@ -1,4 +1,3 @@ -import { features } from "@budibase/backend-core" import env from "./environment" enum AppFeature { @@ -6,7 +5,25 @@ enum AppFeature { AUTOMATIONS = "automations", } -const featureList = features.processFeatureEnvVar( +export function processFeatureEnvVar( + fullList: string[], + featureList?: string +) { + let list + if (!featureList) { + list = fullList + } else { + list = featureList.split(",") + } + for (let feature of list) { + if (!fullList.includes(feature)) { + throw new Error(`Feature: ${feature} is not an allowed option`) + } + } + return list as unknown as T[] +} + +const featureList = processFeatureEnvVar( Object.values(AppFeature), env.APP_FEATURES ) diff --git a/packages/worker/src/api/controllers/global/self.ts b/packages/worker/src/api/controllers/global/self.ts index d762f5168a..ec154adf7f 100644 --- a/packages/worker/src/api/controllers/global/self.ts +++ b/packages/worker/src/api/controllers/global/self.ts @@ -1,6 +1,6 @@ import * as userSdk from "../../../sdk/users" import { - featureFlags, + features, tenancy, db as dbCore, utils, @@ -104,8 +104,8 @@ export async function getSelf(ctx: any) { ctx.body = await groups.enrichUserRolesFromGroups(user) // add the feature flags for this tenant - const tenantId = tenancy.getTenantId() - ctx.body.featureFlags = featureFlags.getTenantFeatureFlags(tenantId) + const flags = await features.fetch() + ctx.body.flags = flags addSessionAttributesToUser(ctx) } diff --git a/packages/worker/src/environment.ts b/packages/worker/src/environment.ts index 9f7baf9e9b..6e36b45a3b 100644 --- a/packages/worker/src/environment.ts +++ b/packages/worker/src/environment.ts @@ -19,8 +19,6 @@ function parseIntSafe(number: any) { } const environment = { - // features - WORKER_FEATURES: process.env.WORKER_FEATURES, // auth MINIO_ACCESS_KEY: process.env.MINIO_ACCESS_KEY, MINIO_SECRET_KEY: process.env.MINIO_SECRET_KEY, diff --git a/packages/worker/src/features.ts b/packages/worker/src/features.ts deleted file mode 100644 index 075b3b81ca..0000000000 --- a/packages/worker/src/features.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { features } from "@budibase/backend-core" -import env from "./environment" - -enum WorkerFeature {} - -const featureList: WorkerFeature[] = features.processFeatureEnvVar( - Object.values(WorkerFeature), - env.WORKER_FEATURES -) - -export function isFeatureEnabled(feature: WorkerFeature) { - return featureList.includes(feature) -}