Skip to content

fix: add feature flags to tenants to selectively apply migrations #633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions migrations/multitenant/0015-migrations-feature-flags.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE tenants ADD COLUMN IF NOT EXISTS migrations_feature_flags text[] NULL;
2 changes: 2 additions & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
@@ -40,6 +40,7 @@
dbSuperUser: string
dbSearchPath: string
dbMigrationStrategy: MultitenantMigrationStrategy
dbMigrationFeatureFlagsEnabled: boolean
dbPostgresVersion?: string
databaseURL: string
databaseSSLRootCert?: string
@@ -299,6 +300,7 @@
),
dbSuperUser: getOptionalConfigFromEnv('DB_SUPER_USER') || 'postgres',
dbMigrationStrategy: getOptionalConfigFromEnv('DB_MIGRATIONS_STRATEGY') || 'on_request',
dbMigrationFeatureFlagsEnabled: getOptionalConfigFromEnv('DB_FEATURE_FLAGS_ENABLED') === 'true',

// Database - Connection
dbSearchPath: getOptionalConfigFromEnv('DATABASE_SEARCH_PATH', 'DB_SEARCH_PATH') || '',
@@ -453,7 +455,7 @@
try {
const parsed = JSON.parse(jwtJWKS)
config.jwtJWKS = parsed
} catch (e: any) {

Check warning on line 458 in src/config.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

'e' is defined but never used

Check warning on line 458 in src/config.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
throw new Error('Unable to parse JWT_JWKS value to JSON')
}
}
8 changes: 7 additions & 1 deletion src/http/routes/admin/tenants.ts
Original file line number Diff line number Diff line change
@@ -31,6 +31,7 @@ const patchSchema = {
serviceKey: { type: 'string' },
tracingMode: { type: 'string' },
disableEvents: { type: 'array', items: { type: 'string' }, nullable: true },
migrationsFeatureFlags: { type: 'array', items: { type: 'string' }, nullable: true },
features: {
type: 'object',
properties: {
@@ -90,6 +91,7 @@ interface tenantDBInterface {
feature_s3_protocol?: boolean
feature_image_transformation?: boolean
image_transformation_max_resolution?: number
migrations_feature_flags?: string[]
}

export default async function routes(fastify: FastifyInstance) {
@@ -163,6 +165,7 @@ export default async function routes(fastify: FastifyInstance) {
migrations_status,
tracing_mode,
disable_events,
migrations_feature_flags,
} = tenant

return {
@@ -192,6 +195,7 @@ export default async function routes(fastify: FastifyInstance) {
migrationStatus: migrations_status,
tracingMode: tracing_mode,
disableEvents: disable_events,
migrationsFeatureFlags: migrations_feature_flags,
}
}
})
@@ -259,6 +263,7 @@ export default async function routes(fastify: FastifyInstance) {
maxConnections,
tracingMode,
disableEvents,
migrationsFeatureFlags,
} = request.body
const { tenantId } = request.params

@@ -284,6 +289,7 @@ export default async function routes(fastify: FastifyInstance) {
: features?.imageTransformation?.maxResolution,
tracing_mode: tracingMode,
disable_events: disableEvents,
migrations_feature_flags: migrationsFeatureFlags,
})
.where('id', tenantId)

@@ -300,7 +306,7 @@ export default async function routes(fastify: FastifyInstance) {
if (e instanceof Error) {
request.executionError = e
}
progressiveMigrations.addTenant(tenantId)
progressiveMigrations.addTenant(tenantId, true)
}
}

97 changes: 61 additions & 36 deletions src/internal/database/migrations/progressive.ts
Original file line number Diff line number Diff line change
@@ -2,6 +2,10 @@ import { logger, logSchema } from '../../monitoring'
import { getTenantConfig, TenantMigrationStatus } from '../tenant'
import { RunMigrationsOnTenants } from '@storage/events'
import { areMigrationsUpToDate } from '@internal/database/migrations/migrate'
import { getConfig } from '../../../config'
import { DBMigration } from '@internal/database/migrations/types'

const { dbMigrationFeatureFlagsEnabled } = getConfig()

export class ProgressiveMigrations {
protected tenants: string[] = []
@@ -48,13 +52,31 @@ export class ProgressiveMigrations {
})
}

addTenant(tenant: string) {
async addTenant(tenant: string, forceMigrate?: boolean) {
const tenantIndex = this.tenants.indexOf(tenant)

if (tenantIndex !== -1) {
return
}

// check feature flags
if (dbMigrationFeatureFlagsEnabled && !forceMigrate) {
const { migrationFeatureFlags, migrationVersion } = await getTenantConfig(tenant)
if (!migrationFeatureFlags || !migrationVersion) {
return
}

// we only want to run migrations for tenants that have the feature flag enabled
// a feature flag can be any migration version that is greater than the current migration version
const migrationFeatureFlagsEnabled = migrationFeatureFlags.some(
(flag) => DBMigration[flag as keyof typeof DBMigration] > DBMigration[migrationVersion]
)

if (!migrationFeatureFlagsEnabled) {
return
}
}

this.tenants.push(tenant)

if (this.tenants.length < this.options.maxSize || this.emittingJobs) {
@@ -95,43 +117,46 @@ export class ProgressiveMigrations {

protected async createJobs(maxJobs: number) {
this.emittingJobs = true
const tenantsBatch = this.tenants.splice(0, maxJobs)
const jobs = await Promise.allSettled(
tenantsBatch.map(async (tenant) => {
const tenantConfig = await getTenantConfig(tenant)
const migrationsUpToDate = await areMigrationsUpToDate(tenant)

if (migrationsUpToDate || tenantConfig.syncMigrationsDone) {
return
}

const scheduleAt = new Date()
scheduleAt.setMinutes(scheduleAt.getMinutes() + 5)
const scheduleForLater =
tenantConfig.migrationStatus === TenantMigrationStatus.FAILED_STALE
? scheduleAt
: undefined

return new RunMigrationsOnTenants({
tenantId: tenant,
scheduleAt: scheduleForLater,
tenant: {
host: '',
ref: tenant,
},
try {
const tenantsBatch = this.tenants.splice(0, maxJobs)
const jobs = await Promise.allSettled(
tenantsBatch.map(async (tenant) => {
const tenantConfig = await getTenantConfig(tenant)
const migrationsUpToDate = await areMigrationsUpToDate(tenant)

if (migrationsUpToDate || tenantConfig.syncMigrationsDone) {
return
}

const scheduleAt = new Date()
scheduleAt.setMinutes(scheduleAt.getMinutes() + 5)
const scheduleForLater =
tenantConfig.migrationStatus === TenantMigrationStatus.FAILED_STALE
? scheduleAt
: undefined

return new RunMigrationsOnTenants({
tenantId: tenant,
scheduleAt: scheduleForLater,
tenant: {
host: '',
ref: tenant,
},
})
})
})
)
)

const validJobs = jobs
.map((job) => {
if (job.status === 'fulfilled' && job.value) {
return job.value
}
})
.filter((job) => job)
const validJobs = jobs
.map((job) => {
if (job.status === 'fulfilled' && job.value) {
return job.value
}
})
.filter((job) => job)

await RunMigrationsOnTenants.batchSend(validJobs as RunMigrationsOnTenants[])
this.emittingJobs = false
await RunMigrationsOnTenants.batchSend(validJobs as RunMigrationsOnTenants[])
} finally {
this.emittingJobs = false
}
}
}
3 changes: 3 additions & 0 deletions src/internal/database/tenant.ts
Original file line number Diff line number Diff line change
@@ -31,6 +31,7 @@ interface TenantConfig {
}
migrationVersion?: keyof typeof DBMigration
migrationStatus?: TenantMigrationStatus
migrationFeatureFlags?: string[]
syncMigrationsDone?: boolean
tracingMode?: string
disableEvents?: string[]
@@ -134,6 +135,7 @@ export async function getTenantConfig(tenantId: string): Promise<TenantConfig> {
migrations_status,
tracing_mode,
disable_events,
migrations_feature_flags,
} = tenant

const serviceKey = decrypt(service_key)
@@ -163,6 +165,7 @@ export async function getTenantConfig(tenantId: string): Promise<TenantConfig> {
migrationVersion: migrations_version,
migrationStatus: migrations_status,
migrationsRun: false,
migrationFeatureFlags: migrations_feature_flags,
tracingMode: tracing_mode,
disableEvents: disable_events,
}

Unchanged files with check annotations Beta

app.get('/metrics', async (req, reply) => {
if (registriesToMerge.length === 0) {
const globalRegistry = appInstance.metrics.client.register
const defaultRegistries = (appInstance.metrics as any).getCustomDefaultMetricsRegistries()

Check warning on line 21 in src/admin-app.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
const routeRegistries = (appInstance.metrics as any).getCustomRouteMetricsRegistries()

Check warning on line 22 in src/admin-app.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
registriesToMerge = Array.from(
new Set([globalRegistry, ...defaultRegistries, ...routeRegistries])
// Fastify errors
if ('statusCode' in error) {
const err = error as FastifyError
return reply.status((error as any).statusCode || 500).send({

Check warning on line 67 in src/http/error-handler.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
statusCode: `${err.statusCode}`,
error: err.name,
message: err.message,
fastify.decorateRequest('jwt', '')
fastify.decorateRequest('jwtPayload', undefined)
fastify.addHook('preHandler', async (request, reply) => {

Check warning on line 28 in src/http/plugins/jwt.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

'reply' is defined but never used
request.jwt = (request.headers.authorization || '').replace(BEARER, '')
if (!request.jwt && request.routeOptions.config.allowInvalidJwt) {
request.jwtPayload = payload
request.owner = payload.sub
request.isAuthenticated = true
} catch (err: any) {

Check warning on line 44 in src/http/plugins/jwt.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
request.jwtPayload = { role: 'anon' }
request.isAuthenticated = false
interface FastifyContextConfig {
operation?: { type: string }
resources?: (req: FastifyRequest<any>) => string[]

Check warning on line 21 in src/http/plugins/log-request.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
}
}
const resources = getFirstDefined<string[]>(
req.resources,
req.routeOptions.config.resources?.(req),
(req.raw as any).resources,

Check warning on line 63 in src/http/plugins/log-request.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
resourceFromParams ? [resourceFromParams] : ([] as string[])
)
const rId = req.id
const cIP = req.ip
const statusCode = options.statusCode
const error = (req.raw as any).executionError || req.executionError

Check warning on line 118 in src/http/plugins/log-request.ts

GitHub Actions / Test / OS ubuntu-20.04 / Node 20

Unexpected any. Specify a different type
const tenantId = req.tenantId
const buildLogMessage = `${tenantId} | ${rMeth} | ${statusCode} | ${cIP} | ${rId} | ${rUrl} | ${uAgent}`