diff --git a/src/commands/clusters/create.ts b/src/commands/clusters/create.ts index 3a47e791f..03d5c7ff6 100644 --- a/src/commands/clusters/create.ts +++ b/src/commands/clusters/create.ts @@ -120,7 +120,7 @@ export default class ClusterCreate extends BaseCommand { const cluster_names = new Set(clusters.map(cluster => cluster.name.toLowerCase())); if (args.cluster && cluster_names.has(args.cluster)) { - console.log(chalk.red('A cluster already exists with the desired name please enter a new one.')); + console.log(chalk.red('A cluster already exists with the desired name. Please enter a new one.')); args.cluster = ''; } diff --git a/src/commands/register.ts b/src/commands/register.ts index 73d6fb89d..17cfc9c44 100644 --- a/src/commands/register.ts +++ b/src/commands/register.ts @@ -278,6 +278,7 @@ export default class ComponentRegister extends BaseCommand { continue; } delete service.debug; // we don't need to compare the debug block for remotely-deployed components + delete service.build; // build block data isn't relevant to remotely-deployed components if (service instanceof ServiceSpec && service.enabled !== undefined && !service.enabled) { continue; } diff --git a/src/dependency-manager/spec/resource-spec.ts b/src/dependency-manager/spec/resource-spec.ts index 1af8a1744..67c2244ad 100644 --- a/src/dependency-manager/spec/resource-spec.ts +++ b/src/dependency-manager/spec/resource-spec.ts @@ -2,14 +2,11 @@ import { IsOptional, ValidateNested } from 'class-validator'; import { JSONSchema } from 'class-validator-jsonschema'; import { Dictionary } from '../utils/dictionary'; import { SecretSpecValue } from './secret-spec'; -import { AnyOf, ArrayOf, ExclusiveOrNeither, ExpressionOr, ExpressionOrString, OneOf, StringOrStringArray } from './utils/json-schema-annotations'; +import { AnyOf, ArrayOf, ExclusiveOrNeither, ExpressionOr, ExpressionOrString, StringOrStringArray } from './utils/json-schema-annotations'; import { ResourceType, Slugs } from './utils/slugs'; @JSONSchema({ description: 'An object containing the details necessary for Architect to build the service via Docker. Whenever a service that specifies a build field is registered with Architect, the CLI will trigger a docker build and replace the build field with a resolvable image.', - errorMessage: { - not: 'Buildpack and Dockerfile cannot be used at the same time', - }, ...ExclusiveOrNeither('buildpack', 'dockerfile'), }) export class BuildSpec { @@ -56,9 +53,6 @@ export class BuildSpec { target?: string; } -@JSONSchema({ - ...OneOf('build', 'image'), -}) export abstract class ResourceSpec { abstract get resource_type(): ResourceType; diff --git a/src/dependency-manager/spec/service-spec.ts b/src/dependency-manager/spec/service-spec.ts index 33d028342..bd4431d02 100644 --- a/src/dependency-manager/spec/service-spec.ts +++ b/src/dependency-manager/spec/service-spec.ts @@ -7,7 +7,7 @@ import { Dictionary } from '../utils/dictionary'; import { LivenessProbeSpec, VolumeSpec } from './common-spec'; import { ResourceSpec } from './resource-spec'; import { transformObject } from './transform/common-transform'; -import { AnyOf, ExclusiveOr, ExpressionOr, ExpressionOrString, RequiredOr } from './utils/json-schema-annotations'; +import { AnyOf, ExclusiveOr, ExclusiveOrNeither, ExpressionOr, ExpressionOrString, RequiredOr } from './utils/json-schema-annotations'; import { ResourceType, Slugs } from './utils/slugs'; @JSONSchema({ @@ -197,6 +197,7 @@ export class ServiceInterfaceSpec { @JSONSchema({ description: 'A runtimes (e.g. daemons, servers, etc.). Each service is independently deployable and scalable. Services are 1:1 with a docker image.', + ...ExclusiveOrNeither('build', 'image'), }) export class ServiceSpec extends ResourceSpec { get resource_type(): ResourceType { diff --git a/src/dependency-manager/spec/task-spec.ts b/src/dependency-manager/spec/task-spec.ts index 81f069938..d54c9a585 100644 --- a/src/dependency-manager/spec/task-spec.ts +++ b/src/dependency-manager/spec/task-spec.ts @@ -2,11 +2,12 @@ import { IsOptional, ValidateNested } from 'class-validator'; import { JSONSchema } from 'class-validator-jsonschema'; import { WithRequired } from '../../common/utils/types'; import { ResourceSpec } from './resource-spec'; -import { ExpressionOrString } from './utils/json-schema-annotations'; +import { ExclusiveOrNeither, ExpressionOrString } from './utils/json-schema-annotations'; import { ResourceType } from './utils/slugs'; @JSONSchema({ description: 'A Task represents a recurring and/or exiting runtime (e.g. crons, schedulers, triggered jobs). Each task will run on its specified schedule and/or be triggerable via the Architect CLI. Tasks are 1:1 with a docker image.', + ...ExclusiveOrNeither('build', 'image'), }) export class TaskSpec extends ResourceSpec { get resource_type(): ResourceType { diff --git a/src/dependency-manager/spec/utils/json-schema-annotations.ts b/src/dependency-manager/spec/utils/json-schema-annotations.ts index 952c6c0a6..384326ca2 100644 --- a/src/dependency-manager/spec/utils/json-schema-annotations.ts +++ b/src/dependency-manager/spec/utils/json-schema-annotations.ts @@ -124,6 +124,9 @@ export const ExclusiveOrNeither = (...properties: string[]): DecoratorSchema => type: 'object', required: [...properties], }, + errorMessage: { + not: `Only one of ${properties.join(', ')} is allowed, or none should exist`, + }, } as DecoratorSchema; }; diff --git a/test/dependency-manager/spec/component-spec.unit.test.ts b/test/dependency-manager/spec/component-spec.unit.test.ts index 9649dfcf6..2cbe32f3c 100644 --- a/test/dependency-manager/spec/component-spec.unit.test.ts +++ b/test/dependency-manager/spec/component-spec.unit.test.ts @@ -6,9 +6,8 @@ import { overrideSpec } from '../../../src/dependency-manager/spec/utils/spec-me import { loadAllTestSpecCombinations } from './partials/spec-test-harness'; describe('component spec unit test', () => { - const all_spec_combinations = loadAllTestSpecCombinations(); - it(`recursively test partial architect components`, () => { + const all_spec_combinations = loadAllTestSpecCombinations(); console.debug(`recursively testing ${all_spec_combinations.length} combined components...`); for (const component of all_spec_combinations) { const source_yml = dumpToYml(component); diff --git a/test/dependency-manager/spec/partials/root/services/_image/image.yml b/test/dependency-manager/spec/partials/root/services/_image/image.yml new file mode 100644 index 000000000..4f780b682 --- /dev/null +++ b/test/dependency-manager/spec/partials/root/services/_image/image.yml @@ -0,0 +1 @@ +heroku/nodejs-hello-world diff --git a/test/dependency-manager/spec/partials/root/services/basic-service.yml b/test/dependency-manager/spec/partials/root/services/basic-service.yml index 3aa91b1bf..6f380a7a5 100644 --- a/test/dependency-manager/spec/partials/root/services/basic-service.yml +++ b/test/dependency-manager/spec/partials/root/services/basic-service.yml @@ -1,4 +1,3 @@ hello: - image: heroku/nodejs-hello-world description: Hello world test service language: javascript diff --git a/test/dependency-manager/spec/partials/root/services/service-with-command-array.yml b/test/dependency-manager/spec/partials/root/services/service-with-command-array.yml index b57c1f977..0e737a6bf 100644 --- a/test/dependency-manager/spec/partials/root/services/service-with-command-array.yml +++ b/test/dependency-manager/spec/partials/root/services/service-with-command-array.yml @@ -1,5 +1,4 @@ hello: - image: heroku/nodejs-hello-world command: - echo "starting up..." - bin/web diff --git a/test/dependency-manager/spec/partials/root/services/service-with-command.yml b/test/dependency-manager/spec/partials/root/services/service-with-command.yml index 6543ca15a..3d33f402f 100644 --- a/test/dependency-manager/spec/partials/root/services/service-with-command.yml +++ b/test/dependency-manager/spec/partials/root/services/service-with-command.yml @@ -1,3 +1,2 @@ hello: - image: heroku/nodejs-hello-world command: bin/web diff --git a/test/dependency-manager/spec/partials/root/services/service-with-entrypoint.yml b/test/dependency-manager/spec/partials/root/services/service-with-entrypoint.yml index 517ee0a93..a7c9b3c0a 100644 --- a/test/dependency-manager/spec/partials/root/services/service-with-entrypoint.yml +++ b/test/dependency-manager/spec/partials/root/services/service-with-entrypoint.yml @@ -1,3 +1,2 @@ hello: - image: heroku/nodejs-hello-world entrypoint: bin/web diff --git a/test/dependency-manager/spec/partials/spec-test-harness.ts b/test/dependency-manager/spec/partials/spec-test-harness.ts index 7f7fb5670..8f729f2e8 100644 --- a/test/dependency-manager/spec/partials/spec-test-harness.ts +++ b/test/dependency-manager/spec/partials/spec-test-harness.ts @@ -3,6 +3,10 @@ import fs from 'fs-extra'; import path from 'path'; import { parseSourceYml } from '../../../../src'; +const xor_spec_properties: string[][] = [ // can't let conflicting properties be joined in one spec + ['build', 'image'], +]; + const lsDirectories = (dir: string) => { return fs.readdirSync(path.resolve(dir), { withFileTypes: true }) .filter(dirent => dirent.isDirectory()) @@ -38,17 +42,44 @@ const recursiveMergeTree = (tree: RecursivePartialsTree): object[] => { const child_partials = recursiveMergeTree(child_tree); const merged_partials = tree.partials.map(current_partial => { - return child_partials.map(child_partial => { + let accums: object[] = []; + for (const child_partial of child_partials) { if (child_key.startsWith('_')) { let accum = current_partial; - for (const current_key of Object.keys(accum)) { - accum[current_key] = deepmerge(accum[current_key], { [child_key.replace('_', '')]: child_partial }); + const resource_name = Object.keys(accum)[0]; + + const spec_child_key = child_key.replace('_', ''); + let xor_mapping_values; + for (const xor_array of xor_spec_properties) { + if (xor_array.includes(spec_child_key)) { + xor_mapping_values = xor_array.filter(v => v !== spec_child_key); + break; + } + } + let alt_accums: object[] = []; + for (const xor_mapping_value of (xor_mapping_values || [])) { + if (Object.keys(accum[resource_name]).includes(xor_mapping_value)) { + const alt_accum = JSON.parse(JSON.stringify(accum)); // deep copy + delete alt_accum[resource_name][xor_mapping_value]; + for (const current_key of Object.keys(alt_accum)) { + alt_accum[current_key] = deepmerge(alt_accum[current_key], { [spec_child_key]: child_partial }); + } + alt_accums.push(alt_accum); + } } - return accum; + + if (!alt_accums.length) { // if not alternative, current partial should be added to the spec + for (const current_key of Object.keys(accum)) { + accum[current_key] = deepmerge(accum[current_key], { [spec_child_key]: child_partial }); + } + } + + accums = accums.concat([accum, ...alt_accums]); } else { - return deepmerge(current_partial, { [child_key]: child_partial }) as object + accums.push(deepmerge(current_partial, { [child_key]: child_partial }) as object); } - }); + } + return accums; }).reduce((accumulator, value) => accumulator.concat(value), []); return merged_partials; diff --git a/test/dependency-manager/validation.test.ts b/test/dependency-manager/validation.test.ts index 0fb826e57..5f241193d 100644 --- a/test/dependency-manager/validation.test.ts +++ b/test/dependency-manager/validation.test.ts @@ -401,6 +401,119 @@ services: }); }); + describe('service config validation', () => { + it(`build and image can't be specified together`, async () => { + const component_config = ` + name: test-component + services: + app: + interfaces: + main: 8080 + build: + context: . + image: postgres:15 + `; + mock_fs({ + '/component.yml': component_config, + }); + const manager = new LocalDependencyManager(axios.create(), 'architect', { + 'test-component': '/component.yml', + }); + let err; + try { + await manager.getGraph([ + await manager.loadComponentSpec('test-component'), + ]); + } catch (e: any) { + err = e; + } + + expect(err).instanceOf(ValidationErrors); + const errors = JSON.parse(err.message) as ValidationError[]; + expect(errors).lengthOf(1); + expect(errors[0].message).includes('Only one of build, image is allowed, or none should exist'); + expect(errors[0].component).eq('test-component'); + expect(errors[0].path).eq('services.app'); + expect(Object.keys(errors[0].value)).to.include('image'); + expect(Object.keys(errors[0].value)).to.include('build'); + expect(process.exitCode).eq(1); + }); + + it(`buildpack and dockerfile can't be specified together`, async () => { + const component_config = ` + name: test-component + services: + app: + interfaces: + main: 8080 + build: + buildpack: true + dockerfile: Dockerfile + `; + mock_fs({ + '/component.yml': component_config, + }); + const manager = new LocalDependencyManager(axios.create(), 'architect', { + 'test-component': '/component.yml', + }); + let err; + try { + await manager.getGraph([ + await manager.loadComponentSpec('test-component'), + ]); + } catch (e: any) { + err = e; + } + + expect(err).instanceOf(ValidationErrors); + const errors = JSON.parse(err.message) as ValidationError[]; + expect(errors).lengthOf(1); + expect(errors[0].message).includes('Only one of buildpack, dockerfile is allowed, or none should exist'); + expect(errors[0].component).eq('test-component'); + expect(errors[0].path).eq('services.app.build'); + expect(Object.keys(errors[0].value)).to.include('buildpack'); + expect(Object.keys(errors[0].value)).to.include('dockerfile'); + expect(process.exitCode).eq(1); + }); + }); + + describe('task config validation', () => { + it(`buildpack and dockerfile can't be specified together`, async () => { + const component_config = ` + name: test-component + tasks: + task: + build: + buildpack: true + dockerfile: Dockerfile + `; + mock_fs({ + '/component.yml': component_config, + }); + const manager = new LocalDependencyManager(axios.create(), 'architect', { + 'test-component': '/component.yml', + }); + let err; + try { + await manager.getGraph([ + await manager.loadComponentSpec('test-component'), + ]); + } catch (e: any) { + err = e; + } + + expect(err).instanceOf(ValidationErrors); + const errors = JSON.parse(err.message) as ValidationError[]; + expect(errors).lengthOf(1); + expect(errors[0].message).includes('Only one of buildpack, dockerfile is allowed, or none should exist'); + expect(errors[0].component).eq('test-component'); + expect(errors[0].path).eq('tasks.task.build'); + expect(Object.keys(errors[0].value)).to.include('dockerfile'); + expect(Object.keys(errors[0].value)).to.include('buildpack'); + expect(process.exitCode).eq(1); + }); + }); + describe('component validation', () => { it('invalid component name', async () => { const component_config = `