From 29d31e6b766886e4fffa41590012a7a1efdd7dbb Mon Sep 17 00:00:00 2001 From: jin-joung Date: Thu, 29 Oct 2026 06:18:49 -0500 Subject: [PATCH] feat(push): skip unchanged services per ignoreFiles/includeFiles policy A push now redeploys only the services whose own policy does not ignore every changed file. Skipped services keep their current image; settled, healthy rows of those services are left out of the source intent's stamp. Pending, failed, or never-deployed rows are always stamped, so a replaced intent's work is carried forward. Adds a service-level includeFiles (watch paths) option and gates the per-service skip behind a new ignoreFilesPerService flag (dry-run logs when off). --- docs/schema/yaml/1.0.0.yaml | 4 + .../lib/__tests__/pushIgnoreFiles.test.ts | 87 ++++++++++++++++++- .../__tests__/mailbox.test.ts | 50 +++++++++++ .../lib/deploymentReconciliation/mailbox.ts | 42 +++++++-- src/server/lib/jsonschema/schemas/1.0.0.json | 6 ++ src/server/lib/pushIgnoreFiles.ts | 75 +++++++++++----- .../yamlSchemas/schema_1_0_0/schema_1_0_0.ts | 1 + src/server/models/yaml/YamlService.ts | 1 + src/server/services/__tests__/build.test.ts | 20 +++++ src/server/services/__tests__/github.test.ts | 64 ++++++++++++++ src/server/services/build.ts | 19 ++-- src/server/services/github.ts | 31 +++++-- 12 files changed, 360 insertions(+), 40 deletions(-) diff --git a/docs/schema/yaml/1.0.0.yaml b/docs/schema/yaml/1.0.0.yaml index 4200cffe..9ef4e013 100644 --- a/docs/schema/yaml/1.0.0.yaml +++ b/docs/schema/yaml/1.0.0.yaml @@ -794,4 +794,8 @@ services: # @param services.ignoreFiles ignoreFiles: # @param services.ignoreFiles[] + - '' + # @param services.includeFiles + includeFiles: + # @param services.includeFiles[] - '' \ No newline at end of file diff --git a/src/server/lib/__tests__/pushIgnoreFiles.test.ts b/src/server/lib/__tests__/pushIgnoreFiles.test.ts index 3c52587e..d66a81aa 100644 --- a/src/server/lib/__tests__/pushIgnoreFiles.test.ts +++ b/src/server/lib/__tests__/pushIgnoreFiles.test.ts @@ -20,6 +20,7 @@ import { hasLifecycleConfigChange, normalizeGithubPath, normalizeIgnoreFiles, + normalizeIncludeFiles, shouldSkipPushDeploy, } from '../pushIgnoreFiles'; @@ -145,6 +146,87 @@ describe('pushIgnoreFiles', () => { }); }); + test('lists the services whose own policy ignores every changed file when others still deploy', () => { + expect( + shouldSkipPushDeploy({ + changedFiles: ['apps/next-web/src/page.tsx'], + servicePolicies: [ + { serviceName: 'next-web', ignoreFiles: ['docs/**'] }, + { serviceName: 'psp-web', ignoreFiles: ['apps/next-web/**'] }, + { serviceName: 'rxsaver-web', ignoreFiles: ['apps/next-web/**'] }, + ], + }) + ).toEqual({ + shouldSkip: false, + reason: 'file_not_ignored', + serviceName: 'next-web', + filePath: 'apps/next-web/src/page.tsx', + skipServices: ['psp-web', 'rxsaver-web'], + }); + }); + + test('keeps a service without a policy deploying while skipping services that ignore the change', () => { + expect( + shouldSkipPushDeploy({ + changedFiles: ['apps/next-web/src/page.tsx'], + servicePolicies: [ + { serviceName: 'worker', ignoreFiles: [] }, + { serviceName: 'psp-web', ignoreFiles: ['apps/next-web/**'] }, + ], + }) + ).toEqual({ + shouldSkip: false, + reason: 'missing_service_policy', + serviceName: 'worker', + skipServices: ['psp-web'], + }); + }); + + test('treats files outside includeFiles as ignored, with ignoreFiles still applied inside them', () => { + const psp = { + serviceName: 'psp-web', + ignoreFiles: ['**/*.spec.ts'], + includeFiles: ['apps/psp/**', 'packages/shared/**', 'pnpm-lock.yaml'], + }; + + expect( + shouldSkipPushDeploy({ + changedFiles: ['packages/accounts/src/a.ts', 'packages/shared/src/a.spec.ts'], + servicePolicies: [psp], + }) + ).toEqual({ shouldSkip: true, reason: 'all_changed_files_ignored' }); + expect(shouldSkipPushDeploy({ changedFiles: ['pnpm-lock.yaml'], servicePolicies: [psp] })).toEqual({ + shouldSkip: false, + reason: 'file_not_ignored', + serviceName: 'psp-web', + filePath: 'pnpm-lock.yaml', + }); + }); + + test('builds an includeFiles-only service policy', () => { + const config = { + version: '1.0.0', + environment: {}, + services: [{ name: 'psp-web', includeFiles: ['apps/psp/**'] }], + } as any; + + expect(getServicePushIgnorePolicy(config, 'psp-web')).toEqual({ + serviceName: 'psp-web', + ignoreFiles: [], + includeFiles: ['apps/psp/**'], + }); + }); + + test('validates includeFiles with a larger pattern cap', () => { + expect(normalizeIncludeFiles(Array.from({ length: 200 }, (_value, index) => `packages/${index}/**`))).toHaveLength( + 200 + ); + expect(() => normalizeIncludeFiles(Array.from({ length: 201 }, (_value, index) => `p/${index}/**`))).toThrow( + 'includeFiles has too many patterns' + ); + expect(() => normalizeIncludeFiles(['../outside/**'])).toThrow('includeFiles patterns cannot traverse'); + }); + test('detects lifecycle config changes by new path', () => { expect(hasLifecycleConfigChange(['docs/readme.md', 'lifecycle.yaml'])).toBe(true); expect(hasLifecycleConfigChange(['docs/lifecycle.yml'])).toBe(false); @@ -154,7 +236,10 @@ describe('pushIgnoreFiles', () => { expect( shouldSkipPushDeploy({ changedFiles: ['./lifecycle.yml'], - servicePolicies: [{ serviceName: 'api', ignoreFiles: ['**/*'] }], + servicePolicies: [ + { serviceName: 'api', ignoreFiles: ['**/*'] }, + { serviceName: 'web', ignoreFiles: [], includeFiles: ['apps/web/**'] }, + ], }) ).toEqual({ shouldSkip: false, diff --git a/src/server/lib/deploymentReconciliation/__tests__/mailbox.test.ts b/src/server/lib/deploymentReconciliation/__tests__/mailbox.test.ts index 28edbf98..764fffa5 100644 --- a/src/server/lib/deploymentReconciliation/__tests__/mailbox.test.ts +++ b/src/server/lib/deploymentReconciliation/__tests__/mailbox.test.ts @@ -25,6 +25,11 @@ jest.mock('server/models/Build', () => ({ const deployStamp: any = { patch: jest.fn(() => deployStamp), where: jest.fn(() => deployStamp), + whereNotIn: jest.fn(() => deployStamp), + select: jest.fn(() => deployStamp), + join: jest.fn(() => deployStamp), + whereIn: jest.fn(() => deployStamp), + whereRaw: jest.fn(() => deployStamp), then: (resolve: (value: number) => void, reject: (reason: unknown) => void) => Promise.resolve(2).then(resolve, reject), }; @@ -506,6 +511,51 @@ describe('deploymentIntentDeployFilter', () => { }); describe('acceptDeploymentIntent row stamping', () => { + const skipIntent = { + type: 'source' as const, + requestId: 'request-skip', + target: 'repository' as const, + githubRepositoryId: 123, + branch: 'main', + sha: 'commit-b', + skipServices: ['psp-web'], + }; + + it('leaves settled, healthy rows of skipped services out of the stamp', async () => { + (Build.query as jest.Mock) + .mockReturnValueOnce(readQuery({ desiredGeneration: '1', acceptedRefs: {} })) + .mockReturnValueOnce(writeQuery()); + + await acceptDeploymentIntent(42, skipIntent); + + expect(deployStamp.whereNotIn).toHaveBeenCalledWith('id', deployStamp); + expect(deployStamp.join).toHaveBeenCalledWith('deployables', 'deployables.id', 'deploys.deployableId'); + expect(deployStamp.where).toHaveBeenCalledWith('deploys.buildId', 42); + expect(deployStamp.whereIn).toHaveBeenCalledWith('deployables.name', ['psp-web']); + expect(deployStamp.whereIn).toHaveBeenCalledWith('deploys.status', [ + DeployStatus.READY, + DeployStatus.DEPLOYED, + DeployStatus.BUILT, + ]); + expect(deployStamp.whereRaw).toHaveBeenCalledWith('?? >= ??', [ + 'deploys.observedGeneration', + 'deploys.desiredGeneration', + ]); + }); + + it('stamps every selected row when no service is skipped or the push selects the whole environment', async () => { + (Build.query as jest.Mock) + .mockReturnValueOnce(readQuery({ desiredGeneration: '1', acceptedRefs: {} })) + .mockReturnValueOnce(writeQuery()) + .mockReturnValueOnce(readQuery({ desiredGeneration: '2', acceptedRefs: {} })) + .mockReturnValueOnce(writeQuery()); + + await acceptDeploymentIntent(42, { ...skipIntent, skipServices: [] }); + await acceptDeploymentIntent(42, { ...skipIntent, requestId: 'request-all', target: 'all' }); + + expect(deployStamp.whereNotIn).not.toHaveBeenCalled(); + }); + it('does not touch Deploy rows when a redelivery is deduplicated', async () => { const existing: AcceptedDeploymentRefs = { 'source:123:main': { diff --git a/src/server/lib/deploymentReconciliation/mailbox.ts b/src/server/lib/deploymentReconciliation/mailbox.ts index a7110b71..ed1c955a 100644 --- a/src/server/lib/deploymentReconciliation/mailbox.ts +++ b/src/server/lib/deploymentReconciliation/mailbox.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import type { PartialModelObject, Transaction } from 'objection'; +import type { PartialModelObject, QueryBuilder, Transaction } from 'objection'; import type { JobDataWithContext } from 'server/lib/logger'; import Build from 'server/models/Build'; import Deploy from 'server/models/Deploy'; @@ -30,6 +30,8 @@ export interface SourcePushIntent { sha: string; /** Delivered push predecessor; used to recognize a temporarily stale branch-head read. */ beforeSha?: string; + /** Services whose push-ignore policy matched every changed file; their healthy rows keep the current image. */ + skipServices?: string[]; } export interface RepositoryRedeployIntent { @@ -102,6 +104,31 @@ export function deploymentIntentDeployFilter(intent: DeploymentIntent): { return { githubRepositoryId: intent.githubRepositoryId }; } +const SKIPPABLE_DEPLOY_STATUSES = [DeployStatus.READY, DeployStatus.DEPLOYED, DeployStatus.BUILT]; + +/** + * Leaves out rows of the intent's skipped services that are settled and healthy. A row that is still pending + * (including one a replaced intent stamped), failed, or never deployed stays selected. + */ +export function excludeSkippedServiceDeploys>( + query: QB, + buildId: number, + intent: DeploymentIntent, + trx?: Transaction +): QB { + if (intent.type !== 'source' || intent.target !== 'repository' || !intent.skipServices?.length) return query; + return query.whereNotIn( + 'id', + Deploy.query(trx) + .select('deploys.id') + .join('deployables', 'deployables.id', 'deploys.deployableId') + .where('deploys.buildId', buildId) + .whereIn('deployables.name', intent.skipServices) + .whereIn('deploys.status', SKIPPABLE_DEPLOY_STATUSES) + .whereRaw('?? >= ??', ['deploys.observedGeneration', 'deploys.desiredGeneration']) + ); +} + /** A stamped row is unowned and queued: the older run can no longer write to it, and its old status is not current. */ export function deployStampForGeneration(generation: number): PartialModelObject { const stamp = { desiredGeneration: generation, runUUID: null, status: DeployStatus.QUEUED, statusMessage: null }; @@ -116,10 +143,15 @@ export async function stampDeploysForIntent( intent: DeploymentIntent, generation: number ): Promise { - return Deploy.query(trx) - .patch(deployStampForGeneration(generation)) - .where({ buildId, active: true, ...deploymentIntentDeployFilter(intent) }) - .where('desiredGeneration', '<', generation); + return excludeSkippedServiceDeploys( + Deploy.query(trx) + .patch(deployStampForGeneration(generation)) + .where({ buildId, active: true, ...deploymentIntentDeployFilter(intent) }) + .where('desiredGeneration', '<', generation), + buildId, + intent, + trx + ); } /** diff --git a/src/server/lib/jsonschema/schemas/1.0.0.json b/src/server/lib/jsonschema/schemas/1.0.0.json index d8945117..dbf550c1 100644 --- a/src/server/lib/jsonschema/schemas/1.0.0.json +++ b/src/server/lib/jsonschema/schemas/1.0.0.json @@ -1559,6 +1559,12 @@ "items": { "type": "string" } + }, + "includeFiles": { + "type": "array", + "items": { + "type": "string" + } } }, "required": [ diff --git a/src/server/lib/pushIgnoreFiles.ts b/src/server/lib/pushIgnoreFiles.ts index 18b8d96c..ae9cb076 100644 --- a/src/server/lib/pushIgnoreFiles.ts +++ b/src/server/lib/pushIgnoreFiles.ts @@ -18,12 +18,16 @@ import picomatch from 'picomatch'; import type { LifecycleConfig } from 'server/models/yaml/Config'; const MAX_IGNORE_PATTERNS = 50; +// A dependency closure in a monorepo can list many package directories. +const MAX_INCLUDE_PATTERNS = 200; const MAX_IGNORE_PATTERN_LENGTH = 200; const LIFECYCLE_CONFIG_FILE_PATHS = new Set(['lifecycle.yaml', 'lifecycle.yml', '.lifecycle.yaml', '.lifecycle.yml']); export interface PushIgnoreServicePolicy { serviceName: string; ignoreFiles: string[]; + /** When set, any changed file outside these patterns is ignored for this service. */ + includeFiles?: string[]; } export interface PushIgnoreDecision { @@ -31,51 +35,61 @@ export interface PushIgnoreDecision { reason: string; serviceName?: string; filePath?: string; + /** Services whose own policy ignores every changed file, when the others still deploy. */ + skipServices?: string[]; } export function normalizeGithubPath(filePath: string): string { return filePath.replace(/\\/g, '/').replace(/^\.\//, '').replace(/^\/+/, ''); } -function validateIgnorePattern(rawPattern: unknown): string { +function validatePattern(rawPattern: unknown, field: string): string { if (typeof rawPattern !== 'string') { - throw new Error(`ignoreFiles patterns must be strings`); + throw new Error(`${field} patterns must be strings`); } const pattern = rawPattern.trim().replace(/\\/g, '/'); if (!pattern) { - throw new Error('ignoreFiles patterns cannot be empty'); + throw new Error(`${field} patterns cannot be empty`); } if (pattern.length > MAX_IGNORE_PATTERN_LENGTH) { - throw new Error(`ignoreFiles pattern exceeds maximum length of ${MAX_IGNORE_PATTERN_LENGTH}: "${pattern}"`); + throw new Error(`${field} pattern exceeds maximum length of ${MAX_IGNORE_PATTERN_LENGTH}: "${pattern}"`); } if (pattern.startsWith('/')) { - throw new Error(`ignoreFiles patterns must be repo-relative: "${pattern}"`); + throw new Error(`${field} patterns must be repo-relative: "${pattern}"`); } if (pattern.split('/').some((segment) => segment === '..')) { - throw new Error(`ignoreFiles patterns cannot traverse directories: "${pattern}"`); + throw new Error(`${field} patterns cannot traverse directories: "${pattern}"`); } return pattern.replace(/^\.\//, ''); } -export function normalizeIgnoreFiles(rawPatterns: unknown): string[] { +function normalizePatterns(rawPatterns: unknown, field: string, maxPatterns: number): string[] { if (rawPatterns == null) { return []; } if (!Array.isArray(rawPatterns)) { - throw new Error('ignoreFiles must be an array of strings'); + throw new Error(`${field} must be an array of strings`); } - if (rawPatterns.length > MAX_IGNORE_PATTERNS) { - throw new Error(`ignoreFiles has too many patterns: ${rawPatterns.length} exceeds ${MAX_IGNORE_PATTERNS}`); + if (rawPatterns.length > maxPatterns) { + throw new Error(`${field} has too many patterns: ${rawPatterns.length} exceeds ${maxPatterns}`); } - return Array.from(new Set(rawPatterns.map(validateIgnorePattern))); + return Array.from(new Set(rawPatterns.map((pattern) => validatePattern(pattern, field)))); +} + +export function normalizeIgnoreFiles(rawPatterns: unknown): string[] { + return normalizePatterns(rawPatterns, 'ignoreFiles', MAX_IGNORE_PATTERNS); +} + +export function normalizeIncludeFiles(rawPatterns: unknown): string[] { + return normalizePatterns(rawPatterns, 'includeFiles', MAX_INCLUDE_PATTERNS); } export function getEffectiveIgnoreFiles(environmentIgnoreFiles: unknown, serviceIgnoreFiles: unknown): string[] { @@ -94,16 +108,24 @@ export function getServicePushIgnorePolicy( } const ignoreFiles = getEffectiveIgnoreFiles(config.environment?.ignoreFiles, service.ignoreFiles); - if (ignoreFiles.length === 0) { + const includeFiles = normalizeIncludeFiles(service.includeFiles); + if (ignoreFiles.length === 0 && includeFiles.length === 0) { return null; } return { serviceName, ignoreFiles, + ...(includeFiles.length > 0 ? { includeFiles } : {}), }; } +function compileIgnoreMatcher(servicePolicy: PushIgnoreServicePolicy): (filePath: string) => boolean { + const isIgnored = servicePolicy.ignoreFiles.length > 0 ? picomatch(servicePolicy.ignoreFiles, { dot: true }) : null; + const isIncluded = servicePolicy.includeFiles?.length ? picomatch(servicePolicy.includeFiles, { dot: true }) : null; + return (filePath) => (isIgnored?.(filePath) ?? false) || (isIncluded != null && !isIncluded(filePath)); +} + export function hasLifecycleConfigChange(changedFiles: string[]): boolean { return changedFiles.some((filePath) => { const normalized = normalizeGithubPath(filePath); @@ -132,22 +154,27 @@ export function shouldSkipPushDeploy({ return { shouldSkip: false, reason: 'no_service_policies' }; } + let deployDecision: PushIgnoreDecision | null = null; + const skipServices: string[] = []; for (const servicePolicy of servicePolicies) { - if (servicePolicy.ignoreFiles.length === 0) { - return { shouldSkip: false, reason: 'missing_service_policy', serviceName: servicePolicy.serviceName }; + const { serviceName } = servicePolicy; + if (servicePolicy.ignoreFiles.length === 0 && !servicePolicy.includeFiles?.length) { + deployDecision ??= { shouldSkip: false, reason: 'missing_service_policy', serviceName }; + continue; } - for (const filePath of normalizedChangedFiles) { - if (!picomatch.isMatch(filePath, servicePolicy.ignoreFiles, { dot: true })) { - return { - shouldSkip: false, - reason: 'file_not_ignored', - serviceName: servicePolicy.serviceName, - filePath, - }; - } + const isIgnored = compileIgnoreMatcher(servicePolicy); + const filePath = normalizedChangedFiles.find((changedFile) => !isIgnored(changedFile)); + if (filePath) { + deployDecision ??= { shouldSkip: false, reason: 'file_not_ignored', serviceName, filePath }; + } else { + skipServices.push(serviceName); } } - return { shouldSkip: true, reason: 'all_changed_files_ignored' }; + if (!deployDecision) { + return { shouldSkip: true, reason: 'all_changed_files_ignored' }; + } + + return skipServices.length > 0 ? { ...deployDecision, skipServices } : deployDecision; } diff --git a/src/server/lib/yamlSchemas/schema_1_0_0/schema_1_0_0.ts b/src/server/lib/yamlSchemas/schema_1_0_0/schema_1_0_0.ts index c7677c15..6c367071 100644 --- a/src/server/lib/yamlSchemas/schema_1_0_0/schema_1_0_0.ts +++ b/src/server/lib/yamlSchemas/schema_1_0_0/schema_1_0_0.ts @@ -143,6 +143,7 @@ const schema_1_0_0 = { properties: { name: { type: 'string' }, ignoreFiles, + includeFiles: ignoreFiles, appShort: { type: 'string' }, defaultUUID: { type: 'string' }, requires: { diff --git a/src/server/models/yaml/YamlService.ts b/src/server/models/yaml/YamlService.ts index 80c0c6fe..f3cac814 100644 --- a/src/server/models/yaml/YamlService.ts +++ b/src/server/models/yaml/YamlService.ts @@ -55,6 +55,7 @@ export interface DevConfig { export interface Service { readonly name: string; readonly ignoreFiles?: string[]; + readonly includeFiles?: string[]; appShort?: string; readonly defaultUUID?: string; readonly requires?: DependencyService[]; diff --git a/src/server/services/__tests__/build.test.ts b/src/server/services/__tests__/build.test.ts index 1b4329b4..64c7ace7 100644 --- a/src/server/services/__tests__/build.test.ts +++ b/src/server/services/__tests__/build.test.ts @@ -1475,6 +1475,26 @@ describe('BuildService deployment reconciliation', () => { }); }); + test('carries skipped services on a repository-scoped push only', async () => { + const { service } = serviceHarness(); + const push = { + buildId: 1, + sourceGithubRepositoryId: 100, + sourceBranch: 'main', + sourceRef: 'commit-c', + skipServices: ['psp-web'], + }; + + await service.enqueueResolveAndDeployBuild({ ...push, githubRepositoryId: 100 }); + await service.enqueueResolveAndDeployBuild(push); + + expect(mockAcceptDeploymentIntent.mock.calls[0][1]).toEqual( + expect.objectContaining({ target: 'repository', skipServices: ['psp-web'] }) + ); + expect(mockAcceptDeploymentIntent.mock.calls[1][1]).toEqual(expect.objectContaining({ target: 'all' })); + expect(mockAcceptDeploymentIntent.mock.calls[1][1]).not.toHaveProperty('skipServices'); + }); + test('uses the legacy trigger ref as the immutable source when a tracked push omits sourceRef', async () => { const { service } = serviceHarness(); diff --git a/src/server/services/__tests__/github.test.ts b/src/server/services/__tests__/github.test.ts index eda9cd64..fd944ff4 100644 --- a/src/server/services/__tests__/github.test.ts +++ b/src/server/services/__tests__/github.test.ts @@ -1044,6 +1044,70 @@ describe('Github Service - handlePushWebhook', () => { expect(mockLoggerInfo).toHaveBeenCalledWith('Push: deploying reason=ignoreFiles_not_matched'); }); + describe('per-service ignoreFiles', () => { + const monorepoConfig = { + version: '1.0.0', + environment: {}, + services: [ + { name: 'next-web', ignoreFiles: ['apps/psp/**'] }, + { name: 'psp-web', includeFiles: ['apps/psp/**', 'packages/**'] }, + ], + }; + + const pushMonorepo = async (features: Record) => { + const buildId = 100; + const nextWeb = { + ...createMockDeploy(buildId, DeployStatus.READY, 1), + deployable: { name: 'next-web', defaultBranchName: 'main' }, + }; + const pspWeb = { + ...createMockDeploy(buildId, DeployStatus.READY, 2), + deployable: { name: 'psp-web', defaultBranchName: 'main' }, + }; + mockDb.services.GlobalConfig.getAllConfigs.mockResolvedValue({ features }); + (YamlService.fetchLifecycleConfig as jest.Mock).mockResolvedValue(monorepoConfig as any); + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([nextWeb, pspWeb]) : createFailedDeploysQuery([]); + }); + + await githubService.handlePushWebhook({ + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + commits: [{ added: [], removed: [], modified: ['apps/next-web/src/page.tsx'] }], + distinct_size: 1, + } as PushEvent); + + return buildId; + }; + + test('redeploys only the services whose policy does not ignore the push', async () => { + const buildId = await pushMonorepo({ ignoreFiles: true, ignoreFilesPerService: true }); + + expect(mockLoggerInfo).toHaveBeenCalledWith('Push: skipped services reason=ignoreFiles'); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith( + 'resolve-deploy', + expect.objectContaining({ buildId, githubRepositoryId: 12345, skipServices: ['psp-web'] }) + ); + expect(mockDb.services.Webhook.webhookQueue.add).not.toHaveBeenCalled(); + }); + + test.each([ + ['the per-service flag is off', { ignoreFiles: true }], + ['ignoreFiles is off', { ignoreFiles: false, ignoreFilesPerService: true }], + ])('dry-runs per-service skips when %s', async (_label, features) => { + await pushMonorepo(features); + + expect(mockLoggerInfo).toHaveBeenCalledWith('Push: dry-run would skip services reason=ignoreFiles'); + const payload = mockDb.services.BuildService.resolveAndDeployBuildQueue.add.mock.calls[0][1]; + expect(payload).toEqual(expect.objectContaining({ githubRepositoryId: 12345 })); + expect(payload).not.toHaveProperty('skipServices'); + }); + }); + test('always redeploys failed deploys without fetching changed files', async () => { const buildId = 100; const mockDeploy = createMockDeploy(buildId); diff --git a/src/server/services/build.ts b/src/server/services/build.ts index 7759aca2..17800a9b 100644 --- a/src/server/services/build.ts +++ b/src/server/services/build.ts @@ -83,6 +83,7 @@ import { acceptDeploymentIntent, deploymentIntentDeployFilter, deploymentIntentSelectsAllDeploys, + excludeSkippedServiceDeploys, deployStampForGeneration, recordIntentConfigFailures, type AcceptedDeploymentIntent, @@ -215,6 +216,8 @@ interface DeploymentReconciliationRequest { sourceGithubRepositoryId?: number | null; sourceRef?: string | null; sourceBeforeRef?: string | null; + /** Repository-scoped pushes only: services whose ignore policy matched every changed file. */ + skipServices?: string[] | null; runUUID?: string | null; origin?: BuildEventOrigin | null; } @@ -330,6 +333,7 @@ export default class BuildService extends BaseService { sourceGithubRepositoryId, sourceRef, sourceBeforeRef, + skipServices, }: DeploymentReconciliationRequest, requestId: string ): DeploymentIntent { @@ -343,6 +347,7 @@ export default class BuildService extends BaseService { branch: sourceBranch, sha: immutableSourceRef, ...(sourceBeforeRef ? { beforeSha: sourceBeforeRef } : {}), + ...(githubRepositoryId != null && skipServices?.length ? { skipServices } : {}), }; } if (githubRepositoryId != null) { @@ -4405,11 +4410,15 @@ export default class BuildService extends BaseService { } private async adoptDeploysForIntent(buildId: number, intent: DeploymentIntent, generation: number): Promise { - await this.db.models.Deploy.query() - .patch(deployStampForGeneration(generation)) - .where({ buildId, active: true, ...deploymentIntentDeployFilter(intent) }) - .where('desiredGeneration', '<', generation) - .where('observedGeneration', '<', generation); + await excludeSkippedServiceDeploys( + this.db.models.Deploy.query() + .patch(deployStampForGeneration(generation)) + .where({ buildId, active: true, ...deploymentIntentDeployFilter(intent) }) + .where('desiredGeneration', '<', generation) + .where('observedGeneration', '<', generation), + buildId, + intent + ); } /** diff --git a/src/server/services/github.ts b/src/server/services/github.ts index f2212fc8..1f7b65ef 100644 --- a/src/server/services/github.ts +++ b/src/server/services/github.ts @@ -45,6 +45,7 @@ import { redisClient } from 'server/lib/dependencies'; import RepositoryService from './repository'; import { getEffectiveIgnoreFiles, + normalizeIncludeFiles, PushIgnoreDecision, PushIgnoreServicePolicy, shouldSkipPushDeploy, @@ -52,6 +53,7 @@ import { const SKIPPED_PUSH_WEBHOOK_STATUSES = new Set([BuildStatus.DEPLOYED, BuildStatus.ERROR, BuildStatus.TORN_DOWN]); const IGNORE_FILES_FEATURE_FLAG = 'ignoreFiles'; +const IGNORE_FILES_PER_SERVICE_FEATURE_FLAG = 'ignoreFilesPerService'; interface PullRequestPatchState { deployLabelPresent: boolean; @@ -429,6 +431,7 @@ export default class GithubService extends Service { return { serviceName, ignoreFiles: getEffectiveIgnoreFiles(lifecycleConfig.environment?.ignoreFiles, service.ignoreFiles), + includeFiles: normalizeIncludeFiles(service.includeFiles), }; }); } @@ -465,14 +468,12 @@ export default class GithubService extends Service { } } - private async isIgnoreFilesFeatureEnabled(): Promise { + private async isIgnoreFilesFeatureEnabled(featureFlag = IGNORE_FILES_FEATURE_FLAG): Promise { try { const { features } = await this.db.services.GlobalConfig.getAllConfigs(); - return features?.[IGNORE_FILES_FEATURE_FLAG] === true; + return features?.[featureFlag] === true; } catch (error) { - getLogger({ error, featureFlag: IGNORE_FILES_FEATURE_FLAG }).warn( - 'Push: ignoreFiles feature flag fetch failed, using dry-run mode' - ); + getLogger({ error, featureFlag }).warn('Push: ignoreFiles feature flag fetch failed, using dry-run mode'); return false; } } @@ -618,6 +619,7 @@ export default class GithubService extends Service { for (const build of buildsToDeploy) { const buildId = build?.id; + let skipServices: string[] | undefined; if (!buildId) { getLogger().error(`Build: id not found for=handlePushWebhook`); } @@ -679,6 +681,24 @@ export default class GithubService extends Service { serviceName: skipDecision.serviceName, filePath: skipDecision.filePath, }).info('Push: deploying reason=ignoreFiles_not_matched'); + + if (skipDecision.skipServices) { + const perServiceEnabled = + (await this.isIgnoreFilesFeatureEnabled()) && + (await this.isIgnoreFilesFeatureEnabled(IGNORE_FILES_PER_SERVICE_FEATURE_FLAG)); + getLogger({ + buildId, + repo: repoName, + branch: branchName, + skipServices: skipDecision.skipServices, + perServiceEnabled, + }).info( + perServiceEnabled + ? 'Push: skipped services reason=ignoreFiles' + : 'Push: dry-run would skip services reason=ignoreFiles' + ); + if (perServiceEnabled) skipServices = skipDecision.skipServices; + } } } else { getLogger({ @@ -701,6 +721,7 @@ export default class GithubService extends Service { ...(!this.isVoidCommit(previousCommit) ? { sourceBeforeRef: previousCommit } : {}), sourceGithubRepositoryId: githubRepositoryId, sourceBranch: branchName, + ...(skipServices ? { skipServices } : {}), }); } } catch (error) {