diff --git a/core/cli/src/install.ts b/core/cli/src/install.ts index d1d714592..1bf7c9963 100644 --- a/core/cli/src/install.ts +++ b/core/cli/src/install.ts @@ -85,12 +85,15 @@ export const loadHookInstallations = async ( }) return installationsWithoutConflicts.map((installations) => { - return installations.map( - ({ hookConstructor, forHook, options }) => { - const hookPlugin = config.hooks[forHook].plugin - return new hookConstructor(logger, forHook, options, config.pluginOptions[hookPlugin.id as keyof PluginOptions]?.options) - } - ) + return installations.map(({ hookConstructor, forHook, options }) => { + const hookPlugin = config.hooks[forHook].plugin + return new hookConstructor( + logger, + forHook, + options, + config.pluginOptions[hookPlugin.id as keyof PluginOptions]?.options + ) + }) }) } @@ -159,9 +162,7 @@ export default async function installHooks(logger: Logger): Promise } if (errors.length) { - const error = new ToolKitError('could not automatically install hooks:') - error.details = errors.map((error) => `- ${error.message}`).join('\n') - throw error + throw new AggregateError(errors, 'could not automatically install hooks') } await updateHashes(config) diff --git a/lib/schemas/src/hooks/circleci.ts b/lib/schemas/src/hooks/circleci.ts index 9caaa9373..2feb28af7 100644 --- a/lib/schemas/src/hooks/circleci.ts +++ b/lib/schemas/src/hooks/circleci.ts @@ -36,6 +36,7 @@ export const CircleCiJob = z pre: [], post: [] }), + splitIntoMatrix: z.boolean().optional(), custom: CircleCiCustom.optional() }) .partial() diff --git a/plugins/circleci/src/circleci-config.ts b/plugins/circleci/src/circleci-config.ts index 8bb43f359..e972976dc 100644 --- a/plugins/circleci/src/circleci-config.ts +++ b/plugins/circleci/src/circleci-config.ts @@ -355,45 +355,118 @@ const mergeInstallationResults = ( return results } -const toolKitOrbPrefix = (job: string, generatedJobs: CircleCIStatePartial['jobs']) => { - if (job.includes('/') || (generatedJobs && job in generatedJobs)) { - return job - } - - return `tool-kit/${job}` +enum ExecutorCount { + None, + One, + Matrix } const generateWorkflowJobs = ( workflow: CircleCiWorkflow, nodeVersions: string[], tagFilterRegex: string, - generatedJobs: CircleCIStatePartial['jobs'] + customJobs?: CircleCiJob[] ): WorkflowJob[] | undefined => { + // HACK:20250106:IM We were previously implicityly prepending a tool-kit/ + // prefix to workflow job names, as it was assumed that they all were + // referencing jobs from the Tool Kit orb. However, many custom plugins + // define custom jobs which need to be referenced too. To avoid a breaking + // change where we require the tool-kit/ prefix to be explicit, let's try an + // infer whether to prepend the prefix or not. + const jobsShouldBeBare = (workflow.jobs ?? []).reduce((acc, job) => { + const shouldBeBare = + // custom jobs won't want the tool-kit/ prefix + (!orbJobs.includes(job.name) && customJobs?.some(({ name }) => job.name === name)) || + // approval jobs don't need to be declared as a custom job before using + job.custom?.type === 'approval' || + // a slash implies an orb job so we don't want to add another prefix + job.name.includes('/') + acc.set(job.name, shouldBeBare) + return acc + }, new Map()) + const toolKitOrbPrefix = (job: string) => (jobsShouldBeBare.get(job) ? job : `tool-kit/${job}`) + const runsOnMultipleNodeVersions = nodeVersions.length > 1 + const getExecutorCount = (job: CircleCiWorkflowJob) => { + const prefixedName = toolKitOrbPrefix(job.name) + const customJob = customJobs?.find(({ name }) => job.name === name) + if ( + prefixedName.startsWith('tool-kit/') || + (runsOnMultipleNodeVersions && customJob && (customJob.splitIntoMatrix ?? true)) + ) { + if (runsOnMultipleNodeVersions && (job.splitIntoMatrix ?? true)) { + return ExecutorCount.Matrix + } else { + return ExecutorCount.One + } + } else { + return ExecutorCount.None + } + } + return workflow.jobs?.map((job) => { - const splitIntoMatrix = runsOnMultipleNodeVersions && (job.splitIntoMatrix ?? true) + const prefixedName = toolKitOrbPrefix(job.name) + + const executorCount = getExecutorCount(job) + let executorParameter + switch (executorCount) { + case ExecutorCount.None: + executorParameter = {} + break + case ExecutorCount.One: + executorParameter = { executor: 'node' } + break + case ExecutorCount.Matrix: + executorParameter = matrixBoilerplate(prefixedName, nodeVersions) + break + } + return { - [toolKitOrbPrefix(job.name, generatedJobs)]: merge( - splitIntoMatrix - ? matrixBoilerplate(toolKitOrbPrefix(job.name, generatedJobs), nodeVersions) - : { - executor: 'node' - }, + [prefixedName]: merge( + executorParameter, { requires: job.requires?.map((required) => { - if (required === 'checkout') { - return required + const getRequiredData = (): [string, ExecutorCount] => { + if (required === 'checkout') { + return [required, ExecutorCount.None] + } + + // HACK:20250106:IM allow plugins to require orb jobs using the + // tool-kit/ prefix as that's what we want to move towards anyway + const normalisedRequired = required.replace(/^tool-kit\//, '') + const requiredOrb = toolKitOrbPrefix(normalisedRequired) + + if (requiredOrb === 'tool-kit/setup') { + return [requiredOrb, runsOnMultipleNodeVersions ? ExecutorCount.Matrix : ExecutorCount.None] + } + + /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- + * if this arrow function is running then the array is defined + */ + const workflowJobs = workflow.jobs! + const requiredJob = workflowJobs.find(({ name: jobName }) => normalisedRequired === jobName) + if (!requiredJob) { + const error = new ToolKitError( + `CircleCI job ${styles.code(job.name)} in workflow ${styles.code( + workflow.name + )} requires a job (${styles.code(required)}) that isn't defined in the workflow` + ) + error.details = `requiring a job that isn't used in a workflow will cause CircleCI to error. valid jobs used in this workflow are:\n${workflowJobs + .map(({ name }) => `- ${name}`) + .join('\n')}` + throw error + } + return [requiredOrb, getExecutorCount(requiredJob)] } - const requiredOrb = toolKitOrbPrefix(required, generatedJobs) - // only need to include a suffix for the required job if it splits - // into a matrix for Node versions - const splitRequiredIntoMatrix = - runsOnMultipleNodeVersions && - (workflow.jobs?.find(({ name: jobName }) => required === jobName)?.splitIntoMatrix ?? true) - if (!splitRequiredIntoMatrix) { - return requiredOrb + + const [requiredName, requiredExecutorCount] = getRequiredData() + if (requiredExecutorCount === ExecutorCount.Matrix) { + return `${requiredName}-${ + executorCount === ExecutorCount.Matrix ? '<< matrix.executor >>' : 'node' + }` + } else { + return requiredName } - return `${requiredOrb}-${splitIntoMatrix ? '<< matrix.executor >>' : 'node'}` }) }, workflow.runOnRelease && job.runOnRelease ? tagFilter(tagFilterRegex) : {}, @@ -416,7 +489,13 @@ const attachWorkspaceStep = { } } -const generateJob = (job: CircleCiJob): JobConfig => ({ +const generateJob = (job: CircleCiJob, nodeVersions: string[]): JobConfig => ({ + ...((job.splitIntoMatrix ?? true) && nodeVersions.length > 1 + ? { + parameters: { executor: { default: 'node', type: 'executor' } }, + executor: '<< parameters.executor >>' + } + : { executor: 'node' }), steps: [ ...(job.workspace?.attach ? [attachWorkspaceStep] : []), ...(job.steps?.pre ?? []), @@ -521,7 +600,7 @@ export default class CircleCi extends Hook< this.options.jobs .filter((job) => !orbJobs.includes(job.name)) .map((job) => { - return [job.name, generateJob(job)] + return [job.name, generateJob(job, nodeVersions)] }) ) } @@ -529,7 +608,7 @@ export default class CircleCi extends Hook< generated.workflows = Object.fromEntries( this.options.workflows.map((workflow) => { const generatedJobs = { - jobs: generateWorkflowJobs(workflow, nodeVersions, tagFilterRegex, generated.jobs ?? {}) + jobs: generateWorkflowJobs(workflow, nodeVersions, tagFilterRegex, this.options.jobs) } return [workflow.name, mergeWithConcatenatedArrays(generatedJobs, workflow.custom)] }) diff --git a/plugins/circleci/test/circleci-config.test.ts b/plugins/circleci/test/circleci-config.test.ts index 7911083ee..e754f6378 100644 --- a/plugins/circleci/test/circleci-config.test.ts +++ b/plugins/circleci/test/circleci-config.test.ts @@ -7,7 +7,6 @@ import type { import { describe, expect, it } from '@jest/globals' import path from 'path' import winston, { Logger } from 'winston' -import * as YAML from 'yaml' import CircleCi from '../src/circleci-config' @@ -24,6 +23,12 @@ const testWorkflowJob: CircleCiWorkflowJob = { splitIntoMatrix: false } +const dependedWorkflowJob: CircleCiWorkflowJob = { + name: 'that-job', + requires: [], + splitIntoMatrix: false +} + const testPrefixedWorkflowJob: CircleCiWorkflowJob = { name: 'slack/approval-notification', requires: ['another/orb'], @@ -43,12 +48,12 @@ const anotherTestWorkflowJob: CircleCiWorkflowJob = { } const configWithWorkflowJob: CircleCiOptions = { - workflows: [{ name: 'tool-kit', jobs: [testWorkflowJob] }], + workflows: [{ name: 'tool-kit', jobs: [dependedWorkflowJob, testWorkflowJob] }], disableBaseConfig: true } const configWithPrefixedWorkflowJob: CircleCiOptions = { - workflows: [{ name: 'tool-kit', jobs: [testPrefixedWorkflowJob] }], + workflows: [{ name: 'tool-kit', jobs: [{ name: 'another/orb', requires: [] }, testPrefixedWorkflowJob] }], disableBaseConfig: true } @@ -128,6 +133,12 @@ describe('CircleCI config hook', () => { "workflows": { "tool-kit": { "jobs": [ + { + "tool-kit/that-job": { + "executor": "node", + "requires": [], + }, + }, { "tool-kit/test-job": { "executor": "node", @@ -154,6 +165,7 @@ describe('CircleCI config hook', () => { { "jobs": { "test-job": { + "executor": "node", "steps": [ { "run": { @@ -180,9 +192,13 @@ describe('CircleCI config hook', () => { "workflows": { "tool-kit": { "jobs": [ + { + "another/orb": { + "requires": [], + }, + }, { "slack/approval-notification": { - "executor": "node", "requires": [ "another/orb", ], @@ -214,6 +230,7 @@ describe('CircleCI config hook', () => { { "jobs": { "test-job": { + "executor": "node", "steps": [ { "run": { @@ -228,8 +245,13 @@ describe('CircleCI config hook', () => { "tool-kit": { "jobs": [ { - "test-job": { + "tool-kit/that-job": { "executor": "node", + "requires": [], + }, + }, + { + "test-job": { "requires": [ "tool-kit/that-job", ], @@ -272,7 +294,9 @@ describe('CircleCI config hook', () => { forHook: 'CircleCi', hookConstructor: CircleCi, options: { - workflows: [{ name: 'test-workflow', jobs: [testWorkflowJob], runOnRelease: true }] + workflows: [ + { name: 'test-workflow', jobs: [dependedWorkflowJob, testWorkflowJob], runOnRelease: true } + ] } } ] @@ -291,7 +315,9 @@ describe('CircleCI config hook', () => { } ], jobs: [testJob], - workflows: [{ name: 'test-workflow', jobs: [testWorkflowJob], runOnRelease: true }] + workflows: [ + { name: 'test-workflow', jobs: [dependedWorkflowJob, testWorkflowJob], runOnRelease: true } + ] }) } ]) @@ -316,7 +342,9 @@ describe('CircleCI config hook', () => { hookConstructor: CircleCi, options: { jobs: [testJob], - workflows: [{ name: 'test-workflow', jobs: [testWorkflowJob], runOnRelease: true }] + workflows: [ + { name: 'test-workflow', jobs: [dependedWorkflowJob, testWorkflowJob], runOnRelease: true } + ] } } ] @@ -329,7 +357,11 @@ describe('CircleCI config hook', () => { options: expect.objectContaining({ jobs: [overriddenTestJob], workflows: [ - { name: 'test-workflow', jobs: [testWorkflowJob, anotherTestWorkflowJob], runOnRelease: true } + { + name: 'test-workflow', + jobs: [dependedWorkflowJob, testWorkflowJob, anotherTestWorkflowJob], + runOnRelease: true + } ] }) } diff --git a/plugins/circleci/test/files/unmanaged/with-workflow-job/.circleci/config.yml b/plugins/circleci/test/files/unmanaged/with-workflow-job/.circleci/config.yml index 8f0355937..f1486f787 100644 --- a/plugins/circleci/test/files/unmanaged/with-workflow-job/.circleci/config.yml +++ b/plugins/circleci/test/files/unmanaged/with-workflow-job/.circleci/config.yml @@ -1,6 +1,9 @@ workflows: tool-kit: jobs: + - tool-kit/that-job: + requires: [] + executor: node - tool-kit/test-job: requires: - tool-kit/that-job