diff --git a/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts b/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts index 2727577..9f4edc7 100644 --- a/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts +++ b/src/handlers/users/apps/appdefinition/AppDefinitionHandler.ts @@ -163,6 +163,74 @@ 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) + + // Build base from existing app definition + const base: UpdateAppDefinitionParams = { + appName, + projectId: existingApp.projectId, + description: existingApp.description, + instanceCount: existingApp.instanceCount, + captainDefinitionRelativeFilePath: + 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) +} + 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..87c0d82 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,27 @@ 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 + }) +})