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

Ephemeral component versions #642

Merged
merged 17 commits into from Aug 4, 2022
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/commands/deploy.ts
@@ -1,13 +1,17 @@
import { CliUx, Flags, Interfaces } from '@oclif/core';
import chalk from 'chalk';
import fs from 'fs';
import inquirer from 'inquirer';
import AccountUtils from '../architect/account/account.utils';
import { EnvironmentUtils } from '../architect/environment/environment.utils';
import PipelineUtils from '../architect/pipeline/pipeline.utils';
import BaseCommand from '../base-command';
import { DeploymentFailedError, PipelineAbortedError, PollingTimeout } from '../common/errors/pipeline-errors';
import DeployUtils from '../common/utils/deploy.utils';
import { buildSpecFromPath } from '../dependency-manager/spec/utils/component-builder';
import { ComponentVersionSlugUtils } from '../dependency-manager/spec/utils/slugs';
import Dev from "./dev";
import ComponentRegister from './register';

export abstract class DeployCommand extends BaseCommand {

Expand Down Expand Up @@ -232,10 +236,25 @@ export default class Deploy extends DeployCommand {
const account = await AccountUtils.getAccount(this.app, flags.account);
const environment = await EnvironmentUtils.getEnvironment(this.app.api, account, flags.environment);

const deployment_dtos = [];
const component_names: string[] = [];
for (const component of components) {
if (fs.existsSync(component)) {
const register = new ComponentRegister([component, '-a', account.name, '-e', environment.name], this.config);
register.app = this.app;
await register.run();
const component_spec = buildSpecFromPath(component);
component_names.push(`${account.name}/${component_spec.name}:${ComponentRegister.getTagFromFlags({ environment: environment.name })}`);
Copy link
Member

Choose a reason for hiding this comment

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

Remove the account name prefix. Since sometimes the account name is part of the name for older customers.

component_names.push(`${component_spec.name}:${ComponentRegister.getTagFromFlags({ environment: environment.name })}`);

} else if (ComponentVersionSlugUtils.Validator.test(component)) {
component_names.push(component);
} else {
throw new Error(`${component} isn't either the name of a component or a path to an existing component file.`);
}
}

const deployment_dtos = [];
for (const component of component_names) {
const deploy_dto = {
component: component,
component,
interfaces: interfaces_map,
recursive: flags.recursive,
values: all_secrets, // TODO: 404: update
Expand Down
31 changes: 22 additions & 9 deletions src/commands/register.ts
Expand Up @@ -10,6 +10,7 @@ import tmp from 'tmp';
import untildify from 'untildify';
import { ArchitectError, buildSpecFromPath, ComponentSlugUtils, Dictionary, dumpToYml, resourceRefToNodeRef, ResourceSlugUtils, ServiceNode, Slugs, validateInterpolation } from '../';
import AccountUtils from '../architect/account/account.utils';
import { EnvironmentUtils } from '../architect/environment/environment.utils';
import BaseCommand from '../base-command';
import LocalDependencyManager from '../common/dependency-manager/local-manager';
import { DockerComposeUtils } from '../common/docker-compose';
Expand All @@ -20,6 +21,8 @@ import { IF_EXPRESSION_REGEX } from '../dependency-manager/spec/utils/interpolat

tmp.setGracefulCleanup();

export const EPHEMERAL_DELIMITER = 'architect-ephemeral';
Copy link
Member

Choose a reason for hiding this comment

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

Should we make it targeted to the environment? Not really a delimiter - more a prefix.

export const ENV_TAG_PREFIX = 'architect.environment.'

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I like that


export default class ComponentRegister extends BaseCommand {
static aliases = ['component:register', 'components:register', 'c:register', 'comp:register'];
static description = 'Register a new Component with Architect Cloud';
Expand All @@ -40,6 +43,7 @@ export default class ComponentRegister extends BaseCommand {
char: 't',
description: 'Tag to give to the new component',
default: 'latest',
exclusive: ['environment'],
}),
},
architecture: {
Expand All @@ -56,6 +60,14 @@ export default class ComponentRegister extends BaseCommand {
description: 'Directory to write build cache to. Do not use in Github Actions: https://docs.architect.io/deployments/automated-previews/#caching-between-workflow-runs',
}),
},
environment: {
Copy link
Member

Choose a reason for hiding this comment

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

should we make this hidden: true?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had considered that, but I don't see why someone shouldn't be able to create an ephemeral component version directly rather than indirectly thorough the deploy command

Copy link
Member

Choose a reason for hiding this comment

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

What would be the reason for them to do that? My inclination is to make it hidden with the option to unhide in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe they just don't want to deploy the component immediately after it's registered? I'll hide it though

non_sensitive: true,
...Flags.string({
char: 'e',
description: 'The name of an environment to register the component version to. If specified, the component version will be removed when the environment is removed',
exclusive: ['tag'],
}),
},
};

static args = [{
Expand All @@ -75,7 +87,7 @@ export default class ComponentRegister extends BaseCommand {
throw new ArchitectError(Slugs.ComponentTagDescription);
}

await this.registerComponent(config_path, flags.tag);
await this.registerComponent(config_path, ComponentRegister.getTagFromFlags(flags));
}

private async registerComponent(config_path: string, tag: string) {
Expand All @@ -89,19 +101,15 @@ export default class ComponentRegister extends BaseCommand {
throw new Error('Component Config must have a name');
}

const context = {
architect: {
build: {
tag: flags.tag,
},
},
};

validateInterpolation(component_spec);

const { component_account_name, component_name } = ComponentSlugUtils.parse(component_spec.name);
const selected_account = await AccountUtils.getAccount(this.app, component_account_name || flags.account);

if (flags.environment) { // will throw an error if a user specifies an environment that doesn't exist
await EnvironmentUtils.getEnvironment(this.app.api, selected_account, flags.environment);
}

const dependency_manager = new LocalDependencyManager(this.app.api);
dependency_manager.environment = 'production';
dependency_manager.account = selected_account.name;
Expand Down Expand Up @@ -256,6 +264,7 @@ export default class ComponentRegister extends BaseCommand {
delete config.metadata;
const component_dto = {
tag,
environment_name: flags.environment,
config,
};

Expand Down Expand Up @@ -313,4 +322,8 @@ export default class ComponentRegister extends BaseCommand {
const { headers } = await registry_client.head(`${name}/manifests/${tag}`);
return headers['docker-content-digest'];
}

public static getTagFromFlags(flags: any): string {
return flags.environment ? `${EPHEMERAL_DELIMITER}-${flags.environment}` : flags.tag;
}
}
39 changes: 39 additions & 0 deletions test/commands/deploy.test.ts
@@ -1,9 +1,12 @@
import { expect } from '@oclif/test';
import sinon, { SinonSpy } from 'sinon';
import { ComponentVersionSlugUtils } from '../../src';
import PipelineUtils from '../../src/architect/pipeline/pipeline.utils';
import Deploy from '../../src/commands/deploy';
import ComponentRegister from '../../src/commands/register';
import DeployUtils from '../../src/common/utils/deploy.utils';
import * as Docker from '../../src/common/utils/docker';
import * as ComponentBuilder from '../../src/dependency-manager/spec/utils/component-builder';
import { app_host } from '../config.json';
import { mockArchitectAuth, MOCK_API_HOST } from '../utils/mocks';

Expand Down Expand Up @@ -50,6 +53,42 @@ describe('remote deploy environment', function () {
expect(ctx.stdout).to.contain('Deployed');
})

remoteDeploy
.stub(ComponentRegister.prototype, 'run', sinon.stub().returns(Promise.resolve()))
.stub(ComponentBuilder, 'buildSpecFromPath', sinon.stub().returns(Promise.resolve()))
.command(['deploy', '-e', environment.name, '-a', account.name, '--auto-approve', 'test/mocks/superset/architect.yml'])
.it('Creates a remote deployment with env and account flags and a path to a component', ctx => {
expect((ComponentRegister.prototype.run as SinonSpy).getCalls().length).to.equal(1);
const build_spec = ComponentBuilder.buildSpecFromPath as SinonSpy;
expect(build_spec.getCalls().length).to.equal(1);
expect(build_spec.firstCall.args[0]).eq('test/mocks/superset/architect.yml');
expect(ctx.stdout).to.contain('Deployed');
})

remoteDeploy
.nock(MOCK_API_HOST, api => api
.post(`/environments/${environment.id}/deploy`)
.reply(200, mock_pipeline))
.nock(MOCK_API_HOST, api => api
.post(`/pipelines/${mock_pipeline.id}/approve`)
.reply(200, {}))
.stub(ComponentRegister.prototype, 'run', sinon.stub().returns(Promise.resolve()))
.stub(ComponentBuilder, 'buildSpecFromPath', sinon.stub().returns(Promise.resolve()))
.stub(ComponentVersionSlugUtils.Validator, 'test', sinon.stub().returns(Promise.resolve()))
.command(['deploy', '-e', environment.name, '-a', account.name, '--auto-approve', 'test/mocks/superset/architect.yml', `${account.name}/superset:latest`])
.it('Creates a remote deployment with env and account flags, a path to a component, and a component version', ctx => {
expect((ComponentRegister.prototype.run as SinonSpy).getCalls().length).to.equal(1);
const build_spec = ComponentBuilder.buildSpecFromPath as SinonSpy;
expect(build_spec.getCalls().length).to.equal(1);
expect(build_spec.firstCall.args[0]).eq('test/mocks/superset/architect.yml');

const slug_validator = ComponentVersionSlugUtils.Validator.test as SinonSpy;
expect(slug_validator.getCalls().length).to.equal(1);
expect(slug_validator.firstCall.args[0]).eq(`${account.name}/superset:latest`);

expect(ctx.stdout).to.contain('Deployed');
})

describe('instance deploys', function () {
remoteDeploy
.command(['deploy', '-e', environment.name, '-a', account.name, '--auto-approve', 'examples/echo:latest@tenant-1'])
Expand Down
25 changes: 25 additions & 0 deletions test/commands/register.test.ts
Expand Up @@ -179,6 +179,31 @@ describe('register', function () {
expect(ctx.stdout).to.contain('Successfully registered component');
});

mockArchitectAuth
.nock(MOCK_API_HOST, api => api
.get(`/accounts/examples`)
.reply(200, mock_account_response)
)
.nock(MOCK_API_HOST, api => api
.post(/\/accounts\/.*\/components/, (body) => body)
.reply(200, (uri, body: any, cb) => {
const contents = yaml.load(fs.readFileSync('examples/gcp-pubsub/pubsub/architect.yml').toString());
expect(body.config).to.deep.equal(contents);
expect(validateSpec(body.config)).to.have.lengthOf(0);
cb(null, body)
})
)
.nock(MOCK_API_HOST, api => api
.get(`/accounts/${mock_account_response.id}/environments/test-env`)
.reply(200)
)
.stdout({ print })
.stderr({ print })
.command(['register', 'examples/gcp-pubsub/pubsub/architect.yml', '-a', 'examples', '-e', 'test-env'])
.it('registers an ephemeral component with an environment specified', ctx => {
expect(ctx.stdout).to.contain('Successfully registered component');
});

mockArchitectAuth
.nock(MOCK_API_HOST, api => api
.get(`/accounts/examples`)
Expand Down