Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dependencies without tags #893

Merged
merged 6 commits into from
May 15, 2023
Merged
14 changes: 10 additions & 4 deletions src/commands/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import yaml from 'js-yaml';
import path from 'path';
import tmp from 'tmp';
import untildify from 'untildify';
import { ArchitectError, buildNodeRef, buildSpecFromPath, ComponentSlugUtils, ComponentSpec, DependencyGraphMutable, Dictionary, dumpToYml, resourceRefToNodeRef, ServiceNode, ServiceSpec, Slugs, TaskNode, validateInterpolation, VolumeSpec } from '../';
import { ArchitectError, buildNodeRef, buildSpecFromPath, ComponentSlugUtils, ComponentSpec, DependencyGraphMutable, DependencySpec, Dictionary, dumpToYml, resourceRefToNodeRef, ServiceNode, ServiceSpec, Slugs, TaskNode, validateInterpolation, VolumeSpec } from '../';
import Account from '../architect/account/account.entity';
import AccountUtils from '../architect/account/account.utils';
import { EnvironmentUtils, GetEnvironmentOptions } from '../architect/environment/environment.utils';
Expand Down Expand Up @@ -441,10 +441,16 @@ export default class ComponentRegister extends BaseCommand {
return flags.environment ? `${ENV_TAG_PREFIX}${flags.environment}` : flags.tag;
}

private async generateDependenciesWarnings(component_dependencies: Dictionary<string>, account_name: string) {
private async generateDependenciesWarnings(component_dependencies: Dictionary<string | DependencySpec>, account_name: string) {
const dependency_arr: string[] = [];
for (const [component_name, tag] of Object.entries(component_dependencies)) {
dependency_arr.push(`${component_name}:${tag}`);
for (const [component_name, tag_or_object] of Object.entries(component_dependencies)) {
if (typeof tag_or_object === 'string') {
dependency_arr.push(`${component_name}:${tag_or_object}`);
} else if (tag_or_object.tag) {
dependency_arr.push(`${component_name}:${tag_or_object.tag}`);
} else {
dependency_arr.push(`${component_name}:latest`);
}
}
const dependencies: Dictionary<{ component: boolean, component_and_version: boolean }> = (await this.app.api.get(`accounts/${account_name}/components-tags`, { params: { components: dependency_arr } })).data;

Expand Down
4 changes: 2 additions & 2 deletions src/common/dependency-manager/local-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ export default class LocalDependencyManager extends DependencyManager {
const component_spec = await this.loadComponentSpec(component_ref, undefined, debug);
component_specs.push(component_spec);

for (const [dep_name, dep_tag] of Object.entries(component_spec.dependencies || {})) {
component_refs_queue.push(`${dep_name}:${dep_tag}`);
for (const dep_name of Object.keys(component_spec.dependencies || {})) {
component_refs_queue.push(`${dep_name}`);
}
}
return component_specs;
Expand Down
4 changes: 2 additions & 2 deletions src/dependency-manager/config/component-config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ComponentInstanceMetadata, ComponentSpec } from '../spec/component-spec';
import { ComponentInstanceMetadata, ComponentSpec, DependencySpec } from '../spec/component-spec';
import { SecretSpecValue } from '../spec/secret-spec';
import { ComponentSlugUtils, ParsedResourceSlug, ResourceSlugUtils, ResourceType, Slugs } from '../spec/utils/slugs';
import { Dictionary } from '../utils/dictionary';
Expand Down Expand Up @@ -35,7 +35,7 @@ export interface ComponentConfig {
services: Dictionary<ServiceConfig>;
databases: Dictionary<DatabaseConfig>;
tasks: Dictionary<TaskConfig>;
dependencies: Dictionary<string>;
dependencies: Dictionary<string | DependencySpec>;
TylerAldrich marked this conversation as resolved.
Show resolved Hide resolved

artifact_image?: string;
}
Expand Down
22 changes: 18 additions & 4 deletions src/dependency-manager/spec/component-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ export interface ComponentInstanceMetadata {
interpolated?: boolean;
}

@JSONSchema({
description: 'An empty object that optionally supports specifying a tag for backwards compatibility.',
})
export class DependencySpec {
@IsOptional()
@JSONSchema({
type: 'string',
pattern: Slugs.ComponentTagValidator.source,
deprecated: true,
})
tag?: string;
}

@JSONSchema({
description: 'Component Interfaces are the primary means by which components advertise their resolvable addresses to others. Interfaces are the only means by which other components can communicate with your component.',
})
Expand Down Expand Up @@ -234,19 +247,20 @@ export class ComponentSpec {
type: 'object',

patternProperties: {
[ComponentSlugUtils.Validator.source]: {
[ComponentSlugUtils.Validator.source]: AnyOf({
type: 'string',
pattern: Slugs.ComponentTagValidator.source,
},
}, DependencySpec),
},

errorMessage: {
additionalProperties: ComponentSlugUtils.Description,
},

description: 'A key-value set of dependencies and their respective tags. Reference each dependency by component name (e.g. `cloud: latest`)',
description: 'A key-value set of dependencies with an empty value. Reference each dependency by component name (e.g. `cloud: {}`)',
})
dependencies?: Dictionary<string>;
@Transform(transformObject(DependencySpec))
dependencies?: Dictionary<string | DependencySpec>;

@IsOptional()
@JSONSchema({
Expand Down
2 changes: 2 additions & 0 deletions src/dependency-manager/spec/utils/json-schema-annotations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export const AnyOf = (...args: any): DecoratorSchema => {
anyOf.push({
$ref: `${REF_PREFIX}${arg.name}`,
});
} else if (typeof arg === 'object' && 'type' in arg && 'pattern' in arg) {
anyOf.push(arg);
Comment on lines +26 to +27
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was needed to specifically support this -

AnyOf({
  type: 'string',
  pattern: Slugs.ComponentTagValidator.source,
}, DependencySpec)

Currently you can only AnyOf any string/object/etc, but not a string with a specific pattern, which this now fixes

} else {
console.error(arg);
throw new Error('Illegal arg for JsonSchema in AnyOf. You must specify either a primitive string or a Type.');
Expand Down
7 changes: 3 additions & 4 deletions src/dependency-manager/spec/utils/json-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,10 @@ export const findDefinition = (pointer: string, schema: SchemaObject): SchemaObj
* @param obj
*/
const recursivelyReplaceDebugRefs = (obj: SchemaObject) => {
if (!obj || typeof obj !== 'object') {
return;
}
for (const k of Object.keys(obj)) {
if (!obj || typeof obj !== 'object') {
return;
}

if (k === '$ref') {
obj[k] = `${REF_PREFIX}${DEBUG_PREFIX}${obj[k].replace(REF_PREFIX, '')}`;
}
Expand Down
4 changes: 2 additions & 2 deletions test/dependency-manager/components.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ describe('components spec v1', function () {
},
},
dependencies: {
'ci': '6.2',
'ci': {},
},
interfaces: {},
};
Expand Down Expand Up @@ -347,7 +347,7 @@ describe('components spec v1', function () {
const worker_node = graph.getNodeByRef(worker_ref) as ServiceNode;
expect(worker_node.config.environment.CONCOURSE_TSA_HOST).eq(web_ref);
expect(worker_node.config.name).to.eq('worker');
expect(worker_node.config.metadata.tag).to.eq('6.2');
expect(worker_node.config.metadata.tag).to.eq('latest');
expect(worker_node.config.metadata.ref).to.eq('ci.services.worker');
});

Expand Down
91 changes: 87 additions & 4 deletions test/dependency-manager/dependencies.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { expect } from '@oclif/test';
import axios from 'axios';
import mock_fs from 'mock-fs';
import { resourceRefToNodeRef, ServiceNode } from '../../src';
import { buildSpecFromYml, resourceRefToNodeRef, ServiceNode, ValidationErrors } from '../../src';
import LocalDependencyManager from '../../src/common/dependency-manager/local-manager';

describe('dependencies', () => {

it('circular dependencies', async () => {
const cloud_config = `
name: cloud
Expand Down Expand Up @@ -63,12 +62,96 @@ describe('dependencies', () => {
expect(app.config.environment).to.deep.equal({
SERVER_ADDR: `http://${server_ref}:8080`,
SERVER_EXT_ADDR: `http://server.arc.localhost`
})
});

const server = graph.getNodeByRef(server_ref) as ServiceNode;
expect(server.config.environment).to.deep.equal({
CLOUD_ADDR: `http://${app_ref}:8080`,
CLOUD_EXT_ADDR: `http://app.arc.localhost`
})
});
});

describe('dependency validation', () => {
it('dependencies with no tag are valid', async () => {
const component_config = `
name: component
dependencies:
server: {}
`;
buildSpecFromYml(component_config);
});

it('dependencies with string tag are still valid', async () => {
const component_config = `
name: component
dependencies:
server: im-a-tag
`;
buildSpecFromYml(component_config);
});

it('dependencies with invalid string tag are still invalid', async () => {
const component_config = `
name: component
dependencies:
server: im-an-invalid-tag!
`;

expect(() => {
buildSpecFromYml(component_config);
}).to.throw(ValidationErrors);
});

it('dependencies with tag as dictionary key is valid', async () => {
const component_config = `
name: component
dependencies:
server:
tag: im-a-tag
`;

buildSpecFromYml(component_config);
});

it('dependencies with invalid tag as dictionary key is invalid', async () => {
const component_config = `
name: component
dependencies:
server:
tag: im-an-invalid-tag!
`;

expect(() => {
buildSpecFromYml(component_config);
}).to.throw(ValidationErrors);
});

it('dependencies with invalid keys are invalid', async () => {
const component_config = `
name: component
dependencies:
server:
foo: bar
baz: bingo
`;

expect(() => {
buildSpecFromYml(component_config);
}).to.throw(ValidationErrors);
});

it('dependencies with valid and invalid keys are invalid', async () => {
const component_config = `
name: component
dependencies:
server:
foo: bar
tag: im-a-tag
`;

expect(() => {
buildSpecFromYml(component_config);
}).to.throw(ValidationErrors);
});
});
});
8 changes: 4 additions & 4 deletions test/dependency-manager/reserved-names.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,17 +316,17 @@ describe('components with reserved_name field set', function () {
services: {
api: {
interfaces: {
main: 8080
main: 8080,
},
environment: {
CONCOURSE_ADDR: '${{ dependencies.ci.interfaces.web.url }}'
}
}
},
dependencies: {
'ci': '6.2'
'ci': {},
},
interfaces: {}
interfaces: {},
};

const concourse_component_config = {
Expand Down Expand Up @@ -388,7 +388,7 @@ describe('components with reserved_name field set', function () {
expect(worker_node.config.environment.CONCOURSE_TSA_HOST).eq(web_ref);
expect(worker_node.config.name).to.eq('worker');
expect(worker_node.ref).to.eq(reserved_name);
expect(worker_node.config.metadata.tag).to.eq('6.2');
expect(worker_node.config.metadata.tag).to.eq('latest');
expect(worker_node.config.metadata.ref).to.eq('ci.services.worker');
});

Expand Down