Skip to content

Commit

Permalink
normalize handling of $! inside cfn tags like !Sub + tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@tavisrudd
tavisrudd committed Nov 29, 2019
1 parent b43905f commit 54b2f79
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 28 deletions.
72 changes: 56 additions & 16 deletions src/preprocess/visitor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,15 @@ export class Visitor {
} else if (node instanceof yaml.Sub) {
return this.visitSub(node, path, env);
} else {
return node.update(this.visitNode(node.data, path, env));
if (_.isArray(node.data)
&& node.data.length === 1
&& node.data[0] instanceof yaml.Tag) {
// unwrap Tags like !ImportValue [!$ someVar]
return node.update(this.visitNode(node.data[0], path, env));
} else {
return node.update(this.visitNode(node.data, path, env));
}

}
}

Expand Down Expand Up @@ -259,7 +267,7 @@ export class Visitor {
if (Number.isSafeInteger(parseInt(part, 10))) {
newKeyParts.push(part)
} else {
const bracketVal = this.visit$include(new yaml.$include(part), `${path} / ${dynamicKey}` , env);
const bracketVal = this.visit$include(new yaml.$include(part), `${path} / ${dynamicKey}`, env);
if (typeof bracketVal === 'number') {
newKeyParts.push(String(bracketVal))
} else if (typeof bracketVal === 'string') {
Expand Down Expand Up @@ -373,6 +381,10 @@ export class Visitor {
}

visit$parseYaml(node: yaml.$parseYaml, path: string, env: Env): AnyButUndefined {
if (!_.isString(node.data)) {
throw new Error(
`Invalid argument to !$parseYaml: expected string, got ${node.data}`);
}
return this.visitNode(yaml.loadString(this.visitString(node.data, path, env), path), path, env);
}

Expand Down Expand Up @@ -446,16 +458,30 @@ export class Visitor {
}

visitRef(node: yaml.Ref, path: string, env: Env): yaml.Ref {
return new yaml.Ref(this.maybeRewriteRef(node.data, path, env));
const refInput = _.isArray(node.data) ? node.data[0] : node.data;
const ref = this.visitNode(refInput, path, env);
if (!_.isString(ref)) {
throw new Error(
`Invalid argument to !Ref: expected string, got ${ref} ${typeof ref}`);
}

return new yaml.Ref(this.maybeRewriteRef(ref, path, env));
}

visitGetAtt(node: yaml.GetAtt, path: string, env: Env): yaml.GetAtt {
if (_.isArray(node.data)) {
const argsArray = _.clone(node.data);
const arg = this.visitNode(node.data, path, env);
if (!(_.isString(arg)
|| (_.isArray(arg) && _.isString(arg[0])))) {
throw new Error(
`Invalid argument to !GetAtt: expected string or list, got ${node.data}
type=${typeof node.data}`);
}
if (_.isArray(arg)) {
const argsArray = _.clone(arg);
argsArray[0] = this.maybeRewriteRef(argsArray[0], path, env);
return new yaml.GetAtt(argsArray);
} else { // it's a string
return new yaml.GetAtt(this.maybeRewriteRef(node.data, path, env));
return new yaml.GetAtt(argsArray.length === 1 ? argsArray[0] : argsArray);
} else { // it should be a string
return new yaml.GetAtt(this.maybeRewriteRef(arg, path, env));
}
}

Expand All @@ -475,13 +501,26 @@ export class Visitor {

visitSub(node: yaml.Sub, path: string, env: Env): yaml.Sub {
if (_.isArray(node.data) && node.data.length === 1) {
return new yaml.Sub(this.visitSubStringTemplate(this.visitNode(node.data[0], path, env), path, env));
const subArg = this.visitNode(node.data[0], path, env);
if (!_.isString(subArg)) {
throw new Error(
`Invalid argument to !Sub: expected string, got ${node.data[0]}`);
}
return new yaml.Sub(this.visitSubStringTemplate(subArg, path, env));
} else if (_.isArray(node.data) && node.data.length === 2) {
const subArg0 = this.visitNode(node.data[0], path, env);
if (!_.isString(subArg0)) {
throw new Error(
`Invalid argument to !Sub: expected string, got ${node.data[0]}: ${subArg0}`);
}
const templateEnv = node.data[1];
const subEnv = mkSubEnv(
env, {...env.$envValues, $globalRefs: _.fromPairs(_.map(_.keys(templateEnv), (k) => [k, true]))},
env, {
...env.$envValues, $globalRefs: _.fromPairs(
_.map(_.keys(templateEnv), (k) => [k, true]))
},
{path});
const template = this.visitSubStringTemplate(this.visitNode(node.data[0], path, env), path, subEnv);
const template = this.visitSubStringTemplate(subArg0, path, subEnv);
return new yaml.Sub([template, this.visitNode(templateEnv, path, env)]);
} else if (_.isString(node.data)) {
return new yaml.Sub(this.visitSubStringTemplate(node.data, path, env));
Expand All @@ -493,7 +532,7 @@ export class Visitor {
visitNode(node: any, path: string, env: Env): any {
const currNode = path.split('.').pop();
// Avoid serializing large `env` data when debug is not enabled
if(logger.isDebugEnabled()) {
if (logger.isDebugEnabled()) {
logger.debug(`entering ${path}:`, {node, nodeType: typeof node, env});
}
const result = (() => {
Expand All @@ -516,7 +555,7 @@ export class Visitor {
return node;
}
})();
if(logger.isDebugEnabled()) {
if (logger.isDebugEnabled()) {
logger.debug(`exiting ${path}:`, {result, node, env});;
}
return result;
Expand Down Expand Up @@ -583,15 +622,15 @@ export class Visitor {
const sub: any = this.visitNode(node[k], appendPath(path, k), env);
for (const k2 in sub) {
// mutate in place to acheive a deep merge
_.merge(res, {[this.visitString(k2, path, env)]: sub[k2]});
_.merge(res, {[this.visitNode(k2, path, env)]: sub[k2]});
}
// TODO handle ref rewriting on the Fn:Ref, Fn:GetAtt type functions
//} else if ( .. Fn:Ref, etc. ) {
} else if (_.includes(extendedCfnDocKeys, k)) {
// we don't want to include things like $imports and $envValues in the output doc
continue;
} else {
res[this.visitString(k, path, env)] = this.visitNode(node[k], appendPath(path, k), env);
res[this.visitNode(k, path, env)] = this.visitNode(node[k], appendPath(path, k), env);
}
}
return res;
Expand All @@ -606,6 +645,7 @@ export class Visitor {
}

visitString(node: string, path: string, env: Env): string {
// NOTE: devs, please assert node is string before calling this
let res: string;
if (node.search(HANDLEBARS_RE) > -1) {
res = this.visitHandlebarsString(node, path, env);
Expand All @@ -632,7 +672,7 @@ export class Visitor {
if (k.indexOf('$merge') === 0) {
const sub: any = visitor.visitNode(node[k], appendPath(path, k), env);
for (const k2 in sub) {
expanded[visitor.visitString(k2, path, env)] = sub[k2];
expanded[visitor.visitNode(k2, path, env)] = sub[k2];
}
} else if (_.includes(extendedCfnDocKeys, k)) {
continue;
Expand Down
68 changes: 61 additions & 7 deletions src/tests/test-yaml-preprocessing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,31 @@ describe('Yaml pre-processing', () => {
}
});

it('leaves AWS Intrinsic functions (!Sub, !Ref, ...) unchanged', () => {
const input = {
GetAtt_a: new yaml.GetAtt('arg0'),
GetAtt_b: {"Fn::GetAtt": 'arg0'},
GetAtt_c: new yaml.GetAtt(['logical', 'attrname']),

GetParam_a: new yaml.customTags['GetParam']('arg0'),
GetParam_b: {"Fn::GetParam": 'arg0'},

GetAZs_a: new yaml.customTags['GetAZs']('arg0'),
GetAZs_b: {"Fn::GetAZs": 'arg0'},

ImportValue_a: new yaml.ImportValue('arg0'),
ImportValue_with_sub: new yaml.ImportValue(new yaml.Sub('arg0')),

Ref_a: new yaml.Ref('arg0'),
Ref_b: {Ref: 'arg0'},

Sub_x: new yaml.Sub('arg0'),
Sub_y: new yaml.Sub(['arg0', {a: 'a'}]),
Sub_z: new yaml.Sub(['${a}', {a: 'a'}])
};
expect(transformNoImport(input)).to.deep.equal(input);
});

});
//////////////////////////////////////////////////////////////////////
describe('$imports:', () => {
Expand Down Expand Up @@ -158,25 +183,54 @@ aref: !$ nested.aref`, mockLoader)).to.deep.equal({aref: 'mock'});
//////////////////////////////////////////////////////////////////////
describe('$defs:', () => {

it('basic usage with !$ works', async () => {
it('basic usage with !$ works', () => {

expect(await transform({$defs: {a: 'b'}, out: '{{a}}'}))
expect(transformNoImport(
{$envValues: {a: 'b'}, out: '{{a}}'}))
.to.deep.equal({out: 'b'});

expect(await transform({$defs: {a: 'b'}, out: new yaml.$include('a')}))
expect(transformNoImport(
{$envValues: {a: 'b'}, out: new yaml.$include('a')}))
.to.deep.equal({out: 'b'});

expect(await transform({$defs: {a: {b: 'c'}}, out: '{{a.b}}'}))
expect(transformNoImport(
{$envValues: {a: {b: 'c'}}, out: '{{a.b}}'}))
.to.deep.equal({out: 'c'});

expect(await transform({
$defs: {a: new yaml.$include('b'), b: 'xref'},
expect(transformNoImport({
$envValues: {a: new yaml.$include('b'), b: 'xref'},
out: new yaml.$include('a')
}))
.to.deep.equal({out: 'xref'});
});
});

it('!$ works inside AWS Intrinsic functions', () => {
for (const [tag_name, ctor] of Object.entries(yaml.cfnIntrinsicTags)) {
try {
expect(transformNoImport({
$envValues: {a: 'abc'},
out: new ctor([new yaml.$include('a')]) // list required
}))
.to.deep.equal({out: new ctor('abc')});

expect(transformNoImport({
$envValues: {a: 'abc'},
out: new ctor('{{a}}')
}))
.to.deep.equal({out: new ctor('abc')});

expect(transformNoImport({
$envValues: {a: '{{xref}}', xref: 'abc'},
out: new ctor('{{a}}')
}))
.to.deep.equal({out: new ctor('abc')});
} catch (err) {
err.message = `${tag_name}: ${err.message}`;
throw err;
}
}
});
});
//////////////////////////////////////////////////////////////////////
describe('{{handlebars}} syntax', () => {

Expand Down
19 changes: 14 additions & 5 deletions src/yaml.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ function mkTagClass(tag_name: string) {

const schemaTypes: jsyaml.Type[] = [];
export const customTags: {[key: string]: typeof Tag} = {};
export const cfnIntrinsicTags: {[key: string]: typeof Tag} = {};

type Resolver = any;

function addCFNTagType(tag_name: string, kind: YamlKind, resolve?: Resolver) {
function addTagType(tag_name: string, kind: YamlKind, resolve?: Resolver) {
const kls = _.has(customTags, tag_name) ? customTags[tag_name] : mkTagClass(tag_name);
customTags[tag_name] = kls;
schemaTypes.push(new jsyaml.Type('!' + tag_name, {
Expand All @@ -43,6 +44,13 @@ function addCFNTagType(tag_name: string, kind: YamlKind, resolve?: Resolver) {
construct: (data: any) => new kls(data),
represent: (node: any) => node.data
}));
return kls;
}

function addCFNTagType(tag_name: string, kind: YamlKind, resolve?: Resolver) {
const kls = addTagType(tag_name, kind, resolve);
cfnIntrinsicTags[tag_name] = kls;
return kls;
}

// http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference.html
Expand All @@ -64,9 +72,10 @@ addCFNTagType('GetAZs', 'scalar');
addCFNTagType('GetAZs', 'mapping');
addCFNTagType('GetAZs', 'sequence');

// TODO add !Transform

// ImportValue will be either a literal string or a !Sub string
export class ImportValue extends Tag<string | object> {}
export class ImportValue extends Tag<string | Sub> {}
customTags.ImportValue = ImportValue;
addCFNTagType('ImportValue', 'scalar');
addCFNTagType('ImportValue', 'mapping');
Expand Down Expand Up @@ -108,9 +117,9 @@ function addCustomTag(name: string | string[], kls: any, resolve?: Resolver) {
customTags[nm] = kls
// add all even if primitive types even if only a subset is valid as
// the error reporting is better handled elsewhere.
addCFNTagType(nm, 'scalar', resolve);
addCFNTagType(nm, 'sequence', resolve);
addCFNTagType(nm, 'mapping', resolve);
addTagType(nm, 'scalar', resolve);
addTagType(nm, 'sequence', resolve);
addTagType(nm, 'mapping', resolve);
}
}

Expand Down

0 comments on commit 54b2f79

Please sign in to comment.