From 413e754a1d91713a63d5a6a5284ffd9474cf12e1 Mon Sep 17 00:00:00 2001 From: vmelikyan Date: Sat, 17 Jan 2026 09:34:36 -0800 Subject: [PATCH 1/2] use mise for local dev --- .mise.toml | 80 ++++++++++++++++++++++++++++++++++++ sysops/tilt/kind-config.yaml | 12 +++--- 2 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 .mise.toml diff --git a/.mise.toml b/.mise.toml new file mode 100644 index 0000000..c738766 --- /dev/null +++ b/.mise.toml @@ -0,0 +1,80 @@ +[tools] +node = "20" +pnpm = "9.15.0" +kubectl = "latest" +helm = "latest" +kind = "latest" +tilt = "latest" + +[env] +_.path = ["./node_modules/.bin"] + +[tasks.dev] +description = "Set up kind cluster and run tilt" +run = """ +#!/usr/bin/env bash +set -e + +CLUSTER_NAME="lfc" +REGISTRY_CONFIG_DIR="/tmp/kind-registry-config/10.96.188.230:5000" + +echo "Setting up registry config for containerd..." +mkdir -p "$REGISTRY_CONFIG_DIR" +cat > "$REGISTRY_CONFIG_DIR/hosts.toml" << 'EOF' +server = "http://10.96.188.230:5000" + +[host."http://10.96.188.230:5000"] + capabilities = ["pull", "resolve", "push"] + skip_verify = true +EOF + +if kind get clusters 2>/dev/null | grep -q "^${CLUSTER_NAME}$"; then + echo "Kind cluster '$CLUSTER_NAME' already exists" +else + echo "Creating kind cluster '$CLUSTER_NAME'..." + kind create cluster --config sysops/tilt/kind-config.yaml --name "$CLUSTER_NAME" +fi + +echo "Switching kubectl context to kind-$CLUSTER_NAME..." +kubectl config use-context "kind-$CLUSTER_NAME" + +echo "Starting tilt..." +tilt up +""" + +[tasks.down] +description = "Stop tilt (cluster remains)" +run = """ +#!/usr/bin/env bash +set -e + +echo "Stopping tilt..." +tilt down + +echo "Tilt stopped. Kind cluster 'lfc' is still running." +echo "Run 'mise run clean' to delete the cluster." +""" + +[tasks.clean] +description = "Delete kind cluster and clean up" +run = """ +#!/usr/bin/env bash +set -e + +CLUSTER_NAME="lfc" + +echo "Stopping tilt (if running)..." +tilt down 2>/dev/null || true + +if kind get clusters 2>/dev/null | grep -q "^${CLUSTER_NAME}$"; then + echo "Deleting kind cluster '$CLUSTER_NAME'..." + kind delete cluster --name "$CLUSTER_NAME" +else + echo "Kind cluster '$CLUSTER_NAME' does not exist" +fi + +echo "Cleaning up registry config..." +rm -rf /tmp/kind-registry-config + +echo "Cleanup complete." +""" diff --git a/sysops/tilt/kind-config.yaml b/sysops/tilt/kind-config.yaml index fe358b2..8b02cb5 100644 --- a/sysops/tilt/kind-config.yaml +++ b/sysops/tilt/kind-config.yaml @@ -16,8 +16,10 @@ kind: Cluster apiVersion: kind.x-k8s.io/v1alpha4 containerdConfigPatches: - |- - [plugins."io.containerd.grpc.v1.cri".registry.configs."10.96.188.230:5000".tls] - plain_http = true - insecure_skip_verify = true - [plugins."io.containerd.grpc.v1.cri".registry.mirrors."10.96.188.230:5000"] - endpoint = ["http://10.96.188.230:5000"] + [plugins."io.containerd.grpc.v1.cri".registry] + config_path = "/etc/containerd/certs.d" +nodes: + - role: control-plane + extraMounts: + - hostPath: /tmp/kind-registry-config + containerPath: /etc/containerd/certs.d From b7507bf0214b98c9f0ea94e9c2ed01ad6cf95dfa Mon Sep 17 00:00:00 2001 From: vmelikyan Date: Sat, 17 Jan 2026 15:27:17 -0800 Subject: [PATCH 2/2] fix githubDeployment and undefined repo url in PRs --- ...8_change_github_deployment_id_to_bigint.ts | 29 ++ .../lib/github/__tests__/deployments.test.ts | 160 ++++++++++- src/server/lib/github/deployments.ts | 83 ++++-- src/server/services/__tests__/deploy.test.ts | 261 ++++++++++++++++++ src/server/services/activityStream.ts | 60 +--- src/server/services/deploy.ts | 58 +++- src/server/services/github.ts | 20 +- 7 files changed, 581 insertions(+), 90 deletions(-) create mode 100644 src/server/db/migrations/008_change_github_deployment_id_to_bigint.ts create mode 100644 src/server/services/__tests__/deploy.test.ts diff --git a/src/server/db/migrations/008_change_github_deployment_id_to_bigint.ts b/src/server/db/migrations/008_change_github_deployment_id_to_bigint.ts new file mode 100644 index 0000000..165e185 --- /dev/null +++ b/src/server/db/migrations/008_change_github_deployment_id_to_bigint.ts @@ -0,0 +1,29 @@ +/** + * Copyright 2025 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 { Knex } from 'knex'; + +export async function up(knex: Knex): Promise { + await knex.schema.alterTable('deploys', (table) => { + table.bigInteger('githubDeploymentId').alter(); + }); +} + +export async function down(knex: Knex): Promise { + await knex.schema.alterTable('deploys', (table) => { + table.integer('githubDeploymentId').alter(); + }); +} diff --git a/src/server/lib/github/__tests__/deployments.test.ts b/src/server/lib/github/__tests__/deployments.test.ts index 4682ea5..bb70ba9 100644 --- a/src/server/lib/github/__tests__/deployments.test.ts +++ b/src/server/lib/github/__tests__/deployments.test.ts @@ -18,12 +18,14 @@ import mockRedisClient from 'server/lib/__mocks__/redisClientMock'; mockRedisClient(); import * as client from 'server/lib/github/client'; +import * as githubIndex from 'server/lib/github/index'; import { createGithubDeployment, createOrUpdateGithubDeployment, deleteGithubDeployment, deleteGithubDeploymentAndEnvironment, deleteGithubEnvironment, + updateDeploymentStatus, } from 'server/lib/github/deployments'; jest.mock('server/services/globalConfig', () => { @@ -40,6 +42,9 @@ jest.mock('server/services/globalConfig', () => { }); jest.mock('server/lib/github/client'); +jest.mock('server/lib/github/index', () => ({ + getPullRequest: jest.fn(), +})); jest.mock('axios'); describe('GitHub Deployment Functions', () => { @@ -98,7 +103,7 @@ describe('GitHub Deployment Functions', () => { await deleteGithubDeploymentAndEnvironment(mockDeploy); expect(mockDeploy.$fetchGraph).toHaveBeenCalledWith('build.pullRequest.repository'); - expect(mockOctokit.request).toHaveBeenCalledTimes(2); + expect(mockOctokit.request).toHaveBeenCalledTimes(3); // markInactive + deleteDeployment + deleteEnvironment }); test('createGithubDeployment - success', async () => { @@ -144,4 +149,157 @@ describe('GitHub Deployment Functions', () => { `DELETE /repos/${mockDeploy.build.pullRequest.repository.fullName}/deployments/${mockDeploy.githubDeploymentId}` ); }); + + test('createOrUpdateGithubDeployment - uses newly created deployment ID for status update', async () => { + const newDeploymentId = 999888; + const mockDeployForIdTest = { + uuid: '1234', + githubDeploymentId: null, + status: 'deployed', + $fetchGraph: jest.fn(), + $query: jest.fn(() => ({ + patch: mockPatch, + })), + build: { + status: 'deployed', + pullRequest: { + repository: { + fullName: 'user/repo', + }, + pullRequestNumber: 123, + branchName: 'feature-branch', + }, + statusMessage: 'Build successful', + }, + }; + + (githubIndex.getPullRequest as jest.Mock).mockResolvedValue({ + data: { head: { sha: 'abc123' } }, + }); + + mockOctokit.request.mockImplementation((url) => { + if (url.includes('POST /repos') && url.includes('/deployments') && !url.includes('/statuses')) { + return Promise.resolve({ data: { id: newDeploymentId } }); + } + if (url.includes('/statuses')) { + return Promise.resolve({ data: {} }); + } + return Promise.resolve({ data: {} }); + }); + + await createOrUpdateGithubDeployment(mockDeployForIdTest); + + expect(mockOctokit.request).toHaveBeenCalledWith( + expect.stringContaining(`/deployments/${newDeploymentId}/statuses`), + expect.any(Object) + ); + }); + + test('createGithubDeployment - sets transient_environment to true', async () => { + const deploymentId = '123456'; + mockOctokit.request.mockResolvedValue({ data: { id: deploymentId } }); + + await createGithubDeployment(mockDeploy, 'abc123'); + + expect(mockOctokit.request).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + data: expect.objectContaining({ + transient_environment: true, + }), + }) + ); + }); + + test('updateDeploymentStatus - maps deployed status to success', async () => { + const mockDeployWithStatus = { + ...mockDeploy, + status: 'deployed', + build: { + ...mockDeploy.build, + status: 'deployed', + }, + }; + mockOctokit.request.mockResolvedValue({ data: {} }); + + await updateDeploymentStatus(mockDeployWithStatus, 12345); + + expect(mockOctokit.request).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + data: expect.objectContaining({ + state: 'success', + }), + }) + ); + }); + + test('updateDeploymentStatus - maps building status to in_progress', async () => { + const mockDeployBuilding = { + ...mockDeploy, + status: 'building', + build: { + ...mockDeploy.build, + status: 'building', + }, + }; + mockOctokit.request.mockResolvedValue({ data: {} }); + + await updateDeploymentStatus(mockDeployBuilding, 12345); + + expect(mockOctokit.request).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + data: expect.objectContaining({ + state: 'in_progress', + }), + }) + ); + }); + + test('updateDeploymentStatus - maps deploy_failed status to failure', async () => { + const mockDeployFailed = { + ...mockDeploy, + status: 'deploy_failed', + build: { + ...mockDeploy.build, + status: 'active', + }, + }; + mockOctokit.request.mockResolvedValue({ data: {} }); + + await updateDeploymentStatus(mockDeployFailed, 12345); + + expect(mockOctokit.request).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + data: expect.objectContaining({ + state: 'failure', + }), + }) + ); + }); + + test('updateDeploymentStatus - maps torn_down status to inactive', async () => { + const mockDeployTornDown = { + ...mockDeploy, + status: 'torn_down', + build: { + ...mockDeploy.build, + status: 'active', + }, + }; + mockOctokit.request.mockResolvedValue({ data: {} }); + + await updateDeploymentStatus(mockDeployTornDown, 12345); + + expect(mockOctokit.request).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + data: expect.objectContaining({ + state: 'inactive', + }), + }) + ); + }); }); diff --git a/src/server/lib/github/deployments.ts b/src/server/lib/github/deployments.ts index b03efe0..dab4a54 100644 --- a/src/server/lib/github/deployments.ts +++ b/src/server/lib/github/deployments.ts @@ -22,26 +22,35 @@ import { getLogger } from 'server/lib/logger'; const githubDeploymentStatuses = { deployed: 'success', + ready: 'success', error: 'failure', + build_failed: 'failure', + deploy_failed: 'failure', config_error: 'error', + torn_down: 'inactive', }; -function lifecycleToGithubStatus(status: string) { - if ( - [DeployStatus.CLONING, DeployStatus.QUEUED, DeployStatus.BUILDING, DeployStatus.BUILT].includes( - status as DeployStatus - ) - ) { +function lifecycleToGithubStatus(status: string): string { + const inProgressStatuses = [ + DeployStatus.CLONING, + DeployStatus.QUEUED, + DeployStatus.BUILDING, + DeployStatus.BUILT, + DeployStatus.DEPLOYING, + DeployStatus.WAITING, + DeployStatus.PENDING, + ]; + + if (inProgressStatuses.includes(status as DeployStatus)) { return 'in_progress'; } - return githubDeploymentStatuses[status]; + return githubDeploymentStatuses[status] || 'error'; } export async function createOrUpdateGithubDeployment(deploy: Deploy) { - getLogger().debug('Creating or updating github deployment'); + getLogger().debug('GitHub deployment: action=create_or_update'); try { - getLogger().info('GitHub: deployment status updated'); await deploy.$fetchGraph('build.pullRequest.repository'); const githubDeploymentId = deploy?.githubDeploymentId; const build = deploy?.build; @@ -63,12 +72,13 @@ export async function createOrUpdateGithubDeployment(deploy: Deploy) { return; } } - await createGithubDeployment(deploy, lastCommit); - if (build?.status === 'deployed') { - await updateDeploymentStatus(deploy, githubDeploymentId); + const newDeployment = await createGithubDeployment(deploy, lastCommit); + const newDeploymentId = newDeployment?.data?.id; + if (newDeploymentId) { + await updateDeploymentStatus(deploy, newDeploymentId); } } catch (error) { - getLogger({ error }).error('GitHub: deployment update failed'); + getLogger().error(`GitHub deployment failed: error=${error.message}`); throw error; } } @@ -76,10 +86,29 @@ export async function createOrUpdateGithubDeployment(deploy: Deploy) { export async function deleteGithubDeploymentAndEnvironment(deploy: Deploy) { if (deploy.githubDeploymentId !== null) { await deploy.$fetchGraph('build.pullRequest.repository'); + await markDeploymentInactive(deploy); await Promise.all([deleteGithubDeployment(deploy), deleteGithubEnvironment(deploy)]); } } +async function markDeploymentInactive(deploy: Deploy) { + const repository = deploy.build.pullRequest.repository; + try { + await cacheRequest(`POST /repos/${repository.fullName}/deployments/${deploy.githubDeploymentId}/statuses`, { + data: { + state: 'inactive', + environment: deploy.uuid, + description: 'Environment torn down', + }, + }); + getLogger().debug(`GitHub deployment: marked inactive deploymentId=${deploy.githubDeploymentId}`); + } catch (error) { + getLogger().warn( + `GitHub deployment: failed to mark inactive deploymentId=${deploy.githubDeploymentId} error=${error.message}` + ); + } +} + export async function createGithubDeployment(deploy: Deploy, ref: string) { const environment = deploy.uuid; const pullRequest = deploy?.build?.pullRequest; @@ -92,7 +121,7 @@ export async function createGithubDeployment(deploy: Deploy, ref: string) { environment, auto_merge: false, required_contexts: [], - transient_environment: false, + transient_environment: true, production_environment: false, }, }); @@ -101,16 +130,13 @@ export async function createGithubDeployment(deploy: Deploy, ref: string) { await deploy.$query().patch({ githubDeploymentId }); return resp; } catch (error) { - getLogger({ - error, - repo: fullName, - }).error('GitHub: deployment create failed'); + getLogger().error(`GitHub deployment create failed: repo=${fullName} error=${error.message}`); throw error; } } export async function deleteGithubDeployment(deploy: Deploy) { - getLogger().debug('Deleting github deployment'); + getLogger().debug('GitHub deployment: action=delete'); if (!deploy?.build) await deploy.$fetchGraph('build.pullRequest.repository'); const resp = await cacheRequest( `DELETE /repos/${deploy.build.pullRequest.repository.fullName}/deployments/${deploy.githubDeploymentId}` @@ -122,20 +148,29 @@ export async function deleteGithubDeployment(deploy: Deploy) { } export async function deleteGithubEnvironment(deploy: Deploy) { - getLogger().debug('Deleting github environment'); if (!deploy?.build) await deploy.$fetchGraph('build.pullRequest.repository'); const repository = deploy.build.pullRequest.repository; + const environmentName = deploy.uuid; try { - await cacheRequest(`DELETE /repos/${repository.fullName}/environments/${deploy.uuid}`); + await cacheRequest(`DELETE /repos/${repository.fullName}/environments/${environmentName}`); + getLogger().debug(`GitHub environment: deleted environment=${environmentName}`); } catch (e) { - if (e.status !== 404) { - throw e; + if (e.status === 404) { + getLogger().debug(`GitHub environment: not found environment=${environmentName}`); + } else if (e.status === 403) { + getLogger().warn( + `GitHub environment: no permission to delete environment=${environmentName} (check GitHub App permissions)` + ); + } else { + getLogger().warn( + `GitHub environment: delete failed environment=${environmentName} status=${e.status} error=${e.message}` + ); } } } export async function updateDeploymentStatus(deploy: Deploy, deploymentId: number) { - getLogger().debug('Updating github deployment status'); + getLogger().debug(`GitHub deployment status: action=update deploymentId=${deploymentId}`); const repository = deploy.build.pullRequest.repository; let buildStatus = determineStatus(deploy); const resp = await cacheRequest(`POST /repos/${repository.fullName}/deployments/${deploymentId}/statuses`, { diff --git a/src/server/services/__tests__/deploy.test.ts b/src/server/services/__tests__/deploy.test.ts new file mode 100644 index 0000000..8ffe241 --- /dev/null +++ b/src/server/services/__tests__/deploy.test.ts @@ -0,0 +1,261 @@ +/** + * Copyright 2025 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 mockRedisClient from 'server/lib/__mocks__/redisClientMock'; +import DeployService from '../deploy'; +import { DeployTypes } from 'shared/constants'; +import { ChartType } from 'server/lib/nativeHelm'; + +mockRedisClient(); + +jest.mock('server/lib/logger', () => ({ + getLogger: jest.fn(() => ({ + error: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + debug: jest.fn(), + child: jest.fn().mockReturnThis(), + })), + withLogContext: jest.fn((ctx, fn) => fn()), + extractContextForQueue: jest.fn(() => ({})), + LogStage: {}, +})); + +jest.mock('server/services/globalConfig', () => ({ + __esModule: true, + default: { + getInstance: jest.fn(() => ({ + getOrgChartName: jest.fn().mockResolvedValue('org-chart'), + })), + }, +})); + +const mockDetermineChartType = jest.fn(); +jest.mock('server/lib/nativeHelm', () => ({ + ...jest.requireActual('server/lib/nativeHelm'), + determineChartType: (...args: any[]) => mockDetermineChartType(...args), +})); + +describe('DeployService - shouldTriggerGithubDeployment', () => { + let deployService: DeployService; + let mockDb: any; + let mockRedis: any; + let mockRedlock: any; + let mockQueueManager: any; + + const createMockDeploy = (overrides: any = {}) => ({ + id: 1, + active: true, + service: { + public: true, + type: DeployTypes.DOCKER, + }, + deployable: { + public: true, + type: DeployTypes.DOCKER, + helm: {}, + }, + build: { + enableFullYaml: true, + }, + ...overrides, + }); + + beforeEach(() => { + jest.clearAllMocks(); + mockDetermineChartType.mockResolvedValue(ChartType.PUBLIC); + + mockDb = { + models: {}, + services: {}, + }; + + mockRedis = {}; + mockRedlock = {}; + + mockQueueManager = { + registerQueue: jest.fn().mockReturnValue({ + add: jest.fn(), + process: jest.fn(), + on: jest.fn(), + }), + }; + + deployService = new DeployService(mockDb, mockRedis, mockRedlock, mockQueueManager); + }); + + describe('deploy type filtering', () => { + test('should return true for DOCKER type', async () => { + const deploy = createMockDeploy({ + deployable: { public: true, type: DeployTypes.DOCKER, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(true); + }); + + test('should return true for GITHUB type', async () => { + const deploy = createMockDeploy({ + deployable: { public: true, type: DeployTypes.GITHUB, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(true); + }); + + test('should return true for CODEFRESH type', async () => { + const deploy = createMockDeploy({ + deployable: { public: true, type: DeployTypes.CODEFRESH, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(true); + }); + + test('should return true for HELM type', async () => { + const deploy = createMockDeploy({ + deployable: { public: true, type: DeployTypes.HELM, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(true); + }); + + test('should return false for CONFIGURATION type', async () => { + const deploy = createMockDeploy({ + deployable: { public: true, type: DeployTypes.CONFIGURATION, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(false); + }); + + test('should return false for AURORA_RESTORE type', async () => { + const deploy = createMockDeploy({ + deployable: { public: true, type: DeployTypes.AURORA_RESTORE, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(false); + }); + }); + + describe('active filtering', () => { + test('should return true when deploy is active', async () => { + const deploy = createMockDeploy({ active: true }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(true); + }); + + test('should return false when deploy is not active', async () => { + const deploy = createMockDeploy({ active: false }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(false); + }); + }); + + describe('public filtering', () => { + test('should return true when deployable is public (fullYaml mode)', async () => { + const deploy = createMockDeploy({ + build: { enableFullYaml: true }, + deployable: { public: true, type: DeployTypes.DOCKER, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(true); + }); + + test('should return false when deployable is not public (fullYaml mode)', async () => { + const deploy = createMockDeploy({ + build: { enableFullYaml: true }, + deployable: { public: false, type: DeployTypes.DOCKER, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(false); + }); + + test('should return true when service is public (classic mode)', async () => { + const deploy = createMockDeploy({ + build: { enableFullYaml: false }, + service: { public: true, type: DeployTypes.DOCKER }, + deployable: { public: false, type: DeployTypes.DOCKER, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(true); + }); + + test('should return false when service is not public (classic mode)', async () => { + const deploy = createMockDeploy({ + build: { enableFullYaml: false }, + service: { public: false, type: DeployTypes.DOCKER }, + deployable: { public: false, type: DeployTypes.DOCKER, helm: {} }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(false); + }); + }); + + describe('org chart handling', () => { + test('should return true for org helm chart even if not explicitly public', async () => { + const deploy = createMockDeploy({ + build: { enableFullYaml: true }, + deployable: { + public: false, + type: DeployTypes.HELM, + helm: { chart: { name: 'org-chart' } }, + }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(true); + }); + + test('should return true for PUBLIC helm chart even if not explicitly public', async () => { + mockDetermineChartType.mockResolvedValue(ChartType.PUBLIC); + const deploy = createMockDeploy({ + build: { enableFullYaml: true }, + deployable: { + public: false, + type: DeployTypes.HELM, + helm: { chart: { name: 'bitnami/jenkins' } }, + }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(true); + }); + + test('should return false for LOCAL helm chart that is not explicitly public', async () => { + mockDetermineChartType.mockResolvedValue(ChartType.LOCAL); + const deploy = createMockDeploy({ + build: { enableFullYaml: true }, + deployable: { + public: false, + type: DeployTypes.HELM, + helm: { chart: { name: './local-chart' } }, + }, + }); + + const result = await deployService['shouldTriggerGithubDeployment'](deploy as any); + expect(result).toBe(false); + }); + }); +}); diff --git a/src/server/services/activityStream.ts b/src/server/services/activityStream.ts index 5e4a1ab..4172f1e 100644 --- a/src/server/services/activityStream.ts +++ b/src/server/services/activityStream.ts @@ -419,18 +419,6 @@ export default class ActivityStream extends BaseService { } if (updateStatus || updateMissionControl) { - const deploysForGithubDeployment = targetGithubRepositoryId - ? deploys.filter((d) => d.githubRepositoryId === targetGithubRepositoryId) - : deploys; - - if (targetGithubRepositoryId) { - getLogger().info( - `Deploy: filtered deployCount=${deploysForGithubDeployment.length} totalCount=${deploys.length} targetRepoId=${targetGithubRepositoryId}` - ); - } - - await this.manageDeployments(build, deploysForGithubDeployment); - const isControlEnabled = await isControlCommentsEnabled(); if (isControlEnabled) { await this.updateMissionControlComment(build, deploys, pullRequest, repository).catch((error) => { @@ -928,7 +916,7 @@ export default class ActivityStream extends BaseService { } message += '\n'; - await build?.$fetchGraph('[deploys.[service, deployable]]'); + await build?.$fetchGraph('[deploys.[service, deployable.repository]]'); deploys = build.deploys; const orgChartName = await GlobalConfigService.getInstance().getOrgChartName(); @@ -1049,7 +1037,7 @@ export default class ActivityStream extends BaseService { message += '| Service | Branch | Link |\n'; message += '|---|---|---|\n'; - await build?.$fetchGraph('[deploys.[service, deployable]]'); + await build?.$fetchGraph('[deploys.[service, deployable.repository]]'); let { deploys } = build; if (deploys.length > 1) { @@ -1133,50 +1121,6 @@ export default class ActivityStream extends BaseService { return message; } - private async manageDeployments(build, deploys) { - const isGithubDeployments = build?.githubDeployments; - if (!isGithubDeployments) return; - const isFullYaml = build?.enableFullYaml; - const orgChartName = await GlobalConfigService.getInstance().getOrgChartName(); - - try { - await Promise.all( - deploys.map(async (deploy) => { - return withLogContext( - { deployUuid: deploy?.uuid, serviceName: deploy?.deployable?.name || deploy?.service?.name }, - async () => { - const deployId = deploy?.id; - const service = deploy?.service; - const deployable = deploy?.deployable; - const isActive = deploy?.active; - const isOrgHelmChart = orgChartName === deployable?.helm?.chart?.name; - const isPublic = isFullYaml ? deployable.public || isOrgHelmChart : service.public; - const serviceType = isFullYaml ? deployable?.type : service?.type; - const isActiveAndPublic = isActive && isPublic; - const isDeploymentType = [DeployTypes.DOCKER, DeployTypes.GITHUB, DeployTypes.CODEFRESH].includes( - serviceType - ); - const isDeployment = isActiveAndPublic && isDeploymentType; - if (!isDeployment) { - getLogger().debug(`Skipping deployment ${deploy?.name}`); - return; - } - await this.db.services.GithubService.githubDeploymentQueue - .add( - 'deployment', - { deployId, action: 'create', ...extractContextForQueue() }, - { delay: 10000, jobId: `deploy-${deployId}` } - ) - .catch((error) => getLogger().warn({ error }, `Deploy: management failed deployId=${deployId}`)); - } - ); - }) - ); - } catch (error) { - getLogger().debug({ error }, 'manageDeployments error'); - } - } - private async purgeFastlyServiceCache(uuid: string) { return withLogContext({ buildUuid: uuid }, async () => { try { diff --git a/src/server/services/deploy.ts b/src/server/services/deploy.ts index 419b4ff..9d0396c 100644 --- a/src/server/services/deploy.ts +++ b/src/server/services/deploy.ts @@ -17,7 +17,7 @@ import BaseService from './_service'; import { Environment, Build, Service, Deploy, Deployable } from 'server/models'; import * as codefresh from 'server/lib/codefresh'; -import { getLogger, withLogContext } from 'server/lib/logger'; +import { getLogger, withLogContext, extractContextForQueue } from 'server/lib/logger'; import hash from 'object-hash'; import { DeployStatus, DeployTypes } from 'shared/constants'; import * as cli from 'server/lib/cli'; @@ -803,7 +803,7 @@ export default class DeployService extends BaseService { const id = deploy?.id; await this.db.models.Deploy.query().where({ id, runUUID }).patch(params); if (deploy.runUUID !== runUUID) { - getLogger().debug({ deployRunUUID: deploy.runUUID, providedRunUUID: runUUID }, 'runUUID mismatch'); + getLogger().debug(`runUUID mismatch: deployRunUUID=${deploy.runUUID} providedRunUUID=${runUUID}`); return; } @@ -811,6 +811,23 @@ export default class DeployService extends BaseService { build = deploy?.build; const pullRequest = build?.pullRequest; + const terminalStatuses = [ + DeployStatus.READY, + DeployStatus.DEPLOYED, + DeployStatus.ERROR, + DeployStatus.BUILD_FAILED, + DeployStatus.DEPLOY_FAILED, + DeployStatus.TORN_DOWN, + ]; + const isTerminalStatus = terminalStatuses.includes(params.status as DeployStatus); + + if (isTerminalStatus && build?.githubDeployments) { + await deploy.$fetchGraph('[service, deployable]'); + if (await this.shouldTriggerGithubDeployment(deploy)) { + await this.triggerGithubDeploymentUpdate(deploy); + } + } + await this.db.services.ActivityStream.updatePullRequestActivityStream( build, [], @@ -827,6 +844,43 @@ export default class DeployService extends BaseService { } } + private async shouldTriggerGithubDeployment(deploy: Deploy): Promise { + if (!deploy?.active) { + getLogger().debug(`GitHub deployment skipped: deployId=${deploy.id} reason=inactive`); + return false; + } + + const { service, deployable, build } = deploy; + const isFullYaml = build?.enableFullYaml; + const serviceType = isFullYaml ? deployable?.type : service?.type; + const validTypes: string[] = [DeployTypes.DOCKER, DeployTypes.GITHUB, DeployTypes.CODEFRESH, DeployTypes.HELM]; + + if (!serviceType || !validTypes.includes(serviceType)) { + getLogger().debug(`GitHub deployment skipped: deployId=${deploy.id} reason=type type=${serviceType}`); + return false; + } + + const orgChartName = await GlobalConfigService.getInstance().getOrgChartName(); + const isOrgHelmChart = orgChartName === deployable?.helm?.chart?.name; + const isPublicChart = serviceType === DeployTypes.HELM && (await determineChartType(deploy)) === ChartType.PUBLIC; + const isPublic = isFullYaml ? deployable?.public || isOrgHelmChart || isPublicChart : service?.public; + + if (!isPublic) { + getLogger().debug(`GitHub deployment skipped: deployId=${deploy.id} reason=private`); + return false; + } + + return true; + } + + private async triggerGithubDeploymentUpdate(deploy: Deploy) { + await this.db.services.GithubService.githubDeploymentQueue + .add('deployment', { deployId: deploy.id, action: 'create', ...extractContextForQueue() }) + .catch((error) => + getLogger().warn(`GitHub deployment queue failed: deployId=${deploy.id} error=${error.message}`) + ); + } + private async patchDeployWithTag({ tag, deploy, initTag, ecrDomain }) { await deploy.$fetchGraph('[build, service, deployable]'); const { build, deployable, service } = deploy; diff --git a/src/server/services/github.ts b/src/server/services/github.ts index ae275fb..b22d62e 100644 --- a/src/server/services/github.ts +++ b/src/server/services/github.ts @@ -455,8 +455,16 @@ export default class GithubService extends Service { return withLogContext({ correlationId, sender, _ddTraceContext, deployUuid: String(deployId) }, async () => { const deploy = await this.db.models.Deploy.query().findById(deployId); + if (!deploy) { + getLogger({ stage: LogStage.DEPLOY_FAILED }).warn( + `GitHub deployment skipped: deployId=${deployId} reason=deploy_not_found` + ); + return; + } try { - getLogger({ stage: LogStage.DEPLOY_STARTING }).debug(`GitHub deployment: ${action}`); + getLogger({ stage: LogStage.DEPLOY_STARTING }).debug( + `GitHub deployment: action=${action} deployId=${deployId}` + ); switch (action) { case 'create': { @@ -471,12 +479,14 @@ export default class GithubService extends Service { throw new Error(`Unknown action: ${action}`); } - getLogger({ stage: LogStage.DEPLOY_COMPLETE }).debug(`GitHub deployment: ${action} completed`); + getLogger({ stage: LogStage.DEPLOY_COMPLETE }).debug( + `GitHub deployment completed: action=${action} deployId=${deployId}` + ); } catch (error) { - getLogger({ stage: LogStage.DEPLOY_FAILED }).warn( - { error }, - `Error processing GitHub deployment job=${job?.id} action=${action}` + getLogger({ stage: LogStage.DEPLOY_FAILED }).error( + `GitHub deployment failed: job=${job?.id} action=${action} error=${error.message}` ); + throw error; } }); };