From a58c75e328f0b4aa0f2ebfb47a1df3275d2b541d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 6 Jan 2025 12:11:14 +0000 Subject: [PATCH 1/3] Expose feature flags type in packages/type so it can be exposed to the frontend. --- .../backend-core/src/context/mainContext.ts | 8 +- packages/backend-core/src/context/types.ts | 2 +- .../backend-core/src/features/features.ts | 135 ++++-------------- .../src/features/tests/features.spec.ts | 28 +--- packages/pro | 2 +- .../src/api/controllers/row/staticFormula.ts | 4 +- packages/server/src/automations/actions.ts | 6 +- .../server/src/automations/steps/openai.ts | 4 +- .../server/src/middleware/zod-validator.ts | 2 +- packages/types/src/api/web/global/self.ts | 3 +- packages/types/src/sdk/featureFlag.ts | 10 +- .../worker/src/api/controllers/global/self.ts | 3 +- 12 files changed, 57 insertions(+), 150 deletions(-) diff --git a/packages/backend-core/src/context/mainContext.ts b/packages/backend-core/src/context/mainContext.ts index 64ba240fa5..e5f20882d3 100644 --- a/packages/backend-core/src/context/mainContext.ts +++ b/packages/backend-core/src/context/mainContext.ts @@ -385,17 +385,17 @@ export function getCurrentContext(): ContextMap | undefined { } } -export function getFeatureFlags>( +export function getFeatureFlags( key: string -): T | undefined { +): Record | undefined { const context = getCurrentContext() if (!context) { return undefined } - return context.featureFlagCache?.[key] as T + return context.featureFlagCache?.[key] } -export function setFeatureFlags(key: string, value: Record) { +export function setFeatureFlags(key: string, value: Record) { const context = getCurrentContext() if (!context) { return diff --git a/packages/backend-core/src/context/types.ts b/packages/backend-core/src/context/types.ts index 5549a47ff7..23598b951e 100644 --- a/packages/backend-core/src/context/types.ts +++ b/packages/backend-core/src/context/types.ts @@ -20,7 +20,7 @@ export type ContextMap = { clients: Record } featureFlagCache?: { - [key: string]: Record + [key: string]: Record } viewToTableCache?: Record } diff --git a/packages/backend-core/src/features/features.ts b/packages/backend-core/src/features/features.ts index 650254fcb2..772bcf5860 100644 --- a/packages/backend-core/src/features/features.ts +++ b/packages/backend-core/src/features/features.ts @@ -2,9 +2,10 @@ import env from "../environment" import * as crypto from "crypto" import * as context from "../context" import { PostHog, PostHogOptions } from "posthog-node" -import { FeatureFlag } from "@budibase/types" import tracer from "dd-trace" import { Duration } from "../utils" +import { cloneDeep } from "lodash" +import { FeatureFlagDefaults } from "@budibase/types" let posthog: PostHog | undefined export function init(opts?: PostHogOptions) { @@ -30,74 +31,6 @@ export function shutdown() { posthog?.shutdown() } -export abstract class Flag { - static boolean(defaultValue: boolean): Flag { - return new BooleanFlag(defaultValue) - } - - static string(defaultValue: string): Flag { - return new StringFlag(defaultValue) - } - - static number(defaultValue: number): Flag { - return new NumberFlag(defaultValue) - } - - protected constructor(public defaultValue: T) {} - - abstract parse(value: any): T -} - -type UnwrapFlag = F extends Flag ? U : never - -export type FlagValues = { - [K in keyof T]: UnwrapFlag -} - -type KeysOfType = { - [K in keyof T]: T[K] extends Flag ? K : never -}[keyof T] - -class BooleanFlag extends Flag { - parse(value: any) { - if (typeof value === "string") { - return ["true", "t", "1"].includes(value.toLowerCase()) - } - - if (typeof value === "boolean") { - return value - } - - throw new Error(`could not parse value "${value}" as boolean`) - } -} - -class StringFlag extends Flag { - parse(value: any) { - if (typeof value === "string") { - return value - } - throw new Error(`could not parse value "${value}" as string`) - } -} - -class NumberFlag extends Flag { - parse(value: any) { - if (typeof value === "number") { - return value - } - - if (typeof value === "string") { - const parsed = parseFloat(value) - if (!isNaN(parsed)) { - return parsed - } - } - - throw new Error(`could not parse value "${value}" as number`) - } -} - export interface EnvFlagEntry { tenantId: string key: string @@ -120,7 +53,7 @@ export function parseEnvFlags(flags: string): EnvFlagEntry[] { return result } -export class FlagSet, T extends { [key: string]: V }> { +export class FlagSet { // This is used to safely cache flags sets in the current request context. // Because multiple sets could theoretically exist, we don't want the cache of // one to leak into another. @@ -130,34 +63,25 @@ export class FlagSet, T extends { [key: string]: V }> { this.setId = crypto.randomUUID() } - defaults(): FlagValues { - return Object.keys(this.flagSchema).reduce((acc, key) => { - const typedKey = key as keyof T - acc[typedKey] = this.flagSchema[key].defaultValue - return acc - }, {} as FlagValues) + defaults(): T { + return cloneDeep(this.flagSchema) } isFlagName(name: string | number | symbol): name is keyof T { return this.flagSchema[name as keyof T] !== undefined } - async get(key: K): Promise[K]> { + async isEnabled(key: K): Promise { const flags = await this.fetch() return flags[key] } - async isEnabled>(key: K): Promise { - const flags = await this.fetch() - return flags[key] - } - - async fetch(): Promise> { + async fetch(): Promise { return await tracer.trace("features.fetch", async span => { - const cachedFlags = context.getFeatureFlags>(this.setId) + const cachedFlags = context.getFeatureFlags(this.setId) if (cachedFlags) { span?.addTags({ fromCache: true }) - return cachedFlags + return cachedFlags as T } const tags: Record = {} @@ -189,7 +113,7 @@ export class FlagSet, T extends { [key: string]: V }> { // @ts-expect-error - TS does not like you writing into a generic type, // but we know that it's okay in this case because it's just an object. - flagValues[key as keyof FlagValues] = value + flagValues[key as keyof T] = value tags[`flags.${key}.source`] = "environment" } @@ -217,11 +141,11 @@ export class FlagSet, T extends { [key: string]: V }> { tags[`readFromPostHog`] = true const personProperties: Record = { tenantId } - const posthogFlags = await posthog.getAllFlagsAndPayloads(userId, { + const posthogFlags = await posthog.getAllFlags(userId, { personProperties, }) - for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { + for (const [name, value] of Object.entries(posthogFlags)) { if (!this.isFlagName(name)) { // We don't want an unexpected PostHog flag to break the app, so we // just log it and continue. @@ -229,19 +153,20 @@ export class FlagSet, T extends { [key: string]: V }> { continue } + if (typeof value !== "boolean") { + console.warn(`Invalid value for posthog flag "${name}": ${value}`) + continue + } + if (flagValues[name] === true || specificallySetFalse.has(name)) { // If the flag is already set to through environment variables, we // don't want to override it back to false here. continue } - const payload = posthogFlags.featureFlagPayloads?.[name] - const flag = this.flagSchema[name] try { - // @ts-expect-error - TS does not like you writing into a generic - // type, but we know that it's okay in this case because it's just - // an object. - flagValues[name] = flag.parse(payload || value) + // @ts-expect-error - TS does not like you writing into a generic type. + flagValues[name] = value tags[`flags.${name}.source`] = "posthog" } catch (err) { // We don't want an invalid PostHog flag to break the app, so we just @@ -262,18 +187,12 @@ export class FlagSet, T extends { [key: string]: V }> { } } -// 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 flagsConfig: Record> = { - [FeatureFlag.DEFAULT_VALUES]: Flag.boolean(true), - [FeatureFlag.AUTOMATION_BRANCHING]: Flag.boolean(true), - [FeatureFlag.AI_CUSTOM_CONFIGS]: Flag.boolean(true), - [FeatureFlag.BUDIBASE_AI]: Flag.boolean(true), - [FeatureFlag.USE_ZOD_VALIDATOR]: Flag.boolean(env.isDev()), -} -export const flags = new FlagSet(flagsConfig) +export const flags = new FlagSet(FeatureFlagDefaults) -type UnwrapPromise = T extends Promise ? U : T -export type FeatureFlags = UnwrapPromise> +export async function isEnabled(flag: keyof typeof FeatureFlagDefaults) { + return await flags.isEnabled(flag) +} + +export async function all() { + return await flags.fetch() +} diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index ced874f4af..f918347eea 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -1,5 +1,5 @@ import { IdentityContext, IdentityType } from "@budibase/types" -import { Flag, FlagSet, FlagValues, init, shutdown } from "../" +import { FlagSet, init, shutdown } from "../" import * as context from "../../context" import environment, { withEnv } from "../../environment" import nodeFetch from "node-fetch" @@ -7,10 +7,8 @@ import nock from "nock" import * as crypto from "crypto" const schema = { - TEST_BOOLEAN: Flag.boolean(false), - TEST_STRING: Flag.string("default value"), - TEST_NUMBER: Flag.number(0), - TEST_BOOLEAN_DEFAULT_TRUE: Flag.boolean(true), + TEST_BOOLEAN: false, + TEST_BOOLEAN_DEFAULT_TRUE: true, } const flags = new FlagSet(schema) @@ -19,7 +17,7 @@ interface TestCase { identity?: Partial environmentFlags?: string posthogFlags?: PostHogFlags - expected?: Partial> + expected?: Partial errorMessage?: string | RegExp } @@ -83,22 +81,6 @@ describe("feature flags", () => { }, expected: { TEST_BOOLEAN: true }, }, - { - it: "should be able to read string flags from PostHog", - posthogFlags: { - featureFlags: { TEST_STRING: true }, - featureFlagPayloads: { TEST_STRING: "test" }, - }, - expected: { TEST_STRING: "test" }, - }, - { - it: "should be able to read numeric flags from PostHog", - posthogFlags: { - featureFlags: { TEST_NUMBER: true }, - featureFlagPayloads: { TEST_NUMBER: "123" }, - }, - expected: { TEST_NUMBER: 123 }, - }, { it: "should not be able to override a negative environment flag from PostHog", environmentFlags: "default:!TEST_BOOLEAN", @@ -177,7 +159,7 @@ describe("feature flags", () => { expect(values).toMatchObject(expected) for (const [key, expectedValue] of Object.entries(expected)) { - const value = await flags.get(key as keyof typeof schema) + const value = await flags.isEnabled(key as keyof typeof schema) expect(value).toBe(expectedValue) } } else { diff --git a/packages/pro b/packages/pro index ae786121d9..9091868986 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit ae786121d923449b0ad5fcbd123d0a9fec28f65e +Subproject commit 9091868986fbc6aae580280f48853482e0b06c6a diff --git a/packages/server/src/api/controllers/row/staticFormula.ts b/packages/server/src/api/controllers/row/staticFormula.ts index 4464b7f44a..b81a164807 100644 --- a/packages/server/src/api/controllers/row/staticFormula.ts +++ b/packages/server/src/api/controllers/row/staticFormula.ts @@ -163,9 +163,9 @@ export async function finaliseRow( contextRows: [enrichedRow], }) const aiEnabled = - ((await features.flags.isEnabled(FeatureFlag.BUDIBASE_AI)) && + ((await features.isEnabled(FeatureFlag.BUDIBASE_AI)) && (await pro.features.isBudibaseAIEnabled())) || - ((await features.flags.isEnabled(FeatureFlag.AI_CUSTOM_CONFIGS)) && + ((await features.isEnabled(FeatureFlag.AI_CUSTOM_CONFIGS)) && (await pro.features.isAICustomConfigsEnabled())) if (aiEnabled) { row = await processAIColumns(table, row, { diff --git a/packages/server/src/automations/actions.ts b/packages/server/src/automations/actions.ts index e5a1c63b7d..1c201d1f64 100644 --- a/packages/server/src/automations/actions.ts +++ b/packages/server/src/automations/actions.ts @@ -105,13 +105,13 @@ if (env.SELF_HOSTED) { export async function getActionDefinitions(): Promise< Record > { - if (await features.flags.isEnabled(FeatureFlag.AUTOMATION_BRANCHING)) { + if (await features.isEnabled(FeatureFlag.AUTOMATION_BRANCHING)) { BUILTIN_ACTION_DEFINITIONS["BRANCH"] = branch.definition } if ( env.SELF_HOSTED || - (await features.flags.isEnabled(FeatureFlag.BUDIBASE_AI)) || - (await features.flags.isEnabled(FeatureFlag.AI_CUSTOM_CONFIGS)) + (await features.isEnabled(FeatureFlag.BUDIBASE_AI)) || + (await features.isEnabled(FeatureFlag.AI_CUSTOM_CONFIGS)) ) { BUILTIN_ACTION_DEFINITIONS["OPENAI"] = openai.definition } diff --git a/packages/server/src/automations/steps/openai.ts b/packages/server/src/automations/steps/openai.ts index 48eaa93057..19595cc0d0 100644 --- a/packages/server/src/automations/steps/openai.ts +++ b/packages/server/src/automations/steps/openai.ts @@ -100,10 +100,10 @@ export async function run({ try { let response const customConfigsEnabled = - (await features.flags.isEnabled(FeatureFlag.AI_CUSTOM_CONFIGS)) && + (await features.isEnabled(FeatureFlag.AI_CUSTOM_CONFIGS)) && (await pro.features.isAICustomConfigsEnabled()) const budibaseAIEnabled = - (await features.flags.isEnabled(FeatureFlag.BUDIBASE_AI)) && + (await features.isEnabled(FeatureFlag.BUDIBASE_AI)) && (await pro.features.isBudibaseAIEnabled()) let llmWrapper diff --git a/packages/server/src/middleware/zod-validator.ts b/packages/server/src/middleware/zod-validator.ts index e8cc2c470a..d57e1c48ff 100644 --- a/packages/server/src/middleware/zod-validator.ts +++ b/packages/server/src/middleware/zod-validator.ts @@ -7,7 +7,7 @@ import { fromZodError } from "zod-validation-error" function validate(schema: AnyZodObject, property: "body" | "params") { // Return a Koa middleware function return async (ctx: Ctx, next: any) => { - if (!(await features.flags.isEnabled(FeatureFlag.USE_ZOD_VALIDATOR))) { + if (!(await features.isEnabled(FeatureFlag.USE_ZOD_VALIDATOR))) { return next() } diff --git a/packages/types/src/api/web/global/self.ts b/packages/types/src/api/web/global/self.ts index 5f21a8ddc5..d2c79bdba3 100644 --- a/packages/types/src/api/web/global/self.ts +++ b/packages/types/src/api/web/global/self.ts @@ -1,3 +1,4 @@ +import { FeatureFlags } from "packages/types/src/sdk" import { DevInfo, User } from "../../../documents" export interface GenerateAPIKeyRequest { @@ -8,5 +9,5 @@ export interface GenerateAPIKeyResponse extends DevInfo {} export interface FetchAPIKeyResponse extends DevInfo {} export interface GetGlobalSelfResponse extends User { - flags?: Record + flags?: FeatureFlags } diff --git a/packages/types/src/sdk/featureFlag.ts b/packages/types/src/sdk/featureFlag.ts index 98e744324c..725ae0feb1 100644 --- a/packages/types/src/sdk/featureFlag.ts +++ b/packages/types/src/sdk/featureFlag.ts @@ -6,6 +6,12 @@ export enum FeatureFlag { USE_ZOD_VALIDATOR = "USE_ZOD_VALIDATOR", } -export interface TenantFeatureFlags { - [key: string]: FeatureFlag[] +export const FeatureFlagDefaults = { + [FeatureFlag.DEFAULT_VALUES]: true, + [FeatureFlag.AUTOMATION_BRANCHING]: true, + [FeatureFlag.AI_CUSTOM_CONFIGS]: true, + [FeatureFlag.BUDIBASE_AI]: true, + [FeatureFlag.USE_ZOD_VALIDATOR]: false, } + +export type FeatureFlags = typeof FeatureFlagDefaults diff --git a/packages/worker/src/api/controllers/global/self.ts b/packages/worker/src/api/controllers/global/self.ts index f8488f526b..eb704ccf02 100644 --- a/packages/worker/src/api/controllers/global/self.ts +++ b/packages/worker/src/api/controllers/global/self.ts @@ -111,8 +111,7 @@ export async function getSelf(ctx: UserCtx) { ctx.body = await groups.enrichUserRolesFromGroups(user) // add the feature flags for this tenant - const flags = await features.flags.fetch() - ctx.body.flags = flags + ctx.body.flags = await features.flags.fetch() addSessionAttributesToUser(ctx) } From c3564528b7c989e8069449251d474651ee9cb2c7 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 6 Jan 2025 12:15:41 +0000 Subject: [PATCH 2/3] Fix types. --- packages/backend-core/src/features/tests/utils.ts | 3 ++- packages/types/src/api/web/global/self.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/features/tests/utils.ts b/packages/backend-core/src/features/tests/utils.ts index cc633c083d..b9281b7f19 100644 --- a/packages/backend-core/src/features/tests/utils.ts +++ b/packages/backend-core/src/features/tests/utils.ts @@ -1,5 +1,6 @@ -import { FeatureFlags, parseEnvFlags } from ".." +import { FeatureFlags } from "@budibase/types" import { setEnv } from "../../environment" +import { parseEnvFlags } from "../features" function getCurrentFlags(): Record> { const result: Record> = {} diff --git a/packages/types/src/api/web/global/self.ts b/packages/types/src/api/web/global/self.ts index d2c79bdba3..9e879e6c3c 100644 --- a/packages/types/src/api/web/global/self.ts +++ b/packages/types/src/api/web/global/self.ts @@ -1,4 +1,4 @@ -import { FeatureFlags } from "packages/types/src/sdk" +import { FeatureFlags } from "@budibase/types" import { DevInfo, User } from "../../../documents" export interface GenerateAPIKeyRequest { From 511328002ee5ff4a33c73df764b3528468eed37d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 6 Jan 2025 12:35:55 +0000 Subject: [PATCH 3/3] Update pro reference. --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 9091868986..32d84f109d 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 9091868986fbc6aae580280f48853482e0b06c6a +Subproject commit 32d84f109d4edc526145472a7446327312151442