From c0c5e8c79ffe6e3051259da562e305661a35dfb2 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Fri, 22 May 2026 11:41:49 -0700 Subject: [PATCH] fix: include repo-less dependencies in scoped deploys --- src/server/lib/__tests__/deployScope.test.ts | 43 +++++ src/server/lib/deployScope.ts | 80 +++++++++ src/server/services/__tests__/build.test.ts | 63 +++++++ src/server/services/__tests__/deploy.test.ts | 166 +++++++++++++++++++ src/server/services/build.ts | 102 +++++++----- src/server/services/deploy.ts | 15 +- 6 files changed, 423 insertions(+), 46 deletions(-) create mode 100644 src/server/lib/__tests__/deployScope.test.ts create mode 100644 src/server/lib/deployScope.ts diff --git a/src/server/lib/__tests__/deployScope.test.ts b/src/server/lib/__tests__/deployScope.test.ts new file mode 100644 index 0000000..4c15d2f --- /dev/null +++ b/src/server/lib/__tests__/deployScope.test.ts @@ -0,0 +1,43 @@ +import { normalizeRepositoryId, scopedDeployableNames } from '../deployScope'; + +describe('deploy scope helpers', () => { + test('preserves null repository IDs instead of coercing them to zero', () => { + expect(normalizeRepositoryId(null)).toBeNull(); + expect(normalizeRepositoryId(undefined)).toBeNull(); + expect(normalizeRepositoryId('')).toBeNull(); + expect(normalizeRepositoryId(123)).toBe(123); + expect(normalizeRepositoryId('123')).toBe(123); + }); + + test('includes YAML-only Docker dependencies for a scoped repository deploy', () => { + const scopedNames = scopedDeployableNames( + [ + { name: 'sponsored-benefits', repositoryId: 1154960313 }, + { + name: 'sbs-localstack', + repositoryId: null, + dependsOnDeployableName: 'sponsored-benefits', + }, + { name: 'unrelated-localstack', repositoryId: null, dependsOnDeployableName: 'other-service' }, + { name: 'external-partners', repositoryId: 932333620 }, + ], + 1154960313 + ); + + expect(Array.from(scopedNames).sort()).toEqual(['sbs-localstack', 'sponsored-benefits']); + }); + + test('includes explicit deployment dependencies for a scoped repository deploy', () => { + const scopedNames = scopedDeployableNames( + [ + { name: 'api', repositoryId: 1, deploymentDependsOn: ['db', 'localstack'] }, + { name: 'db', repositoryId: null }, + { name: 'localstack', repositoryId: null }, + { name: 'worker', repositoryId: 2 }, + ], + 1 + ); + + expect(Array.from(scopedNames).sort()).toEqual(['api', 'db', 'localstack']); + }); +}); diff --git a/src/server/lib/deployScope.ts b/src/server/lib/deployScope.ts new file mode 100644 index 0000000..b303624 --- /dev/null +++ b/src/server/lib/deployScope.ts @@ -0,0 +1,80 @@ +export interface DeployScopeNode { + name?: string | null; + repositoryId?: number | string | null; + dependsOnDeployableName?: string | null; + deploymentDependsOn?: string[] | string | null; +} + +export function normalizeRepositoryId(repositoryId: number | string | null | undefined): number | null { + if (repositoryId == null || repositoryId === '') { + return null; + } + + const parsed = Number(repositoryId); + return Number.isFinite(parsed) ? parsed : null; +} + +function deploymentDependencyNames(node: DeployScopeNode | undefined): string[] { + const dependencies = node?.deploymentDependsOn; + if (!dependencies) { + return []; + } + + if (Array.isArray(dependencies)) { + return dependencies; + } + + try { + const parsed = JSON.parse(dependencies); + return Array.isArray(parsed) ? parsed : []; + } catch { + return []; + } +} + +export function scopedDeployableNames(deployables: DeployScopeNode[], githubRepositoryId?: number | null): Set { + if (!githubRepositoryId) { + return new Set(deployables.map((deployable) => deployable.name).filter(Boolean) as string[]); + } + + const targetRepositoryId = normalizeRepositoryId(githubRepositoryId); + const byName = new Map(); + deployables.forEach((deployable) => { + if (deployable.name) { + byName.set(deployable.name, deployable); + } + }); + + const included = new Set( + deployables + .filter((deployable) => normalizeRepositoryId(deployable.repositoryId) === targetRepositoryId) + .map((deployable) => deployable.name) + .filter(Boolean) as string[] + ); + + let changed = true; + while (changed) { + changed = false; + + for (const deployable of deployables) { + const name = deployable.name; + if (!name || included.has(name)) { + continue; + } + + const requiredByIncluded = deployable.dependsOnDeployableName + ? included.has(deployable.dependsOnDeployableName) + : false; + const explicitlyRequiredByIncluded = Array.from(included).some((includedName) => + deploymentDependencyNames(byName.get(includedName)).includes(name) + ); + + if (requiredByIncluded || explicitlyRequiredByIncluded) { + included.add(name); + changed = true; + } + } + } + + return included; +} diff --git a/src/server/services/__tests__/build.test.ts b/src/server/services/__tests__/build.test.ts index 758ca7a..67e1d1b 100644 --- a/src/server/services/__tests__/build.test.ts +++ b/src/server/services/__tests__/build.test.ts @@ -1094,3 +1094,66 @@ describe('BuildService queue fingerprinting', () => { ); }); }); + +describe('BuildService running image updates', () => { + test('updates repo-less dependency deploys during a scoped repository deploy', async () => { + const appPatch = jest.fn().mockResolvedValue(undefined); + const dependencyPatch = jest.fn().mockResolvedValue(undefined); + const unrelatedPatch = jest.fn().mockResolvedValue(undefined); + const buildService = new BuildService( + { models: {}, services: {} } as any, + {} as any, + {} as any, + { + registerQueue: jest.fn(() => ({ + add: jest.fn(), + process: jest.fn(), + on: jest.fn(), + })), + } as any + ); + const build = { + deploys: [ + { + uuid: 'api-deploy', + githubRepositoryId: 100, + dockerImage: 'api:latest', + deployable: { + name: 'api', + repositoryId: 100, + }, + $query: jest.fn(() => ({ patch: appPatch })), + }, + { + uuid: 'api-localstack-deploy', + githubRepositoryId: null, + dockerImage: 'localstack:latest', + deployable: { + name: 'api-localstack', + repositoryId: null, + dependsOnDeployableName: 'api', + }, + $query: jest.fn(() => ({ patch: dependencyPatch })), + }, + { + uuid: 'worker-deploy', + githubRepositoryId: 200, + dockerImage: 'worker:latest', + deployable: { + name: 'worker', + repositoryId: 200, + }, + $query: jest.fn(() => ({ patch: unrelatedPatch })), + }, + ], + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }; + + await (buildService as any).updateDeploysImageDetails(build, 100); + + expect(build.$fetchGraph).toHaveBeenCalledWith('deploys.[service, deployable]'); + expect(appPatch).toHaveBeenCalledWith({ isRunningLatest: true, runningImage: 'api:latest' }); + expect(dependencyPatch).toHaveBeenCalledWith({ isRunningLatest: true, runningImage: 'localstack:latest' }); + expect(unrelatedPatch).not.toHaveBeenCalled(); + }); +}); diff --git a/src/server/services/__tests__/deploy.test.ts b/src/server/services/__tests__/deploy.test.ts index 26c4864..fedb2e5 100644 --- a/src/server/services/__tests__/deploy.test.ts +++ b/src/server/services/__tests__/deploy.test.ts @@ -777,3 +777,169 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { }); }); }); + +describe('DeployService - scoped deploy initialization', () => { + test('creates repo-less Docker deploys with a null githubRepositoryId', async () => { + const patch = jest.fn().mockResolvedValue(undefined); + const createdDeploy = { + id: 99, + $query: jest.fn(() => ({ patch })), + $setRelated: jest.fn(), + }; + const deployQuery: any = { + where: jest.fn(() => deployQuery), + withGraphFetched: jest.fn().mockResolvedValue([]), + then: (resolve: (value: any[]) => void, reject: (reason: unknown) => void) => + Promise.resolve([]).then(resolve, reject), + }; + const build = { + id: 1449, + uuid: 'good-dev-0', + enableFullYaml: true, + deployables: [ + { + id: 215190, + name: 'sponsored-benefits', + serviceId: null, + repositoryId: 1154960313, + branchName: 'main', + defaultTag: 'main', + type: DeployTypes.HELM, + active: true, + }, + { + id: 215191, + name: 'sbs-localstack', + serviceId: null, + repositoryId: null, + branchName: 'main', + defaultTag: '4.10', + type: DeployTypes.DOCKER, + active: true, + dependsOnDeployableName: 'sponsored-benefits', + }, + ], + deploys: [{ deployableId: 215190 }, { deployableId: 215191 }], + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }; + const mockDb = { + models: { + Deploy: { + query: jest.fn(() => deployQuery), + findOne: jest.fn().mockResolvedValue(null), + create: jest.fn().mockResolvedValue(createdDeploy), + }, + }, + services: { + Deploy: { + hostForDeployableDeploy: jest.fn(() => 'sbs-localstack-good-dev-0.lifecycle.test'), + }, + }, + }; + + const deployService = new DeployService( + mockDb as any, + {} as any, + {} as any, + { + registerQueue: jest.fn().mockReturnValue({ add: jest.fn(), process: jest.fn(), on: jest.fn() }), + } as any + ); + + await deployService.findOrCreateDeploys({} as any, build as any, 1154960313); + + expect(mockDb.models.Deploy.create).toHaveBeenCalledWith( + expect.objectContaining({ + githubRepositoryId: null, + }) + ); + expect(patch).toHaveBeenCalledWith( + expect.objectContaining({ + tag: '4.10', + }) + ); + }); + + test('patches repo-less Docker dependencies during a repo-scoped deploy', async () => { + const appPatch = jest.fn().mockResolvedValue(undefined); + const dependencyPatch = jest.fn().mockResolvedValue(undefined); + const existingDeploys = [ + { + deployableId: 1, + $query: jest.fn(() => ({ patch: appPatch })), + $setRelated: jest.fn(), + }, + { + deployableId: 2, + $query: jest.fn(() => ({ patch: dependencyPatch })), + $setRelated: jest.fn(), + }, + ]; + const deployQuery: any = { + where: jest.fn(() => deployQuery), + withGraphFetched: jest.fn().mockResolvedValue(existingDeploys), + then: (resolve: (value: any[]) => void, reject: (reason: unknown) => void) => + Promise.resolve(existingDeploys).then(resolve, reject), + }; + const build = { + id: 1449, + uuid: 'good-dev-0', + enableFullYaml: true, + deployables: [ + { + id: 1, + name: 'sponsored-benefits', + repositoryId: 1154960313, + branchName: 'main', + defaultTag: 'main', + type: DeployTypes.HELM, + active: true, + }, + { + id: 2, + name: 'sbs-localstack', + repositoryId: null, + branchName: 'main', + defaultTag: '4.10', + type: DeployTypes.DOCKER, + active: true, + dependsOnDeployableName: 'sponsored-benefits', + }, + ], + deploys: existingDeploys, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }; + const mockDb = { + models: { + Deploy: { + query: jest.fn(() => deployQuery), + findOne: jest.fn(), + create: jest.fn(), + }, + }, + services: { + Deploy: { + hostForDeployableDeploy: jest.fn((deploy: any, deployable: any) => `${deployable.name}.lifecycle.test`), + }, + }, + }; + + const deployService = new DeployService( + mockDb as any, + {} as any, + {} as any, + { + registerQueue: jest.fn().mockReturnValue({ add: jest.fn(), process: jest.fn(), on: jest.fn() }), + } as any + ); + + await deployService.findOrCreateDeploys({} as any, build as any, 1154960313); + + expect(appPatch).toHaveBeenCalled(); + expect(dependencyPatch).toHaveBeenCalledWith( + expect.objectContaining({ + tag: '4.10', + }) + ); + }); +}); diff --git a/src/server/services/build.ts b/src/server/services/build.ts index 7d78fb8..e918b03 100644 --- a/src/server/services/build.ts +++ b/src/server/services/build.ts @@ -51,6 +51,7 @@ import { getYamlFileContentFromBranch } from 'server/lib/github'; import WebhookService from './webhook'; import { compactStatusMessage, statusMessageFromError } from 'server/lib/terminalFailure'; import OverrideService, { type BuildServiceOverrideState } from './override'; +import { scopedDeployableNames } from 'server/lib/deployScope'; const tracer = Tracer.getInstance(); tracer.initialize('build-service'); @@ -121,19 +122,37 @@ export default class BuildService extends BaseService { return deploy.service?.name || deploy.deployable?.name || deploy.uuid || String(deploy.id || ''); } + private selectDeploysForRepositoryScope(deploys: Deploy[], githubRepositoryId?: number | null): Deploy[] { + if (!githubRepositoryId) { + return deploys; + } + + const scopedNames = scopedDeployableNames( + deploys.map((deploy) => ({ + name: deploy.deployable?.name || deploy.service?.name || deploy.uuid, + repositoryId: deploy.deployable?.repositoryId ?? deploy.githubRepositoryId, + dependsOnDeployableName: deploy.deployable?.dependsOnDeployableName ?? null, + deploymentDependsOn: deploy.deployable?.deploymentDependsOn ?? null, + })), + githubRepositoryId + ); + + return deploys.filter((deploy) => + scopedNames.has(deploy.deployable?.name || deploy.service?.name || deploy.uuid || String(deploy.id || '')) + ); + } + private buildFingerprintPayload(build: Build, githubRepositoryId?: number) { - const deploys = (build.deploys || []) - .filter((deploy) => !githubRepositoryId || deploy.githubRepositoryId === githubRepositoryId) - .map((deploy) => ({ - key: this.getBuildFingerprintDeployKey(build, deploy), - githubRepositoryId: deploy.githubRepositoryId ?? null, - branchName: deploy.branchName ?? null, - active: deploy.active ?? true, - publicUrl: deploy.publicUrl ?? null, - env: deploy.env || {}, - initEnv: deploy.initEnv || {}, - commentBranchName: deploy.deployable?.commentBranchName ?? null, - })); + const deploys = this.selectDeploysForRepositoryScope(build.deploys || [], githubRepositoryId).map((deploy) => ({ + key: this.getBuildFingerprintDeployKey(build, deploy), + githubRepositoryId: deploy.githubRepositoryId ?? null, + branchName: deploy.branchName ?? null, + active: deploy.active ?? true, + publicUrl: deploy.publicUrl ?? null, + env: deploy.env || {}, + initEnv: deploy.initEnv || {}, + commentBranchName: deploy.deployable?.commentBranchName ?? null, + })); return { buildId: build.id, @@ -1419,16 +1438,15 @@ export default class BuildService extends BaseService { if (!buildId) { getLogger().error('Build: id missing for=deployCLIServices'); } - const deploys = await Deploy.query() - .where({ buildId, ...(githubRepositoryId ? { githubRepositoryId } : {}) }) - .withGraphFetched({ service: true, deployable: true }); - if (!deploys || deploys.length === 0) return false; + const deploys = await Deploy.query().where({ buildId }).withGraphFetched({ service: true, deployable: true }); + const scopedDeploys = this.selectDeploysForRepositoryScope(deploys, githubRepositoryId); + if (!scopedDeploys || scopedDeploys.length === 0) return false; try { if (build?.enableFullYaml) { return _.every( await Promise.all( - deploys - .filter((d) => d.active && CLIDeployTypes.has(d.deployable.type)) + scopedDeploys + .filter((d) => d.active && d.deployable && CLIDeployTypes.has(d.deployable.type)) .map(async (deploy) => { if (!deploy) { getLogger().debug(`Deploy is undefined in deployCLIServices: deploysLength=${deploys.length}`); @@ -1451,7 +1469,7 @@ export default class BuildService extends BaseService { } else { return _.every( await Promise.all( - deploys + scopedDeploys .filter((d) => d.active && CLIDeployTypes.has(d.service.type)) .map(async (deploy) => { if (deploy === undefined) { @@ -1492,18 +1510,19 @@ export default class BuildService extends BaseService { const deploys = await Deploy.query() .where({ buildId, - ...(githubRepositoryId ? { githubRepositoryId } : {}), }) .withGraphFetched({ service: true, deployable: true, }); + const scopedDeploys = this.selectDeploysForRepositoryScope(deploys, githubRepositoryId); if (build?.enableFullYaml) { try { - const deploysToBuild = deploys.filter((d) => { + const deploysToBuild = scopedDeploys.filter((d) => { return ( d.active && + d.deployable && (d.deployable.type === DeployTypes.DOCKER || d.deployable.type === DeployTypes.GITHUB || d.deployable.type === DeployTypes.HELM) @@ -1518,7 +1537,7 @@ export default class BuildService extends BaseService { const results = await Promise.all( deploysToBuild.map(async (deploy, index) => { if (deploy === undefined) { - getLogger().debug(`Deploy is undefined in buildImages: deploysLength=${build.deploys.length}`); + getLogger().debug(`Deploy is undefined in buildImages: deploysLength=${build.deploys?.length}`); } await deploy.$query().patchAndFetch({ deployPipelineId: null, @@ -1539,7 +1558,7 @@ export default class BuildService extends BaseService { } else { try { const results = await Promise.all( - deploys + scopedDeploys .filter((d) => { getLogger().debug( `Check service type for docker builds: deployUuid=${d.uuid} serviceType=${d.service?.type}` @@ -1610,7 +1629,6 @@ export default class BuildService extends BaseService { const allDeploys = await Deploy.query() .where({ buildId, - ...(githubRepositoryId ? { githubRepositoryId } : {}), }) .withGraphFetched({ service: { @@ -1619,11 +1637,16 @@ export default class BuildService extends BaseService { deployable: true, }); - const activeDeploys = allDeploys.filter((d) => d.active); + const activeDeploys = this.selectDeploysForRepositoryScope(allDeploys, githubRepositoryId).filter( + (d) => d.active + ); // Generate manifests for GitHub/Docker/CLI deploys for (const deploy of activeDeploys) { - const deployType = deploy.deployable.type; + const deployType = deploy.deployable?.type; + if (!deployType) { + continue; + } if ( deployType === DeployTypes.GITHUB || deployType === DeployTypes.DOCKER || @@ -1647,9 +1670,10 @@ export default class BuildService extends BaseService { // Use DeploymentManager for all active deploys (both Helm and GitHub types) if (activeDeploys.length > 0) { // we should ignore Codefresh and Configuration services here since we dont deploy anything - const managedDeploys = activeDeploys.filter( - (d) => d.deployable.type !== DeployTypes.CODEFRESH && d.deployable.type !== DeployTypes.CONFIGURATION - ); + const managedDeploys = activeDeploys.filter((d) => { + const deployType = d.deployable?.type; + return deployType && deployType !== DeployTypes.CODEFRESH && deployType !== DeployTypes.CONFIGURATION; + }); const deploymentManager = new DeploymentManager(managedDeploys); await deploymentManager.deploy(); } @@ -1661,12 +1685,12 @@ export default class BuildService extends BaseService { }); // Legacy manifest generation for backwards compatibility - const githubTypeDeploys = activeDeploys.filter( - (d) => - d.deployable.type === DeployTypes.GITHUB || - d.deployable.type === DeployTypes.DOCKER || - CLIDeployTypes.has(d.deployable.type) - ); + const githubTypeDeploys = activeDeploys.filter((d) => { + const deployType = d.deployable?.type; + return deployType + ? deployType === DeployTypes.GITHUB || deployType === DeployTypes.DOCKER || CLIDeployTypes.has(deployType) + : false; + }); if (githubTypeDeploys.length > 0) { const legacyManifest = k8s.generateManifest({ @@ -1769,11 +1793,9 @@ export default class BuildService extends BaseService { return environments; } - private async updateDeploysImageDetails(build: Build, githubRepositoryId?: number) { - await build?.$fetchGraph('deploys'); - const deploys = githubRepositoryId - ? build.deploys.filter((d) => d.githubRepositoryId === githubRepositoryId) - : build.deploys; + private async updateDeploysImageDetails(build: Build, githubRepositoryId?: number | null) { + await build?.$fetchGraph('deploys.[service, deployable]'); + const deploys = this.selectDeploysForRepositoryScope(build.deploys || [], githubRepositoryId); await Promise.all( deploys.map((deploy) => deploy.$query().patch({ isRunningLatest: true, runningImage: deploy?.dockerImage })) ); diff --git a/src/server/services/deploy.ts b/src/server/services/deploy.ts index 24a0a91..e90cd39 100644 --- a/src/server/services/deploy.ts +++ b/src/server/services/deploy.ts @@ -42,6 +42,7 @@ import { SecretProcessor } from 'server/services/secretProcessor'; import { fallbackDeployStatusMessage, statusMessageFromError } from 'server/lib/terminalFailure'; import { isNativeBuilderEngine } from 'server/lib/buildEngines'; import { SecretProvidersConfig } from 'server/services/types/globalConfig'; +import { normalizeRepositoryId, scopedDeployableNames } from 'server/lib/deployScope'; export interface DeployOptions { ownerId?: number; @@ -76,7 +77,7 @@ export default class DeployService extends BaseService { async findOrCreateDeploys(environment: Environment, build: Build, githubRepositoryId?: number): Promise { await build?.$fetchGraph('[deployables.[repository]]'); - const { deployables } = build; + const deployables = build.deployables || []; if (build?.enableFullYaml) { const buildId = build?.id; @@ -87,13 +88,14 @@ export default class DeployService extends BaseService { const existingDeploys = await this.db.models.Deploy.query().where({ buildId }).withGraphFetched('deployable'); const existingDeployMap = new Map(existingDeploys.map((d) => [d.deployableId, d])); + const deployableNamesInScope = scopedDeployableNames(deployables, githubRepositoryId); await Promise.all( deployables.map(async (deployable) => { const uuid = `${deployable.name}-${build?.uuid}`; const patchFields: Objection.PartialModelObject = {}; - const deployableRepositoryId = Number(deployable.repositoryId); - const isTargetRepo = !githubRepositoryId || deployableRepositoryId === githubRepositoryId; + const deployableRepositoryId = normalizeRepositoryId(deployable.repositoryId); + const isTargetRepo = !githubRepositoryId || deployableNamesInScope.has(deployable.name); let deploy = existingDeployMap.get(deployable.id) ?? null; if (!deploy) { @@ -109,10 +111,11 @@ export default class DeployService extends BaseService { } } + if (!isTargetRepo) { + return; + } + if (deploy != null) { - if (!isTargetRepo) { - return; - } patchFields.deployableId = deployable?.id ?? null; patchFields.publicUrl = this.db.services.Deploy.hostForDeployableDeploy(deploy, deployable); patchFields.internalHostname = uuid;