From 38a45dfbe782953c423a2f5d85316aefdc69b921 Mon Sep 17 00:00:00 2001 From: Ivn Nv Date: Fri, 30 Jan 2026 13:28:40 -0500 Subject: [PATCH 1/3] Add PATCH /update/ endpoint for partial app definition updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing POST /update/ replaces all fields — omitted fields are reset to defaults (e.g. envVars becomes [], instanceCount becomes 0). This makes simple operations like scaling dangerous: sending only {appName, instanceCount} wipes all environment variables. The new PATCH /update/ endpoint fetches the existing app definition and merges only the explicitly provided fields. Omitted fields retain their current values. Example — scale without touching env vars: PATCH /api/v2/user/apps/appDefinitions/update/ {"appName": "my-app", "instanceCount": 1} The POST endpoint is unchanged — full backward compatibility. --- .../appdefinition/AppDefinitionHandler.ts | 115 +++++++++ .../apps/appdefinition/AppDefinitionRouter.ts | 27 ++ tests/PatchAppDefinition.test.ts | 241 ++++++++++++++++++ 3 files changed, 383 insertions(+) create mode 100644 tests/PatchAppDefinition.test.ts diff --git a/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts b/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts index 2727577..90209a0 100644 --- a/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts +++ b/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts @@ -163,6 +163,121 @@ export interface UpdateAppDefinitionParams { appDeployTokenConfig?: AppDeployTokenConfig } +/** + * Partially update an app definition by merging provided fields with existing values. + * Only fields explicitly included in the request body are updated; + * omitted fields retain their current values. + * + * This is safer than the full update (POST /update/) for operations like + * scaling instance count, where you don't want to accidentally reset + * env vars or other settings. + */ +export async function patchAppDefinition( + appName: string, + patch: Record, + dataStore: DataStore, + serviceManager: ServiceManager +): Promise { + if (!appName) { + throw ApiStatusCodes.createError( + ApiStatusCodes.ILLEGAL_PARAMETER, + 'appName is required' + ) + } + + // Fetch existing app definition to use as base + const existingApp = await dataStore + .getAppsDataStore() + .getAppDefinition(appName) + + // Merge: use patch value if explicitly provided, otherwise keep existing + const merged: UpdateAppDefinitionParams = { + appName, + projectId: + patch.projectId !== undefined + ? `${patch.projectId}` + : existingApp.projectId, + description: + patch.description !== undefined + ? `${patch.description}` + : existingApp.description, + instanceCount: + patch.instanceCount !== undefined + ? patch.instanceCount as number | string + : existingApp.instanceCount, + captainDefinitionRelativeFilePath: + patch.captainDefinitionRelativeFilePath !== undefined + ? `${patch.captainDefinitionRelativeFilePath}` + : existingApp.captainDefinitionRelativeFilePath, + envVars: + patch.envVars !== undefined + ? (patch.envVars as IAppEnvVar[]) + : existingApp.envVars, + volumes: + patch.volumes !== undefined + ? (patch.volumes as IAppVolume[]) + : existingApp.volumes, + tags: + patch.tags !== undefined + ? (patch.tags as IAppTag[]) + : existingApp.tags, + nodeId: + patch.nodeId !== undefined + ? `${patch.nodeId}` + : existingApp.nodeId, + notExposeAsWebApp: + patch.notExposeAsWebApp !== undefined + ? !!patch.notExposeAsWebApp + : existingApp.notExposeAsWebApp, + containerHttpPort: + patch.containerHttpPort !== undefined + ? (patch.containerHttpPort as number | string) + : existingApp.containerHttpPort, + httpAuth: + patch.httpAuth !== undefined + ? patch.httpAuth + : (existingApp as any).httpAuth, + forceSsl: + patch.forceSsl !== undefined + ? !!patch.forceSsl + : existingApp.forceSsl, + ports: + patch.ports !== undefined + ? (patch.ports as IAppPort[]) + : existingApp.ports, + repoInfo: + patch.appPushWebhook !== undefined + ? (patch.appPushWebhook as any)?.repoInfo + : existingApp.appPushWebhook?.repoInfo, + customNginxConfig: + patch.customNginxConfig !== undefined + ? `${patch.customNginxConfig}` + : existingApp.customNginxConfig, + redirectDomain: + patch.redirectDomain !== undefined + ? `${patch.redirectDomain}` + : existingApp.redirectDomain, + preDeployFunction: + patch.preDeployFunction !== undefined + ? `${patch.preDeployFunction}` + : existingApp.preDeployFunction, + serviceUpdateOverride: + patch.serviceUpdateOverride !== undefined + ? `${patch.serviceUpdateOverride}` + : existingApp.serviceUpdateOverride, + websocketSupport: + patch.websocketSupport !== undefined + ? !!patch.websocketSupport + : existingApp.websocketSupport, + appDeployTokenConfig: + patch.appDeployTokenConfig !== undefined + ? (patch.appDeployTokenConfig as AppDeployTokenConfig) + : existingApp.appDeployTokenConfig, + } + + return updateAppDefinition(merged, serviceManager) +} + export async function updateAppDefinition( params: UpdateAppDefinitionParams, serviceManager: ServiceManager diff --git a/src/routes/user/apps/appdefinition/AppDefinitionRouter.ts b/src/routes/user/apps/appdefinition/AppDefinitionRouter.ts index c99a6ca..83bc2fb 100644 --- a/src/routes/user/apps/appdefinition/AppDefinitionRouter.ts +++ b/src/routes/user/apps/appdefinition/AppDefinitionRouter.ts @@ -3,6 +3,7 @@ import ApiStatusCodes from '../../../../api/ApiStatusCodes' import BaseApi from '../../../../api/BaseApi' import { getAllAppDefinitions, + patchAppDefinition, registerAppDefinition, updateAppDefinition, } from '../../../../handlers/users/apps/appdefinition/AppDefinitionHandler' @@ -314,4 +315,30 @@ router.post('/update/', function (req, res, next) { .catch(ApiStatusCodes.createCatcher(res)) }) +// Partial update - only provided fields are changed, omitted fields keep existing values +router.patch('/update/', function (req, res, next) { + const dataStore = + InjectionExtractor.extractUserFromInjected(res).user.dataStore + const serviceManager = + InjectionExtractor.extractUserFromInjected(res).user.serviceManager + + const appName = req.body.appName + + if (!appName) { + res.send( + new BaseApi( + ApiStatusCodes.ILLEGAL_PARAMETER, + 'appName is required' + ) + ) + return + } + + return patchAppDefinition(appName, req.body, dataStore, serviceManager) + .then(function (result) { + res.send(new BaseApi(ApiStatusCodes.STATUS_OK, result.message)) + }) + .catch(ApiStatusCodes.createCatcher(res)) +}) + export default router diff --git a/tests/PatchAppDefinition.test.ts b/tests/PatchAppDefinition.test.ts new file mode 100644 index 0000000..8668546 --- /dev/null +++ b/tests/PatchAppDefinition.test.ts @@ -0,0 +1,241 @@ +/** + * TEST FILE: PatchAppDefinition.test.ts + * + * Tests the `patchAppDefinition` handler which partially updates an app + * definition by merging only the provided fields with existing values. + * + * This ensures that operations like scaling (changing instanceCount) do not + * accidentally wipe env vars, volumes, ports, or other app configuration. + */ + +import { patchAppDefinition } from '../src/handlers/users/apps/appdefinition/AppDefinitionHandler' +import { IAppDef } from '../src/models/AppDefinition' + +const mockExistingApp: IAppDef = { + description: 'My test app', + deployedVersion: 5, + notExposeAsWebApp: false, + hasPersistentData: false, + hasDefaultSubDomainSsl: true, + containerHttpPort: 3000, + captainDefinitionRelativeFilePath: './captain-definition', + forceSsl: true, + websocketSupport: true, + nodeId: 'node-abc', + instanceCount: 2, + preDeployFunction: 'console.log("pre")', + serviceUpdateOverride: '', + customNginxConfig: '', + redirectDomain: '', + networks: ['captain-overlay-network'], + customDomain: [], + tags: [{ tagName: 'production' }], + ports: [{ containerPort: 3000, hostPort: 3000, protocol: 'tcp' }], + volumes: [ + { + containerPath: '/data', + volumeName: 'app-data', + }, + ], + envVars: [ + { key: 'API_HOST', value: 'https://api.example.com' }, + { key: 'API_KEY', value: 'secret-key-123' }, + { key: 'DB_URL', value: 'postgres://localhost/mydb' }, + ], + versions: [], + appDeployTokenConfig: { enabled: true, appDeployToken: 'tok-123' }, + appPushWebhook: { + tokenVersion: 'v1', + repoInfo: { + repo: 'my-repo', + branch: 'main', + user: 'my-user', + password: 'my-pass', + }, + pushWebhookToken: 'webhook-tok', + }, + httpAuth: { user: 'admin', password: 'pass123' }, +} + +// Track what updateAppDefinition receives via the serviceManager mock +let capturedUpdateArgs: any[] = [] + +const mockDataStore = { + getAppsDataStore: () => ({ + getAppDefinition: jest.fn().mockResolvedValue(mockExistingApp), + }), +} as any + +const mockServiceManager = { + updateAppDefinition: jest.fn().mockImplementation((...args: any[]) => { + capturedUpdateArgs = args + return Promise.resolve() + }), + ensureNotBuilding: jest.fn().mockResolvedValue(undefined), + dataStore: mockDataStore, +} as any + +describe('patchAppDefinition', () => { + beforeEach(() => { + jest.clearAllMocks() + capturedUpdateArgs = [] + }) + + it('should preserve all existing fields when only instanceCount is provided', async () => { + await patchAppDefinition( + 'my-app', + { appName: 'my-app', instanceCount: 1 }, + mockDataStore, + mockServiceManager + ) + + expect(mockServiceManager.updateAppDefinition).toHaveBeenCalledTimes(1) + + // serviceManager.updateAppDefinition positional args: + // 0=appName, 1=projectId, 2=description, 3=instanceCount, 4=captainDefPath, + // 5=envVars, 6=volumes, 7=tags, 8=nodeId, 9=notExposeAsWebApp, + // 10=containerHttpPort, 11=httpAuth, 12=forceSsl, 13=ports, + // 14=repoInfo, 15=customNginxConfig, 16=redirectDomain, + // 17=preDeployFunction, 18=serviceUpdateOverride, 19=websocketSupport, + // 20=appDeployTokenConfig + const args = capturedUpdateArgs + expect(args[0]).toBe('my-app') // appName + expect(args[3]).toBe(1) // instanceCount — patched + + // envVars (arg 5) should be preserved + expect(args[5]).toEqual(mockExistingApp.envVars) + expect(args[5]).toHaveLength(3) + + // volumes (arg 6) preserved + expect(args[6]).toEqual(mockExistingApp.volumes) + + // tags (arg 7) preserved + expect(args[7]).toEqual(mockExistingApp.tags) + + // forceSsl (arg 12) preserved + expect(args[12]).toBe(true) + + // websocketSupport (arg 19) preserved + expect(args[19]).toBe(true) + }) + + it('should preserve env vars when scaling to zero', async () => { + await patchAppDefinition( + 'my-app', + { appName: 'my-app', instanceCount: 0 }, + mockDataStore, + mockServiceManager + ) + + const args = capturedUpdateArgs + expect(args[3]).toBe(0) // instanceCount + expect(args[5]).toEqual(mockExistingApp.envVars) // envVars preserved + expect(args[5]).toHaveLength(3) + }) + + it('should update only the provided fields', async () => { + await patchAppDefinition( + 'my-app', + { + appName: 'my-app', + instanceCount: 3, + description: 'Updated description', + forceSsl: false, + }, + mockDataStore, + mockServiceManager + ) + + const args = capturedUpdateArgs + expect(args[3]).toBe(3) // instanceCount — patched + expect(args[2]).toBe('Updated description') // description — patched + expect(args[12]).toBe(false) // forceSsl — patched + + // Non-provided fields preserved + expect(args[5]).toEqual(mockExistingApp.envVars) + expect(args[6]).toEqual(mockExistingApp.volumes) + expect(args[19]).toBe(true) // websocketSupport preserved + }) + + it('should allow updating env vars explicitly', async () => { + const newEnvVars = [{ key: 'NEW_VAR', value: 'new-value' }] + + await patchAppDefinition( + 'my-app', + { appName: 'my-app', envVars: newEnvVars }, + mockDataStore, + mockServiceManager + ) + + const args = capturedUpdateArgs + expect(args[5]).toEqual(newEnvVars) // envVars — patched + expect(args[5]).toHaveLength(1) + expect(args[3]).toBe(2) // instanceCount preserved from existing + }) + + it('should allow setting envVars to empty array explicitly', async () => { + await patchAppDefinition( + 'my-app', + { appName: 'my-app', envVars: [] }, + mockDataStore, + mockServiceManager + ) + + const args = capturedUpdateArgs + expect(args[5]).toEqual([]) // envVars — explicitly emptied + expect(args[3]).toBe(2) // instanceCount preserved + expect(args[6]).toEqual(mockExistingApp.volumes) // volumes preserved + }) + + it('should throw error when appName is missing', async () => { + await expect( + patchAppDefinition( + '', + { instanceCount: 1 }, + mockDataStore, + mockServiceManager + ) + ).rejects.toThrow() + }) + + it('should preserve httpAuth from existing app', async () => { + await patchAppDefinition( + 'my-app', + { appName: 'my-app', instanceCount: 1 }, + mockDataStore, + mockServiceManager + ) + + const args = capturedUpdateArgs + // httpAuth is arg 11 + expect(args[11]).toEqual(mockExistingApp.httpAuth) + }) + + it('should handle multiple fields updated at once', async () => { + await patchAppDefinition( + 'my-app', + { + appName: 'my-app', + instanceCount: 5, + containerHttpPort: 8080, + websocketSupport: false, + notExposeAsWebApp: true, + redirectDomain: 'example.com', + }, + mockDataStore, + mockServiceManager + ) + + const args = capturedUpdateArgs + expect(args[3]).toBe(5) // instanceCount + expect(args[10]).toBe(8080) // containerHttpPort + expect(args[19]).toBe(false) // websocketSupport + expect(args[9]).toBe(true) // notExposeAsWebApp + expect(args[16]).toBe('example.com') // redirectDomain + + // Non-provided preserved + expect(args[5]).toEqual(mockExistingApp.envVars) + expect(args[12]).toBe(true) // forceSsl + expect(args[2]).toBe('My test app') // description + }) +}) From 60a1fd880bd429f39e5f0cd723c683ac30a0e825 Mon Sep 17 00:00:00 2001 From: Ivn Nv Date: Fri, 30 Jan 2026 21:33:47 -0500 Subject: [PATCH 2/3] fix null case / format --- .../apps/appdefinition/AppDefinitionHandler.ts | 18 +++++++++--------- .../apps/appdefinition/AppDefinitionRouter.ts | 5 +---- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts b/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts index 90209a0..42f2473 100644 --- a/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts +++ b/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts @@ -195,19 +195,19 @@ export async function patchAppDefinition( appName, projectId: patch.projectId !== undefined - ? `${patch.projectId}` + ? `${patch.projectId ?? ''}` : existingApp.projectId, description: patch.description !== undefined - ? `${patch.description}` + ? `${patch.description ?? ''}` : existingApp.description, instanceCount: patch.instanceCount !== undefined - ? patch.instanceCount as number | string + ? (patch.instanceCount as number | string) : existingApp.instanceCount, captainDefinitionRelativeFilePath: patch.captainDefinitionRelativeFilePath !== undefined - ? `${patch.captainDefinitionRelativeFilePath}` + ? `${patch.captainDefinitionRelativeFilePath ?? ''}` : existingApp.captainDefinitionRelativeFilePath, envVars: patch.envVars !== undefined @@ -223,7 +223,7 @@ export async function patchAppDefinition( : existingApp.tags, nodeId: patch.nodeId !== undefined - ? `${patch.nodeId}` + ? `${patch.nodeId ?? ''}` : existingApp.nodeId, notExposeAsWebApp: patch.notExposeAsWebApp !== undefined @@ -251,19 +251,19 @@ export async function patchAppDefinition( : existingApp.appPushWebhook?.repoInfo, customNginxConfig: patch.customNginxConfig !== undefined - ? `${patch.customNginxConfig}` + ? `${patch.customNginxConfig ?? ''}` : existingApp.customNginxConfig, redirectDomain: patch.redirectDomain !== undefined - ? `${patch.redirectDomain}` + ? `${patch.redirectDomain ?? ''}` : existingApp.redirectDomain, preDeployFunction: patch.preDeployFunction !== undefined - ? `${patch.preDeployFunction}` + ? `${patch.preDeployFunction ?? ''}` : existingApp.preDeployFunction, serviceUpdateOverride: patch.serviceUpdateOverride !== undefined - ? `${patch.serviceUpdateOverride}` + ? `${patch.serviceUpdateOverride ?? ''}` : existingApp.serviceUpdateOverride, websocketSupport: patch.websocketSupport !== undefined diff --git a/src/routes/user/apps/appdefinition/AppDefinitionRouter.ts b/src/routes/user/apps/appdefinition/AppDefinitionRouter.ts index 83bc2fb..87c0d82 100644 --- a/src/routes/user/apps/appdefinition/AppDefinitionRouter.ts +++ b/src/routes/user/apps/appdefinition/AppDefinitionRouter.ts @@ -326,10 +326,7 @@ router.patch('/update/', function (req, res, next) { if (!appName) { res.send( - new BaseApi( - ApiStatusCodes.ILLEGAL_PARAMETER, - 'appName is required' - ) + new BaseApi(ApiStatusCodes.ILLEGAL_PARAMETER, 'appName is required') ) return } From 78d57c93ea6c411aaf7ab8a1f6bf27e3543b19d5 Mon Sep 17 00:00:00 2001 From: Ivn Nv Date: Fri, 30 Jan 2026 21:49:18 -0500 Subject: [PATCH 3/3] more maintainable implementation --- .../appdefinition/AppDefinitionHandler.ts | 115 ++++++------------ 1 file changed, 34 insertions(+), 81 deletions(-) diff --git a/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts b/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts index 42f2473..9f4edc7 100644 --- a/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts +++ b/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts @@ -190,91 +190,44 @@ export async function patchAppDefinition( .getAppsDataStore() .getAppDefinition(appName) - // Merge: use patch value if explicitly provided, otherwise keep existing - const merged: UpdateAppDefinitionParams = { + // Build base from existing app definition + const base: UpdateAppDefinitionParams = { appName, - projectId: - patch.projectId !== undefined - ? `${patch.projectId ?? ''}` - : existingApp.projectId, - description: - patch.description !== undefined - ? `${patch.description ?? ''}` - : existingApp.description, - instanceCount: - patch.instanceCount !== undefined - ? (patch.instanceCount as number | string) - : existingApp.instanceCount, + projectId: existingApp.projectId, + description: existingApp.description, + instanceCount: existingApp.instanceCount, captainDefinitionRelativeFilePath: - patch.captainDefinitionRelativeFilePath !== undefined - ? `${patch.captainDefinitionRelativeFilePath ?? ''}` - : existingApp.captainDefinitionRelativeFilePath, - envVars: - patch.envVars !== undefined - ? (patch.envVars as IAppEnvVar[]) - : existingApp.envVars, - volumes: - patch.volumes !== undefined - ? (patch.volumes as IAppVolume[]) - : existingApp.volumes, - tags: - patch.tags !== undefined - ? (patch.tags as IAppTag[]) - : existingApp.tags, - nodeId: - patch.nodeId !== undefined - ? `${patch.nodeId ?? ''}` - : existingApp.nodeId, - notExposeAsWebApp: - patch.notExposeAsWebApp !== undefined - ? !!patch.notExposeAsWebApp - : existingApp.notExposeAsWebApp, - containerHttpPort: - patch.containerHttpPort !== undefined - ? (patch.containerHttpPort as number | string) - : existingApp.containerHttpPort, - httpAuth: - patch.httpAuth !== undefined - ? patch.httpAuth - : (existingApp as any).httpAuth, - forceSsl: - patch.forceSsl !== undefined - ? !!patch.forceSsl - : existingApp.forceSsl, - ports: - patch.ports !== undefined - ? (patch.ports as IAppPort[]) - : existingApp.ports, - repoInfo: - patch.appPushWebhook !== undefined - ? (patch.appPushWebhook as any)?.repoInfo - : existingApp.appPushWebhook?.repoInfo, - customNginxConfig: - patch.customNginxConfig !== undefined - ? `${patch.customNginxConfig ?? ''}` - : existingApp.customNginxConfig, - redirectDomain: - patch.redirectDomain !== undefined - ? `${patch.redirectDomain ?? ''}` - : existingApp.redirectDomain, - preDeployFunction: - patch.preDeployFunction !== undefined - ? `${patch.preDeployFunction ?? ''}` - : existingApp.preDeployFunction, - serviceUpdateOverride: - patch.serviceUpdateOverride !== undefined - ? `${patch.serviceUpdateOverride ?? ''}` - : existingApp.serviceUpdateOverride, - websocketSupport: - patch.websocketSupport !== undefined - ? !!patch.websocketSupport - : existingApp.websocketSupport, - appDeployTokenConfig: - patch.appDeployTokenConfig !== undefined - ? (patch.appDeployTokenConfig as AppDeployTokenConfig) - : existingApp.appDeployTokenConfig, + existingApp.captainDefinitionRelativeFilePath, + envVars: existingApp.envVars, + volumes: existingApp.volumes, + tags: existingApp.tags, + nodeId: existingApp.nodeId, + notExposeAsWebApp: existingApp.notExposeAsWebApp, + containerHttpPort: existingApp.containerHttpPort, + httpAuth: (existingApp as any).httpAuth, + forceSsl: existingApp.forceSsl, + ports: existingApp.ports, + repoInfo: existingApp.appPushWebhook?.repoInfo, + customNginxConfig: existingApp.customNginxConfig, + redirectDomain: existingApp.redirectDomain, + preDeployFunction: existingApp.preDeployFunction, + serviceUpdateOverride: existingApp.serviceUpdateOverride, + websocketSupport: existingApp.websocketSupport, + appDeployTokenConfig: existingApp.appDeployTokenConfig, } + // Extract only defined patch fields, mapping to UpdateAppDefinitionParams keys + const overrides: Partial = {} + for (const key of Object.keys(patch)) { + if (key === 'appPushWebhook') { + overrides.repoInfo = (patch.appPushWebhook as any)?.repoInfo + } else if (key in base) { + ;(overrides as any)[key] = patch[key] + } + } + + const merged: UpdateAppDefinitionParams = { ...base, ...overrides } + return updateAppDefinition(merged, serviceManager) }