Merge pull request #14353 from Budibase/license-in-feature-flags

Make it possible to get flag values from the license.
This commit is contained in:
Sam Rose 2024-08-12 10:19:55 +01:00 committed by GitHub
commit 5a28c23a97
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 232 additions and 200 deletions

View File

@ -1,12 +1,12 @@
import env from "../environment" import env from "../environment"
import * as context from "../context" import * as context from "../context"
import { PostHog, PostHogOptions } from "posthog-node" import { PostHog, PostHogOptions } from "posthog-node"
import { IdentityType } from "@budibase/types" import { IdentityType, UserCtx } from "@budibase/types"
import tracer from "dd-trace" import tracer from "dd-trace"
let posthog: PostHog | undefined let posthog: PostHog | undefined
export function init(opts?: PostHogOptions) { export function init(opts?: PostHogOptions) {
if (env.POSTHOG_TOKEN) { if (env.POSTHOG_TOKEN && env.POSTHOG_API_HOST) {
posthog = new PostHog(env.POSTHOG_TOKEN, { posthog = new PostHog(env.POSTHOG_TOKEN, {
host: env.POSTHOG_API_HOST, host: env.POSTHOG_API_HOST,
...opts, ...opts,
@ -83,40 +83,40 @@ class NumberFlag extends Flag<number> {
} }
export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> { export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
private readonly flags: T constructor(private readonly flagSchema: T) {}
constructor(flags: T) {
this.flags = flags
}
defaults(): FlagValues<T> { defaults(): FlagValues<T> {
return Object.keys(this.flags).reduce((acc, key) => { return Object.keys(this.flagSchema).reduce((acc, key) => {
const typedKey = key as keyof T const typedKey = key as keyof T
acc[typedKey] = this.flags[key].defaultValue acc[typedKey] = this.flagSchema[key].defaultValue
return acc return acc
}, {} as FlagValues<T>) }, {} as FlagValues<T>)
} }
isFlagName(name: string | number | symbol): name is keyof T { isFlagName(name: string | number | symbol): name is keyof T {
return this.flags[name as keyof T] !== undefined return this.flagSchema[name as keyof T] !== undefined
} }
async get<K extends keyof T>(key: K): Promise<FlagValues<T>[K]> { async get<K extends keyof T>(
const flags = await this.fetch() key: K,
ctx?: UserCtx
): Promise<FlagValues<T>[K]> {
const flags = await this.fetch(ctx)
return flags[key] return flags[key]
} }
async isEnabled<K extends KeysOfType<T, boolean>>(key: K): Promise<boolean> { async isEnabled<K extends KeysOfType<T, boolean>>(
const flags = await this.fetch() key: K,
ctx?: UserCtx
): Promise<boolean> {
const flags = await this.fetch(ctx)
return flags[key] return flags[key]
} }
async fetch(): Promise<FlagValues<T>> { async fetch(ctx?: UserCtx): Promise<FlagValues<T>> {
return await tracer.trace("features.fetch", async span => { return await tracer.trace("features.fetch", async span => {
const tags: Record<string, any> = {} const tags: Record<string, any> = {}
const flagValues = this.defaults()
const flags = this.defaults()
const currentTenantId = context.getTenantId() const currentTenantId = context.getTenantId()
const specificallySetFalse = new Set<string>() const specificallySetFalse = new Set<string>()
@ -140,39 +140,64 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
throw new Error(`Feature: ${feature} is not an allowed option`) throw new Error(`Feature: ${feature} is not an allowed option`)
} }
if (typeof flags[feature] !== "boolean") { if (typeof flagValues[feature] !== "boolean") {
throw new Error(`Feature: ${feature} is not a boolean`) throw new Error(`Feature: ${feature} is not a boolean`)
} }
// @ts-ignore // @ts-expect-error - TS does not like you writing into a generic type,
flags[feature] = value // but we know that it's okay in this case because it's just an object.
flagValues[feature] = value
tags[`flags.${feature}.source`] = "environment" tags[`flags.${feature}.source`] = "environment"
} }
} }
const license = ctx?.user?.license
if (license) {
for (const feature of license.features) {
if (!this.isFlagName(feature)) {
continue
}
if (
flagValues[feature] === true ||
specificallySetFalse.has(feature)
) {
// If the flag is already set to through environment variables, we
// don't want to override it back to false here.
continue
}
// @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[feature] = true
tags[`flags.${feature}.source`] = "license"
}
}
const identity = context.getIdentity() const identity = context.getIdentity()
if (posthog && identity?.type === IdentityType.USER) { if (posthog && identity?.type === IdentityType.USER) {
const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id)
for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { for (const [name, value] of Object.entries(posthogFlags.featureFlags)) {
const flag = this.flags[name] if (!this.isFlagName(name)) {
if (!flag) {
// We don't want an unexpected PostHog flag to break the app, so we // We don't want an unexpected PostHog flag to break the app, so we
// just log it and continue. // just log it and continue.
console.warn(`Unexpected posthog flag "${name}": ${value}`) console.warn(`Unexpected posthog flag "${name}": ${value}`)
continue continue
} }
if (flags[name] === true || specificallySetFalse.has(name)) { if (flagValues[name] === true || specificallySetFalse.has(name)) {
// If the flag is already set to through environment variables, we // If the flag is already set to through environment variables, we
// don't want to override it back to false here. // don't want to override it back to false here.
continue continue
} }
const payload = posthogFlags.featureFlagPayloads?.[name] const payload = posthogFlags.featureFlagPayloads?.[name]
const flag = this.flagSchema[name]
try { try {
// @ts-ignore // @ts-expect-error - TS does not like you writing into a generic
flags[name] = flag.parse(payload || value) // type, but we know that it's okay in this case because it's just
// an object.
flagValues[name] = flag.parse(payload || value)
tags[`flags.${name}.source`] = "posthog" tags[`flags.${name}.source`] = "posthog"
} catch (err) { } catch (err) {
// We don't want an invalid PostHog flag to break the app, so we just // We don't want an invalid PostHog flag to break the app, so we just
@ -182,12 +207,12 @@ export class FlagSet<V extends Flag<any>, T extends { [key: string]: V }> {
} }
} }
for (const [key, value] of Object.entries(flags)) { for (const [key, value] of Object.entries(flagValues)) {
tags[`flags.${key}.value`] = value tags[`flags.${key}.value`] = value
} }
span?.addTags(tags) span?.addTags(tags)
return flags return flagValues
}) })
} }
} }

View File

@ -1,7 +1,7 @@
import { IdentityContext, IdentityType } from "@budibase/types" import { IdentityContext, IdentityType, UserCtx } from "@budibase/types"
import { Flag, FlagSet, FlagValues, init } from "../" import { Flag, FlagSet, FlagValues, init } from "../"
import { context } from "../.." import { context } from "../.."
import { setEnv, withEnv } from "../../environment" import environment, { withEnv } from "../../environment"
import nodeFetch from "node-fetch" import nodeFetch from "node-fetch"
import nock from "nock" import nock from "nock"
@ -12,207 +12,214 @@ const schema = {
} }
const flags = new FlagSet(schema) const flags = new FlagSet(schema)
interface TestCase {
it: string
identity?: Partial<IdentityContext>
environmentFlags?: string
posthogFlags?: PostHogFlags
licenseFlags?: Array<string>
expected?: Partial<FlagValues<typeof schema>>
errorMessage?: string | RegExp
}
interface PostHogFlags {
featureFlags?: Record<string, boolean>
featureFlagPayloads?: Record<string, string>
}
function mockPosthogFlags(flags: PostHogFlags) {
nock("https://us.i.posthog.com")
.post("/decide/?v=3", body => {
return body.token === "test" && body.distinct_id === "us_1234"
})
.reply(200, flags)
.persist()
}
describe("feature flags", () => { describe("feature flags", () => {
interface TestCase { beforeEach(() => {
tenant: string nock.cleanAll()
flags: string })
expected: Partial<FlagValues<typeof schema>>
}
it.each<TestCase>([ it.each<TestCase>([
{ {
tenant: "tenant1", it: "should should find a simple boolean flag in the environment",
flags: "tenant1:TEST_BOOLEAN", environmentFlags: "default:TEST_BOOLEAN",
expected: { TEST_BOOLEAN: true }, expected: { TEST_BOOLEAN: true },
}, },
{ {
tenant: "tenant1", it: "should should find a simple netgative boolean flag in the environment",
flags: "tenant1:!TEST_BOOLEAN", environmentFlags: "default:!TEST_BOOLEAN",
expected: { TEST_BOOLEAN: false }, expected: { TEST_BOOLEAN: false },
}, },
{ {
tenant: "tenant1", it: "should should match stars in the environment",
flags: "*:TEST_BOOLEAN", environmentFlags: "*:TEST_BOOLEAN",
expected: { TEST_BOOLEAN: true }, expected: { TEST_BOOLEAN: true },
}, },
{ {
tenant: "tenant1", it: "should not match a different tenant's flags",
flags: "tenant2:TEST_BOOLEAN", environmentFlags: "otherTenant:TEST_BOOLEAN",
expected: { TEST_BOOLEAN: false }, expected: { TEST_BOOLEAN: false },
}, },
{ {
tenant: "tenant1", it: "should return the defaults when no flags are set",
flags: "", expected: flags.defaults(),
},
{
it: "should fail when an environment flag is not recognised",
environmentFlags: "default:TEST_BOOLEAN,default:FOO",
errorMessage: "Feature: FOO is not an allowed option",
},
{
it: "should be able to read boolean flags from PostHog",
posthogFlags: {
featureFlags: { TEST_BOOLEAN: true },
},
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",
posthogFlags: {
featureFlags: { TEST_BOOLEAN: true },
},
expected: { TEST_BOOLEAN: false },
},
{
it: "should not be able to override a positive environment flag from PostHog",
environmentFlags: "default:TEST_BOOLEAN",
posthogFlags: {
featureFlags: {
TEST_BOOLEAN: false,
},
},
expected: { TEST_BOOLEAN: true },
},
{
it: "should be able to set boolean flags through the license",
licenseFlags: ["TEST_BOOLEAN"],
expected: { TEST_BOOLEAN: true },
},
{
it: "should not be able to override a negative environment flag from license",
environmentFlags: "default:!TEST_BOOLEAN",
licenseFlags: ["TEST_BOOLEAN"],
expected: { TEST_BOOLEAN: false },
},
{
it: "should not error on unrecognised PostHog flag",
posthogFlags: {
featureFlags: { UNDEFINED: true },
},
expected: flags.defaults(),
},
{
it: "should not error on unrecognised license flag",
licenseFlags: ["UNDEFINED"],
expected: flags.defaults(), expected: flags.defaults(),
}, },
])( ])(
'should find flags $expected for $tenant with string "$flags"', "$it",
({ tenant, flags: envFlags, expected }) => async ({
context.doInTenant(tenant, async () => identity,
withEnv({ TENANT_FEATURE_FLAGS: envFlags }, async () => { environmentFlags,
const values = await flags.fetch() posthogFlags,
licenseFlags,
expected,
errorMessage,
}) => {
const env: Partial<typeof environment> = {
TENANT_FEATURE_FLAGS: environmentFlags,
}
if (posthogFlags) {
mockPosthogFlags(posthogFlags)
env.POSTHOG_TOKEN = "test"
env.POSTHOG_API_HOST = "https://us.i.posthog.com"
}
const ctx = { user: { license: { features: licenseFlags || [] } } }
await withEnv(env, async () => {
// We need to pass in node-fetch here otherwise nock won't get used
// because posthog-node uses axios under the hood.
init({ fetch: nodeFetch })
const fullIdentity: IdentityContext = {
_id: "us_1234",
tenantId: "default",
type: IdentityType.USER,
email: "test@example.com",
firstName: "Test",
lastName: "User",
...identity,
}
await context.doInIdentityContext(fullIdentity, async () => {
if (errorMessage) {
await expect(flags.fetch(ctx as UserCtx)).rejects.toThrow(
errorMessage
)
} else if (expected) {
const values = await flags.fetch(ctx as UserCtx)
expect(values).toMatchObject(expected) expect(values).toMatchObject(expected)
for (const [key, expectedValue] of Object.entries(expected)) { for (const [key, expectedValue] of Object.entries(expected)) {
const value = await flags.get(key as keyof typeof schema) const value = await flags.get(
key as keyof typeof schema,
ctx as UserCtx
)
expect(value).toBe(expectedValue) expect(value).toBe(expectedValue)
} }
}) } else {
) throw new Error("No expected value")
) }
})
interface FailedTestCase { })
tenant: string
flags: string
expected: string | RegExp
} }
it.each<FailedTestCase>([
{
tenant: "tenant1",
flags: "tenant1:TEST_BOOLEAN,tenant1:FOO",
expected: "Feature: FOO is not an allowed option",
},
])(
"should fail with message \"$expected\" for $tenant with string '$flags'",
({ tenant, flags: envFlags, expected }) =>
context.doInTenant(tenant, () =>
withEnv({ TENANT_FEATURE_FLAGS: envFlags }, () =>
expect(flags.fetch()).rejects.toThrow(expected)
)
)
) )
describe("posthog", () => { it("should not error if PostHog is down", async () => {
const identity: IdentityContext = { const identity: IdentityContext = {
_id: "us_1234", _id: "us_1234",
tenantId: "tenant1", tenantId: "default",
type: IdentityType.USER, type: IdentityType.USER,
email: "test@example.com", email: "test@example.com",
firstName: "Test", firstName: "Test",
lastName: "User", lastName: "User",
} }
let cleanup: () => void
beforeAll(() => {
cleanup = setEnv({ POSTHOG_TOKEN: "test" })
})
afterAll(() => {
cleanup()
})
beforeEach(() => {
nock.cleanAll()
// We need to pass in node-fetch here otherwise nock won't get used
// because posthog-node uses axios under the hood.
init({ fetch: nodeFetch })
})
function mockFlags(flags: {
featureFlags?: Record<string, boolean>
featureFlagPayloads?: Record<string, any>
}) {
nock("https://us.i.posthog.com") nock("https://us.i.posthog.com")
.post("/decide/?v=3", body => { .post("/decide/?v=3", body => {
return body.token === "test" && body.distinct_id === "us_1234" return body.token === "test" && body.distinct_id === "us_1234"
}) })
.reply(200, flags) .reply(503)
} .persist()
it("should be able to read flags from posthog", async () => {
mockFlags({
featureFlags: {
TEST_BOOLEAN: true,
},
})
await context.doInIdentityContext(identity, async () => {
const values = await flags.fetch()
expect(values.TEST_BOOLEAN).toBe(true)
})
})
it("should be able to read flags from posthog with payloads", async () => {
mockFlags({
featureFlags: {
TEST_STRING: true,
},
featureFlagPayloads: {
TEST_STRING: "test payload",
},
})
await context.doInIdentityContext(identity, async () => {
const values = await flags.fetch()
expect(values.TEST_STRING).toBe("test payload")
})
})
it("should be able to read flags from posthog with numbers", async () => {
mockFlags({
featureFlags: {
TEST_NUMBER: true,
},
featureFlagPayloads: {
TEST_NUMBER: 123,
},
})
await context.doInIdentityContext(identity, async () => {
const values = await flags.fetch()
expect(values.TEST_NUMBER).toBe(123)
})
})
it("should not fail when a flag is not known", async () => {
mockFlags({
featureFlags: {
_SOME_RANDOM_FLAG: true,
},
})
await context.doInIdentityContext(identity, async () => {
await expect(flags.fetch()).resolves.not.toThrow()
})
})
it("should not override flags set in the environment", async () => {
mockFlags({
featureFlags: {
TEST_BOOLEAN: false,
},
})
await withEnv( await withEnv(
{ TENANT_FEATURE_FLAGS: `${identity.tenantId}:TEST_BOOLEAN` }, { POSTHOG_TOKEN: "test", POSTHOG_API_HOST: "https://us.i.posthog.com" },
async () => { async () => {
await context.doInIdentityContext(identity, async () => { await context.doInIdentityContext(identity, async () => {
const values = await flags.fetch() await flags.fetch()
expect(values.TEST_BOOLEAN).toBe(true)
}) })
} }
) )
}) })
it("should not override flags set in the environment with a ! prefix", async () => {
mockFlags({
featureFlags: {
TEST_BOOLEAN: true,
},
})
await withEnv(
{ TENANT_FEATURE_FLAGS: `${identity.tenantId}:!TEST_BOOLEAN` },
async () => {
await context.doInIdentityContext(identity, async () => {
const values = await flags.fetch()
expect(values.TEST_BOOLEAN).toBe(false)
})
}
)
})
})
}) })

@ -1 +1 @@
Subproject commit 7fe713e51afea77c8024579582a4e1a4ec1b55b3 Subproject commit 94747fd5bb67c218244bb60b9540f3a6f1c3f6f1