Skip to content

Commit

Permalink
fix(revert): Revert changes to spec involving build/image
Browse files Browse the repository at this point in the history
This reverts commit b08f29f.
  • Loading branch information
mueschm authored Apr 12, 2023
1 parent 6eb973e commit 11a2e36
Show file tree
Hide file tree
Showing 14 changed files with 22 additions and 162 deletions.
2 changes: 1 addition & 1 deletion src/commands/clusters/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '';
}

Expand Down
1 change: 0 additions & 1 deletion src/commands/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@ 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;
}
Expand Down
8 changes: 7 additions & 1 deletion src/dependency-manager/spec/resource-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ 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, StringOrStringArray } from './utils/json-schema-annotations';
import { AnyOf, ArrayOf, ExclusiveOrNeither, ExpressionOr, ExpressionOrString, OneOf, 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 {
Expand Down Expand Up @@ -53,6 +56,9 @@ export class BuildSpec {
target?: string;
}

@JSONSchema({
...OneOf('build', 'image'),
})
export abstract class ResourceSpec {
abstract get resource_type(): ResourceType;

Expand Down
3 changes: 1 addition & 2 deletions src/dependency-manager/spec/service-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, ExclusiveOrNeither, ExpressionOr, ExpressionOrString, RequiredOr } from './utils/json-schema-annotations';
import { AnyOf, ExclusiveOr, ExpressionOr, ExpressionOrString, RequiredOr } from './utils/json-schema-annotations';
import { ResourceType, Slugs } from './utils/slugs';

@JSONSchema({
Expand Down Expand Up @@ -197,7 +197,6 @@ 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 {
Expand Down
3 changes: 1 addition & 2 deletions src/dependency-manager/spec/task-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ import { IsOptional, ValidateNested } from 'class-validator';
import { JSONSchema } from 'class-validator-jsonschema';
import { WithRequired } from '../../common/utils/types';
import { ResourceSpec } from './resource-spec';
import { ExclusiveOrNeither, ExpressionOrString } from './utils/json-schema-annotations';
import { 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 {
Expand Down
3 changes: 0 additions & 3 deletions src/dependency-manager/spec/utils/json-schema-annotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,6 @@ 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;
};

Expand Down
3 changes: 2 additions & 1 deletion test/dependency-manager/spec/component-spec.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ 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);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
hello:
image: heroku/nodejs-hello-world
description: Hello world test service
language: javascript
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
hello:
image: heroku/nodejs-hello-world
command:
- echo "starting up..."
- bin/web
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
hello:
image: heroku/nodejs-hello-world
command: bin/web
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
hello:
image: heroku/nodejs-hello-world
entrypoint: bin/web
43 changes: 6 additions & 37 deletions test/dependency-manager/spec/partials/spec-test-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ 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())
Expand Down Expand Up @@ -42,44 +38,17 @@ const recursiveMergeTree = (tree: RecursivePartialsTree): object[] => {
const child_partials = recursiveMergeTree(child_tree);

const merged_partials = tree.partials.map(current_partial => {
let accums: object[] = [];
for (const child_partial of child_partials) {
return child_partials.map(child_partial => {
if (child_key.startsWith('_')) {
let accum = current_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);
}
for (const current_key of Object.keys(accum)) {
accum[current_key] = deepmerge(accum[current_key], { [child_key.replace('_', '')]: child_partial });
}

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]);
return accum;
} else {
accums.push(deepmerge(current_partial, { [child_key]: child_partial }) as object);
return deepmerge(current_partial, { [child_key]: child_partial }) as object
}
}
return accums;
});
}).reduce((accumulator, value) => accumulator.concat(value), []);

return merged_partials;
Expand Down
113 changes: 0 additions & 113 deletions test/dependency-manager/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,119 +401,6 @@ 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 = `
Expand Down

0 comments on commit 11a2e36

Please sign in to comment.