From cfdc322146de6a3f061be3c65532f019a3b1ab05 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Mon, 11 Aug 2025 11:37:19 -0700 Subject: [PATCH 1/7] use git org from app_setup config for codefresh steps - for now using the config from app_setup.org. this still can be empty with current setup - when we build the onboarding flow, maybe make this required so this config value can never be empty! --- .../lib/codefresh/__fixtures__/codefresh.ts | 1 - .../__tests__/kubeContextStep.test.ts | 124 ++++++++++++++++++ src/server/lib/codefresh/constants.ts | 2 - src/server/lib/codefresh/index.ts | 7 +- src/server/lib/codefresh/types.ts | 1 + .../utils/__tests__/generateYaml.test.ts | 16 ++- .../codefresh/utils/__tests__/index.test.ts | 8 +- .../codefresh/utils/determineCheckoutStep.ts | 3 +- .../lib/codefresh/utils/generateYaml.ts | 3 +- src/server/lib/codefresh/utils/index.ts | 4 +- src/server/lib/helm/helm.ts | 7 +- src/server/services/deploy.ts | 8 +- src/server/services/types/globalConfig.ts | 1 + 13 files changed, 166 insertions(+), 19 deletions(-) create mode 100644 src/server/lib/codefresh/__tests__/kubeContextStep.test.ts diff --git a/src/server/lib/codefresh/__fixtures__/codefresh.ts b/src/server/lib/codefresh/__fixtures__/codefresh.ts index 0ffa62a8..61de44cb 100644 --- a/src/server/lib/codefresh/__fixtures__/codefresh.ts +++ b/src/server/lib/codefresh/__fixtures__/codefresh.ts @@ -40,7 +40,6 @@ export const deploy = { export const checkoutStep = { fail_fast: true, - git: CF.CHECKOUT.GIT, repo, revision, stage: CF.CHECKOUT.CHECKOUT_STAGE, diff --git a/src/server/lib/codefresh/__tests__/kubeContextStep.test.ts b/src/server/lib/codefresh/__tests__/kubeContextStep.test.ts new file mode 100644 index 00000000..fd53ad0d --- /dev/null +++ b/src/server/lib/codefresh/__tests__/kubeContextStep.test.ts @@ -0,0 +1,124 @@ +/** + * 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'; +mockRedisClient(); + +// Mock the GlobalConfigService before importing +jest.mock('server/services/globalConfig', () => ({ + __esModule: true, + default: { + getInstance: jest.fn(), + }, +})); + +// Mock shared/config before importing +jest.mock('shared/config', () => ({ + ENVIRONMENT: 'production', +})); + +import { kubeContextStep } from 'server/lib/codefresh'; +import GlobalConfigService from 'server/services/globalConfig'; + +const MockedGlobalConfigService = GlobalConfigService as jest.MockedClass; + +describe('kubeContextStep', () => { + let mockGetAllConfigs: jest.Mock; + let mockInstance: Partial; + + beforeEach(() => { + jest.clearAllMocks(); + + mockGetAllConfigs = jest.fn(); + mockInstance = { + getAllConfigs: mockGetAllConfigs, + }; + + (MockedGlobalConfigService.getInstance as jest.Mock).mockReturnValue(mockInstance); + }); + + describe('gitOrg handling', () => { + const context = 'test-context'; + const cluster = 'test-cluster'; + + it('should use the org value when app_setup.org is a valid string', async () => { + mockGetAllConfigs.mockResolvedValue({ + app_setup: { org: 'test-org' }, + }); + + const result = await kubeContextStep({ context, cluster }); + + expect(result).toEqual({ + title: 'Set kube context', + type: 'test-org/kube-context:0.0.2', + arguments: { + app: context, + cluster, + aws_access_key_id: '${{DEPLOYMENT_AWS_ACCESS_KEY_ID}}', + aws_secret_access_key: '${{DEPLOYMENT_AWS_SECRET_ACCESS_KEY}}', + }, + }); + }); + + it('should use fallback when app_setup does not exist', async () => { + mockGetAllConfigs.mockResolvedValue({}); + + const result = await kubeContextStep({ context, cluster }); + + expect(result.type).toBe('REPLACE_ME_ORG/kube-context:0.0.2'); + }); + + it('should use fallback when app_setup.org is undefined', async () => { + mockGetAllConfigs.mockResolvedValue({ + app_setup: {}, + }); + + const result = await kubeContextStep({ context, cluster }); + + expect(result.type).toBe('REPLACE_ME_ORG/kube-context:0.0.2'); + }); + + it('should use fallback when app_setup.org is an empty string', async () => { + mockGetAllConfigs.mockResolvedValue({ + app_setup: { org: '' }, + }); + + const result = await kubeContextStep({ context, cluster }); + + expect(result.type).toBe('REPLACE_ME_ORG/kube-context:0.0.2'); + }); + + it('should use fallback when app_setup.org is whitespace only', async () => { + mockGetAllConfigs.mockResolvedValue({ + app_setup: { org: ' ' }, + }); + + const result = await kubeContextStep({ context, cluster }); + + expect(result.type).toBe('REPLACE_ME_ORG/kube-context:0.0.2'); + }); + + it('should trim whitespace from valid org values', async () => { + mockGetAllConfigs.mockResolvedValue({ + app_setup: { org: ' test-org ' }, + }); + + const result = await kubeContextStep({ context, cluster }); + + expect(result.type).toBe('test-org/kube-context:0.0.2'); + }); + }); +}); diff --git a/src/server/lib/codefresh/constants.ts b/src/server/lib/codefresh/constants.ts index 6be9b1bc..713ffb39 100644 --- a/src/server/lib/codefresh/constants.ts +++ b/src/server/lib/codefresh/constants.ts @@ -20,8 +20,6 @@ export const CODEFRESH_PATH = `${TMP_PATH}/codefresh`; export const CF = { CHECKOUT: { - // this will be the Codefresh git org - GIT: 'REPLACE_ME_ORG', PATH: `${TMP_PATH}/codefresh`, CHECKOUT_STAGE: 'Checkout', TYPE: 'git-clone', diff --git a/src/server/lib/codefresh/index.ts b/src/server/lib/codefresh/index.ts index 792badff..ef354030 100644 --- a/src/server/lib/codefresh/index.ts +++ b/src/server/lib/codefresh/index.ts @@ -118,7 +118,7 @@ export const triggerPipeline = async ( return buildId; }; -export function kubeContextStep({ context, cluster }: { context: string; cluster: string }) { +export async function kubeContextStep({ context, cluster }: { context: string; cluster: string }) { let awsAccessKeyId = '${{DEPLOYMENT_AWS_ACCESS_KEY_ID}}'; let awsSecretAccessKey = '${{DEPLOYMENT_AWS_SECRET_ACCESS_KEY}}'; @@ -126,11 +126,12 @@ export function kubeContextStep({ context, cluster }: { context: string; cluster awsAccessKeyId = '${{STG_AWS_ACCESS_KEY_ID}}'; awsSecretAccessKey = '${{STG_AWS_SECRET_ACCESS_KEY}}'; } - + const { app_setup } = await GlobalConfigService.getInstance().getAllConfigs(); + const gitOrg = (app_setup?.org && app_setup.org.trim()) || 'REPLACE_ME_ORG'; return { title: 'Set kube context', // this is a custom step setup to update kube context - type: 'REPLACE_ME_IF_NEEDED/kube-context:0.0.2', + type: `${gitOrg}/kube-context:0.0.2`, arguments: { app: context, cluster, diff --git a/src/server/lib/codefresh/types.ts b/src/server/lib/codefresh/types.ts index 58b9f111..1e5377b9 100644 --- a/src/server/lib/codefresh/types.ts +++ b/src/server/lib/codefresh/types.ts @@ -27,6 +27,7 @@ export type ContainerBuildOptions = { dockerfilePath: string; ecrDomain: string; envVars: Record; + gitOrg?: string; initDockerfilePath: string; repo: string; revision: string; diff --git a/src/server/lib/codefresh/utils/__tests__/generateYaml.test.ts b/src/server/lib/codefresh/utils/__tests__/generateYaml.test.ts index 578efa31..5ab46a57 100644 --- a/src/server/lib/codefresh/utils/__tests__/generateYaml.test.ts +++ b/src/server/lib/codefresh/utils/__tests__/generateYaml.test.ts @@ -39,12 +39,24 @@ describe('generateYaml', () => { it('should generate yaml', () => { constructBuildArgs = jest.spyOn(utils, 'constructBuildArgs').mockReturnValue([]); - generateCheckoutStepSpy = jest.spyOn(utils, 'generateCheckoutStep').mockReturnValue(checkoutStep); + generateCheckoutStepSpy = jest.spyOn(utils, 'generateCheckoutStep').mockReturnValue({ + ...checkoutStep, + git: 'REPLACE_ME_ORG', + }); generateBuildStepSpy = jest.spyOn(utils, 'generateBuildStep').mockReturnValue(buildStep); generateAfterBuildStepSpy = jest.spyOn(utils, 'generateAfterBuildStep').mockReturnValue(afterBuildStep); constructStagesSpy = jest.spyOn(utils, 'constructStages').mockReturnValue(['Checkout', 'Build', 'PostBuild']); const result = generateYaml(generateYamlOptions); - expect(result).toEqual(yamlContent); + expect(result).toEqual({ + ...yamlContent, + steps: { + ...yamlContent.steps, + Checkout: { + ...checkoutStep, + git: 'REPLACE_ME_ORG', + }, + }, + }); expect(constructBuildArgs).toHaveBeenCalledWith({}); expect(generateCheckoutStepSpy).toHaveBeenCalled(); expect(generateBuildStepSpy).toHaveBeenCalled(); diff --git a/src/server/lib/codefresh/utils/__tests__/index.test.ts b/src/server/lib/codefresh/utils/__tests__/index.test.ts index 0f2cc476..a6af5649 100644 --- a/src/server/lib/codefresh/utils/__tests__/index.test.ts +++ b/src/server/lib/codefresh/utils/__tests__/index.test.ts @@ -40,8 +40,12 @@ describe('constructBuildArgs', () => { }); test('generateCheckoutStep', () => { - const result = utils.generateCheckoutStep(revision, repo); - expect(result).toEqual(checkoutStep); + const gitOrg = 'test-git-org'; + const result = utils.generateCheckoutStep(revision, repo, gitOrg); + expect(result).toEqual({ + ...checkoutStep, + git: gitOrg, + }); }); test('generateBuildStep', () => { diff --git a/src/server/lib/codefresh/utils/determineCheckoutStep.ts b/src/server/lib/codefresh/utils/determineCheckoutStep.ts index 6f1808c9..812257f7 100644 --- a/src/server/lib/codefresh/utils/determineCheckoutStep.ts +++ b/src/server/lib/codefresh/utils/determineCheckoutStep.ts @@ -16,4 +16,5 @@ import { generateCheckoutStep } from 'server/lib/codefresh/utils'; -export const determineCheckoutStep = (revision: string, repo: string) => generateCheckoutStep(revision, repo); +export const determineCheckoutStep = (revision: string, repo: string, gitOrg: string) => + generateCheckoutStep(revision, repo, gitOrg); diff --git a/src/server/lib/codefresh/utils/generateYaml.ts b/src/server/lib/codefresh/utils/generateYaml.ts index a152f336..f513b86c 100644 --- a/src/server/lib/codefresh/utils/generateYaml.ts +++ b/src/server/lib/codefresh/utils/generateYaml.ts @@ -28,6 +28,7 @@ export const generateYaml = (options: ContainerBuildOptions) => { const { ecrRepo, envVars, + gitOrg = 'REPLACE_ME_ORG', repo, revision, tag, @@ -81,7 +82,7 @@ export const generateYaml = (options: ContainerBuildOptions) => { mode: 'parallel', stages: constructStages({ initDockerfilePath, afterBuildPipelineId }), steps: { - Checkout: generateCheckoutStep(revision, repo), + Checkout: generateCheckoutStep(revision, repo, gitOrg), Build: generateBuildStep(buildOptions), ...(initDockerfilePath && { InitContainer: generateBuildStep({ diff --git a/src/server/lib/codefresh/utils/index.ts b/src/server/lib/codefresh/utils/index.ts index 902ec969..d00e2ecc 100644 --- a/src/server/lib/codefresh/utils/index.ts +++ b/src/server/lib/codefresh/utils/index.ts @@ -23,10 +23,10 @@ export const constructBuildArgs = (envVars = {}) => { return envVarsItems?.length > 0 ? Object.keys(envVars).map((k) => `${k}=\${{${k}}}`) : []; }; -export const generateCheckoutStep = (revision: string, repo: string) => ({ +export const generateCheckoutStep = (revision: string, repo: string, gitOrg: string) => ({ ...CF_CHECKOUT_STEP, working_directory: '.', - git: CF.CHECKOUT.GIT, + git: gitOrg, repo, revision, type: CF.CHECKOUT.TYPE, diff --git a/src/server/lib/helm/helm.ts b/src/server/lib/helm/helm.ts index 49a03046..083e58ac 100644 --- a/src/server/lib/helm/helm.ts +++ b/src/server/lib/helm/helm.ts @@ -321,7 +321,7 @@ export async function generateCodefreshRunCommand(deploy: Deploy): Promise> { const configs = await GlobalConfigService.getInstance().getAllConfigs(); - const kubeContext = kubeContextStep({ context: deploy.uuid, cluster: configs.lifecycleDefaults.deployCluster }); + const kubeContext = await kubeContextStep({ context: deploy.uuid, cluster: configs.lifecycleDefaults.deployCluster }); const helmDeploy = await helmDeployStep(deploy); delete helmDeploy.working_directory; kubeContext['stage'] = 'Checkout'; @@ -374,10 +374,11 @@ export async function generateHelmCodefreshYamlWithCheckout(deploy: Deploy): Pro const repositoryName = deploy?.repository?.fullName; const revision = deploy.sha; + const gitOrg = (configs?.app_setup?.org && configs.app_setup.org.trim()) || 'REPLACE_ME_ORG'; - const Checkout = generateCheckoutStep(revision, repositoryName); + const Checkout = generateCheckoutStep(revision, repositoryName, gitOrg); delete Checkout.stage; - const kubeContext = kubeContextStep({ context: deploy.uuid, cluster: configs.lifecycleDefaults.deployCluster }); + const kubeContext = await kubeContextStep({ context: deploy.uuid, cluster: configs.lifecycleDefaults.deployCluster }); const addHelmReleaseDeletionStep = deploy?.build?.isStatic ? configs?.deletePendingHelmReleaseStep?.static_delete : configs?.deletePendingHelmReleaseStep?.delete; diff --git a/src/server/services/deploy.ts b/src/server/services/deploy.ts index 811da9a8..1d838cea 100644 --- a/src/server/services/deploy.ts +++ b/src/server/services/deploy.ts @@ -639,8 +639,9 @@ export default class DeployService extends BaseService { logger.debug(`${uuidText} Tags exist check for ${deploy.uuid}: ${tagsExist}`); - const { lifecycleDefaults } = await GlobalConfigService.getInstance().getAllConfigs(); + const { lifecycleDefaults, app_setup } = await GlobalConfigService.getInstance().getAllConfigs(); const { ecrDomain, ecrRegistry: registry } = lifecycleDefaults; + const gitOrg = (app_setup?.org && app_setup.org.trim()) || 'REPLACE_ME_ORG'; if (!ecrDomain || !registry) { logger.child({ lifecycleDefaults }).error(`[BUILD ${deploy.uuid}] Missing ECR config to build image`); await this.patchAndUpdateActivityFeed(deploy, { status: DeployStatus.ERROR }, runUUID); @@ -657,6 +658,7 @@ export default class DeployService extends BaseService { ecrRepo, envVars: envVariables, dockerfilePath, + gitOrg, tag, revision: fullSha, repo: repositoryName, @@ -905,8 +907,9 @@ export default class DeployService extends BaseService { logger.debug(`${uuidText} Auto-appended service name to ECR path: ${ecrRepo}`); } - const { lifecycleDefaults } = await GlobalConfigService.getInstance().getAllConfigs(); + const { lifecycleDefaults, app_setup } = await GlobalConfigService.getInstance().getAllConfigs(); const { ecrDomain, ecrRegistry: registry } = lifecycleDefaults; + const gitOrg = (app_setup?.org && app_setup.org.trim()) || 'REPLACE_ME_ORG'; if (!ecrDomain || !registry) { logger.child({ lifecycleDefaults }).error(`[BUILD ${deploy.uuid}] Missing ECR config to build image`); await this.patchAndUpdateActivityFeed(deploy, { status: DeployStatus.ERROR }, runUUID); @@ -939,6 +942,7 @@ export default class DeployService extends BaseService { ecrDomain, envVars: deploy.env, dockerfilePath, + gitOrg, tag, revision: fullSha, repo: repositoryName, diff --git a/src/server/services/types/globalConfig.ts b/src/server/services/types/globalConfig.ts index bc44f4ae..4c871b56 100644 --- a/src/server/services/types/globalConfig.ts +++ b/src/server/services/types/globalConfig.ts @@ -45,6 +45,7 @@ export type AppSetup = { restarted: boolean; url: string; state: string; + org?: string; }; export type RoleSettings = { From a100c4332d1348d8d2f4e6f492bbdf9f7ffc9584 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Mon, 11 Aug 2025 14:37:47 -0700 Subject: [PATCH 2/7] fix: suffix for image_name should not be needed no history on why we added this for now, so to handle external ECR setups, we dont want to add a suffix to the image name generated. we need to test this change with codefresh and buildkit to make sure this doesnt break anything --- src/server/services/deploy.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/server/services/deploy.ts b/src/server/services/deploy.ts index 1d838cea..fec9539a 100644 --- a/src/server/services/deploy.ts +++ b/src/server/services/deploy.ts @@ -627,12 +627,6 @@ export default class DeployService extends BaseService { const initTag = generateDeployTag({ prefix: 'lfc-init', sha: shortSha, envVarsHash }); let ecrRepo = deployable?.ecr; - const serviceName = deploy.build?.enableFullYaml ? deployable?.name : deploy.service?.name; - if (serviceName && ecrRepo && !ecrRepo.endsWith(`/${serviceName}`)) { - ecrRepo = `${ecrRepo}/${serviceName}`; - logger.debug(`${uuidText} Auto-appended service name to ECR path: ${ecrRepo}`); - } - const tagsExist = (await codefresh.tagExists({ tag, ecrRepo, uuid })) && (!initDockerfilePath || (await codefresh.tagExists({ tag: initTag, ecrRepo, uuid }))); @@ -901,12 +895,6 @@ export default class DeployService extends BaseService { const initTag = generateDeployTag({ prefix: 'lfc-init', sha: shortSha, envVarsHash }); let ecrRepo = deployable?.ecr; - const serviceName = deploy.build?.enableFullYaml ? deployable?.name : deploy.service?.name; - if (serviceName && ecrRepo && !ecrRepo.endsWith(`/${serviceName}`)) { - ecrRepo = `${ecrRepo}/${serviceName}`; - logger.debug(`${uuidText} Auto-appended service name to ECR path: ${ecrRepo}`); - } - const { lifecycleDefaults, app_setup } = await GlobalConfigService.getInstance().getAllConfigs(); const { ecrDomain, ecrRegistry: registry } = lifecycleDefaults; const gitOrg = (app_setup?.org && app_setup.org.trim()) || 'REPLACE_ME_ORG'; From 33583cb3d607d3009c1a9667e8ac359283875066 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Mon, 11 Aug 2025 16:29:28 -0700 Subject: [PATCH 3/7] fix: normalize git branch name for build and deploy job manifests --- src/server/lib/kubernetes/jobFactory.ts | 8 +++-- src/server/lib/kubernetes/utils.ts | 45 +++++++++++++++++++++++++ src/server/lib/nativeHelm/helm.ts | 1 + src/server/lib/nativeHelm/utils.ts | 6 ++-- 4 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 src/server/lib/kubernetes/utils.ts diff --git a/src/server/lib/kubernetes/jobFactory.ts b/src/server/lib/kubernetes/jobFactory.ts index 67a296b4..c332b42a 100644 --- a/src/server/lib/kubernetes/jobFactory.ts +++ b/src/server/lib/kubernetes/jobFactory.ts @@ -15,6 +15,7 @@ */ import { V1Job } from '@kubernetes/client-node'; +import { normalizeKubernetesLabelValue } from './utils'; export interface JobConfig { name: string; @@ -134,7 +135,7 @@ export function createBuildJob(config: BuildJobConfig): V1Job { 'lc-deploy-uuid': config.deployUuid, 'lc-build-id': String(config.buildId), 'git-sha': config.shortSha, - 'git-branch': config.branch, + 'git-branch': normalizeKubernetesLabelValue(config.branch), 'builder-engine': config.engine, 'build-method': 'native', }, @@ -153,6 +154,7 @@ export interface HelmJobConfig { namespace: string; serviceAccount: string; serviceName: string; + buildUUID: string; isStatic: boolean; timeout?: number; gitUsername?: string; @@ -174,13 +176,13 @@ export function createHelmJob(config: HelmJobConfig): V1Job { const timeout = config.timeout || 1800; // 30 minutes default const labels: Record = { - 'lc-uuid': config.name.split('-')[0], + 'lc-uuid': config.buildUUID, service: config.serviceName, }; if (config.deployMetadata) { labels['git-sha'] = config.deployMetadata.sha; - labels['git-branch'] = config.deployMetadata.branch; + labels['git-branch'] = normalizeKubernetesLabelValue(config.deployMetadata.branch); labels['deploy-id'] = config.deployMetadata.deployId || ''; labels['deployable-id'] = config.deployMetadata.deployableId; } diff --git a/src/server/lib/kubernetes/utils.ts b/src/server/lib/kubernetes/utils.ts new file mode 100644 index 00000000..07017cf8 --- /dev/null +++ b/src/server/lib/kubernetes/utils.ts @@ -0,0 +1,45 @@ +/** + * 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. + */ + +/** + * Normalizes a string to be valid for Kubernetes labels. + * Kubernetes label values must match the regex: (([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? + * This means they must start and end with alphanumeric characters, and can contain + * hyphens, underscores, and dots in the middle. + */ +export function normalizeKubernetesLabelValue(value: string): string { + if (!value) return ''; + + // replace invalid characters with hyphens + let normalized = value.replace(/[^A-Za-z0-9\-_.]/g, '-'); + + // remove leading/trailing non-alphanumeric characters + normalized = normalized.replace(/^[^A-Za-z0-9]+|[^A-Za-z0-9]+$/g, ''); + + // if empty return 'unknown' + if (!normalized) { + return 'unknown'; + } + + // truncate if too long (k8s labels have a 63 character limit) + if (normalized.length > 63) { + normalized = normalized.substring(0, 63); + // handle trailing non-alphanumeric after truncate + normalized = normalized.replace(/[^A-Za-z0-9]+$/, ''); + } + + return normalized; +} diff --git a/src/server/lib/nativeHelm/helm.ts b/src/server/lib/nativeHelm/helm.ts index 0dac9058..3ff708b7 100644 --- a/src/server/lib/nativeHelm/helm.ts +++ b/src/server/lib/nativeHelm/helm.ts @@ -181,6 +181,7 @@ export async function generateHelmManifest(deploy: Deploy, jobId: string, option namespace: options.namespace, serviceAccount: serviceAccountName, serviceName: deploy.deployable.name, + buildUUID: build.uuid, isStatic: build.isStatic, gitUsername: GIT_USERNAME, gitToken, diff --git a/src/server/lib/nativeHelm/utils.ts b/src/server/lib/nativeHelm/utils.ts index 9a4f0d12..706a5889 100644 --- a/src/server/lib/nativeHelm/utils.ts +++ b/src/server/lib/nativeHelm/utils.ts @@ -27,6 +27,7 @@ import { import { HelmConfigBuilder } from 'server/lib/config/ConfigBuilder'; import rootLogger from 'server/lib/logger'; import { shellPromise } from 'server/lib/shell'; +import { normalizeKubernetesLabelValue } from 'server/lib/kubernetes/utils'; const logger = rootLogger.child({ filename: 'lib/nativeHelm/utils.ts', @@ -558,6 +559,7 @@ export function createHelmJob( isStatic: boolean, serviceAccountName: string = 'default', serviceName: string, + buildUUID: string, deployMetadata?: { sha: string; branch: string; @@ -571,13 +573,13 @@ export function createHelmJob( const labels: Record = { 'app.kubernetes.io/name': 'native-helm', 'app.kubernetes.io/component': 'deployment', - 'lc-uuid': name.split('-')[0], + 'lc-uuid': buildUUID, service: serviceName, }; if (deployMetadata) { labels['git-sha'] = deployMetadata.sha; - labels['git-branch'] = deployMetadata.branch; + labels['git-branch'] = normalizeKubernetesLabelValue(deployMetadata.branch); labels['deploy-id'] = deployMetadata.deployId || ''; labels['deployable-id'] = deployMetadata.deployableId; } From 178b54f01551649b02e564afd259dbcc0c010327 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Tue, 12 Aug 2025 12:09:09 -0700 Subject: [PATCH 4/7] fix: add version to public and org chart's nativeHelm deploy manifest --- .../lib/nativeHelm/__tests__/helm.test.ts | 196 +++++++++++++++++- src/server/lib/nativeHelm/helm.ts | 10 +- src/server/lib/nativeHelm/utils.ts | 13 +- 3 files changed, 209 insertions(+), 10 deletions(-) diff --git a/src/server/lib/nativeHelm/__tests__/helm.test.ts b/src/server/lib/nativeHelm/__tests__/helm.test.ts index e1400429..e5ee82b1 100644 --- a/src/server/lib/nativeHelm/__tests__/helm.test.ts +++ b/src/server/lib/nativeHelm/__tests__/helm.test.ts @@ -392,15 +392,116 @@ describe('Native Helm', () => { ['auth.username=admin', 'auth.password=secret'], [], ChartType.PUBLIC, - '--version 12.9.0 --wait', - 'oci://ghcr.io/myorg/charts/postgresql' + '--wait', + 'oci://ghcr.io/myorg/charts/postgresql', + undefined, + '12.9.0' ); expect(result).toContain('helm upgrade --install my-release oci://ghcr.io/myorg/charts/postgresql'); expect(result).toContain('--namespace my-namespace'); + expect(result).toContain('--version 12.9.0'); expect(result).toContain('--set "auth.username=admin"'); expect(result).toContain('--set "auth.password=secret"'); - expect(result).toContain('--version 12.9.0 --wait'); + expect(result).toContain('--wait'); + }); + + it('should add chart version for PUBLIC charts when version is specified', () => { + const result = constructHelmCommand( + 'upgrade --install', + 'bitnami/postgresql', + 'my-release', + 'my-namespace', + ['auth.username=postgres_user'], + [], + ChartType.PUBLIC, + undefined, + 'https://charts.bitnami.com/bitnami', + undefined, + '12.9.0' + ); + + expect(result).toContain('helm upgrade --install my-release bitnami/postgresql'); + expect(result).toContain('--namespace my-namespace'); + expect(result).toContain('--version 12.9.0'); + expect(result).toContain('--set "auth.username=postgres_user"'); + }); + + it('should not add chart version for LOCAL charts even when version is specified', () => { + const result = constructHelmCommand( + 'upgrade --install', + './my-local-chart', + 'my-release', + 'my-namespace', + [], + [], + ChartType.LOCAL, + undefined, + undefined, + undefined, + '1.0.0' + ); + + expect(result).toContain('helm upgrade --install my-release ./my-local-chart'); + expect(result).toContain('--namespace my-namespace'); + expect(result).not.toContain('--version'); + }); + + it('should not add chart version for PUBLIC charts when version is not specified', () => { + const result = constructHelmCommand( + 'upgrade --install', + 'bitnami/postgresql', + 'my-release', + 'my-namespace', + [], + [], + ChartType.PUBLIC, + undefined, + 'https://charts.bitnami.com/bitnami' + ); + + expect(result).toContain('helm upgrade --install my-release bitnami/postgresql'); + expect(result).toContain('--namespace my-namespace'); + expect(result).not.toContain('--version'); + }); + + it('should add chart version for ORG_CHART when version is specified', () => { + const result = constructHelmCommand( + 'upgrade --install', + 'helm-app', + 'my-release', + 'my-namespace', + ['deployment.appImage=myapp:latest'], + [], + ChartType.ORG_CHART, + undefined, + 'cm://h.cfcr.io/helm/default', + undefined, + '2.0.9' + ); + + expect(result).toContain('helm upgrade --install my-release helm-app'); + expect(result).toContain('--namespace my-namespace'); + expect(result).toContain('--version 2.0.9'); + expect(result).toContain('--set "deployment.appImage=myapp:latest"'); + }); + + it('should not add chart version for ORG_CHART when version is not specified', () => { + const result = constructHelmCommand( + 'upgrade --install', + 'helm-app', + 'my-release', + 'my-namespace', + [], + [], + ChartType.ORG_CHART, + undefined, + 'cm://h.cfcr.io/helm/default' + ); + + expect(result).toContain('helm upgrade --install my-release helm-app'); + expect(result).toContain('--namespace my-namespace'); + expect(result).not.toContain('--version'); }); }); @@ -417,7 +518,8 @@ describe('Native Helm', () => { ChartType.PUBLIC, '--force --timeout 60m0s --wait', 'https://charts.example.com', - '--wait --timeout 30m' // defaultArgs + '--wait --timeout 30m', + '1.2.3' ); expect(result.name).toBe('helm-deploy'); @@ -653,4 +755,90 @@ describe('Native Helm', () => { expect(customValues).toContain('commonLabels.name=build-123'); }); }); + + describe('Chart Version Integration', () => { + it('should include chart version from global config for PUBLIC charts', async () => { + mockGetAllConfigs.mockResolvedValue({ + postgresql: { + version: '3.7.2', + chart: { + name: 'postgresql', + repoUrl: 'https://charts.bitnami.com/bitnami', + version: '12.9.0', + values: ['auth.username=postgres_user'], + }, + }, + }); + + const result = await createHelmContainer( + 'no-repo', + 'postgresql', + 'test-release', + 'test-namespace', + '3.12.0', + ['auth.username=postgres_user'], + [], + ChartType.PUBLIC, + undefined, + 'https://charts.bitnami.com/bitnami', + undefined, + '12.9.0' + ); + + expect(result.args[0]).toContain('--version 12.9.0'); + expect(result.args[0]).toContain('bitnami/postgresql'); + }); + + it('should include chart version from global config for ORG_CHART', async () => { + mockGetOrgChartName.mockResolvedValue('helm-app'); + mockGetAllConfigs.mockResolvedValue({ + 'helm-app': { + version: '3.7.2', + chart: { + name: 'helm-app', + repoUrl: 'cm://h.cfcr.io/helm/default', + version: '2.0.9', + values: ['deployment.customNodeAffinity.enabled=true'], + }, + }, + }); + + const deploy = { + uuid: 'test-uuid', + dockerImage: 'myapp:v1.2.3', + env: {}, + deployable: { + buildUUID: 'build-123', + helm: { + chart: { name: 'helm-app' }, + docker: { app: {} }, + }, + }, + build: { + commentRuntimeEnv: {}, + }, + } as any; + + const chartType = await determineChartType(deploy); + expect(chartType).toBe(ChartType.ORG_CHART); + + const result = await createHelmContainer( + 'no-repo', + 'helm-app', + 'test-release', + 'test-namespace', + '3.7.2', + ['deployment.appImage=myapp:v1.2.3', 'version=v1.2.3'], + [], + ChartType.ORG_CHART, + undefined, + 'cm://h.cfcr.io/helm/default', + undefined, + '2.0.9' + ); + + expect(result.args[0]).toContain('--version 2.0.9'); + expect(result.args[0]).toContain('helm-app'); + }); + }); }); diff --git a/src/server/lib/nativeHelm/helm.ts b/src/server/lib/nativeHelm/helm.ts index 3ff708b7..8a6f1ac6 100644 --- a/src/server/lib/nativeHelm/helm.ts +++ b/src/server/lib/nativeHelm/helm.ts @@ -74,7 +74,8 @@ export async function createHelmContainer( chartType: ChartType, args?: string, chartRepoUrl?: string, - defaultArgs?: string + defaultArgs?: string, + chartVersion?: string ): Promise { const script = generateHelmInstallScript( repoName, @@ -86,7 +87,8 @@ export async function createHelmContainer( chartType, args, chartRepoUrl, - defaultArgs + defaultArgs, + chartVersion ); return { @@ -136,6 +138,7 @@ export async function generateHelmManifest(deploy: Deploy, jobId: string, option const { mergeHelmConfigWithGlobal } = await import('./utils'); const mergedHelmConfig = await mergeHelmConfigWithGlobal(deploy); const chartRepoUrl = mergedHelmConfig.chart?.repoUrl; + const chartVersion = mergedHelmConfig.chart?.version; const helmArgs = mergedHelmConfig.args; const defaultArgs = mergedHelmConfig.nativeHelm?.defaultArgs; @@ -150,7 +153,8 @@ export async function generateHelmManifest(deploy: Deploy, jobId: string, option helmConfig.chartType, helmArgs, chartRepoUrl, - defaultArgs + defaultArgs, + chartVersion ); const volumeConfig = { diff --git a/src/server/lib/nativeHelm/utils.ts b/src/server/lib/nativeHelm/utils.ts index 706a5889..7faaee13 100644 --- a/src/server/lib/nativeHelm/utils.ts +++ b/src/server/lib/nativeHelm/utils.ts @@ -205,7 +205,8 @@ export function constructHelmCommand( chartType: ChartType, args?: string, chartRepoUrl?: string, - defaultArgs?: string + defaultArgs?: string, + chartVersion?: string ): string { let command = `helm ${action} ${releaseName}`; @@ -231,6 +232,10 @@ export function constructHelmCommand( command += ` --namespace ${namespace}`; + if (chartVersion && (chartType === ChartType.PUBLIC || chartType === ChartType.ORG_CHART)) { + command += ` --version ${chartVersion}`; + } + customValues.forEach((value) => { const equalIndex = value.indexOf('='); if (equalIndex > -1) { @@ -269,7 +274,8 @@ export function generateHelmInstallScript( chartType: ChartType, args?: string, chartRepoUrl?: string, - defaultArgs?: string + defaultArgs?: string, + chartVersion?: string ): string { const helmCommand = constructHelmCommand( 'upgrade --install', @@ -281,7 +287,8 @@ export function generateHelmInstallScript( chartType, args, chartRepoUrl, - defaultArgs + defaultArgs, + chartVersion ); let script = ` From 3cc917dcce3b11d4ad7eaa4d06abdf3bb8cc4a65 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Tue, 12 Aug 2025 13:51:28 -0700 Subject: [PATCH 5/7] fix: add --reset-values flag for nativeHelm seed row --- src/server/db/migrations/001_seed.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/db/migrations/001_seed.ts b/src/server/db/migrations/001_seed.ts index e661f5f3..8bdf3a22 100644 --- a/src/server/db/migrations/001_seed.ts +++ b/src/server/db/migrations/001_seed.ts @@ -447,7 +447,7 @@ export async function up(knex: Knex): Promise { INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('lifecycleDefaults', '{"defaultUUID":"dev-0","defaultPublicUrl":"dev-0.app.0env.com","cfStepType":"helm:1.1.12","ecrDomain":"${ IS_DEV ? '10.96.188.230:5000' : 'distribution.0env.com' }","ecrRegistry":"default","buildPipeline":"","deployCluster":"lifecycle-gke","helmDeployPipeline":"replace_me"}', now(), now(), null, 'Default values for lifecycle'); - INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('helmDefaults', '{"version":"3.12.0","nativeHelm":{"enabled":true,"defaultArgs":"--wait --timeout 30m","defaultHelmVersion":"3.12.0"}}', now(), now(), null, 'Default configuration for helm deployments.'); + INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('helmDefaults', '{"version":"3.12.0","nativeHelm":{"enabled":true,"defaultArgs":"--wait --reset-values --timeout 30m","defaultHelmVersion":"3.12.0"}}', now(), now(), null, 'Default configuration for helm deployments.'); INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('socat-tunneller', '{"version":"3.7.2","args":"--force --timeout 60m0s --wait","action":"install","chart":{"name":"isotoma/socat-tunneller","repoUrl":" https://isotoma.github.io/charts","version":"0.2.0","values":[],"valueFiles":[]},"label":"podAnnotations","tolerations":"tolerations","affinity":"affinity","nodeSelector":"nodeSelector"}', now(), now(), null, 'soca-tunneller configuration for db-tunnels with helm'); INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('lifecycleIgnores', '{"github":{"branches":[],"events":["closed","deleted"],"organizations":[],"botUsers":[]}}', now(), now(), null, 'Data values for Lifecycle to ignore'); INSERT INTO global_config (key, config, "createdAt", "updatedAt", "deletedAt", description) VALUES ('deletePendingHelmReleaseStep', '{"delete":true,"static_delete":true}', now(), now(), null, 'If deletePendingHelmReleaseStep is set to true'); From 91d276bc141c4804a51e1fb8acef6ad27954d83a Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Tue, 12 Aug 2025 14:11:53 -0700 Subject: [PATCH 6/7] fix: duration for failed jobs in deployLogs table --- .../api/v1/builds/[uuid]/services/[name]/deployLogs.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/pages/api/v1/builds/[uuid]/services/[name]/deployLogs.ts b/src/pages/api/v1/builds/[uuid]/services/[name]/deployLogs.ts index df725e11..e15cc2e7 100644 --- a/src/pages/api/v1/builds/[uuid]/services/[name]/deployLogs.ts +++ b/src/pages/api/v1/builds/[uuid]/services/[name]/deployLogs.ts @@ -100,8 +100,13 @@ async function getDeploymentJobs(serviceName: string, namespace: string): Promis if (startedAt) { const startTime = new Date(startedAt).getTime(); - const endTime = completedAt ? new Date(completedAt).getTime() : Date.now(); - duration = Math.floor((endTime - startTime) / 1000); + + if (completedAt) { + const endTime = new Date(completedAt).getTime(); + duration = Math.floor((endTime - startTime) / 1000); + } else if (status === 'Active') { + duration = Math.floor((Date.now() - startTime) / 1000); + } } let podName: string | undefined; From 60e9b2414ffd0e8851e946f699ac928bc326c72b Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Tue, 12 Aug 2025 14:13:59 -0700 Subject: [PATCH 7/7] fix: duration for failed jobs in buildLogs api --- .../api/v1/builds/[uuid]/services/[name]/buildLogs.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/pages/api/v1/builds/[uuid]/services/[name]/buildLogs.ts b/src/pages/api/v1/builds/[uuid]/services/[name]/buildLogs.ts index 7695ef73..32476b75 100644 --- a/src/pages/api/v1/builds/[uuid]/services/[name]/buildLogs.ts +++ b/src/pages/api/v1/builds/[uuid]/services/[name]/buildLogs.ts @@ -87,8 +87,13 @@ async function getNativeBuildJobs(serviceName: string, namespace: string): Promi if (startedAt) { const startTime = new Date(startedAt).getTime(); - const endTime = completedAt ? new Date(completedAt).getTime() : Date.now(); - duration = Math.floor((endTime - startTime) / 1000); + + if (completedAt) { + const endTime = new Date(completedAt).getTime(); + duration = Math.floor((endTime - startTime) / 1000); + } else if (status === 'Active') { + duration = Math.floor((Date.now() - startTime) / 1000); + } } let podName: string | undefined;