diff --git a/src/server/services/__tests__/build.test.ts b/src/server/services/__tests__/build.test.ts index 48ce891..5215c21 100644 --- a/src/server/services/__tests__/build.test.ts +++ b/src/server/services/__tests__/build.test.ts @@ -863,6 +863,73 @@ describe('BuildService stale deploy reconciliation', () => { expect(build.$fetchGraph).toHaveBeenCalledWith('[deployables, deploys]'); }); + test('never reaps a service whose YAML could not be resolved', async () => { + createService( + [ + { id: 1, name: 'unreadable-dep', resolvedFromRepositoryId: targetRepoId }, + { id: 3, name: 'worker-old', resolvedFromRepositoryId: targetRepoId }, + ], + [{ id: 78, uuid: 'worker-old-build-1', deployableId: 3 }] + ); + const build = createBuild(); + + await (buildService as any).reconcileDeletedDeployables(build, { + canReconcile: true, + deployables: [], + unresolvedServiceNames: ['unreadable-dep'], + unresolvedRepositoryIds: [], + reconcileEligibleDeployables: [ + { name: 'worker-new', source: 'yaml', reconcileEligible: true, resolvedFromRepositoryId: targetRepoId }, + ], + }); + + // 'unreadable-dep' is absent from the expected set only because its config could not be read. + expect(mockDeleteServiceRows).toHaveBeenCalledWith({ buildId: 10, deployableIds: [3] }); + }); + + test('never reaps deployables owned by a repository whose YAML could not be read', async () => { + createService( + [ + { id: 5, name: 'dep-child', resolvedFromRepositoryId: otherRepoId }, + { id: 3, name: 'worker-old', resolvedFromRepositoryId: targetRepoId }, + ], + [{ id: 78, uuid: 'worker-old-build-1', deployableId: 3 }] + ); + const build = createBuild(); + + await (buildService as any).reconcileDeletedDeployables(build, { + canReconcile: true, + deployables: [], + unresolvedServiceNames: [], + unresolvedRepositoryIds: [otherRepoId], + reconcileEligibleDeployables: [ + { name: 'worker-new', source: 'yaml', reconcileEligible: true, resolvedFromRepositoryId: targetRepoId }, + ], + }); + + // 'requires:' children of an unreadable repository are never enumerated, so they must be protected. + expect(mockDeleteServiceRows).toHaveBeenCalledWith({ buildId: 10, deployableIds: [3] }); + }); + + test('an unresolved dependency no longer blocks reaping the rest of the environment', async () => { + const staleDeploy = { id: 78, uuid: 'worker-old-build-1', deployableId: 3 }; + createService([{ id: 3, name: 'worker-old', resolvedFromRepositoryId: targetRepoId }], [staleDeploy]); + const build = createBuild(); + + await (buildService as any).reconcileDeletedDeployables(build, { + canReconcile: true, + deployables: [], + unresolvedServiceNames: ['unrelated-archived-dep'], + unresolvedRepositoryIds: [otherRepoId], + reconcileEligibleDeployables: [ + { name: 'worker-new', source: 'yaml', reconcileEligible: true, resolvedFromRepositoryId: targetRepoId }, + ], + }); + + expect(mockCleanupDeploy).toHaveBeenCalledWith(staleDeploy, { mode: 'service' }); + expect(mockDeleteServiceRows).toHaveBeenCalledWith({ buildId: 10, deployableIds: [3] }); + }); + test('repo-filtered reconciliation removes only deployables from the triggering repository scope', async () => { const staleDeploy = { id: 79, uuid: 'target-old-build-1', deployableId: 4 }; createService([{ id: 4, name: 'target-old', resolvedFromRepositoryId: targetRepoId }], [staleDeploy]); diff --git a/src/server/services/__tests__/deployableSourceSeam.test.ts b/src/server/services/__tests__/deployableSourceSeam.test.ts index 90f9972..e2e3684 100644 --- a/src/server/services/__tests__/deployableSourceSeam.test.ts +++ b/src/server/services/__tests__/deployableSourceSeam.test.ts @@ -377,6 +377,151 @@ describe('deployable source seam (PR vs API build)', () => { ); }); + it('records an unresolvable dependency repository instead of vetoing reconciliation', async () => { + const service = makeService(); + const rootRepository = { githubRepositoryId: 42, fullName: 'org/root' }; + mockRepositoryWhereNull.mockResolvedValue(rootRepository); + mockFetchLifecycleConfigByRepository.mockResolvedValue({ + environment: { + defaultServices: [{ name: 'archived-dep', repository: 'org/archived', branch: 'main' }], + optionalServices: [], + }, + services: [], + }); + mockResolveRepository.mockResolvedValue({ + githubRepositoryId: 77, + fullName: 'org/archived', + deletedAt: '2026-01-01T00:00:00Z', + }); + const build: any = { + id: 9, + triggerType: 'github_pr', + githubRepositoryId: 42, + branchName: 'main', + configSha: 'root-config-sha', + deploys: [], + environment: { id: 5 }, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }; + const unresolvedServiceNames = new Set(); + const unresolvedRepositoryIds = new Set(); + + const result = await (service as any).updateOrCreateDeployableUsingYamlConfig( + new Map(), + 9, + 'uuid-9', + null, + build, + undefined, + null, + null, + undefined, + unresolvedServiceNames, + unresolvedRepositoryIds + ); + + expect(result).toBe(true); + expect(Array.from(unresolvedServiceNames)).toEqual(['archived-dep']); + expect(Array.from(unresolvedRepositoryIds)).toEqual([77]); + }); + + it('records the repository when a remote service fails exact-name resolution', async () => { + const service = makeService(); + const rootRepository = { githubRepositoryId: 42, fullName: 'org/root' }; + const dependencyRepository = { githubRepositoryId: 99, fullName: 'org/dependency' }; + mockRepositoryWhereNull.mockResolvedValue(rootRepository); + mockFetchLifecycleConfigByRepository + .mockResolvedValueOnce({ + environment: { + defaultServices: [{ name: 'dependency-api', repository: 'org/dependency', branch: 'main' }], + optionalServices: [], + }, + services: [], + }) + .mockResolvedValueOnce({ services: [{ name: 'renamed-api' }] }); + mockResolveRepository.mockResolvedValue(dependencyRepository); + mockResolveExactEnvironmentService.mockReturnValue(null); + const build: any = { + id: 9, + triggerType: 'github_pr', + githubRepositoryId: 42, + branchName: 'main', + configSha: 'root-config-sha', + deploys: [], + environment: { id: 5 }, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }; + const unresolvedServiceNames = new Set(); + const unresolvedRepositoryIds = new Set(); + + const result = await (service as any).updateOrCreateDeployableUsingYamlConfig( + new Map(), + 9, + 'uuid-9', + null, + build, + undefined, + null, + null, + undefined, + unresolvedServiceNames, + unresolvedRepositoryIds + ); + + expect(result).toBe(true); + expect(Array.from(unresolvedServiceNames)).toEqual(['dependency-api']); + // The `requires:` recursion never ran, so the repository must be protected too. + expect(Array.from(unresolvedRepositoryIds)).toEqual([99]); + }); + + it('records a legacy serviceId reference instead of vetoing reconciliation', async () => { + const service = makeService(); + const rootRepository = { githubRepositoryId: 42, fullName: 'org/root' }; + (service as any).db.models.Repository = { + query: jest.fn(() => ({ + findOne: jest.fn(() => ({ whereNull: jest.fn().mockResolvedValue(rootRepository) })), + })), + }; + mockRepositoryWhereNull.mockResolvedValue(rootRepository); + mockFetchLifecycleConfigByRepository.mockResolvedValue({ + environment: { + defaultServices: [{ name: 'legacy-db-service', serviceId: 47 }, { name: 'api' }], + optionalServices: [], + }, + services: [{ name: 'api' }], + }); + mockResolveExactEnvironmentService.mockReturnValue({ service: { name: 'api' }, requiredServices: [] }); + jest.spyOn(service, 'updateOrCreateDeployableAttributesUsingYAMLConfig').mockResolvedValue(undefined); + const build: any = { + id: 9, + triggerType: 'github_pr', + githubRepositoryId: 42, + branchName: 'main', + configSha: 'root-config-sha', + deploys: [], + environment: { id: 5 }, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }; + const unresolvedServiceNames = new Set(); + + const result = await (service as any).updateOrCreateDeployableUsingYamlConfig( + new Map(), + 9, + 'uuid-9', + null, + build, + 42, + null, + 'main', + 42, + unresolvedServiceNames, + new Set() + ); + + expect(result).toBe(true); + expect(Array.from(unresolvedServiceNames)).toEqual(['legacy-db-service']); + }); + it('fails closed before YAML import when the targeted repository has no live row', async () => { const service = makeService(); const filterWhereNull = jest.fn().mockResolvedValue(undefined); diff --git a/src/server/services/build.ts b/src/server/services/build.ts index b0a335d..e41b093 100644 --- a/src/server/services/build.ts +++ b/src/server/services/build.ts @@ -2362,6 +2362,11 @@ export default class BuildService extends BaseService { expectedGeneration?: number ) { if (!reconciliationResult?.canReconcile) { + getLogger({ + buildUuid: build.uuid, + filterGithubRepositoryId, + sourceBranch, + }).warn('Stale deploy reconciliation: skipped reason=configNotFullyResolved'); return; } @@ -2409,7 +2414,18 @@ export default class BuildService extends BaseService { !sourceBranch || (deployable.commentBranchName ?? deployable.branchName) === sourceBranch ); - const staleDeployables = existingDeployables.filter((deployable) => !expectedNames.has(deployable.name)); + // A service whose config we could not read is an unknown, not a deletion — never reap it. + const unresolvedNames = new Set(reconciliationResult.unresolvedServiceNames ?? []); + const unresolvedRepositoryIds = new Set(reconciliationResult.unresolvedRepositoryIds ?? []); + const staleDeployables = existingDeployables.filter( + (deployable) => + !expectedNames.has(deployable.name) && + !unresolvedNames.has(deployable.name) && + !( + deployable.resolvedFromRepositoryId != null && + unresolvedRepositoryIds.has(deployable.resolvedFromRepositoryId) + ) + ); if (staleDeployables.length === 0) { getLogger({ diff --git a/src/server/services/deployable.ts b/src/server/services/deployable.ts index 1b7f7e7..5e875a9 100644 --- a/src/server/services/deployable.ts +++ b/src/server/services/deployable.ts @@ -40,6 +40,10 @@ export interface DeployableReconciliationResult { canReconcile: boolean; reconcileEligibleDeployables: DeployableReconciliationEntry[]; filterGithubRepositoryId?: number | null; + /** Services named in YAML that could not be fully resolved; never reaped as stale. */ + unresolvedServiceNames: string[]; + /** Repositories whose YAML could not be read; their deployables are never reaped as stale. */ + unresolvedRepositoryIds: number[]; } export interface DeployableAttributes { @@ -432,10 +436,11 @@ export default class DeployableService extends BaseService { filterGithubRepositoryId?: number, sourceRef?: string | null, sourceBranch?: string | null, - sourceGithubRepositoryId: number | null | undefined = filterGithubRepositoryId + sourceGithubRepositoryId: number | null | undefined = filterGithubRepositoryId, + unresolvedServiceNames: Set = new Set(), + unresolvedRepositoryIds: Set = new Set() ): Promise { try { - let allReferencedYamlConfigsResolved = true; let sourceRepository: Repository | null = null; let rootBranch: string | null = null; let rootBaseConfigRef: string | null = null; @@ -462,6 +467,14 @@ export default class DeployableService extends BaseService { filterRepositoryFullName = filterRepo?.fullName?.toLowerCase() ?? null; } + // A service we could not fully resolve is an unknown, not a deletion. Recording it (and the + // repository whose config we could not read) keeps stale-service reaping scoped to the + // services we did resolve, instead of vetoing reconciliation for the whole environment. + const markUnresolved = (name: string | undefined, repositoryId?: number | string | null) => { + if (name != null) unresolvedServiceNames.add(name); + if (repositoryId != null) unresolvedRepositoryIds.add(Number(repositoryId)); + }; + const targetsSource = (repository: Repository | null | undefined, branchName: string | null | undefined) => filterGithubRepositoryId == null || (repository?.githubRepositoryId != null && @@ -490,7 +503,7 @@ export default class DeployableService extends BaseService { services.map(async (yamlEnvService) => { try { if (yamlEnvService.serviceId != null) { - if (filterGithubRepositoryId != null && sourceBranch != null) targetAttributionFailed = true; + markUnresolved(yamlEnvService.name); getLogger({ buildUUID, service: yamlEnvService.name, serviceId: yamlEnvService.serviceId }).warn( 'serviceId references in lifecycle.yaml are no longer supported; skipping service ' + yamlEnvService.name @@ -535,7 +548,7 @@ export default class DeployableService extends BaseService { if (yamlEnvService.repository.toLowerCase() === filterRepositoryFullName) { targetAttributionFailed = true; } - allReferencedYamlConfigsResolved = false; + markUnresolved(yamlEnvService.name, repository?.githubRepositoryId); getLogger({ buildUUID, service: yamlEnvService.name }).warn( 'Deployable: referenced repository is not live; skipping service' ); @@ -603,14 +616,15 @@ export default class DeployableService extends BaseService { build ); } else { - if (filterGithubRepositoryId != null && sourceBranch != null) targetAttributionFailed = true; + // The `requires:` recursion above is gated on a resolved service, so this service's + // inner dependencies were never enumerated either. Protect them by repository. + markUnresolved(yamlEnvService.name, repository?.githubRepositoryId); getLogger({ buildUUID, service: yamlEnvService.name }).warn( 'Service cannot be found in yaml configuration. Is it referenced via the Lifecycle database?' ); } } else { - allReferencedYamlConfigsResolved = false; - if (filterGithubRepositoryId != null && sourceBranch != null) targetAttributionFailed = true; + markUnresolved(yamlEnvService.name, repository?.githubRepositoryId); getLogger({ buildUUID, deployUUID: deploy?.uuid, repository: repository?.fullName }).warn( `Unable to locate YAML config file from ${repository?.fullName}:${branchName}. Is this a database service?` ); @@ -721,7 +735,7 @@ export default class DeployableService extends BaseService { ); } } - return allReferencedYamlConfigsResolved && targetAttributionResolved && !targetAttributionFailed; + return targetAttributionResolved && !targetAttributionFailed; } } else { getLogger({ buildUUID }).warn('Build source repository or ref missing'); @@ -761,6 +775,8 @@ export default class DeployableService extends BaseService { // Temporary storage for all the deployable configurations in memory const deployableServices: Map = new Map(); + const unresolvedServiceNames = new Set(); + const unresolvedRepositoryIds = new Set(); try { if (pullRequest != null || hasBuildSource) { if (pullRequest != null && pullRequest.branchName == null) { @@ -777,7 +793,9 @@ export default class DeployableService extends BaseService { filterGithubRepositoryId, sourceRef, sourceBranch, - sourceGithubRepositoryId + sourceGithubRepositoryId, + unresolvedServiceNames, + unresolvedRepositoryIds ); // Finally, Upsert the deployables into the database @@ -802,6 +820,8 @@ export default class DeployableService extends BaseService { deployables, canReconcile, filterGithubRepositoryId: filterGithubRepositoryId ?? null, + unresolvedServiceNames: Array.from(unresolvedServiceNames), + unresolvedRepositoryIds: Array.from(unresolvedRepositoryIds), reconcileEligibleDeployables: Array.from(deployableServices.values()) .filter((deployable) => deployable.reconcileEligible) .map((deployable) => ({