From fb9a65290786cc72fc4a3f3c75a4c16ace4d57d6 Mon Sep 17 00:00:00 2001 From: Vigneshraj Sekar Babu Date: Tue, 28 Apr 2026 09:10:55 -0700 Subject: [PATCH] feat: add global builder engine default --- src/server/lib/buildEngines.ts | 9 + src/server/lib/config/ConfigBuilder.ts | 5 +- .../config/__tests__/ConfigBuilder.test.ts | 6 + src/server/lib/helm/__tests__/helm.test.ts | 2 +- src/server/lib/jsonschema/schemas/1.0.0.json | 14 +- src/server/lib/nativeBuild/index.ts | 3 +- .../lib/nativeHelm/__tests__/helm.test.ts | 2 +- .../lib/yamlSchemas/schema_1_0_0/docker.ts | 2 +- src/server/models/yaml/YamlService.ts | 16 +- src/server/services/__tests__/deploy.test.ts | 111 ++++++++++++- .../services/__tests__/deployable.test.ts | 156 +++++++++++++++++- src/server/services/deploy.ts | 3 +- src/server/services/deployable.ts | 5 +- src/server/services/types/globalConfig.ts | 2 + 14 files changed, 321 insertions(+), 15 deletions(-) create mode 100644 src/server/lib/buildEngines.ts diff --git a/src/server/lib/buildEngines.ts b/src/server/lib/buildEngines.ts new file mode 100644 index 00000000..7d0b391f --- /dev/null +++ b/src/server/lib/buildEngines.ts @@ -0,0 +1,9 @@ +export const NATIVE_BUILDER_ENGINES = ['buildkit', 'kaniko'] as const; +export const BUILDER_ENGINES = [...NATIVE_BUILDER_ENGINES, 'ci'] as const; + +export type NativeBuilderEngine = (typeof NATIVE_BUILDER_ENGINES)[number]; +export type BuilderEngine = (typeof BUILDER_ENGINES)[number]; + +export function isNativeBuilderEngine(engine: unknown): engine is NativeBuilderEngine { + return NATIVE_BUILDER_ENGINES.includes(engine as NativeBuilderEngine); +} diff --git a/src/server/lib/config/ConfigBuilder.ts b/src/server/lib/config/ConfigBuilder.ts index ff051671..bade3c4f 100644 --- a/src/server/lib/config/ConfigBuilder.ts +++ b/src/server/lib/config/ConfigBuilder.ts @@ -20,6 +20,7 @@ import { NativeHelmConfig as GlobalNativeHelmConfig, NativeHelmPostRendererConfig as GlobalNativeHelmPostRendererConfig, } from 'server/services/types/globalConfig'; +import { BuilderEngine } from 'server/lib/buildEngines'; export type HelmPostRendererConfig = GlobalNativeHelmPostRendererConfig; export type NativeHelmConfig = Partial; @@ -39,7 +40,7 @@ export interface HelmConfig { } export interface BuildConfig { - engine?: 'buildkit' | 'kaniko'; + engine?: BuilderEngine; serviceAccount?: string; jobTimeout?: number; resources?: { @@ -171,7 +172,7 @@ export class HelmConfigBuilder extends ConfigBuilder { } export class BuildConfigBuilder extends ConfigBuilder { - setEngine(engine: 'buildkit' | 'kaniko'): BuildConfigBuilder { + setEngine(engine: BuilderEngine): BuildConfigBuilder { this.set('engine', engine); return this; } diff --git a/src/server/lib/config/__tests__/ConfigBuilder.test.ts b/src/server/lib/config/__tests__/ConfigBuilder.test.ts index d12de440..f8a73b9e 100644 --- a/src/server/lib/config/__tests__/ConfigBuilder.test.ts +++ b/src/server/lib/config/__tests__/ConfigBuilder.test.ts @@ -142,6 +142,12 @@ describe('ConfigBuilder', () => { endpoint: 'tcp://buildkit-custom:1234', }); }); + + it('sets ci as an explicit build engine', () => { + const config = new BuildConfigBuilder().setEngine('ci').build(); + + expect(config.engine).toBe('ci'); + }); }); describe('GlobalConfigBuilder', () => { diff --git a/src/server/lib/helm/__tests__/helm.test.ts b/src/server/lib/helm/__tests__/helm.test.ts index eefcc442..39ca9205 100644 --- a/src/server/lib/helm/__tests__/helm.test.ts +++ b/src/server/lib/helm/__tests__/helm.test.ts @@ -336,7 +336,7 @@ describe('Helm tests', () => { buildUUID: 'build-123', port: 8080, builder: { - engine: 'docker', + engine: 'ci', }, helm: { chart: { name: 'lifecycle-app', values: [] }, diff --git a/src/server/lib/jsonschema/schemas/1.0.0.json b/src/server/lib/jsonschema/schemas/1.0.0.json index 1dc67d2e..b23d9ec1 100644 --- a/src/server/lib/jsonschema/schemas/1.0.0.json +++ b/src/server/lib/jsonschema/schemas/1.0.0.json @@ -376,7 +376,12 @@ "additionalProperties": true, "properties": { "engine": { - "type": "string" + "type": "string", + "enum": [ + "buildkit", + "kaniko", + "ci" + ] }, "resources": { "type": "object", @@ -809,7 +814,12 @@ "additionalProperties": true, "properties": { "engine": { - "type": "string" + "type": "string", + "enum": [ + "buildkit", + "kaniko", + "ci" + ] }, "resources": { "type": "object", diff --git a/src/server/lib/nativeBuild/index.ts b/src/server/lib/nativeBuild/index.ts index 46027d12..28ead1c7 100644 --- a/src/server/lib/nativeBuild/index.ts +++ b/src/server/lib/nativeBuild/index.ts @@ -19,6 +19,7 @@ import { getLogger, withSpan, withLogContext } from '../logger'; import { ensureNamespaceExists } from './utils'; import { buildWithEngine, NativeBuildOptions } from './engines'; import { ensureServiceAccountForJob } from '../kubernetes/common/serviceAccount'; +import { isNativeBuilderEngine } from '../buildEngines'; export type { NativeBuildOptions } from './engines'; @@ -51,7 +52,7 @@ export async function buildWithNative(deploy: Deploy, options: NativeBuildOption let result: NativeBuildResult; - if (builderEngine === 'buildkit' || builderEngine === 'kaniko') { + if (isNativeBuilderEngine(builderEngine)) { getLogger().debug(`Build: using ${builderEngine} engine`); result = await buildWithEngine(deploy, buildOptions, builderEngine); } else { diff --git a/src/server/lib/nativeHelm/__tests__/helm.test.ts b/src/server/lib/nativeHelm/__tests__/helm.test.ts index bbb19b41..91721932 100644 --- a/src/server/lib/nativeHelm/__tests__/helm.test.ts +++ b/src/server/lib/nativeHelm/__tests__/helm.test.ts @@ -953,7 +953,7 @@ describe('Native Helm', () => { buildUUID: 'build-123', port: 8080, builder: { - engine: 'docker', + engine: 'ci', }, helm: { chart: { name: 'lifecycle-app', values: [] }, diff --git a/src/server/lib/yamlSchemas/schema_1_0_0/docker.ts b/src/server/lib/yamlSchemas/schema_1_0_0/docker.ts index 4b1ecfb8..37c200fa 100644 --- a/src/server/lib/yamlSchemas/schema_1_0_0/docker.ts +++ b/src/server/lib/yamlSchemas/schema_1_0_0/docker.ts @@ -25,7 +25,7 @@ export const docker = { type: 'object', additionalProperties: true, properties: { - engine: { type: 'string' }, + engine: { type: 'string', enum: ['buildkit', 'kaniko', 'ci'] }, resources: { type: 'object', additionalProperties: false, diff --git a/src/server/models/yaml/YamlService.ts b/src/server/models/yaml/YamlService.ts index a902a204..43027f0a 100644 --- a/src/server/models/yaml/YamlService.ts +++ b/src/server/models/yaml/YamlService.ts @@ -21,6 +21,7 @@ import GlobalConfigService from 'server/services/globalConfig'; import { DeployTypes, FeatureFlags, NO_DEFAULT_ENV_UUID } from 'shared/constants'; import Build from '../Build'; import { DomainDefaults, NativeHelmConfig } from 'server/services/types/globalConfig'; +import { BuilderEngine } from 'server/lib/buildEngines'; export interface Service001 { readonly github: { @@ -291,7 +292,7 @@ export interface ServiceDiskConfig { } export interface Builder { - readonly engine?: string; + readonly engine?: BuilderEngine; readonly resources?: { readonly requests?: Record; readonly limits?: Record; @@ -413,6 +414,19 @@ export function hasLifecycleManagedDockerBuild(service: Service): boolean { } } +export function getEffectiveBuilder(service: Service, defaultEngine?: BuilderEngine): Builder { + const builder = getBuilder(service) ?? {}; + + if (builder.engine || !defaultEngine || !hasLifecycleManagedDockerBuild(service)) { + return builder; + } + + return { + ...builder, + engine: defaultEngine, + }; +} + /** * Helper function to quickly retrieve the environment variables from varies Lifecycle service type (because they are in different locations within the JSON structure). * @param service Valid Lifecycle Service YAML model diff --git a/src/server/services/__tests__/deploy.test.ts b/src/server/services/__tests__/deploy.test.ts index c505fa4e..574dd97c 100644 --- a/src/server/services/__tests__/deploy.test.ts +++ b/src/server/services/__tests__/deploy.test.ts @@ -23,6 +23,14 @@ import * as github from 'server/lib/github'; mockRedisClient(); const mockCliDeploy = jest.fn(); +const mockCodefreshBuildImage = jest.fn(); +const mockCodefreshGetLogs = jest.fn(); +const mockCodefreshGetRepositoryTag = jest.fn(); +const mockCodefreshTagExists = jest.fn(); +const mockCodefreshWaitForImage = jest.fn(); +const mockBuildWithNative = jest.fn(); +const mockGlobalConfigGetAllConfigs = jest.fn(); +const mockGlobalConfigGetOrgChartName = jest.fn(); jest.mock('server/lib/logger', () => ({ getLogger: jest.fn(() => ({ @@ -41,11 +49,24 @@ jest.mock('server/services/globalConfig', () => ({ __esModule: true, default: { getInstance: jest.fn(() => ({ - getOrgChartName: jest.fn().mockResolvedValue('org-chart'), + getAllConfigs: (...args: any[]) => mockGlobalConfigGetAllConfigs(...args), + getOrgChartName: (...args: any[]) => mockGlobalConfigGetOrgChartName(...args), })), }, })); +jest.mock('server/lib/codefresh', () => ({ + buildImage: (...args: any[]) => mockCodefreshBuildImage(...args), + getLogs: (...args: any[]) => mockCodefreshGetLogs(...args), + getRepositoryTag: (...args: any[]) => mockCodefreshGetRepositoryTag(...args), + tagExists: (...args: any[]) => mockCodefreshTagExists(...args), + waitForImage: (...args: any[]) => mockCodefreshWaitForImage(...args), +})); + +jest.mock('server/lib/nativeBuild', () => ({ + buildWithNative: (...args: any[]) => mockBuildWithNative(...args), +})); + const mockDetermineChartType = jest.fn(); jest.mock('server/lib/nativeHelm', () => ({ ...jest.requireActual('server/lib/nativeHelm'), @@ -91,6 +112,25 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { beforeEach(() => { jest.clearAllMocks(); mockCliDeploy.mockReset(); + mockCodefreshBuildImage.mockReset(); + mockCodefreshGetLogs.mockReset(); + mockCodefreshGetRepositoryTag.mockReset(); + mockCodefreshTagExists.mockReset(); + mockCodefreshWaitForImage.mockReset(); + mockBuildWithNative.mockReset(); + mockGlobalConfigGetOrgChartName.mockResolvedValue('org-chart'); + mockGlobalConfigGetAllConfigs.mockResolvedValue({ + lifecycleDefaults: { + buildPipeline: 'sample/build-image', + deployCluster: 'test-cluster', + ecrDomain: '123456789012.dkr.ecr.us-west-2.amazonaws.com', + ecrRegistry: 'sample-registry', + }, + app_setup: { + org: 'example-org', + }, + buildDefaults: {}, + }); mockDetermineChartType.mockResolvedValue(ChartType.PUBLIC); mockDb = { @@ -352,6 +392,75 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { ); }); + test('buildImageForHelmAndGithub uses Codefresh when builder engine is ci', async () => { + (github.getSHAForBranch as jest.Mock).mockResolvedValue('abcdef1234567890'); + mockCodefreshTagExists.mockResolvedValue(false); + mockCodefreshBuildImage.mockResolvedValue('codefresh-build-123'); + mockCodefreshWaitForImage.mockResolvedValue(false); + mockCodefreshGetLogs.mockResolvedValue('codefresh logs'); + const patchSpy = jest.spyOn(deployService, 'patchAndUpdateActivityFeed').mockResolvedValue(undefined); + const deployPatch = jest.fn().mockResolvedValue(undefined); + const deploy = { + uuid: 'sample-service-build', + runUUID: 'run-1', + branchName: 'feature-branch', + env: {}, + initEnv: {}, + dockerImage: 'old-image', + service: { + name: 'sample-service', + }, + build: { + id: 1, + uuid: 'sample-build', + namespace: 'env-sample', + enableFullYaml: true, + commentRuntimeEnv: {}, + enabledFeatures: [], + pullRequest: { + githubLogin: 'sample-user', + }, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }, + deployable: { + name: 'sample-service', + type: DeployTypes.GITHUB, + dockerfilePath: './Dockerfile', + initDockerfilePath: null, + env: {}, + ecr: 'sample/app-images', + dockerBuildPipelineName: 'sample/build-image', + builder: { + engine: 'ci', + }, + repository: { + fullName: 'example-org/example-repo', + }, + $fetchGraph: jest.fn().mockResolvedValue(undefined), + }, + reload: jest.fn().mockResolvedValue(undefined), + $query: jest.fn(() => ({ + patch: deployPatch, + })), + }; + + const result = await deployService.buildImageForHelmAndGithub(deploy as any, 'run-1'); + + expect(result).toBe(false); + expect(mockBuildWithNative).not.toHaveBeenCalled(); + expect(mockCodefreshBuildImage).toHaveBeenCalledTimes(1); + expect(mockCodefreshBuildImage).toHaveBeenCalledWith( + expect.objectContaining({ + buildPipelineName: 'sample/build-image', + dockerfilePath: './Dockerfile', + repo: 'example-org/example-repo', + }) + ); + expect(deployPatch).toHaveBeenCalledWith({ buildPipelineId: 'codefresh-build-123' }); + expect(deployPatch).toHaveBeenCalledWith({ buildOutput: 'codefresh logs' }); + expect(patchSpy).toHaveBeenLastCalledWith(deploy, { status: DeployStatus.BUILD_FAILED }, 'run-1'); + }); + test('deployAurora records failures with the newly assigned runUUID', async () => { const patchSpy = jest.spyOn(deployService, 'patchAndUpdateActivityFeed').mockResolvedValue(undefined); jest.spyOn(deployService as any, 'findExistingAuroraDatabase').mockResolvedValue(null); diff --git a/src/server/services/__tests__/deployable.test.ts b/src/server/services/__tests__/deployable.test.ts index 04cb1216..30eabfd6 100644 --- a/src/server/services/__tests__/deployable.test.ts +++ b/src/server/services/__tests__/deployable.test.ts @@ -50,11 +50,13 @@ const domainDefaults = { grpc: 'lifecycle-grpc.example.com', }; -const mockedGetAllConfigs = jest.fn().mockResolvedValue({ +const globalConfigs = { lifecycleDefaults: lifecycleDefaults, serviceDefaults: serviceDefaults, domainDefaults: domainDefaults, -}); +}; + +const mockedGetAllConfigs = jest.fn().mockResolvedValue(globalConfigs); const mockedInstance = { getAllConfigs: mockedGetAllConfigs, @@ -66,6 +68,11 @@ describe('Deployable Service', () => { describe('generateAttributesFromYamlConfig', () => { const deployableService: DeployableService = new DeployableService(null, null, null); + beforeEach(() => { + mockedGetAllConfigs.mockResolvedValue(globalConfigs); + mockedInstance.isFeatureEnabled.mockResolvedValue(false); + }); + test('Generates from Github Service Type Configuration', async () => { const githubService: YamlService.GithubService = { name: 'github-app', @@ -423,5 +430,150 @@ describe('Deployable Service', () => { expect(result.readinessHttpGetPort).toBeUndefined(); expect(result.readinessHttpGetPath).toBeUndefined(); }); + + test('inherits global buildkit engine for Github docker services', async () => { + mockedGetAllConfigs.mockResolvedValue({ + ...globalConfigs, + buildDefaults: { engine: 'buildkit' }, + }); + const githubService: YamlService.GithubService = { + name: 'github-app', + github: { + repository: 'example-org/example-service', + branchName: 'unit-test', + docker: { + defaultTag: 'main', + app: { + dockerfilePath: 'app1/app.Dockerfile', + ports: [8080], + }, + }, + }, + }; + + // @ts-ignore + const result: DeployableAttributes = await deployableService.generateAttributesFromYamlConfig( + 100, + 'unit-test-12345', + '1234567890', + 'unit-test', + githubService + ); + + expect(result.builder).toEqual({ engine: 'buildkit' }); + }); + + test('inherits global buildkit engine for Helm docker services', async () => { + mockedGetAllConfigs.mockResolvedValue({ + ...globalConfigs, + buildDefaults: { engine: 'buildkit' }, + 'sample-chart': { + chart: { + name: 'sample-chart', + values: [], + }, + }, + }); + const helmService = { + name: 'helm-app', + helm: { + cfStepType: 'helm', + repository: 'example-org/example-service', + branchName: 'unit-test', + chart: { + name: 'sample-chart', + values: [], + }, + docker: { + defaultTag: 'main', + app: { + dockerfilePath: 'app1/app.Dockerfile', + ports: [8080], + }, + }, + }, + } as unknown as YamlService.Service; + + // @ts-ignore + const result: DeployableAttributes = await deployableService.generateAttributesFromYamlConfig( + 100, + 'unit-test-12345', + '1234567890', + 'unit-test', + helmService + ); + + expect(result.builder).toEqual({ engine: 'buildkit' }); + }); + + test('service builder engine ci overrides global buildkit and persists ci', async () => { + mockedGetAllConfigs.mockResolvedValue({ + ...globalConfigs, + buildDefaults: { engine: 'buildkit' }, + }); + const githubService: YamlService.GithubService = { + name: 'github-app', + github: { + repository: 'example-org/example-service', + branchName: 'unit-test', + docker: { + defaultTag: 'main', + builder: { + engine: 'ci', + }, + app: { + dockerfilePath: 'app1/app.Dockerfile', + ports: [8080], + }, + }, + }, + }; + + // @ts-ignore + const result: DeployableAttributes = await deployableService.generateAttributesFromYamlConfig( + 100, + 'unit-test-12345', + '1234567890', + 'unit-test', + githubService + ); + + expect(result.builder).toEqual({ engine: 'ci' }); + }); + + test('service builder engine kaniko overrides global buildkit', async () => { + mockedGetAllConfigs.mockResolvedValue({ + ...globalConfigs, + buildDefaults: { engine: 'buildkit' }, + }); + const githubService: YamlService.GithubService = { + name: 'github-app', + github: { + repository: 'example-org/example-service', + branchName: 'unit-test', + docker: { + defaultTag: 'main', + builder: { + engine: 'kaniko', + }, + app: { + dockerfilePath: 'app1/app.Dockerfile', + ports: [8080], + }, + }, + }, + }; + + // @ts-ignore + const result: DeployableAttributes = await deployableService.generateAttributesFromYamlConfig( + 100, + 'unit-test-12345', + '1234567890', + 'unit-test', + githubService + ); + + expect(result.builder).toEqual({ engine: 'kaniko' }); + }); }); }); diff --git a/src/server/services/deploy.ts b/src/server/services/deploy.ts index e080a061..f15ce6bf 100644 --- a/src/server/services/deploy.ts +++ b/src/server/services/deploy.ts @@ -40,6 +40,7 @@ import { ChartType, determineChartType } from 'server/lib/nativeHelm'; import { parseSecretRefsFromEnv } from 'server/lib/secretRefs'; import { SecretProcessor } from 'server/services/secretProcessor'; import { fallbackDeployStatusMessage, statusMessageFromError } from 'server/lib/terminalFailure'; +import { isNativeBuilderEngine } from 'server/lib/buildEngines'; export interface DeployOptions { ownerId?: number; @@ -1088,7 +1089,7 @@ export default class DeployService extends BaseService { deployCluster: lifecycleDefaults.deployCluster, }; - if (['buildkit', 'kaniko'].includes(deployable.builder?.engine)) { + if (isNativeBuilderEngine(deployable.builder?.engine)) { getLogger().info(`Image: building engine=${deployable.builder.engine}`); let buildSecretNames: string[] = []; diff --git a/src/server/services/deployable.ts b/src/server/services/deployable.ts index c7f20bfa..a0519b88 100644 --- a/src/server/services/deployable.ts +++ b/src/server/services/deployable.ts @@ -290,7 +290,8 @@ export default class DeployableService extends BaseService { } } } - const { serviceDefaults, lifecycleDefaults, domainDefaults } = await GlobalConfigService.getInstance().getAllConfigs(); + const { serviceDefaults, lifecycleDefaults, domainDefaults, buildDefaults } = + await GlobalConfigService.getInstance().getAllConfigs(); //TODO check and throw error here? const defaultUUID = lifecycleDefaults.defaultUUID; const dockerBuildPipelineName = @@ -373,7 +374,7 @@ export default class DeployableService extends BaseService { dependsOnDeployableName, helm: await YamlService.getHelmConfigFromYaml(service), deploymentDependsOn: service.deploymentDependsOn || [], - builder: YamlService.getBuilder(service) ?? {}, + builder: YamlService.getEffectiveBuilder(service, buildDefaults?.engine) ?? {}, envLens: await YamlService.getEnvLens(service), }; } diff --git a/src/server/services/types/globalConfig.ts b/src/server/services/types/globalConfig.ts index 4dc4f017..fc7b97b0 100644 --- a/src/server/services/types/globalConfig.ts +++ b/src/server/services/types/globalConfig.ts @@ -15,6 +15,7 @@ */ import { Helm } from 'server/models/yaml'; +import { BuilderEngine } from 'server/lib/buildEngines'; export type GlobalConfig = { lifecycleDefaults: LifecycleDefaults; @@ -203,6 +204,7 @@ export type NativeHelmConfig = { }; export type BuildDefaults = { + engine?: BuilderEngine; jobTimeout?: number; serviceAccount?: string; cacheRegistry?: string;