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

fix(cx-api): improve compatibility messages for cli <=> app #2676

Merged
merged 3 commits into from
May 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/@aws-cdk/cdk/test/test.synthesis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export = {

// THEN
test.same(app.run(), session); // same session if we run() again
test.deepEqual(list(session.directory), [ 'manifest.json' ]);
test.deepEqual(list(session.directory), [ 'cdk.out', 'manifest.json' ]);
test.deepEqual(readJson(session.directory, 'manifest.json').artifacts, {});
test.done();
},
Expand All @@ -40,6 +40,7 @@ export = {

// THEN
test.deepEqual(list(session.directory), [
'cdk.out',
'manifest.json',
'one-stack.template.json'
]);
Expand Down Expand Up @@ -71,6 +72,7 @@ export = {

// THEN
test.deepEqual(list(session.directory), [
'cdk.out',
'foo.json',
'manifest.json',
'one-stack.template.json'
Expand Down
9 changes: 7 additions & 2 deletions packages/@aws-cdk/cx-api/lib/cloud-assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export class CloudAssemblyBuilder {

// we leverage the fact that outdir is long-lived to avoid staging assets into it
// that were already staged (copying can be expensive). this is achieved by the fact
// that assets use a source hash as their name. other artifacts, and the manifest,
// that assets use a source hash as their name. other artifacts, and the manifest itself,
// will overwrite existing files as needed.

if (fs.existsSync(this.outdir)) {
Expand All @@ -143,6 +143,11 @@ export class CloudAssemblyBuilder {
const manifestFilePath = path.join(this.outdir, MANIFEST_FILE);
fs.writeFileSync(manifestFilePath, JSON.stringify(manifest, undefined, 2));

// "backwards compatibility": in order for the old CLI to tell the user they
// need a new version, we'll emit the legacy manifest with only "version".
// this will result in an error "CDK Toolkit >= 0.33.0 is required in order to interact with this program."
fs.writeFileSync(path.join(this.outdir, 'cdk.out'), JSON.stringify({ version: CLOUD_ASSEMBLY_VERSION }));

return new CloudAssembly(this.outdir);
}
}
Expand Down Expand Up @@ -201,4 +206,4 @@ function filterUndefined(obj: any): any {

function ignore(_x: any) {
return;
}
}
5 changes: 3 additions & 2 deletions packages/@aws-cdk/cx-api/lib/versioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ export function verifyManifestVersion(manifetVersion: string) {

// if framework > cli, we require a newer cli version
if (semver.gt(frameworkVersion, toolkitVersion)) {
throw new Error(`CLI >= ${frameworkVersion} is required to interact with this app`);
throw new Error(`CDK CLI >= ${frameworkVersion} is required to interact with this app`);
}

// if framework < cli, we require a newer framework version
if (semver.lt(frameworkVersion, toolkitVersion)) {
throw new Error(`App used framework v${frameworkVersion} but it must be >= v${CLOUD_ASSEMBLY_VERSION} in order to interact with this CLI`);
throw new Error(
`CDK CLI can only be used with apps created by CDK >= ${CLOUD_ASSEMBLY_VERSION}`);
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/cx-api/test/cloud-assembly.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,6 @@ test('fails for invalid dependencies', () => {

test('verifyManifestVersion', () => {
verifyManifestVersion('0.33.0');
expect(() => verifyManifestVersion('0.31.0')).toThrow('App used framework v0.31.0 but it must be >= v0.33.0 in order to interact with this CLI');
expect(() => verifyManifestVersion('0.34.0')).toThrow('CLI >= 0.34.0 is required to interact with this app');
expect(() => verifyManifestVersion('0.31.0')).toThrow('CDK CLI can only be used with apps created by CDK >= 0.33.0');
expect(() => verifyManifestVersion('0.34.0')).toThrow('CDK CLI >= 0.34.0 is required to interact with this app');
});