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: correctly emit quoted YAML for account numbers #1105

Merged
merged 2 commits into from
Nov 8, 2018
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: 2 additions & 2 deletions packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import 'source-map-support/register';
import cdk = require('@aws-cdk/cdk');
import child_process = require('child_process');
import fs = require('fs-extra');
import YAML = require('js-yaml');
import os = require('os');
import path = require('path');
import YAML = require('yaml');

import { isStackConstructor, parseApplet } from '../lib/applet-helpers';

Expand All @@ -25,7 +25,7 @@ async function main() {
}

// read applet(s) properties from the provided file
const fileContents = YAML.safeLoad(await fs.readFile(appletFile, { encoding: 'utf-8' }));
const fileContents = YAML.parse(await fs.readFile(appletFile, { encoding: 'utf-8' }));
if (typeof fileContents !== 'object') {
throw new Error(`${appletFile}: should contain a YAML object`);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/applet-js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@
"license": "Apache-2.0",
"devDependencies": {
"@types/fs-extra": "^5.0.4",
"@types/js-yaml": "^3.11.2",
"@types/yaml": "^1.0.0",
"cdk-build-tools": "^0.15.1",
"pkglint": "^0.15.1"
},
"dependencies": {
"@aws-cdk/cdk": "^0.15.1",
"fs-extra": "^7.0.0",
"source-map-support": "^0.5.6",
"js-yaml": "^3.12.0"
"yaml": "^1.0.1"
},
"repository": {
"url": "https://github.com/awslabs/aws-cdk.git",
Expand Down
16 changes: 3 additions & 13 deletions packages/aws-cdk/bin/cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import 'source-map-support/register';
import cxapi = require('@aws-cdk/cx-api');
import colors = require('colors/safe');
import fs = require('fs-extra');
import YAML = require('js-yaml');
import minimatch = require('minimatch');
import util = require('util');
import yargs = require('yargs');
Expand All @@ -19,6 +18,7 @@ import { interactive } from '../lib/interactive';
import { data, debug, error, highlight, print, setVerbose, success, warning } from '../lib/logging';
import { PluginHost } from '../lib/plugin';
import { parseRenames } from '../lib/renames';
import { deserializeStructure, serializeStructure } from '../lib/serialize';
import { DEFAULTS, PER_USER_DEFAULTS, Settings } from '../lib/settings';
import { VERSION } from '../lib/version';

Expand Down Expand Up @@ -609,11 +609,7 @@ async function initCommandLine() {

/* Attempt to parse YAML, fall back to JSON. */
function parseTemplate(text: string): any {
try {
return YAML.safeLoad(text);
} catch (e) {
return JSON.parse(text);
}
return deserializeStructure(text);
}
}

Expand Down Expand Up @@ -679,13 +675,7 @@ async function initCommandLine() {
}

function toJsonOrYaml(object: any): string {
if (argv.json) {
const noFiltering = undefined;
const indentWidth = 2;
return JSON.stringify(object, noFiltering, indentWidth);
} else {
return YAML.safeDump(object);
}
return serializeStructure(object, argv.json);
}
}

Expand Down
12 changes: 6 additions & 6 deletions packages/aws-cdk/integ-tests/test-cdk-synth.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ setup

assert "cdk synth cdk-toolkit-integration-test-1" <<HERE
Resources:
topic69831491:
Type: 'AWS::SNS::Topic'
topic69831491:
Type: AWS::SNS::Topic

HERE

assert "cdk synth cdk-toolkit-integration-test-2" <<HERE
Resources:
topic152D84A37:
Type: 'AWS::SNS::Topic'
topic2A4FB547F:
Type: 'AWS::SNS::Topic'
topic152D84A37:
Type: AWS::SNS::Topic
topic2A4FB547F:
Type: AWS::SNS::Topic

HERE

Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import cxapi = require('@aws-cdk/cx-api');
import aws = require('aws-sdk');
import colors = require('colors/safe');
import YAML = require('js-yaml');
import uuid = require('uuid');
import { prepareAssets } from '../assets';
import { debug, error, print } from '../logging';
import { toYAML } from '../serialize';
import { Mode } from './aws-auth/credentials';
import { ToolkitInfo } from './toolkit-info';
import { describeStack, stackExists, stackFailedCreating, waitForChangeSet, waitForStack } from './util/cloudformation';
Expand Down Expand Up @@ -113,7 +113,7 @@ async function getStackOutputs(cfn: aws.CloudFormation, stackName: string): Prom
* @param toolkitInfo information about the toolkit stack
*/
async function makeBodyParameter(stack: cxapi.SynthesizedStack, toolkitInfo?: ToolkitInfo): Promise<TemplateBodyParameter> {
const templateJson = YAML.safeDump(stack.template, { indent: 4, flowLevel: 16 });
const templateJson = toYAML(stack.template);
if (toolkitInfo) {
const s3KeyPrefix = `cdk/${stack.name}/`;
const s3KeySuffix = '.yml';
Expand Down
38 changes: 38 additions & 0 deletions packages/aws-cdk/lib/serialize.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import YAML = require('yaml');

/**
* Stringify to YAML
*/
export function toYAML(obj: any): string {
return YAML.stringify(obj, { schema: 'yaml-1.1' });
}

/**
* Parse YAML
*/
export function fromYAML(str: string): any {
return YAML.parse(str, { schema: 'yaml-1.1' });
}

/**
* Parse either YAML or JSON
*/
export function deserializeStructure(str: string) {
try {
return fromYAML(str);
} catch (e) {
// This shouldn't really ever happen I think, but it's the code we had so I'm leaving it.
return JSON.parse(str);
}
}

/**
* Serialize to either YAML or JSON
*/
export function serializeStructure(object: any, json: boolean) {
if (json) {
return JSON.stringify(object, undefined, 2);
} else {
return toYAML(object);
}
}
4 changes: 2 additions & 2 deletions packages/aws-cdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"@types/semver": "^5.5.0",
"@types/uuid": "^3.4.3",
"@types/yargs": "^8.0.3",
"@types/js-yaml": "^3.11.2",
"@types/yaml": "^1.0.0",
"cdk-build-tools": "^0.15.1",
"mockery": "^2.1.0",
"pkglint": "^0.15.1"
Expand All @@ -54,7 +54,7 @@
"colors": "^1.2.1",
"decamelize": "^2.0.0",
"fs-extra": "^4.0.2",
"js-yaml": "^3.12.0",
"yaml": "^1.0.1",
"json-diff": "^0.3.1",
"minimatch": ">=3.0",
"promptly": "^0.2.0",
Expand Down
33 changes: 18 additions & 15 deletions packages/aws-cdk/test/test.yaml.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,47 @@
import { Test } from 'nodeunit';
import { toYAML } from '../lib/serialize';

import YAML = require('js-yaml');

function yamlStringify(obj: any) {
return YAML.dump(obj);
}
// Preferred quote of the YAML library
const q = '"';

export = {
'quote the word "ON"'(test: Test) {
// NON NEGOTIABLE! If not quoted, will be interpreted as the boolean TRUE

// tslint:disable-next-line:no-console
const output = yamlStringify({
const output = toYAML({
notABoolean: "ON"
});

test.equals(output.trim(), `notABoolean: 'ON'`);
test.equals(output.trim(), `notABoolean: ${q}ON${q}`);

test.done();
},

'quote number-like strings with a leading 0'(test: Test) {
const output = yamlStringify({
const output = toYAML({
leadingZero: "012345"
});

test.equals(output.trim(), `leadingZero: '012345'`);
test.equals(output.trim(), `leadingZero: ${q}012345${q}`);

test.done();
},

'do not quote octal numbers that arent really octal'(test: Test) {
// Under contention: this seems to be okay, pyyaml parses it
// correctly. Unsure of what CloudFormation does about it.
// This is a contentious one, and something that might have changed in YAML1.2 vs YAML1.1
//
// One could make the argument that a sequence of characters that couldn't ever
// be an octal value doesn't need to be quoted, and pyyaml parses it correctly.
//
// However, CloudFormation's parser interprets it as a decimal number (eating the
// leading 0) if it's unquoted, so that's the behavior we're testing for.

const output = yamlStringify({
const output = toYAML({
leadingZero: "0123456789"
});

test.equals(output.trim(), `leadingZero: 0123456789`);
test.equals(output.trim(), `leadingZero: ${q}0123456789${q}`);

test.done();
},
Expand All @@ -48,14 +51,14 @@ export = {
//
// 'yaml' fails this.

const output = yamlStringify({
const output = toYAML({
colons: ['arn', ':', 'aws']
});

test.equals(output.trim(), [
'colons:',
' - arn',
` - ':'`,
` - ${q}:${q}`,
' - aws'
].join('\n'));

Expand Down