From 210c99f40077f2f7f113c3749ab8612cbb15d956 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Sun, 26 Apr 2026 14:29:34 -0700 Subject: [PATCH] feat: add ignoreFiles push redeploy skips --- docs/schema/yaml/1.0.0.yaml | 8 + .../lib/__tests__/pushIgnoreFiles.test.ts | 145 +++++++ src/server/lib/github/__tests__/index.test.ts | 143 +++++-- src/server/lib/github/__tests__/utils.test.ts | 11 +- src/server/lib/github/cacheRequest.ts | 22 +- src/server/lib/github/index.ts | 86 ++++ src/server/lib/jsonschema/schemas/1.0.0.json | 14 +- src/server/lib/pushIgnoreFiles.ts | 153 +++++++ .../yamlSchemas/schema_1_0_0/schema_1_0_0.ts | 7 + src/server/models/yaml/YamlEnvironment.ts | 1 + src/server/models/yaml/YamlService.ts | 1 + .../models/yaml/tests/YamlService.test.ts | 23 ++ src/server/models/yaml/types.ts | 4 + src/server/services/__tests__/github.test.ts | 388 +++++++++++++++++- src/server/services/github.ts | 216 +++++++++- 15 files changed, 1181 insertions(+), 41 deletions(-) create mode 100644 src/server/lib/__tests__/pushIgnoreFiles.test.ts create mode 100644 src/server/lib/pushIgnoreFiles.ts diff --git a/docs/schema/yaml/1.0.0.yaml b/docs/schema/yaml/1.0.0.yaml index a7dc2371..315c6f32 100644 --- a/docs/schema/yaml/1.0.0.yaml +++ b/docs/schema/yaml/1.0.0.yaml @@ -113,6 +113,10 @@ environment: branch: '' # @param environment.agentSession.skills.path (required) path: '' + # @param environment.ignoreFiles + ignoreFiles: + # @param environment.ignoreFiles[] + - '' # @section services services: # @param services[] @@ -778,3 +782,7 @@ services: branch: '' # @param services.dev.agentSession.skills.path (required) path: '' + # @param services.ignoreFiles + ignoreFiles: + # @param services.ignoreFiles[] + - '' \ No newline at end of file diff --git a/src/server/lib/__tests__/pushIgnoreFiles.test.ts b/src/server/lib/__tests__/pushIgnoreFiles.test.ts new file mode 100644 index 00000000..f7dc526e --- /dev/null +++ b/src/server/lib/__tests__/pushIgnoreFiles.test.ts @@ -0,0 +1,145 @@ +/** + * Copyright 2026 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + getEffectiveIgnoreFiles, + getServicePushIgnorePolicy, + hasLifecycleConfigChange, + normalizeIgnoreFiles, + shouldSkipPushDeploy, +} from '../pushIgnoreFiles'; + +describe('pushIgnoreFiles', () => { + test('merges environment and service ignoreFiles uniquely', () => { + expect(getEffectiveIgnoreFiles(['docs/**', '**/*.spec.ts'], ['docs/**', '**/*.stories.tsx'])).toEqual([ + 'docs/**', + '**/*.spec.ts', + '**/*.stories.tsx', + ]); + }); + + test('rejects invalid ignore patterns', () => { + expect(() => normalizeIgnoreFiles([''])).toThrow('cannot be empty'); + expect(() => normalizeIgnoreFiles(['/tmp/**'])).toThrow('repo-relative'); + expect(() => normalizeIgnoreFiles(['../secrets/**'])).toThrow('traverse'); + expect(() => normalizeIgnoreFiles(['docs/..'])).toThrow('traverse'); + expect(() => normalizeIgnoreFiles([123])).toThrow('must be strings'); + expect(() => normalizeIgnoreFiles(Array.from({ length: 51 }, (_value, index) => `docs/${index}.md`))).toThrow( + 'too many patterns' + ); + expect(() => normalizeIgnoreFiles(['a'.repeat(201)])).toThrow('exceeds maximum length'); + }); + + test('builds service policy from config inheritance', () => { + const policy = getServicePushIgnorePolicy( + { + version: '1.0.0', + environment: { ignoreFiles: ['docs/**'] }, + services: [ + { + name: 'api', + ignoreFiles: ['**/*.spec.ts'], + }, + ], + } as any, + 'api' + ); + + expect(policy).toEqual({ + serviceName: 'api', + ignoreFiles: ['docs/**', '**/*.spec.ts'], + }); + }); + + test('builds service-only policy and returns null when no policy exists', () => { + const config = { + version: '1.0.0', + environment: {}, + services: [ + { + name: 'api', + ignoreFiles: ['docs/**'], + }, + { + name: 'worker', + }, + ], + } as any; + + expect(getServicePushIgnorePolicy(config, 'api')).toEqual({ + serviceName: 'api', + ignoreFiles: ['docs/**'], + }); + expect(getServicePushIgnorePolicy(config, 'worker')).toBeNull(); + expect(getServicePushIgnorePolicy(config, 'missing')).toBeNull(); + }); + + test('matches paths case-sensitively with broad glob support', () => { + expect( + shouldSkipPushDeploy({ + changedFiles: ['src/api.spec.ts', '.github/workflows/test.yml'], + servicePolicies: [{ serviceName: 'api', ignoreFiles: ['**/*'] }], + }) + ).toEqual({ shouldSkip: true, reason: 'all_changed_files_ignored' }); + + expect( + shouldSkipPushDeploy({ + changedFiles: ['src/API.spec.ts'], + servicePolicies: [{ serviceName: 'api', ignoreFiles: ['src/api.spec.ts'] }], + }) + ).toEqual({ + shouldSkip: false, + reason: 'file_not_ignored', + serviceName: 'api', + filePath: 'src/API.spec.ts', + }); + }); + + test('requires every changed file to match every affected service policy', () => { + expect( + shouldSkipPushDeploy({ + changedFiles: ['docs/readme.md', 'src/api.ts'], + servicePolicies: [{ serviceName: 'api', ignoreFiles: ['docs/**'] }], + }) + ).toEqual({ + shouldSkip: false, + reason: 'file_not_ignored', + serviceName: 'api', + filePath: 'src/api.ts', + }); + }); + + test('fails open when changed files or service policies are missing', () => { + expect(shouldSkipPushDeploy({ changedFiles: [], servicePolicies: [] })).toEqual({ + shouldSkip: false, + reason: 'no_changed_files', + }); + expect( + shouldSkipPushDeploy({ + changedFiles: ['docs/readme.md'], + servicePolicies: [], + }) + ).toEqual({ + shouldSkip: false, + reason: 'no_service_policies', + }); + }); + + test('detects lifecycle config changes by new path', () => { + expect(hasLifecycleConfigChange(['docs/readme.md', 'lifecycle.yaml'])).toBe(true); + expect(hasLifecycleConfigChange(['docs/lifecycle.yml'])).toBe(false); + }); +}); diff --git a/src/server/lib/github/__tests__/index.test.ts b/src/server/lib/github/__tests__/index.test.ts index 56947a65..d34c2a37 100644 --- a/src/server/lib/github/__tests__/index.test.ts +++ b/src/server/lib/github/__tests__/index.test.ts @@ -26,8 +26,11 @@ import { verifyWebhookSignature, getSHAForBranch, checkIfCommentExists, + getChangedFilesFromPushPayload, + getChangedFilesForPush, } from 'server/lib/github'; import * as client from 'server/lib/github/client'; +import { cacheRequest } from 'server/lib/github/cacheRequest'; jest.mock('server/services/globalConfig', () => { const RedisMock = { @@ -43,6 +46,7 @@ jest.mock('server/services/globalConfig', () => { }); jest.mock('server/lib/github/client'); +jest.mock('server/lib/github/cacheRequest'); jest.mock('server/lib/github/utils'); jest.mock('server/lib/logger', () => ({ getLogger: jest.fn().mockReturnValue({ @@ -52,7 +56,11 @@ jest.mock('server/lib/logger', () => ({ warn: jest.fn(), }), })); -import { getLogger, rootLogger as logger } from 'server/lib/logger'; +import { getLogger } from 'server/lib/logger'; + +beforeEach(() => { + jest.clearAllMocks(); +}); test('createOrUpdatePullRequestComment success', async () => { jest.spyOn(client, 'createOctokitClient').mockResolvedValue({ @@ -71,37 +79,32 @@ test('createOrUpdatePullRequestComment success', async () => { }); test('getPullRequest success', async () => { - jest.spyOn(client, 'createOctokitClient').mockResolvedValue({ - request: jest.fn().mockResolvedValue({ data: 'foo' }), - }); - const result = await getPullRequest('foo', 'bar', 1, 123, logger); + (cacheRequest as jest.Mock).mockResolvedValue({ data: 'foo' }); + const result = await getPullRequest('foo', 'bar', 1, 123); expect(result.data).toEqual('foo'); }); test('getPullRequestByRepositoryFullName success', async () => { - jest.spyOn(client, 'createOctokitClient').mockResolvedValue({ - request: jest.fn().mockResolvedValue({ data: 'foo' }), - }); - const result = await getPullRequestByRepositoryFullName('example-org/example-repo', 123, 1); + (cacheRequest as jest.Mock).mockResolvedValue({ data: 'foo' }); + const result = await getPullRequestByRepositoryFullName('example-org/example-repo', 123); expect(result.data).toEqual('foo'); }); test('getPullRequestByRepositoryFullName failure', async () => { - jest.spyOn(client, 'createOctokitClient').mockResolvedValue({ - request: jest.fn().mockRejectedValue(new Error('error')), - }); - await expect(getPullRequestByRepositoryFullName('example-org/example-repo', 123, 1)).rejects.toThrow(); + (cacheRequest as jest.Mock).mockRejectedValue(new Error('error')); + await expect(getPullRequestByRepositoryFullName('example-org/example-repo', 123)).rejects.toThrow(); }); test('getPullRequestByRepositoryFullName invalid repository name', async () => { - await expect(getPullRequestByRepositoryFullName('foo', 123, 1)).rejects.toThrow(); + (cacheRequest as jest.Mock).mockRejectedValue(new Error('error')); + await expect(getPullRequestByRepositoryFullName('foo', 123)).rejects.toThrow(); }); test('createDeploy success', async () => { jest.spyOn(client, 'createOctokitClient').mockResolvedValue({ request: jest.fn().mockResolvedValue({ data: 'foo' }), }); - const result = await createDeploy('foo', 'bar', 'main', 1); + const result = await createDeploy({ repositoryId: 1, owner: 'foo', name: 'bar', branch: 'main', installationId: 1 }); expect(result.data).toEqual('foo'); }); @@ -109,7 +112,9 @@ test('createDeploy failure', async () => { jest.spyOn(client, 'createOctokitClient').mockResolvedValue({ request: jest.fn().mockRejectedValue(new Error('error')), }); - await expect(createDeploy('foo', 'bar', 'main', 1)).rejects.toThrow(); + await expect( + createDeploy({ repositoryId: 1, owner: 'foo', name: 'bar', branch: 'main', installationId: 1 }) + ).rejects.toThrow(); }); test('verifyWebhookSignature false', async () => { @@ -119,7 +124,7 @@ test('verifyWebhookSignature false', async () => { }, rawBody: 'foo', }; - const result = await verifyWebhookSignature(req as unknown as client.WebhookRequest, '123'); + const result = await verifyWebhookSignature(req as any); expect(result).toEqual(false); }); @@ -127,7 +132,7 @@ test('verifyWebhookSignature missing header', async () => { const req = { body: { foo: 'bar' }, }; - const result = await verifyWebhookSignature(req as unknown as NextApiRequest); + const result = await verifyWebhookSignature(req as any); expect(result).toEqual(false); }); @@ -150,9 +155,7 @@ test('getSHAForBranch failure', async () => { test('checkIfCommentExists to return true', async () => { const mockComments = [{ body: 'This is a test comment' }, { body: `This comment contains the uniqueIdentifier` }]; - jest.spyOn(client, 'createOctokitClient').mockResolvedValue({ - request: jest.fn().mockResolvedValue({ data: mockComments }), - }); + (cacheRequest as jest.Mock).mockResolvedValue({ data: mockComments }); const result = await checkIfCommentExists({ fullName: 'example-org/example-repo', pullRequestNumber: 123, @@ -165,9 +168,7 @@ test('checkIfCommentExists to return true', async () => { test('checkIfCommentExists to return false', async () => { const mockComments = [{ body: 'This is a test comment' }, { body: `This comment contains the not` }]; - jest.spyOn(client, 'createOctokitClient').mockResolvedValue({ - request: jest.fn().mockResolvedValue({ data: mockComments }), - }); + (cacheRequest as jest.Mock).mockResolvedValue({ data: mockComments }); const result = await checkIfCommentExists({ fullName: 'example-org/example-repo', pullRequestNumber: 123, @@ -175,3 +176,97 @@ test('checkIfCommentExists to return false', async () => { }); expect(result).toBe(false); }); + +test('getChangedFilesForPush returns current filenames from compare responses', async () => { + (cacheRequest as jest.Mock).mockResolvedValue({ + headers: { + 'x-ratelimit-remaining': '4999', + 'x-ratelimit-reset': '1770000000', + }, + data: { + files: [ + { + filename: 'src/new-name.ts', + previous_filename: 'src/old-name.ts', + status: 'renamed', + }, + ], + }, + }); + + const result = await getChangedFilesForPush({ + fullName: 'example-org/example-repo', + before: 'before-sha', + after: 'after-sha', + }); + + expect(cacheRequest).toHaveBeenCalledWith('GET /repos/example-org/example-repo/compare/before-sha...after-sha'); + expect(result).toEqual({ canSkip: true, files: ['src/new-name.ts'] }); +}); + +test('getChangedFilesFromPushPayload returns unique added and modified files', () => { + expect( + getChangedFilesFromPushPayload({ + commits: [ + { + added: ['src/new.ts'], + modified: ['docs/readme.md'], + }, + { + modified: ['docs/readme.md', 'src/app.ts'], + }, + ], + commitCount: 2, + }) + ).toEqual({ canSkip: true, files: ['src/new.ts', 'docs/readme.md', 'src/app.ts'] }); +}); + +test('getChangedFilesFromPushPayload falls back for incomplete or removed-file payloads', () => { + expect( + getChangedFilesFromPushPayload({ + commits: [{ modified: ['src/app.ts'] }], + commitCount: 2, + }) + ).toEqual({ canSkip: false, files: [], reason: 'payload_commits_incomplete' }); + + expect( + getChangedFilesFromPushPayload({ + commits: [{ removed: ['src/old.ts'] }], + commitCount: 1, + }) + ).toEqual({ canSkip: false, files: [], reason: 'payload_has_removed_files' }); +}); + +test('getChangedFilesForPush fails open for large compare file lists', async () => { + (cacheRequest as jest.Mock).mockResolvedValue({ + data: { + files: Array.from({ length: 300 }, (_value, index) => ({ + filename: `file-${index}.ts`, + })), + }, + }); + + await expect( + getChangedFilesForPush({ + fullName: 'example-org/example-repo', + before: 'before-sha', + after: 'after-sha', + }) + ).resolves.toEqual({ canSkip: false, files: [], reason: 'large_or_incomplete_compare' }); +}); + +test('getChangedFilesForPush fails open when compare cannot provide filenames', async () => { + (cacheRequest as jest.Mock).mockResolvedValue({ + data: { + files: [{ filename: 'src/api.ts' }, { status: 'removed' }], + }, + }); + + await expect( + getChangedFilesForPush({ + fullName: 'example-org/example-repo', + before: 'before-sha', + after: 'after-sha', + }) + ).resolves.toEqual({ canSkip: false, files: [], reason: 'missing_file_names' }); +}); diff --git a/src/server/lib/github/__tests__/utils.test.ts b/src/server/lib/github/__tests__/utils.test.ts index 031536cd..9959dd97 100644 --- a/src/server/lib/github/__tests__/utils.test.ts +++ b/src/server/lib/github/__tests__/utils.test.ts @@ -35,7 +35,16 @@ jest.mock('server/services/globalConfig', () => { jest.mock('server/lib/github/client'); -jest.mock('server/lib/logger/rootLogger'); +jest.mock('server/lib/logger/rootLogger', () => ({ + __esModule: true, + default: { + child: jest.fn(() => ({ + debug: jest.fn(), + error: jest.fn(), + info: jest.fn(), + })), + }, +})); import { rootLogger as logger } from 'server/lib/logger'; diff --git a/src/server/lib/github/cacheRequest.ts b/src/server/lib/github/cacheRequest.ts index 04a511bd..733678fe 100644 --- a/src/server/lib/github/cacheRequest.ts +++ b/src/server/lib/github/cacheRequest.ts @@ -56,14 +56,22 @@ export async function cacheRequest( ); await cache.expire(cacheKey, GITHUB_API_CACHE_EXPIRATION_SECONDS); + getLogger({ + endpoint, + cacheHit: false, + rateLimitRemaining: respHeaders?.['x-ratelimit-remaining'], + rateLimitReset: respHeaders?.['x-ratelimit-reset'], + }).debug('GitHub: cache request fetched'); + return resp; } catch (error) { if (error?.status === 304) { const cachedData = cached?.data; try { if (!cachedData) throw new Error('No cached data'); - const data = JSON.parse(cached?.data); - return { data }; + const data = JSON.parse(cachedData); + getLogger({ endpoint, cacheHit: true }).debug('GitHub: cache request hit'); + return { data, cacheHit: true }; } catch (error) { return cacheRequest(endpoint, requestData, { cache, ignoreCache: true }); } @@ -71,8 +79,14 @@ export async function cacheRequest( getLogger().info(`GitHub: cache request not found endpoint=${endpoint}`); throw new Error('Resource not found'); } else { - getLogger().error({ error }, `GitHub: cache request failed endpoint=${endpoint}`); - throw new Error('GitHub API request failed'); + const errorHeaders = error?.response?.headers || error?.headers; + getLogger({ + error, + endpoint, + rateLimitRemaining: errorHeaders?.['x-ratelimit-remaining'], + rateLimitReset: errorHeaders?.['x-ratelimit-reset'], + }).error('GitHub: cache request failed'); + throw Object.assign(new Error('GitHub API request failed'), { headers: errorHeaders }); } } } diff --git a/src/server/lib/github/index.ts b/src/server/lib/github/index.ts index 804e62eb..b1f03bf7 100644 --- a/src/server/lib/github/index.ts +++ b/src/server/lib/github/index.ts @@ -28,6 +28,48 @@ import { getRefForBranchName } from 'server/lib/github/utils'; import { Deploy } from 'server/models'; import { LifecycleYamlConfigOptions } from 'server/models/yaml/types'; +const MAX_COMPARE_FILES = 300; + +export interface ChangedFilesForPushResult { + canSkip: boolean; + files: string[]; + reason?: string; +} + +interface PushPayloadCommitFiles { + added?: string[]; + removed?: string[]; + modified?: string[]; +} + +export function getChangedFilesFromPushPayload({ + commits, + commitCount, +}: { + commits?: PushPayloadCommitFiles[]; + commitCount?: number; +}): ChangedFilesForPushResult { + if (!Array.isArray(commits) || commits.length === 0) { + return { canSkip: false, files: [], reason: 'payload_missing_commits' }; + } + + if (typeof commitCount === 'number' && commits.length < commitCount) { + return { canSkip: false, files: [], reason: 'payload_commits_incomplete' }; + } + + if (commits.some((commit) => Array.isArray(commit.removed) && commit.removed.length > 0)) { + return { canSkip: false, files: [], reason: 'payload_has_removed_files' }; + } + + const changedFiles = commits.flatMap((commit) => [...(commit.added || []), ...(commit.modified || [])]); + const uniqueChangedFiles = Array.from(new Set(changedFiles.filter(Boolean))); + if (uniqueChangedFiles.length === 0) { + return { canSkip: false, files: [], reason: 'payload_missing_files' }; + } + + return { canSkip: true, files: uniqueChangedFiles }; +} + export async function getRepositoryByFullName(fullName: string, installationId: number) { try { const client = await createOctokitClient({ installationId, caller: 'getRepositoryByFullName' }); @@ -239,6 +281,50 @@ export async function getSHAForBranch(branchName: string, owner: string, name: s } } +export async function getChangedFilesForPush({ + fullName, + before, + after, +}: { + fullName: string; + before: string; + after: string; +}): Promise { + try { + const response = await cacheRequest(`GET /repos/${fullName}/compare/${before}...${after}`); + const files = response?.data?.files; + if (!Array.isArray(files) || files.length === 0) { + return { canSkip: false, files: [], reason: 'no_files' }; + } + + if (files.length >= MAX_COMPARE_FILES) { + getLogger({ repo: fullName, fileCount: files.length }).info( + 'GitHub: compare file list may be incomplete, redeploying' + ); + return { canSkip: false, files: [], reason: 'large_or_incomplete_compare' }; + } + + const changedFiles = files + .map((file) => file?.filename) + .filter((fileName): fileName is string => typeof fileName === 'string' && fileName.length > 0); + + if (changedFiles.length !== files.length) { + return { canSkip: false, files: [], reason: 'missing_file_names' }; + } + + return { canSkip: true, files: changedFiles }; + } catch (error) { + const headers = error?.response?.headers || error?.headers; + getLogger({ + error, + repo: fullName, + rateLimitRemaining: headers?.['x-ratelimit-remaining'], + rateLimitReset: headers?.['x-ratelimit-reset'], + }).warn('GitHub: compare fetch failed, redeploying'); + return { canSkip: false, files: [], reason: 'compare_fetch_failed' }; + } +} + export async function getYamlFileContent({ fullName, branch = '', sha = '', isJSON = false }) { try { const identifier = sha?.length > 0 ? sha : branch; diff --git a/src/server/lib/jsonschema/schemas/1.0.0.json b/src/server/lib/jsonschema/schemas/1.0.0.json index 77aa2e73..1dc67d2e 100644 --- a/src/server/lib/jsonschema/schemas/1.0.0.json +++ b/src/server/lib/jsonschema/schemas/1.0.0.json @@ -243,6 +243,12 @@ } } } + }, + "ignoreFiles": { + "type": "array", + "items": { + "type": "string" + } } } }, @@ -1507,6 +1513,12 @@ "image", "command" ] + }, + "ignoreFiles": { + "type": "array", + "items": { + "type": "string" + } } }, "required": [ @@ -1515,4 +1527,4 @@ } } } -} +} \ No newline at end of file diff --git a/src/server/lib/pushIgnoreFiles.ts b/src/server/lib/pushIgnoreFiles.ts new file mode 100644 index 00000000..18b8d96c --- /dev/null +++ b/src/server/lib/pushIgnoreFiles.ts @@ -0,0 +1,153 @@ +/** + * Copyright 2026 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import picomatch from 'picomatch'; +import type { LifecycleConfig } from 'server/models/yaml/Config'; + +const MAX_IGNORE_PATTERNS = 50; +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[]; +} + +export interface PushIgnoreDecision { + shouldSkip: boolean; + reason: string; + serviceName?: string; + filePath?: string; +} + +export function normalizeGithubPath(filePath: string): string { + return filePath.replace(/\\/g, '/').replace(/^\.\//, '').replace(/^\/+/, ''); +} + +function validateIgnorePattern(rawPattern: unknown): string { + if (typeof rawPattern !== 'string') { + throw new Error(`ignoreFiles patterns must be strings`); + } + + const pattern = rawPattern.trim().replace(/\\/g, '/'); + if (!pattern) { + throw new Error('ignoreFiles 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}"`); + } + + if (pattern.startsWith('/')) { + throw new Error(`ignoreFiles patterns must be repo-relative: "${pattern}"`); + } + + if (pattern.split('/').some((segment) => segment === '..')) { + throw new Error(`ignoreFiles patterns cannot traverse directories: "${pattern}"`); + } + + return pattern.replace(/^\.\//, ''); +} + +export function normalizeIgnoreFiles(rawPatterns: unknown): string[] { + if (rawPatterns == null) { + return []; + } + + if (!Array.isArray(rawPatterns)) { + throw new Error('ignoreFiles 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}`); + } + + return Array.from(new Set(rawPatterns.map(validateIgnorePattern))); +} + +export function getEffectiveIgnoreFiles(environmentIgnoreFiles: unknown, serviceIgnoreFiles: unknown): string[] { + return Array.from( + new Set([...normalizeIgnoreFiles(environmentIgnoreFiles), ...normalizeIgnoreFiles(serviceIgnoreFiles)]) + ); +} + +export function getServicePushIgnorePolicy( + config: LifecycleConfig, + serviceName: string +): PushIgnoreServicePolicy | null { + const service = config?.services?.find((candidate) => candidate.name === serviceName); + if (!service) { + return null; + } + + const ignoreFiles = getEffectiveIgnoreFiles(config.environment?.ignoreFiles, service.ignoreFiles); + if (ignoreFiles.length === 0) { + return null; + } + + return { + serviceName, + ignoreFiles, + }; +} + +export function hasLifecycleConfigChange(changedFiles: string[]): boolean { + return changedFiles.some((filePath) => { + const normalized = normalizeGithubPath(filePath); + return LIFECYCLE_CONFIG_FILE_PATHS.has(normalized); + }); +} + +export function shouldSkipPushDeploy({ + changedFiles, + servicePolicies, +}: { + changedFiles: string[]; + servicePolicies: PushIgnoreServicePolicy[]; +}): PushIgnoreDecision { + const normalizedChangedFiles = changedFiles.map(normalizeGithubPath).filter(Boolean); + + if (normalizedChangedFiles.length === 0) { + return { shouldSkip: false, reason: 'no_changed_files' }; + } + + if (hasLifecycleConfigChange(normalizedChangedFiles)) { + return { shouldSkip: false, reason: 'lifecycle_config_changed' }; + } + + if (servicePolicies.length === 0) { + return { shouldSkip: false, reason: 'no_service_policies' }; + } + + for (const servicePolicy of servicePolicies) { + if (servicePolicy.ignoreFiles.length === 0) { + return { shouldSkip: false, reason: 'missing_service_policy', serviceName: servicePolicy.serviceName }; + } + + for (const filePath of normalizedChangedFiles) { + if (!picomatch.isMatch(filePath, servicePolicy.ignoreFiles, { dot: true })) { + return { + shouldSkip: false, + reason: 'file_not_ignored', + serviceName: servicePolicy.serviceName, + filePath, + }; + } + } + } + + return { shouldSkip: true, reason: 'all_changed_files_ignored' }; +} 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 93aee5df..f1d394be 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 @@ -72,6 +72,11 @@ const agentSessionSkills = { items: agentSessionSkillRef, }; +const ignoreFiles = { + type: 'array', + items: { type: 'string' }, +}; + const schema_1_0_0 = { id: 'schema-1.0.0', type: 'object', @@ -83,6 +88,7 @@ const schema_1_0_0 = { additionalProperties: false, properties: { enabledFeatures: { type: 'array' }, + ignoreFiles, autoDeploy: { type: 'boolean' }, githubDeployments: { type: 'boolean' }, useGithubStatusComment: { type: 'boolean' }, @@ -136,6 +142,7 @@ const schema_1_0_0 = { additionalProperties: false, properties: { name: { type: 'string' }, + ignoreFiles, appShort: { type: 'string' }, defaultUUID: { type: 'string' }, requires: { diff --git a/src/server/models/yaml/YamlEnvironment.ts b/src/server/models/yaml/YamlEnvironment.ts index cacb0ef7..bb0f76f2 100644 --- a/src/server/models/yaml/YamlEnvironment.ts +++ b/src/server/models/yaml/YamlEnvironment.ts @@ -33,6 +33,7 @@ export interface AgentSessionEnvironmentConfig { } export interface Environment { + readonly ignoreFiles?: string[]; readonly defaultServices?: YamlService.DependencyService[]; readonly optionalServices?: YamlService.DependencyService[]; readonly webhooks?: YamlService.Webhook[]; diff --git a/src/server/models/yaml/YamlService.ts b/src/server/models/yaml/YamlService.ts index a98335ed..a902a204 100644 --- a/src/server/models/yaml/YamlService.ts +++ b/src/server/models/yaml/YamlService.ts @@ -53,6 +53,7 @@ export interface DevConfig { export interface Service { readonly name: string; + readonly ignoreFiles?: string[]; appShort?: string; readonly defaultUUID?: string; readonly requires?: DependencyService[]; diff --git a/src/server/models/yaml/tests/YamlService.test.ts b/src/server/models/yaml/tests/YamlService.test.ts index 216eeb1b..36d4d11b 100644 --- a/src/server/models/yaml/tests/YamlService.test.ts +++ b/src/server/models/yaml/tests/YamlService.test.ts @@ -279,6 +279,29 @@ services: expect(() => new YamlConfigValidator().validate_1_0_0(config)).not.toThrow(); }); + + test('accepts environment-level and service-level ignoreFiles', () => { + const parser = new YamlConfigParser(); + const config = parser.parseYamlConfigFromString(`--- +version: '1.0.0' +environment: + ignoreFiles: + - 'docs/**' +services: + - name: 'githubApp' + ignoreFiles: + - '**/*.spec.ts' + github: + repository: 'org/foobar' + branchName: 'main' + docker: + defaultTag: 'main' + app: + dockerfilePath: 'app1/app.Dockerfile' +`); + + expect(() => new YamlConfigValidator().validate_1_0_0(config)).not.toThrow(); + }); }); describe('isGithubService', () => { diff --git a/src/server/models/yaml/types.ts b/src/server/models/yaml/types.ts index 19469585..57e1c7af 100644 --- a/src/server/models/yaml/types.ts +++ b/src/server/models/yaml/types.ts @@ -14,9 +14,12 @@ * limitations under the License. */ +import type { Service as LifecycleYamlService } from './YamlService'; + export type LifecycleYamlConfigEnvironment = { autoDeploy?: boolean; githubDeployments?: boolean; + ignoreFiles?: string[]; defaultServices?: YamlService[]; optionalServices?: YamlService[]; webhooks?: YamlWebhook[]; @@ -40,6 +43,7 @@ export type LifecycleYamlConfigEnvironment = { export type LifecycleYamlConfigOptions = { environment: LifecycleYamlConfigEnvironment; + services?: LifecycleYamlService[]; }; export type YamlService = { diff --git a/src/server/services/__tests__/github.test.ts b/src/server/services/__tests__/github.test.ts index 5fcb4b5a..2f019952 100644 --- a/src/server/services/__tests__/github.test.ts +++ b/src/server/services/__tests__/github.test.ts @@ -17,9 +17,10 @@ import mockRedisClient from 'server/lib/__mocks__/redisClientMock'; import Github from '../github'; import RepositoryService from '../repository'; -import { DeployStatus, PullRequestStatus } from 'shared/constants'; +import { BuildStatus, DeployStatus, PullRequestStatus } from 'shared/constants'; import { PushEvent } from '@octokit/webhooks-types'; import * as githubLib from 'server/lib/github'; +import * as YamlService from 'server/models/yaml'; mockRedisClient(); @@ -30,6 +31,10 @@ const mockIsLifecycleLabel = jest.fn(); const mockHasDeployLabel = jest.fn(); const mockEnableKillSwitch = jest.fn(); const mockIsStaging = jest.fn().mockReturnValue(false); +const mockLoggerError = jest.fn(); +const mockLoggerInfo = jest.fn(); +const mockLoggerWarn = jest.fn(); +const mockLoggerDebug = jest.fn(); jest.mock('server/lib/utils', () => ({ ...jest.requireActual('server/lib/utils'), @@ -41,10 +46,10 @@ jest.mock('server/lib/utils', () => ({ jest.mock('server/lib/logger', () => ({ getLogger: jest.fn(() => ({ - error: jest.fn(), - info: jest.fn(), - warn: jest.fn(), - debug: jest.fn(), + error: mockLoggerError, + info: mockLoggerInfo, + warn: mockLoggerWarn, + debug: mockLoggerDebug, child: jest.fn().mockReturnThis(), })), withLogContext: jest.fn((_ctx, fn) => fn()), @@ -53,10 +58,17 @@ jest.mock('server/lib/logger', () => ({ })); jest.mock('server/lib/github', () => ({ + ...jest.requireActual('server/lib/github'), getYamlFileContent: jest.fn(), + getChangedFilesForPush: jest.fn(), verifyWebhookSignature: jest.fn(() => true), })); +jest.mock('server/models/yaml', () => ({ + ...jest.requireActual('server/models/yaml'), + fetchLifecycleConfig: jest.fn(), +})); + const createDedupeAwareResolveEnqueue = (queueAdd: jest.Mock) => { const queuedKeys = new Set(); @@ -387,6 +399,7 @@ describe('Github Service - handlePushWebhook', () => { let mockRedis: any; let mockRedlock: any; let mockQueueManager: any; + const mockGetChangedFilesForPush = githubLib.getChangedFilesForPush as jest.Mock; const createMockPushEvent = ( repoId: number = 12345, @@ -406,6 +419,7 @@ describe('Github Service - handlePushWebhook', () => { const createMockBuild = (buildId: number, buildUuid: string = 'test-uuid-123') => ({ id: buildId, uuid: buildUuid, + status: BuildStatus.DEPLOYED, enableFullYaml: false, trackDefaultBranches: true, pullRequest: { @@ -423,15 +437,30 @@ describe('Github Service - handlePushWebhook', () => { githubRepositoryId: 12345, build: createMockBuild(buildId), service: { + name: 'api', branchName: 'main', }, deployable: { + name: 'api', defaultBranchName: 'main', }, }); + const createAllDeploysQuery = (deploys: any[]) => ({ + where: jest.fn().mockReturnThis(), + whereNot: jest.fn().mockReturnThis(), + withGraphFetched: jest.fn().mockResolvedValue(deploys), + }); + + const createFailedDeploysQuery = (deploys: any[]) => ({ + where: jest.fn().mockReturnThis(), + whereIn: jest.fn().mockResolvedValue(deploys), + }); + beforeEach(() => { jest.clearAllMocks(); + mockGetChangedFilesForPush.mockReset(); + (YamlService.fetchLifecycleConfig as jest.Mock).mockReset(); const mockResolveQueueAdd = jest.fn().mockResolvedValue(undefined); const mockEnqueueResolveAndDeployBuild = createDedupeAwareResolveEnqueue(mockResolveQueueAdd); @@ -451,6 +480,18 @@ describe('Github Service - handlePushWebhook', () => { add: mockResolveQueueAdd, }, }, + Webhook: { + webhookQueue: { + add: jest.fn().mockResolvedValue(undefined), + }, + }, + GlobalConfig: { + getAllConfigs: jest.fn().mockResolvedValue({ + features: { + ignoreFiles: true, + }, + }), + }, }, }; @@ -612,6 +653,343 @@ describe('Github Service - handlePushWebhook', () => { }); }); + test('updates latestCommit for a matching previous commit', async () => { + const patch = jest.fn().mockResolvedValue(undefined); + const buildId = 100; + const mockBuild = { + ...createMockBuild(buildId), + pullRequest: { + status: PullRequestStatus.CLOSED, + deployOnUpdate: true, + }, + }; + const mockDeploy = { ...createMockDeploy(buildId), build: mockBuild }; + + mockDb.models.PullRequest.findOne.mockResolvedValue({ + $query: jest.fn().mockReturnValue({ patch }), + }); + mockDb.models.Deploy.query.mockReturnValue(createAllDeploysQuery([mockDeploy])); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockDb.models.PullRequest.findOne).toHaveBeenCalledWith({ latestCommit: 'previous-commit' }); + expect(patch).toHaveBeenCalledWith({ latestCommit: 'latest-commit' }); + expect(mockGetChangedFilesForPush).not.toHaveBeenCalled(); + }); + + test('skips deploys when every changed file matches ignoreFiles and queues deployed webhooks', async () => { + const buildId = 100; + const mockDeploy = createMockDeploy(buildId); + mockGetChangedFilesForPush.mockResolvedValue({ canSkip: true, files: ['docs/readme.md'] }); + (YamlService.fetchLifecycleConfig as jest.Mock).mockResolvedValue({ + version: '1.0.0', + environment: { ignoreFiles: ['docs/**'] }, + services: [{ name: 'api' }], + } as any); + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([mockDeploy]) : createFailedDeploysQuery([]); + }); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + commits: [ + { + added: [], + removed: [], + modified: ['docs/readme.md'], + }, + ], + distinct_size: 1, + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockGetChangedFilesForPush).not.toHaveBeenCalled(); + expect(YamlService.fetchLifecycleConfig).toHaveBeenCalledWith('test/repo', 'main'); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).not.toHaveBeenCalled(); + expect(mockDb.services.Webhook.webhookQueue.add).toHaveBeenCalledWith('webhook', { buildId }); + }); + + test('dry-runs ignoreFiles skips when the feature flag is false', async () => { + const buildId = 100; + const mockDeploy = createMockDeploy(buildId); + mockDb.services.GlobalConfig.getAllConfigs.mockResolvedValue({ features: { ignoreFiles: false } }); + (YamlService.fetchLifecycleConfig as jest.Mock).mockResolvedValue({ + version: '1.0.0', + environment: { ignoreFiles: ['docs/**'] }, + services: [{ name: 'api' }], + } as any); + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([mockDeploy]) : createFailedDeploysQuery([]); + }); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + commits: [ + { + added: [], + removed: [], + modified: ['docs/readme.md'], + }, + ], + distinct_size: 1, + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockDb.services.GlobalConfig.getAllConfigs).toHaveBeenCalled(); + expect(mockLoggerInfo).toHaveBeenCalledWith('Push: dry-run would skip deploy reason=ignoreFiles'); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { + buildId, + githubRepositoryId: 12345, + }); + expect(mockDb.services.Webhook.webhookQueue.add).not.toHaveBeenCalled(); + }); + + test('dry-runs ignoreFiles skips when the feature flag is missing', async () => { + const buildId = 100; + const mockDeploy = createMockDeploy(buildId); + mockDb.services.GlobalConfig.getAllConfigs.mockResolvedValue({ features: {} }); + (YamlService.fetchLifecycleConfig as jest.Mock).mockResolvedValue({ + version: '1.0.0', + environment: { ignoreFiles: ['docs/**'] }, + services: [{ name: 'api' }], + } as any); + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([mockDeploy]) : createFailedDeploysQuery([]); + }); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + commits: [ + { + added: [], + removed: [], + modified: ['docs/readme.md'], + }, + ], + distinct_size: 1, + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockLoggerInfo).toHaveBeenCalledWith('Push: dry-run would skip deploy reason=ignoreFiles'); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { + buildId, + githubRepositoryId: 12345, + }); + expect(mockDb.services.Webhook.webhookQueue.add).not.toHaveBeenCalled(); + }); + + test('falls back to compare when push payload cannot safely provide changed files', async () => { + const buildId = 100; + const mockDeploy = createMockDeploy(buildId); + mockGetChangedFilesForPush.mockResolvedValue({ canSkip: true, files: ['docs/readme.md'] }); + (YamlService.fetchLifecycleConfig as jest.Mock).mockResolvedValue({ + version: '1.0.0', + environment: { ignoreFiles: ['docs/**'] }, + services: [{ name: 'api' }], + } as any); + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([mockDeploy]) : createFailedDeploysQuery([]); + }); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + commits: [ + { + added: [], + removed: ['src/old.ts'], + modified: [], + }, + ], + distinct_size: 1, + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockGetChangedFilesForPush).toHaveBeenCalledWith({ + fullName: 'test/repo', + before: 'previous-commit', + after: 'latest-commit', + }); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).not.toHaveBeenCalled(); + expect(mockDb.services.Webhook.webhookQueue.add).toHaveBeenCalledWith('webhook', { buildId }); + }); + + test('redeploys when ignoreFiles config cannot be fetched', async () => { + const buildId = 100; + const mockDeploy = createMockDeploy(buildId); + mockGetChangedFilesForPush.mockResolvedValue({ canSkip: true, files: ['docs/readme.md'] }); + (YamlService.fetchLifecycleConfig as jest.Mock).mockRejectedValue(new Error('fetch failed')); + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([mockDeploy]) : createFailedDeploysQuery([]); + }); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { + buildId, + githubRepositoryId: 12345, + }); + expect(mockDb.services.Webhook.webhookQueue.add).not.toHaveBeenCalled(); + }); + + test('redeploys when an affected service has no ignoreFiles policy', async () => { + const buildId = 100; + const mockDeploy = createMockDeploy(buildId); + (YamlService.fetchLifecycleConfig as jest.Mock).mockResolvedValue({ + version: '1.0.0', + environment: {}, + services: [{ name: 'api' }], + } as any); + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([mockDeploy]) : createFailedDeploysQuery([]); + }); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + commits: [ + { + added: [], + removed: [], + modified: ['docs/readme.md'], + }, + ], + distinct_size: 1, + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { + buildId, + githubRepositoryId: 12345, + }); + expect(mockDb.services.Webhook.webhookQueue.add).not.toHaveBeenCalled(); + expect(mockLoggerInfo).toHaveBeenCalledWith('Push: deploying reason=ignoreFiles_not_matched'); + }); + + test('always redeploys failed deploys without fetching changed files', async () => { + const buildId = 100; + const mockDeploy = createMockDeploy(buildId); + const mockFailedDeploy = { id: 1, status: DeployStatus.ERROR, buildId }; + + let queryCount = 0; + mockDb.models.Deploy.query.mockImplementation(() => { + queryCount++; + return queryCount === 1 ? createAllDeploysQuery([mockDeploy]) : createFailedDeploysQuery([mockFailedDeploy]); + }); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockGetChangedFilesForPush).not.toHaveBeenCalled(); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { + buildId, + }); + }); + + test('queues skipped-push webhooks only for supported current build statuses', async () => { + await (githubService as any).queueWebhooksForSkippedPush({ id: 1, status: BuildStatus.DEPLOYED }); + await (githubService as any).queueWebhooksForSkippedPush({ id: 2, status: BuildStatus.ERROR }); + await (githubService as any).queueWebhooksForSkippedPush({ id: 3, status: BuildStatus.TORN_DOWN }); + await (githubService as any).queueWebhooksForSkippedPush({ id: 4, status: BuildStatus.BUILT }); + + expect(mockDb.services.Webhook.webhookQueue.add).toHaveBeenCalledTimes(3); + expect(mockDb.services.Webhook.webhookQueue.add).toHaveBeenNthCalledWith(1, 'webhook', { buildId: 1 }); + expect(mockDb.services.Webhook.webhookQueue.add).toHaveBeenNthCalledWith(2, 'webhook', { buildId: 2 }); + expect(mockDb.services.Webhook.webhookQueue.add).toHaveBeenNthCalledWith(3, 'webhook', { buildId: 3 }); + }); + + test('redeploys static environments without fetching changed files', async () => { + const buildId = 701; + const repoBuilder = { + from: jest.fn().mockReturnThis(), + select: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + }; + const prBuilder = { + from: jest.fn().mockReturnThis(), + select: jest.fn().mockReturnThis(), + where: jest.fn().mockReturnThis(), + whereIn: jest.fn((_column, callback) => { + callback(repoBuilder); + return prBuilder; + }), + }; + const buildQuery = { + whereIn: jest.fn((_column, callback) => { + callback(prBuilder); + return buildQuery; + }), + andWhere: jest.fn().mockReturnThis(), + first: jest.fn().mockResolvedValue({ id: buildId }), + }; + + mockDb.models.Build = { query: jest.fn().mockReturnValue(buildQuery) }; + mockDb.models.PullRequest.tableName = 'pull_requests'; + mockDb.models.Repository = { tableName: 'repositories' }; + mockDb.models.Deploy.query.mockReturnValue(createAllDeploysQuery([])); + + const pushEvent = { + ...createMockPushEvent(), + before: 'previous-commit', + after: 'latest-commit', + } as PushEvent; + + await githubService.handlePushWebhook(pushEvent); + + expect(mockGetChangedFilesForPush).not.toHaveBeenCalled(); + expect(mockDb.services.BuildService.resolveAndDeployBuildQueue.add).toHaveBeenCalledWith('resolve-deploy', { + buildId, + }); + }); + test('should not add to queue when PR is closed', async () => { const buildId = 100; const mockBuild = { diff --git a/src/server/services/github.ts b/src/server/services/github.ts index 20f66467..218f46c6 100644 --- a/src/server/services/github.ts +++ b/src/server/services/github.ts @@ -30,22 +30,39 @@ import { PullRequestStatus, FallbackLabels, DeployStatus, + BuildStatus, } from 'shared/constants'; import { QUEUE_NAMES } from 'shared/config'; import { NextApiRequest } from 'next'; import * as github from 'server/lib/github'; -import { Repository, Build, PullRequest } from 'server/models'; +import { Repository, Build, PullRequest, Deploy } from 'server/models'; +import * as YamlService from 'server/models/yaml'; import { LifecycleYamlConfigOptions } from 'server/models/yaml/types'; import { createOrUpdateGithubDeployment, deleteGithubDeploymentAndEnvironment } from 'server/lib/github/deployments'; import { enableKillSwitch, isStaging, hasDeployLabel, isLifecycleLabel } from 'server/lib/utils'; import { redisClient } from 'server/lib/dependencies'; import RepositoryService from './repository'; +import { + getEffectiveIgnoreFiles, + PushIgnoreDecision, + PushIgnoreServicePolicy, + shouldSkipPushDeploy, +} from 'server/lib/pushIgnoreFiles'; + +const SKIPPED_PUSH_WEBHOOK_STATUSES = new Set([BuildStatus.DEPLOYED, BuildStatus.ERROR, BuildStatus.TORN_DOWN]); +const IGNORE_FILES_FEATURE_FLAG = 'ignoreFiles'; interface PullRequestPatchState { deployLabelPresent: boolean; deployOnUpdate: boolean; } +type LifecycleConfigCache = Map>; +type PushEventWithCommitCounts = PushEvent & { + distinct_size?: number; + size?: number; +}; + export default class GithubService extends Service { private readonly repositoryService = new RepositoryService(this.db, this.redis, this.redlock, this.queueManager); @@ -359,13 +376,149 @@ export default class GithubService extends Service { } }; - handlePushWebhook = async ({ ref, before: previousCommit, after: latestCommit, repository }: PushEvent) => { + private async getLifecycleConfigForPush( + repoName: string, + branchName: string, + lifecycleConfigCache: LifecycleConfigCache + ): Promise { + const cacheKey = `${repoName}:${branchName}`; + if (!lifecycleConfigCache.has(cacheKey)) { + lifecycleConfigCache.set(cacheKey, YamlService.fetchLifecycleConfig(repoName, branchName)); + } + + return lifecycleConfigCache.get(cacheKey)!; + } + + private getDeployServiceName(deploy: Deploy): string | null { + return (deploy.build?.enableFullYaml ? deploy.deployable?.name : deploy.service?.name) ?? null; + } + + private async getPushIgnoreServicePolicies({ + repoName, + branchName, + deploys, + lifecycleConfigCache, + }: { + repoName: string; + branchName: string; + deploys: Deploy[]; + lifecycleConfigCache: LifecycleConfigCache; + }): Promise { + const lifecycleConfig = await this.getLifecycleConfigForPush(repoName, branchName, lifecycleConfigCache); + if (!lifecycleConfig) { + throw new Error(`Lifecycle config not found for ${repoName}:${branchName}`); + } + + return deploys.map((deploy) => { + const serviceName = this.getDeployServiceName(deploy); + if (!serviceName) { + throw new Error(`Deploy ${deploy.id} does not have a service name`); + } + + const service = lifecycleConfig.services?.find((candidate) => candidate.name === serviceName); + if (!service) { + throw new Error(`Service ${serviceName} not found in ${repoName}:${branchName}`); + } + + return { + serviceName, + ignoreFiles: getEffectiveIgnoreFiles(lifecycleConfig.environment?.ignoreFiles, service.ignoreFiles), + }; + }); + } + + private async shouldSkipBuildDeployForPush({ + repoName, + branchName, + build, + affectedDeploys, + changedFiles, + lifecycleConfigCache, + }: { + repoName: string; + branchName: string; + build: Build; + affectedDeploys: Deploy[]; + changedFiles: string[]; + lifecycleConfigCache: LifecycleConfigCache; + }): Promise { + try { + const servicePolicies = await this.getPushIgnoreServicePolicies({ + repoName, + branchName, + deploys: affectedDeploys, + lifecycleConfigCache, + }); + + return shouldSkipPushDeploy({ changedFiles, servicePolicies }); + } catch (error) { + getLogger({ error, buildId: build.id, repo: repoName, branch: branchName }).warn( + 'Push: ignoreFiles policy resolution failed, redeploying' + ); + return { shouldSkip: false, reason: 'policy_resolution_failed' }; + } + } + + private async isIgnoreFilesFeatureEnabled(): Promise { + try { + const { features } = await this.db.services.GlobalConfig.getAllConfigs(); + return features?.[IGNORE_FILES_FEATURE_FLAG] === true; + } catch (error) { + getLogger({ error, featureFlag: IGNORE_FILES_FEATURE_FLAG }).warn( + 'Push: ignoreFiles feature flag fetch failed, using dry-run mode' + ); + return false; + } + } + + private async queueWebhooksForSkippedPush(build: Build): Promise { + if (!SKIPPED_PUSH_WEBHOOK_STATUSES.has(build.status)) { + getLogger({ buildId: build.id, status: build.status }).info('Push: skipped deploy without webhook'); + return; + } + + await this.db.services.Webhook.webhookQueue.add('webhook', { + buildId: build.id, + ...extractContextForQueue(), + }); + getLogger({ buildId: build.id, status: build.status }).info('Push: skipped deploy and queued webhooks'); + } + + handlePushWebhook = async (pushEvent: PushEvent) => { + const { ref, before: previousCommit, after: latestCommit, repository } = pushEvent; + const pushEventWithCounts = pushEvent as PushEventWithCommitCounts; const { id: githubRepositoryId, full_name: repoName } = repository; const branchName = ref.split('refs/heads/')[1]; if (!branchName) return; const hasVoidCommit = [previousCommit, latestCommit].some((commit) => this.isVoidCommit(commit)); getLogger({}).debug(`Push event repo=${repoName} branch=${branchName}`); const models = this.db.models; + let changedFilesForPush: github.ChangedFilesForPushResult | null = null; + const lifecycleConfigCache: LifecycleConfigCache = new Map(); + + const loadChangedFilesForPush = async () => { + if (!changedFilesForPush) { + changedFilesForPush = github.getChangedFilesFromPushPayload({ + commits: pushEvent.commits, + commitCount: pushEventWithCounts.distinct_size ?? pushEventWithCounts.size, + }); + + if (!changedFilesForPush.canSkip) { + getLogger({ + repo: repoName, + branch: branchName, + reason: changedFilesForPush.reason, + }).info('Push: changed files unavailable from payload, falling back to compare'); + changedFilesForPush = await github.getChangedFilesForPush({ + fullName: repoName, + before: previousCommit, + after: latestCommit, + }); + } + } + + return changedFilesForPush; + }; try { if (!hasVoidCommit) { @@ -438,6 +591,57 @@ export default class GithubService extends Service { } if (!hasFailedDeploys) { + if (!hasVoidCommit) { + const changedFilesResult = await loadChangedFilesForPush(); + if (changedFilesResult.canSkip) { + const affectedDeploys = deploysToRebuild.filter((deploy) => deploy.build?.id === buildId); + const skipDecision = await this.shouldSkipBuildDeployForPush({ + repoName, + branchName, + build, + affectedDeploys, + changedFiles: changedFilesResult.files, + lifecycleConfigCache, + }); + + if (skipDecision.shouldSkip) { + const ignoreFilesEnabled = await this.isIgnoreFilesFeatureEnabled(); + getLogger({ + buildId, + repo: repoName, + branch: branchName, + reason: skipDecision.reason, + ignoreFilesEnabled, + }).info( + ignoreFilesEnabled + ? 'Push: skipped deploy reason=ignoreFiles' + : 'Push: dry-run would skip deploy reason=ignoreFiles' + ); + + if (ignoreFilesEnabled) { + await this.queueWebhooksForSkippedPush(build); + continue; + } + } else { + getLogger({ + buildId, + repo: repoName, + branch: branchName, + reason: skipDecision.reason, + serviceName: skipDecision.serviceName, + filePath: skipDecision.filePath, + }).info('Push: deploying reason=ignoreFiles_not_matched'); + } + } else { + getLogger({ + buildId, + repo: repoName, + branch: branchName, + reason: changedFilesResult.reason, + }).info('Push: deploying reason=changed_files_unavailable'); + } + } + getLogger().info(`Push: deploying repo=${repoName} branch=${branchName}`); } @@ -453,10 +657,10 @@ export default class GithubService extends Service { }; /** - * okay! most times the static environment builds are in a separate repo. because of this, we will not have a deploy with this repo's - * github repository id and branch name causing pushes to this branch to not trigger a redeploy. - * Ideally when a service is added or removed in a static env branch, we want to rebuild the whole environment. - * this is a patch to achieve this + * Static environment builds are often defined in a separate config repo, so they may not have deploy records + * for the pushed repo and branch. In that case, rebuild the whole static environment for matching default-branch pushes. + * Static environments intentionally bypass ignoreFiles diff checks because their service graph can change from + * the config repo itself, and redeploying is the safer default. */ handlePushForStaticEnv = async ({ githubRepositoryId,