diff --git a/packages/@aws-cdk/aws-ssm/package.json b/packages/@aws-cdk/aws-ssm/package.json index 969afb616dcf7..526a296ba3c8c 100644 --- a/packages/@aws-cdk/aws-ssm/package.json +++ b/packages/@aws-cdk/aws-ssm/package.json @@ -39,6 +39,11 @@ "cdk-build": { "cloudformation": "AWS::SSM" }, + "cfn2ts": { + "rename": { + "AWS::SSM::Association/Name": "DocumentName" + } + }, "keywords": [ "aws", "cdk", diff --git a/packages/@aws-cdk/aws-ssm/test/test.ssm-document.ts b/packages/@aws-cdk/aws-ssm/test/test.ssm-document.ts new file mode 100644 index 0000000000000..31e4db52db95e --- /dev/null +++ b/packages/@aws-cdk/aws-ssm/test/test.ssm-document.ts @@ -0,0 +1,23 @@ +import { expect, haveResource } from '@aws-cdk/assert'; +import cdk = require('@aws-cdk/cdk'); +import { Test } from 'nodeunit'; +import ssm = require('../lib'); + +export = { + 'association name is rendered properly in L1 construct'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + + // WHEN + new ssm.cloudformation.AssociationResource(stack, 'Assoc', { + documentName: 'document', + }); + + // THEN + expect(stack).to(haveResource('AWS::SSM::Association', { + Name: 'document', + })); + + test.done(); + } +}; \ No newline at end of file diff --git a/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.json b/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.json deleted file mode 100644 index 8ce9d9a3deb39..0000000000000 --- a/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "ResourceTypes": { - "AWS::SSM::Association": { - "patch": { - "description": "rename +Name+ to +DocumentName+ to clearly indicate what the attribute is for", - "operations": [ - { - "from": "/Properties/Name", - "op": "move", - "path": "/Properties/DocumentName" - } - ] - } - } - } -} diff --git a/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.md b/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.md deleted file mode 100644 index 9c311b0dbe502..0000000000000 --- a/packages/@aws-cdk/cfnspec/spec-source/500_SSM_AssociationName_patch.md +++ /dev/null @@ -1,9 +0,0 @@ -We discovered that SSM::Assocations have an AssociationName and a Name parameter, which introduced a conflict. - -Apart from that we have to resolve the conflict, the original heuristics were even wrong: - -- Name is NOT the name of the resource to be created, it's the name of the SSM document to associate with. -- AssociationName IS the name of the resource to be created, that's fine, it's the name that we would have -picked for that attribute anyway. - -We rename "Name" to "DocumentName" to clearly indicate what the Name attribute is for. diff --git a/tools/cfn2ts/lib/codegen.ts b/tools/cfn2ts/lib/codegen.ts index ffd18a56ef6ec..f559be52859f1 100644 --- a/tools/cfn2ts/lib/codegen.ts +++ b/tools/cfn2ts/lib/codegen.ts @@ -24,7 +24,7 @@ export default class CodeGenerator { * @param moduleName the name of the module (used to determine the file name). * @param spec CloudFormation resource specification */ - constructor(moduleName: string, private readonly spec: schema.Specification) { + constructor(moduleName: string, private readonly spec: schema.Specification, private readonly renames: {[key: string]: string}) { this.outputFile = `${moduleName}.generated.ts`; this.code.openFile(this.outputFile); @@ -117,23 +117,26 @@ export default class CodeGenerator { this.code.closeBlock(); } - private emitPropsType(resourceName: genspec.CodeName, spec: schema.ResourceType): genspec.CodeName | undefined { - if (!spec.Properties || Object.keys(spec.Properties).length === 0) { return; } - const name = genspec.CodeName.forResourceProperties(resourceName); + /** + * Emit the XxxProps interface for a resource + */ + private emitPropsType(resourceName: genspec.CodeName, spec: schema.ResourceType): PropsInterfaceType { + if (!spec.Properties || Object.keys(spec.Properties).length === 0) { return { attributesTable: {}}; } + const codeName = genspec.CodeName.forResourceProperties(resourceName); this.docLink(spec.Documentation); - this.code.openBlock(`export interface ${name.className}`); + this.code.openBlock(`export interface ${codeName.className}`); - const conversionTable = this.emitPropsTypeProperties(resourceName.specName!, spec.Properties); + const attributesTable = this.emitPropsTypeProperties(resourceName.specName!, spec.Properties); this.code.closeBlock(); this.code.line(); - this.emitValidator(name, spec.Properties, conversionTable); + this.emitValidator(codeName, spec.Properties, attributesTable); this.code.line(); - this.emitCloudFormationMapper(name, spec.Properties, conversionTable); + this.emitCloudFormationMapper(codeName, spec.Properties, attributesTable); - return name; + return { codeName, attributesTable }; } /** @@ -143,28 +146,37 @@ export default class CodeGenerator { */ private emitPropsTypeProperties(resourceName: SpecName, propertiesSpec: { [name: string]: schema.Property }): Dictionary { const propertyMap: Dictionary = {}; - - // Sanity check that our renamed "Name" is not going to conflict with a real property - const renamedNameProperty = resourceNameProperty(resourceName); - const lowerNames = Object.keys(propertiesSpec).map(s => s.toLowerCase()); - if (lowerNames.indexOf('name') !== -1 && lowerNames.indexOf(renamedNameProperty.toLowerCase()) !== -1) { - // tslint:disable-next-line:max-line-length - throw new Error(`Oh gosh, we want to rename ${resourceName.fqn}'s 'Name' property to '${renamedNameProperty}', but that property already exists! We need to find a solution to this problem.`); - } + const finalNames = new Set(); Object.keys(propertiesSpec).sort(propertyComparator).forEach(propName => { const originalName = propName; const propSpec = propertiesSpec[propName]; const additionalDocs = resourceName.relativeName(propName).fqn; + // Apply property renames + const renameKey = `${resourceName.fqn}/${propName}`; + if (renameKey in this.renames) { + // User-defined rename + propName = this.renames[renameKey]; + } + if (propName.toLocaleLowerCase() === 'name') { - propName = renamedNameProperty; + // We like to include the resource name in the "name" property + propName = resourceNameProperty(resourceName); + } + + if (originalName !== propName) { // tslint:disable-next-line:no-console - console.error(`Renamed property 'Name' of ${resourceName.fqn} to '${renamedNameProperty}'`); + console.error(`[${resourceName.fqn}] Renamed property '${originalName}' to '${propName}'`); } + if (finalNames.has(propName)) { + throw new Error(`Oh gosh, there was already a property named ${propName}!`); + } + finalNames.add(propName); const resourceCodeName = genspec.CodeName.forResource(resourceName); const newName = this.emitProperty(resourceCodeName, propName, propSpec, quoteCode(additionalDocs)); + propertyMap[originalName] = newName; }); return propertyMap; @@ -265,7 +277,7 @@ export default class CodeGenerator { this.code.line(` * @param properties the properties of this ${quoteCode(resourceName.className)}`); this.code.line(' */'); const optionalProps = spec.Properties && !Object.values(spec.Properties).some(p => p.Required); - const propsArgument = propsType ? `, properties${optionalProps ? '?' : ''}: ${propsType.className}` : ''; + const propsArgument = propsType.codeName ? `, properties${optionalProps ? '?' : ''}: ${propsType.codeName.className}` : ''; this.code.openBlock(`constructor(parent: ${CONSTRUCT_CLASS}, name: string${propsArgument})`); this.code.line(`super(parent, name, { type: ${resourceName.className}.resourceTypeName${propsType ? ', properties' : ''} });`); // verify all required properties @@ -273,8 +285,8 @@ export default class CodeGenerator { for (const pname of Object.keys(spec.Properties)) { const prop = spec.Properties[pname]; if (prop.Required) { - const propName = pname.toLocaleLowerCase() === 'name' ? resourceNameProperty(resourceName.specName!) : pname; - this.code.line(`${CORE}.requireProperty(properties, '${genspec.cloudFormationToScriptName(propName)}', this);`); + const attributeName = propsType.attributesTable[pname]; + this.code.line(`${CORE}.requireProperty(properties, '${genspec.cloudFormationToScriptName(attributeName)}', this);`); } } } @@ -304,9 +316,9 @@ export default class CodeGenerator { this.code.closeBlock(); - if (propsType) { + if (propsType.codeName) { this.code.line(); - this.emitCloudFormationPropertiesOverride(propsType); + this.emitCloudFormationPropertiesOverride(propsType.codeName); } this.closeClass(resourceName); @@ -619,6 +631,25 @@ export default class CodeGenerator { } } +/** + * Represents a PropsType that has been generated + */ +interface PropsInterfaceType { + /** + * Name in the code of the XxxProps type + * + * Missing if no type got generated because there were no properties. + */ + codeName?: genspec.CodeName; + + /** + * Name conversion table for properties on this props type + * + * Maps cfn name -> attribute name. + */ + attributesTable: Dictionary; +} + /** * Return a comma-separated list of validator functions for the given types */ diff --git a/tools/cfn2ts/lib/index.ts b/tools/cfn2ts/lib/index.ts index d06c845b6ec6f..f4e08b598889c 100644 --- a/tools/cfn2ts/lib/index.ts +++ b/tools/cfn2ts/lib/index.ts @@ -14,7 +14,23 @@ export default async function(scope: string, outPath: string, force: boolean) { } const name = packageName(scope); - const generator = new CodeGenerator(name, spec); + // Read package.json in the current directory to determine renames + const renames: {[key: string]: string} = {}; + let packageJson; + try { + packageJson = require(path.resolve(process.cwd(), 'package.json')); + } catch (e) { + // Nothing + } + if (packageJson && packageJson.cfn2ts && packageJson.cfn2ts.rename) { + // tslint:disable-next-line:no-console + console.error('Reading renames from package.json'); + for (const [key, value] of Object.entries(packageJson.cfn2ts.rename)) { + renames[key] = value as any; + } + } + + const generator = new CodeGenerator(name, spec, renames); if (!force && await generator.upToDate(outPath)) { // tslint:disable-next-line:no-console