From f45593176c3f467c1ebcc1562ae769c3600872d6 Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Tue, 16 Apr 2024 15:47:45 +0300 Subject: [PATCH] feat: start extracting project from session object (#6856) Previously, we were extracting the project from the token, but now we will retrieve it from the session, which contains the full list of projects. This change also resolves an issue we encountered when the token was a multi-project token, formatted as []:dev:token. Previously, it was unable to display the exact list of projects. Now, it will show the exact project names. --- .../__snapshots__/create-config.test.ts.snap | 1 + src/lib/db/client-applications-store.ts | 51 ++++++++--- src/lib/db/index.ts | 1 + src/lib/features/metrics/instance/models.ts | 1 + src/lib/features/metrics/instance/register.ts | 25 ++++-- src/lib/features/metrics/shared/schema.ts | 1 + src/lib/types/experimental.ts | 7 +- src/server-dev.ts | 1 + src/test/e2e/api/admin/metrics.e2e.test.ts | 84 ++++++++++++++++++- 9 files changed, 150 insertions(+), 22 deletions(-) diff --git a/src/lib/__snapshots__/create-config.test.ts.snap b/src/lib/__snapshots__/create-config.test.ts.snap index 273155a128..b803013285 100644 --- a/src/lib/__snapshots__/create-config.test.ts.snap +++ b/src/lib/__snapshots__/create-config.test.ts.snap @@ -132,6 +132,7 @@ exports[`should create default config 1`] = ` }, "migrationLock": true, "outdatedSdksBanner": false, + "parseProjectFromSession": false, "personalAccessTokensKillSwitch": false, "projectListFilterMyProjects": false, "projectOverviewRefactor": false, diff --git a/src/lib/db/client-applications-store.ts b/src/lib/db/client-applications-store.ts index b3502c8ed5..428edfc83e 100644 --- a/src/lib/db/client-applications-store.ts +++ b/src/lib/db/client-applications-store.ts @@ -10,6 +10,7 @@ import type { Logger, LogProvider } from '../logger'; import type { Db } from './db'; import type { IApplicationOverview } from '../features/metrics/instance/models'; import { applySearchFilters } from '../features/feature-search/search-utils'; +import type { IFlagResolver } from '../types'; const COLUMNS = [ 'app_name', @@ -110,14 +111,6 @@ const remapRow = (input) => { return temp; }; -const remapUsageRow = (input) => { - return { - app_name: input.appName, - project: input.project || '*', - environment: input.environment || '*', - }; -}; - export default class ClientApplicationsStore implements IClientApplicationsStore { @@ -125,24 +118,32 @@ export default class ClientApplicationsStore private logger: Logger; - constructor(db: Db, eventBus: EventEmitter, getLogger: LogProvider) { + private flagResolver: IFlagResolver; + + constructor( + db: Db, + eventBus: EventEmitter, + getLogger: LogProvider, + flagResolver: IFlagResolver, + ) { this.db = db; + this.flagResolver = flagResolver; this.logger = getLogger('client-applications-store.ts'); } async upsert(details: Partial): Promise { const row = remapRow(details); await this.db(TABLE).insert(row).onConflict('app_name').merge(); - const usageRow = remapUsageRow(details); + const usageRows = this.remapUsageRow(details); await this.db(TABLE_USAGE) - .insert(usageRow) + .insert(usageRows) .onConflict(['app_name', 'project', 'environment']) .merge(); } async bulkUpsert(apps: Partial[]): Promise { const rows = apps.map(remapRow); - const usageRows = apps.map(remapUsageRow); + const usageRows = apps.flatMap(this.remapUsageRow); await this.db(TABLE).insert(rows).onConflict('app_name').merge(); await this.db(TABLE_USAGE) .insert(usageRows) @@ -420,4 +421,30 @@ export default class ClientApplicationsStore }, }; } + + private remapUsageRow = (input) => { + if (this.flagResolver.isEnabled('parseProjectFromSession')) { + if (!input.projects || input.projects.length === 0) { + return [ + { + app_name: input.appName, + project: '*', + environment: input.environment || '*', + }, + ]; + } else { + return input.projects.map((project) => ({ + app_name: input.appName, + project: project, + environment: input.environment || '*', + })); + } + } else { + return { + app_name: input.appName, + project: input.project || '*', + environment: input.environment || '*', + }; + } + }; } diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index 7a97999e30..3fc41b3a99 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -65,6 +65,7 @@ export const createStores = ( db, eventBus, getLogger, + config.flagResolver, ), clientInstanceStore: new ClientInstanceStore(db, eventBus, getLogger), clientMetricsStoreV2: new ClientMetricsStoreV2( diff --git a/src/lib/features/metrics/instance/models.ts b/src/lib/features/metrics/instance/models.ts index 6d0efe33fc..99b598d358 100644 --- a/src/lib/features/metrics/instance/models.ts +++ b/src/lib/features/metrics/instance/models.ts @@ -28,6 +28,7 @@ export interface IApplication { instances?: IClientInstance[]; seenToggles?: Record; project?: string; + projects?: string[]; environment?: string; links?: Record; } diff --git a/src/lib/features/metrics/instance/register.ts b/src/lib/features/metrics/instance/register.ts index 00dda75d1f..d3150acc31 100644 --- a/src/lib/features/metrics/instance/register.ts +++ b/src/lib/features/metrics/instance/register.ts @@ -1,6 +1,6 @@ import type { Response } from 'express'; import Controller from '../../../routes/controller'; -import type { IUnleashServices } from '../../../types'; +import type { IFlagResolver, IUnleashServices } from '../../../types'; import type { IUnleashConfig } from '../../../types/option'; import type { Logger } from '../../../logger'; import type ClientInstanceService from './instance-service'; @@ -24,6 +24,8 @@ export default class RegisterController extends Controller { openApiService: OpenApiService; + flagResolver: IFlagResolver; + constructor( { clientInstanceService, @@ -35,6 +37,7 @@ export default class RegisterController extends Controller { this.logger = config.getLogger('/api/client/register'); this.clientInstanceService = clientInstanceService; this.openApiService = openApiService; + this.flagResolver = config.flagResolver; this.route({ method: 'post', @@ -62,7 +65,7 @@ export default class RegisterController extends Controller { }); } - private static resolveEnvironment( + private resolveEnvironment( user: IUser | IApiUser, data: Partial, ) { @@ -76,7 +79,14 @@ export default class RegisterController extends Controller { return 'default'; } - private static extractProjectFromRequest( + private resolveProject(user: IUser | IApiUser) { + if (user instanceof ApiUser) { + return user.projects; + } + return ['default']; + } + + private extractProjectFromRequest( req: IAuthRequest, ) { const token = req.get('Authorisation') || req.headers.authorization; @@ -91,8 +101,13 @@ export default class RegisterController extends Controller { res: Response, ): Promise { const { body: data, ip: clientIp, user } = req; - data.environment = RegisterController.resolveEnvironment(user, data); - data.project = RegisterController.extractProjectFromRequest(req); + data.environment = this.resolveEnvironment(user, data); + if (this.flagResolver.isEnabled('parseProjectFromSession')) { + data.projects = this.resolveProject(user); + } else { + data.project = this.extractProjectFromRequest(req); + } + await this.clientInstanceService.registerClient(data, clientIp); res.header('X-Unleash-Version', version).status(202).end(); } diff --git a/src/lib/features/metrics/shared/schema.ts b/src/lib/features/metrics/shared/schema.ts index 54a8098ff3..5896dee9ee 100644 --- a/src/lib/features/metrics/shared/schema.ts +++ b/src/lib/features/metrics/shared/schema.ts @@ -92,4 +92,5 @@ export const clientRegisterSchema = joi interval: joi.number().required(), environment: joi.string().optional(), project: joi.string().optional(), + projects: joi.array().optional().items(joi.string()), }); diff --git a/src/lib/types/experimental.ts b/src/lib/types/experimental.ts index 32e86fe040..5ee59943b6 100644 --- a/src/lib/types/experimental.ts +++ b/src/lib/types/experimental.ts @@ -58,7 +58,8 @@ export type IFlagKey = | 'bearerTokenMiddleware' | 'projectOverviewRefactorFeedback' | 'featureLifecycle' - | 'projectListFilterMyProjects'; + | 'projectListFilterMyProjects' + | 'parseProjectFromSession'; export type IFlags = Partial<{ [key in IFlagKey]: boolean | Variant }>; @@ -287,6 +288,10 @@ const flags: IFlags = { process.env.UNLEASH_EXPERIMENTAL_PROJECTS_LIST_MY_PROJECTS, false, ), + parseProjectFromSession: parseEnvVarBoolean( + process.env.UNLEASH_EXPERIMENTAL_PARSE_PROJECT_FROM_SESSION, + false, + ), }; export const defaultExperimentalOptions: IExperimentalOptions = { diff --git a/src/server-dev.ts b/src/server-dev.ts index 402fbf0d25..28ff7362ee 100644 --- a/src/server-dev.ts +++ b/src/server-dev.ts @@ -55,6 +55,7 @@ process.nextTick(async () => { projectOverviewRefactorFeedback: true, featureLifecycle: true, projectListFilterMyProjects: true, + parseProjectFromSession: true, }, }, authentication: { diff --git a/src/test/e2e/api/admin/metrics.e2e.test.ts b/src/test/e2e/api/admin/metrics.e2e.test.ts index 6704254df1..cb6722a885 100644 --- a/src/test/e2e/api/admin/metrics.e2e.test.ts +++ b/src/test/e2e/api/admin/metrics.e2e.test.ts @@ -4,13 +4,31 @@ import { setupAppWithCustomConfig, } from '../../helpers/test-helper'; import getLogger from '../../../fixtures/no-logger'; +import { ApiTokenType } from '../../../../lib/types/models/api-token'; let app: IUnleashTest; let db: ITestDb; beforeAll(async () => { - db = await dbInit('metrics_serial', getLogger, {}); - app = await setupAppWithCustomConfig(db.stores, {}, db.rawDatabase); + db = await dbInit('metrics_serial', getLogger, { + experimental: { + flags: { + parseProjectFromSession: true, + }, + }, + }); + app = await setupAppWithCustomConfig( + db.stores, + { + experimental: { + flags: { + strictSchemaValidation: true, + parseProjectFromSession: true, + }, + }, + }, + db.rawDatabase, + ); }); beforeEach(async () => { @@ -52,7 +70,7 @@ beforeEach(async () => { appName: 'usage-app', strategies: ['default'], description: 'Some desc', - project: 'default', + projects: ['default'], environment: 'dev', }); }); @@ -123,6 +141,64 @@ test('should get list of application usage', async () => { ); expect(application).toMatchObject({ appName: 'usage-app', - usage: [{ project: 'default', environments: ['dev'] }], + usage: [ + { + project: 'default', + environments: ['dev'], + }, + ], + }); +}); + +test('should save multiple projects from token', async () => { + await db.reset(); + await db.stores.projectStore.create({ + id: 'mainProject', + name: 'mainProject', + }); + + const multiProjectToken = + await app.services.apiTokenService.createApiTokenWithProjects({ + type: ApiTokenType.CLIENT, + projects: ['default', 'mainProject'], + environment: 'default', + tokenName: 'tester', + }); + + await app.request + .post('/api/client/register') + .set('Authorization', multiProjectToken.secret) + .send({ + appName: 'multi-project-app', + instanceId: 'instance-1', + strategies: ['default'], + started: Date.now(), + interval: 10, + }); + + await app.services.clientInstanceService.bulkAdd(); + + const { body } = await app.request + .get('/api/admin/metrics/applications') + .expect('Content-Type', /json/) + .expect(200); + + expect(body).toMatchObject({ + applications: [ + { + appName: 'multi-project-app', + usage: [ + { + environments: ['default'], + project: 'default', + }, + { + environments: ['default'], + project: 'mainProject', + }, + ], + }, + ], + total: 1, }); });