From 10b0665ad88c37918d6d3050917ab697106a2d37 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 22 Apr 2019 15:06:57 +0300 Subject: [PATCH 1/6] refactor(awslint): XxxReflection classes Implement a "Reflection" class for each rule family (modules, constructs, resources) which reflect on the type system and expose a relevant API for linter rules. The rational is that this increases our ability to reuse the model by other rule families. For example, the "ResourceReflection" extends "ConstructReflection" and adds more information that's specific to AWS resource constructs. --- tools/awslint/bin/awslint.ts | 6 + tools/awslint/lib/cfn-resources.ts | 87 --------- tools/awslint/lib/rules/common.ts | 1 + tools/awslint/lib/rules/construct.ts | 171 +++++++++++------ tools/awslint/lib/rules/index.ts | 4 +- tools/awslint/lib/rules/module.ts | 2 +- tools/awslint/lib/rules/resource.ts | 273 ++++++++++++++------------- tools/awslint/lib/util.ts | 15 -- 8 files changed, 274 insertions(+), 285 deletions(-) delete mode 100644 tools/awslint/lib/cfn-resources.ts create mode 100644 tools/awslint/lib/rules/common.ts delete mode 100644 tools/awslint/lib/util.ts diff --git a/tools/awslint/bin/awslint.ts b/tools/awslint/bin/awslint.ts index c3165543a4748..10ea203e9aef8 100644 --- a/tools/awslint/bin/awslint.ts +++ b/tools/awslint/bin/awslint.ts @@ -8,6 +8,7 @@ import yargs = require('yargs'); import { constructLinter, DiagnosticLevel, moduleLinter, resourceLinter } from '../lib'; const LINTERS = [ moduleLinter, constructLinter, resourceLinter ]; +let stackTrace = false; async function main() { const argv = yargs @@ -43,6 +44,8 @@ async function main() { const args = argv.argv; + stackTrace = args.verbose || args.debug; + if (args._.length > 1) { argv.showHelp(); process.exit(1); @@ -208,6 +211,9 @@ async function main() { main().catch(e => { console.error(colors.red(e.message)); + if (stackTrace) { + console.error(e.stack); + } process.exit(1); }); diff --git a/tools/awslint/lib/cfn-resources.ts b/tools/awslint/lib/cfn-resources.ts deleted file mode 100644 index 5707058b0dcf2..0000000000000 --- a/tools/awslint/lib/cfn-resources.ts +++ /dev/null @@ -1,87 +0,0 @@ -import reflect = require('jsii-reflect'); -import { isConstruct } from './util'; - -export interface CfnResourceSpec { - /** - * The full AWS CloudFormation name of the resource (i.e. `AWS::S3::Bucket`) - */ - fullname: string; - - /** - * The AWS CloudFormation namespace of the resource (i.e. `AWS::S3`) - */ - namespace: string; - - /** - * The base name of the resource (i.e. `Bucket`) - */ - basename: string; - - /** - * The documentation link - */ - doc: string; - - /** - * The set of CloudFormation attributes this resource has (i.e. `bucketArn`, `queueUrl`) - */ - attributes: string[]; -} - -/** - * Given a jsii assembly, extracts all CloudFormation resources from the module. - * @param a - */ -export function findCfnResources(assembly: reflect.Assembly): CfnResourceSpec[] { - - return assembly.classes.filter(c => isCfnResource(c)).map(layer1 => { - const basename = layer1.name.substr('Cfn'.length); - const docLink = layer1.docs.docs.see || ''; - - // HACK: extract full CFN name from initializer docs - const initializerDoc = (layer1.initializer && layer1.initializer.docs.docs.summary) || ''; - const out = /a new `([^`]+)`/.exec(initializerDoc); - const fullname = out && out[1]; - if (!fullname) { - throw new Error(`Unable to extract CloudFormation resource name from initializer documentation of ${layer1}`); - } - - const namespace = fullname.split('::').slice(0, 2).join('::'); - - const resource: CfnResourceSpec = { - namespace, - fullname, - doc: docLink, - basename, - attributes: parseResourceAttributes(layer1) - }; - - return resource; - }); - - function parseResourceAttributes(cfnResourceClass: reflect.ClassType) { - return cfnResourceClass.ownProperties.filter(p => (p.docs.docs.custom || {}).cloudformationAttribute).map(p => p.name); - } - - function isCfnResource(c: reflect.ClassType) { - const cdkAssembly = '@aws-cdk/cdk'; - if (!c.system.includesAssembly(cdkAssembly)) { - return false; - } - const resourceBaseClass = c.system.findFqn(`${cdkAssembly}.CfnResource`); - - if (!isConstruct(c)) { - return false; - } - - if (c.base !== resourceBaseClass) { - return false; - } - - if (!c.name.startsWith('Cfn')) { - return false; - } - - return true; - } -} diff --git a/tools/awslint/lib/rules/common.ts b/tools/awslint/lib/rules/common.ts new file mode 100644 index 0000000000000..48401e5277419 --- /dev/null +++ b/tools/awslint/lib/rules/common.ts @@ -0,0 +1 @@ +export const CORE_MODULE = '@aws-cdk/cdk'; diff --git a/tools/awslint/lib/rules/construct.ts b/tools/awslint/lib/rules/construct.ts index 719bf4a4a021e..5920b7d1dcb4c 100644 --- a/tools/awslint/lib/rules/construct.ts +++ b/tools/awslint/lib/rules/construct.ts @@ -1,33 +1,101 @@ import reflect = require('jsii-reflect'); import { Linter, MethodSignatureParameterExpectation } from '../linter'; -import { CONSTRUCT_FQN, isConstruct } from '../util'; - -interface ConstructLinterContext { - readonly construct: reflect.ClassType; - readonly ts: reflect.TypeSystem; - hasProps?: boolean; // indicates if the ctor accepts any "props" or just two arguments - props?: reflect.InterfaceType; - initializer?: reflect.Initializer; -} +import { CORE_MODULE } from './common'; + +const CONSTRUCT_FQN = `${CORE_MODULE}.Construct`; +const CONSTRUCT_INTERFACE_FQN = `${CORE_MODULE}.IConstruct`; + +export const constructLinter = new Linter(assembly => assembly.classes + .filter(t => ConstructReflection.isConstructClass(t)) + .map(construct => new ConstructReflection(construct))); + +export class ConstructReflection { + /** + * Determines if a class is a construct. + */ + public static isConstructClass(c: reflect.ClassType) { + if (!c.system.includesAssembly(CORE_MODULE)) { + return false; + } + + if (!c.isClassType()) { + return false; + } + + if (c.abstract) { + return false; + } + + return c.extends(c.system.findFqn(CONSTRUCT_FQN)); + } + + public static findAllConstructs(assembly: reflect.Assembly) { + return assembly.classes + .filter(c => ConstructReflection.isConstructClass(c)) + .map(c => new ConstructReflection(c)); + } + + public readonly fqn: string; + public readonly interfaceFqn: string; + public readonly propsFqn: string; + public readonly interfaceType?: reflect.InterfaceType; + public readonly propsType?: reflect.InterfaceType; + public readonly initializer?: reflect.Initializer; + public readonly hasPropsArgument: boolean; + public readonly rootClass: reflect.ClassType; // cdk.Construct + public readonly sys: reflect.TypeSystem; + + constructor(public readonly classType: reflect.ClassType) { + this.fqn = classType.fqn; + this.sys = classType.system; + this.interfaceFqn = `${classType.assembly.name}.I${classType.name}`; + this.propsFqn = `${classType.assembly.name}.${classType.name}Props`; + this.interfaceType = this.tryFindInterface(); + this.propsType = this.tryFindProps(); + this.initializer = classType.initializer; + this.hasPropsArgument = this.initializer != null && this.initializer.parameters.length >= 3; + this.rootClass = this.sys.findClass(CONSTRUCT_FQN); + } + + private tryFindInterface() { + const sys = this.classType.system; + const found = sys.tryFindFqn(this.interfaceFqn); + if (!found) { + return undefined; + } + + if (!found.isInterfaceType()) { + throw new Error(`Expecting type ${this.interfaceFqn} to be an interface`); + } -export const constructLinter = new Linter(assembly => assembly.classes - .filter(t => isConstruct(t)) - .map(construct => ({ - construct, - ts: construct.system - }))); + return found; + } + + private tryFindProps() { + const found = this.sys.tryFindFqn(this.propsFqn); + if (!found) { + return undefined; + } + + if (!found.isInterfaceType()) { + throw new Error(`Expecrting props struct ${this.propsFqn} to be an interface`); + } + + return found; + } +} constructLinter.add({ code: 'construct-ctor', message: 'signature of all construct constructors should be "scope, id, props"', eval: e => { // only applies to non abstract classes - if (e.ctx.construct.abstract) { + if (e.ctx.classType.abstract) { return; } - const initializer = e.ctx.construct.initializer; - if (!e.assert(initializer, e.ctx.construct.fqn)) { + const initializer = e.ctx.initializer; + if (!e.assert(initializer, e.ctx.fqn)) { return; } @@ -49,15 +117,11 @@ constructLinter.add({ expectedParams.push({ name: 'props', }); - - e.ctx.hasProps = true; } e.assertSignature(initializer, { parameters: expectedParams }); - - e.ctx.initializer = initializer; } }); @@ -65,35 +129,16 @@ constructLinter.add({ code: 'props-struct-name', message: 'all constructs must have a props struct', eval: e => { - if (!e.ctx.construct) { - return; - } - - if (!e.ctx.hasProps) { + if (!e.ctx.hasPropsArgument) { return; } // abstract classes are exempt - if (e.ctx.construct.abstract) { - return; - } - - const fqn = `${e.ctx.construct.assembly.name}.${e.ctx.construct.name}Props`; - - if (!e.assert(e.ctx.ts.tryFindFqn(fqn), fqn)) { - return; - } - - const iface = e.ctx.ts.findInterface(fqn); - if (!e.assert(iface, fqn)) { - return; - } - - if (!e.assert(iface.isInterfaceType(), fqn)) { + if (e.ctx.classType.abstract) { return; } - e.ctx.props = iface; + e.assert(e.ctx.propsType, e.ctx.interfaceFqn); } }); @@ -101,12 +146,13 @@ constructLinter.add({ code: 'construct-ctor-props-type', message: 'construct "props" type should use the props struct %s', eval: e => { - if (!e.ctx.props) { return; } - if (!e.ctx.construct) { return; } if (!e.ctx.initializer) { return; } if (e.ctx.initializer.parameters.length < 3) { return; } - e.assert(e.ctx.initializer.parameters[2].type.type === e.ctx.props, e.ctx.construct.fqn, e.ctx.props.fqn); + e.assert( + e.ctx.initializer.parameters[2].type.type === e.ctx.propsType, + e.ctx.fqn, + e.ctx.propsFqn); } }); @@ -114,17 +160,36 @@ constructLinter.add({ code: 'construct-ctor-props-optional', message: 'construct "props" must be optional since all props are optional', eval: e => { - if (!e.ctx.props) { return; } - if (!e.ctx.construct) { return; } + if (!e.ctx.propsType) { return; } if (!e.ctx.initializer) { return; } - if (e.ctx.initializer.parameters.length < 3) { return; } + if (!e.ctx.hasPropsArgument) { return; } // this rule applies only if all properties are optional - const allOptional = e.ctx.props.allProperties.every(p => p.optional); + const allOptional = e.ctx.propsType.allProperties.every(p => p.optional); if (!allOptional) { return; } - e.assert(e.ctx.initializer.parameters[2].optional, e.ctx.construct.fqn); + e.assert(e.ctx.initializer.parameters[2].optional, e.ctx.fqn); + } +}); + +constructLinter.add({ + code: 'construct-interface-extends-iconstruct', + message: 'construct interface must extend core.IConstruct', + eval: e => { + if (!e.ctx.interfaceType) { return; } + const interfaceBase = e.ctx.sys.findInterface(CONSTRUCT_INTERFACE_FQN); + e.assert(e.ctx.interfaceType.extends(interfaceBase), e.ctx.interfaceType.fqn); + } +}); + +constructLinter.add({ + code: 'construct-base-is-private', + message: 'Construct base class (FooBase) should not be public', + eval: e => { + const baseTypeFqn = `${e.ctx.fqn}Base`; + const found = e.ctx.sys.tryFindFqn(baseTypeFqn); + e.assert(!found, baseTypeFqn); } -}); \ No newline at end of file +}); diff --git a/tools/awslint/lib/rules/index.ts b/tools/awslint/lib/rules/index.ts index 065133c0eb94c..33524721968fe 100644 --- a/tools/awslint/lib/rules/index.ts +++ b/tools/awslint/lib/rules/index.ts @@ -1,3 +1,3 @@ -export * from './module'; export * from './construct'; -export * from './resource'; +export * from './module'; +export * from './resource'; \ No newline at end of file diff --git a/tools/awslint/lib/rules/module.ts b/tools/awslint/lib/rules/module.ts index c733e8749c4f7..25a792cae029e 100644 --- a/tools/awslint/lib/rules/module.ts +++ b/tools/awslint/lib/rules/module.ts @@ -1,6 +1,6 @@ import reflect = require('jsii-reflect'); -import { findCfnResources } from '../cfn-resources'; import { Linter } from '../linter'; +import { findCfnResources } from './resource'; interface ModuleLinterContext { readonly assembly: reflect.Assembly; diff --git a/tools/awslint/lib/rules/resource.ts b/tools/awslint/lib/rules/resource.ts index 32ffeeccc0ca5..e3da0f7f482aa 100644 --- a/tools/awslint/lib/rules/resource.ts +++ b/tools/awslint/lib/rules/resource.ts @@ -1,27 +1,98 @@ import reflect = require('jsii-reflect'); -import { CfnResourceSpec, findCfnResources } from '../cfn-resources'; import { Linter } from '../linter'; -import { CONSTRUCT_FQN, CONSTRUCT_INTERFACE_FQN } from '../util'; - -export const resourceLinter = new Linter(assembly => { - return findCfnResources(assembly).map(cfn => ({ - assembly, - resource: cfn, - ts: assembly.system, - })); -}); +import { CORE_MODULE } from './common'; +import { ConstructReflection } from './construct'; +const CFN_RESOURCE_BASE_CLASS_FQN = `${CORE_MODULE}.CfnResource`; +const RESOURCE_BASE_CLASS_FQN = `${CORE_MODULE}.Resource`; +const RESOURCE_BASE_INTERFACE_FQN = `${CORE_MODULE}.IResource`; const GRANT_RESULT_FQN = '@aws-cdk/aws-iam.Grant'; -interface ResourceLinterContext { - readonly ts: reflect.TypeSystem; - readonly resource: CfnResourceSpec; - readonly assembly: reflect.Assembly; - resourceClass?: reflect.ClassType; - resourcePropsInterface?: reflect.InterfaceType; - resourceInterface?: reflect.InterfaceType; - importPropsInterface?: reflect.InterfaceType; - resourceAttributes?: reflect.Property[]; +export const resourceLinter = new Linter(assembly => findCfnResources(assembly)); + +export class ResourceReflection { + public static isResourceConstruct(construct: ConstructReflection) { + const classType = construct.classType; + const baseResource = classType.system.findClass(RESOURCE_BASE_CLASS_FQN); + return classType.extends(baseResource); + } + + /** + * @returns all resource constructs + */ + public static findAllResources(assembly: reflect.Assembly) { + return findCfnResources(assembly).filter(r => r.hasConstruct); + } + + public readonly fullname: string; // AWS::S3::Bucket + public readonly namespace: string; // AWS::S3 + public readonly basename: string; // Bucket + public readonly doc: string; // link to CloudFormation docs + public readonly attributeNames: string[]; // bucketArn, bucketName, queueUrl + public readonly attributes: reflect.Property[]; // actual attribute props + public readonly fqn: string; // expected fqn of resource class + + public readonly assembly: reflect.Assembly; + public readonly sys: reflect.TypeSystem; + + private readonly _construct?: ConstructReflection; // the resource construct + + constructor(public readonly cfnResource: reflect.ClassType) { + this.sys = cfnResource.system; + this.basename = cfnResource.name.substr('Cfn'.length); + this.doc = cfnResource.docs.docs.see || ''; + + // HACK: extract full CFN name from initializer docs + const initializerDoc = (cfnResource.initializer && cfnResource.initializer.docs.docs.summary) || ''; + const out = /a new `([^`]+)`/.exec(initializerDoc); + const fullname = out && out[1]; + if (!fullname) { + throw new Error(`Unable to extract CloudFormation resource name from initializer documentation of ${cfnResource}`); + } + this.fullname = fullname; + this.namespace = fullname.split('::').slice(0, 2).join('::'); + this.attributeNames = parseResourceAttributes(cfnResource); + + function parseResourceAttributes(cfnResourceClass: reflect.ClassType) { + return cfnResourceClass.ownProperties.filter(p => (p.docs.docs.custom || {}).cloudformationAttribute).map(p => p.name); + } + + this.assembly = cfnResource.assembly; + + this.fqn = `${this.assembly.name}.${this.basename}`; + this._construct = ConstructReflection + .findAllConstructs(this.assembly) + .find(c => c.fqn === this.fqn); + + this.attributes = this.findAttributeProperties(); + } + + public get construct(): ConstructReflection { + if (!this._construct) { + throw new Error(`Resource ${this.fullname} does not have a corresponding AWS construct`); + } + return this._construct; + } + + public get hasConstruct(): boolean { + return !!this._construct; + } + + private findAttributeProperties() { + if (!this.hasConstruct) { + return []; + } + + const resiult = new Array(); + for (const attr of this.attributeNames) { + const attribute = this.construct.classType.allProperties.find(p => p.name === attr); + if (attribute) { + resiult.push(attribute); + } + } + + return resiult; + } } resourceLinter.add({ @@ -29,13 +100,7 @@ resourceLinter.add({ message: 'every resource must have a resource class (L2)', warning: true, eval: e => { - const resourceFqn = `${e.ctx.assembly.name}.${e.ctx.resource.basename}`; - const resourceClass = e.ctx.ts.classes.find(c => c.fqn === resourceFqn); - if (!e.assert(resourceClass, resourceFqn)) { - return; - } - - e.ctx.resourceClass = resourceClass; + e.assert(e.ctx.hasConstruct, e.ctx.fqn); } }); @@ -43,27 +108,20 @@ resourceLinter.add({ code: 'resource-class-extends-resource', message: `resource classes must extend "cdk.Resource" directly or indirectly`, eval: e => { - if (!e.ctx.resourceClass) { return; } - const resourceBase = e.ctx.ts.findClass('@aws-cdk/cdk.Resource'); - e.assert(e.ctx.resourceClass.extends(resourceBase), e.ctx.resourceClass.fqn); + if (!e.ctx.hasConstruct) { return; } + + const resourceBase = e.ctx.sys.findClass(RESOURCE_BASE_CLASS_FQN); + e.assert(e.ctx.construct.classType.extends(resourceBase), e.ctx.construct.fqn); } }); resourceLinter.add({ code: 'resource-interface', - message: 'every resource must have a resource interface', warning: true, + message: 'every resource must have a resource interface', eval: e => { - if (!e.ctx.resourceClass) { return; } - - // first, let's look up the IFoo interface - const interfaceFqn = `${e.ctx.assembly.name}.I${e.ctx.resource.basename}`; - const resourceInterface = e.ctx.ts.interfaces.find(i => i.fqn === interfaceFqn); - if (!e.assert(resourceInterface, interfaceFqn)) { - return; - } - - e.ctx.resourceInterface = resourceInterface; + if (!e.ctx.hasConstruct) { return; } + e.assert(e.ctx.construct.interfaceType, e.ctx.construct.fqn); } }); @@ -71,34 +129,11 @@ resourceLinter.add({ code: 'resource-interface-extends-resource', message: 'construct interfaces of AWS resources must extend cdk.IResource', eval: e => { - if (!e.ctx.resourceInterface) { return; } - const interfaceBase = e.ctx.ts.findInterface('@aws-cdk/cdk.IResource'); - e.assert(e.ctx.resourceInterface.extends(interfaceBase), e.ctx.resourceInterface.fqn); - } -}); + const resourceInterface = e.ctx.hasConstruct && e.ctx.construct.interfaceType; + if (!resourceInterface) { return; } -resourceLinter.add({ - code: 'import-props-interface', - message: 'every resource must have an "FooImportProps" interface', - eval: e => { - if (!e.ctx.resourceInterface) { - return; - } - - const attrsFqn = `${e.ctx.assembly.name}.${e.ctx.resource.basename}ImportProps`; - const importPropsInterface = e.ctx.ts.interfaces.find(c => c.fqn === attrsFqn); - if (e.assert(importPropsInterface, attrsFqn)) { - e.ctx.importPropsInterface = importPropsInterface; - } - } -}); - -resourceLinter.add({ - code: 'resource-interface-extends-construct', - message: 'resource interface must extend cdk.IConstruct', - eval: e => { - if (!e.ctx.resourceInterface) { return; } - e.assert(e.ctx.resourceInterface.getInterfaces(true).some(i => i.fqn === CONSTRUCT_INTERFACE_FQN), e.ctx.resourceInterface.fqn); + const interfaceBase = e.ctx.sys.findInterface(RESOURCE_BASE_INTERFACE_FQN); + e.assert(resourceInterface.extends(interfaceBase), resourceInterface.fqn); } }); @@ -106,19 +141,13 @@ resourceLinter.add({ code: 'resource-attribute', message: 'resources must represent all attributes as properties', eval: e => { - if (!e.ctx.resourceInterface) { return; } - - // verify that the interface has all attributes as readonly properties - const resourceAttributes = new Array(); - for (const attr of e.ctx.resource.attributes) { - const attribute: reflect.Property | undefined = e.ctx.resourceInterface.ownProperties.find(p => p.name === attr); - const scope: string = e.ctx.resourceInterface.fqn + '.' + attr; - if (e.assert(attribute, scope)) { - resourceAttributes.push(attribute); - } - } + const resourceInterface = e.ctx.hasConstruct && e.ctx.construct.interfaceType; + if (!resourceInterface) { return; } - e.ctx.resourceAttributes = resourceAttributes; + for (const name of e.ctx.attributeNames) { + const found = e.ctx.attributes.find(a => a.name === name); + e.assert(found, `${e.ctx.fqn}.${name}`); + } } }); @@ -126,70 +155,60 @@ resourceLinter.add({ code: 'resource-attribute-immutable', message: 'resource attributes must be immutable (readonly)', eval: e => { - if (!e.ctx.resourceAttributes) { return; } - for (const att of e.ctx.resourceAttributes) { + for (const att of e.ctx.attributes) { e.assert(att.immutable, att.parentType.fqn + '.' + att.name); } } }); resourceLinter.add({ - code: 'import', - message: 'resource class must have a static "import" method', + code: 'grant-result', + message: `"grant" method must return ${GRANT_RESULT_FQN}`, eval: e => { - if (!e.ctx.resourceClass) { return; } - if (!e.ctx.resourceInterface) { return; } - if (!e.ctx.importPropsInterface) { return; } + if (!e.ctx.hasConstruct) { return; } - const importMethod = e.ctx.resourceClass.ownMethods.find(m => m.static && m.name === 'import'); - if (!e.assert(importMethod, e.ctx.resourceClass.fqn)) { - return; - } + const grantResultType = e.ctx.sys.findFqn(GRANT_RESULT_FQN); + const grantMethods = e.ctx.construct.classType.allMethods.filter(m => m.name.startsWith('grant')); - e.assertSignature(importMethod, { - returns: e.ctx.resourceInterface, - parameters: [ - { name: 'scope', type: CONSTRUCT_FQN }, - { name: 'id', type: 'string' }, - { name: 'props', type: e.ctx.importPropsInterface.fqn } - ], - }); + for (const grantMethod of grantMethods) { + e.assertSignature(grantMethod, { + returns: grantResultType + }); + } } }); -resourceLinter.add({ - code: 'export', - message: 'resource interface must have an "export" method', - eval: e => { - if (!e.ctx.resourceInterface) { return; } - - const exportMethod = e.ctx.resourceInterface.ownMethods.find(m => m.name === 'export'); - if (!e.assert(exportMethod, e.ctx.resourceInterface.fqn)) { - return; +/** + * Given a jsii assembly, extracts all CloudFormation resources from CFN classes + */ +export function findCfnResources(assembly: reflect.Assembly): ResourceReflection[] { + return assembly.classes.filter(c => isCfnResource(c)).map(layer1 => { + return new ResourceReflection(layer1); + }); + + function isCfnResource(c: reflect.ClassType) { + if (!c.system.includesAssembly(CORE_MODULE)) { + return false; } - if (!e.ctx.importPropsInterface) { return; } - - e.assertSignature(exportMethod, { - returns: e.ctx.importPropsInterface, - parameters: [] - }); - } -}); + // skip CfnResource itself + if (c.fqn === CFN_RESOURCE_BASE_CLASS_FQN) { + return false; + } -resourceLinter.add({ - code: 'grant-result', - message: `"grant" method must return ${GRANT_RESULT_FQN}`, - eval: e => { - if (!e.ctx.resourceClass) { return; } + if (!ConstructReflection.isConstructClass(c)) { + return false; + } - const grantResultType = e.ctx.ts.findFqn(GRANT_RESULT_FQN); - const grantMethods = e.ctx.resourceClass.allMethods.filter(m => m.name.startsWith('grant')); + const cfnResourceClass = c.system.findFqn(CFN_RESOURCE_BASE_CLASS_FQN); + if (!c.extends(cfnResourceClass)) { + return false; + } - for (const grantMethod of grantMethods) { - e.assertSignature(grantMethod, { - returns: grantResultType - }); + if (!c.name.startsWith('Cfn')) { + return false; } + + return true; } -}); \ No newline at end of file +} diff --git a/tools/awslint/lib/util.ts b/tools/awslint/lib/util.ts deleted file mode 100644 index 01057d14e0644..0000000000000 --- a/tools/awslint/lib/util.ts +++ /dev/null @@ -1,15 +0,0 @@ -import reflect = require('jsii-reflect'); - -export const CONSTRUCT_ASSEMBLY = '@aws-cdk/cdk'; -export const CONSTRUCT_FQN = `${CONSTRUCT_ASSEMBLY}.Construct`; -export const CONSTRUCT_INTERFACE_FQN = `${CONSTRUCT_ASSEMBLY}.IConstruct`; - -export function isConstruct(c: reflect.ClassType) { - if (!c.system.includesAssembly(CONSTRUCT_ASSEMBLY)) { - return false; - } - const constructClass = c.system.findFqn(CONSTRUCT_FQN); - const bases = c.getAncestors(); - const root = bases[bases.length - 1]; - return root === constructClass; -} \ No newline at end of file From 9bdadd5589a196a047d8b42754432a1a554f5c2d Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 22 Apr 2019 23:13:38 +0300 Subject: [PATCH 2/6] full build passes --- .../@aws-cdk/aws-apigateway/lib/restapi.ts | 40 +- .../aws-cloudfront/lib/web_distribution.ts | 4 +- .../aws-codedeploy/lib/lambda/application.ts | 6 +- .../lib/lambda/deployment-config.ts | 67 +-- .../lib/lambda/deployment-group.ts | 7 +- .../aws-codedeploy/lib/server/application.ts | 6 +- .../lib/server/deployment-config.ts | 73 +-- .../lib/server/deployment-group.ts | 8 +- packages/@aws-cdk/aws-codedeploy/lib/utils.ts | 29 +- packages/@aws-cdk/aws-codedeploy/package.json | 2 +- .../lib/codedeploy/server-deploy-action.ts | 2 +- packages/@aws-cdk/aws-ec2/lib/index.ts | 1 - .../@aws-cdk/aws-ec2/lib/security-group.ts | 53 +- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 484 +++++++++++++++++- packages/@aws-cdk/aws-kinesis/lib/stream.ts | 2 +- packages/@aws-cdk/aws-kms/lib/key.ts | 2 +- packages/@aws-cdk/aws-logs/lib/log-group.ts | 2 +- packages/@aws-cdk/aws-s3/lib/bucket.ts | 146 +++--- .../aws-s3/test/integ.bucket.domain-name.ts | 4 +- packages/@aws-cdk/aws-ssm/lib/parameter.ts | 2 +- tools/awslint/lib/rules/construct.ts | 10 - 21 files changed, 687 insertions(+), 263 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts index ec5eaa8bb4dbb..e62cca6120b57 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts @@ -12,6 +12,11 @@ export interface RestApiImportProps { * The REST API ID of an existing REST API resource. */ readonly restApiId: string; + + /** + * The resource ID of the root resource. + */ + readonly restApiRootResourceId?: string; } export interface IRestApi extends IResource { @@ -20,6 +25,11 @@ export interface IRestApi extends IResource { */ readonly restApiId: string; + /** + * The resource ID of the root resource. + */ + readonly restApiRootResourceId: string; + /** * Exports a REST API resource from this stack. * @returns REST API props that can be imported to another stack. @@ -162,7 +172,16 @@ export class RestApi extends Resource implements IRestApi { * @param props Imported rest API properties */ public static import(scope: Construct, id: string, props: RestApiImportProps): IRestApi { - return new ImportedRestApi(scope, id, props); + class Import extends Construct implements IRestApi { + public restApiId = props.restApiId; + public get restApiRootResourceId() { + if (!props.restApiRootResourceId) { throw new Error(`Imported REST API does not have "restApiRootResourceId"`); } + return props.restApiRootResourceId; + } + public export() { return props; } + } + + return new Import(scope, id); } /** @@ -170,6 +189,11 @@ export class RestApi extends Resource implements IRestApi { */ public readonly restApiId: string; + /** + * The resource ID of the root resource. + */ + public readonly restApiRootResourceId: string; + /** * API Gateway deployment that represents the latest changes of the API. * This resource will be automatically updated every time the REST API model changes. @@ -377,20 +401,6 @@ export enum EndpointType { Private = 'PRIVATE' } -class ImportedRestApi extends Construct implements IRestApi { - public restApiId: string; - - constructor(scope: Construct, id: string, private readonly props: RestApiImportProps) { - super(scope, id); - - this.restApiId = props.restApiId; - } - - public export() { - return this.props; - } -} - class RootResource extends ResourceBase { public readonly parentResource?: IRestApiResource; public readonly resourceApi: RestApi; diff --git a/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts b/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts index 2a466978e6530..da76fc22b2611 100644 --- a/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts +++ b/packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts @@ -566,7 +566,7 @@ export class CloudFrontWebDistribution extends cdk.Construct implements route53. const originProperty: CfnDistribution.OriginProperty = { id: originId, domainName: originConfig.s3OriginSource - ? originConfig.s3OriginSource.s3BucketSource.domainName + ? originConfig.s3OriginSource.s3BucketSource.bucketDomainName : originConfig.customOriginSource!.domainName, originPath: originConfig.originPath, originCustomHeaders: originHeaders.length > 0 ? originHeaders : undefined, @@ -660,7 +660,7 @@ export class CloudFrontWebDistribution extends cdk.Construct implements route53. distributionConfig = { ...distributionConfig, logging: { - bucket: this.loggingBucket.domainName, + bucket: this.loggingBucket.bucketDomainName, includeCookies: props.loggingConfig.includeCookies || false, prefix: props.loggingConfig.prefix } diff --git a/packages/@aws-cdk/aws-codedeploy/lib/lambda/application.ts b/packages/@aws-cdk/aws-codedeploy/lib/lambda/application.ts index dd1f2b72e9e56..e998a7f20ec0c 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/lambda/application.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/lambda/application.ts @@ -1,6 +1,6 @@ import cdk = require("@aws-cdk/cdk"); import { CfnApplication } from "../codedeploy.generated"; -import { applicationNameToArn } from "../utils"; +import { arnForApplication } from "../utils"; /** * Represents a reference to a CodeDeploy Application deploying to AWS Lambda. @@ -60,7 +60,7 @@ export class LambdaApplication extends cdk.Construct implements ILambdaApplicati }); this.applicationName = resource.ref; - this.applicationArn = applicationNameToArn(this.applicationName, this); + this.applicationArn = arnForApplication(this.applicationName); } public export(): LambdaApplicationImportProps { @@ -92,7 +92,7 @@ class ImportedLambdaApplication extends cdk.Construct implements ILambdaApplicat super(scope, id); this.applicationName = props.applicationName; - this.applicationArn = applicationNameToArn(props.applicationName, this); + this.applicationArn = arnForApplication(props.applicationName); } public export(): LambdaApplicationImportProps { diff --git a/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-config.ts b/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-config.ts index 416b1cbf77b82..704c4d764e1ab 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-config.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-config.ts @@ -1,5 +1,5 @@ import cdk = require('@aws-cdk/cdk'); -import { arnForDeploymentConfigName } from '../utils'; +import { arnForDeploymentConfig } from '../utils'; /** * The Deployment Configuration of a Lambda Deployment Group. @@ -12,19 +12,7 @@ import { arnForDeploymentConfigName } from '../utils'; */ export interface ILambdaDeploymentConfig { readonly deploymentConfigName: string; - deploymentConfigArn(scope: cdk.IConstruct): string; -} - -class DefaultLambdaDeploymentConfig implements ILambdaDeploymentConfig { - public readonly deploymentConfigName: string; - - constructor(deploymentConfigName: string) { - this.deploymentConfigName = `CodeDeployDefault.Lambda${deploymentConfigName}`; - } - - public deploymentConfigArn(scope: cdk.IConstruct): string { - return arnForDeploymentConfigName(this.deploymentConfigName, scope); - } + readonly deploymentConfigArn: string; } /** @@ -41,24 +29,6 @@ export interface LambdaDeploymentConfigImportProps { readonly deploymentConfigName: string; } -class ImportedLambdaDeploymentConfig extends cdk.Construct implements ILambdaDeploymentConfig { - public readonly deploymentConfigName: string; - - constructor(scope: cdk.Construct, id: string, private readonly props: LambdaDeploymentConfigImportProps) { - super(scope, id); - - this.deploymentConfigName = props.deploymentConfigName; - } - - public deploymentConfigArn(scope: cdk.IConstruct): string { - return arnForDeploymentConfigName(this.deploymentConfigName, scope); - } - - public export() { - return this.props; - } -} - /** * A custom Deployment Configuration for a Lambda Deployment Group. * @@ -67,29 +37,36 @@ class ImportedLambdaDeploymentConfig extends cdk.Construct implements ILambdaDep * (private constructor) and does not extend {@link cdk.Construct} */ export class LambdaDeploymentConfig { - public static readonly AllAtOnce: ILambdaDeploymentConfig = new DefaultLambdaDeploymentConfig('AllAtOnce'); - public static readonly Canary10Percent30Minutes: ILambdaDeploymentConfig = new DefaultLambdaDeploymentConfig('Canary10Percent30Minutes'); - public static readonly Canary10Percent5Minutes: ILambdaDeploymentConfig = new DefaultLambdaDeploymentConfig('Canary10Percent5Minutes'); - public static readonly Canary10Percent10Minutes: ILambdaDeploymentConfig = new DefaultLambdaDeploymentConfig('Canary10Percent10Minutes'); - public static readonly Canary10Percent15Minutes: ILambdaDeploymentConfig = new DefaultLambdaDeploymentConfig('Canary10Percent15Minutes'); - public static readonly Linear10PercentEvery10Minutes: ILambdaDeploymentConfig = new DefaultLambdaDeploymentConfig('Linear10PercentEvery10Minutes'); - public static readonly Linear10PercentEvery1Minute: ILambdaDeploymentConfig = new DefaultLambdaDeploymentConfig('Linear10PercentEvery1Minute'); - public static readonly Linear10PercentEvery2Minutes: ILambdaDeploymentConfig = new DefaultLambdaDeploymentConfig('Linear10PercentEvery2Minutes'); - public static readonly Linear10PercentEvery3Minutes: ILambdaDeploymentConfig = new DefaultLambdaDeploymentConfig('Linear10PercentEvery3Minutes'); + public static readonly AllAtOnce = deploymentConfig('CodeDeployDefault.LambdaAllAtOnce'); + public static readonly Canary10Percent30Minutes = deploymentConfig('CodeDeployDefault.LambdaCanary10Percent30Minutes'); + public static readonly Canary10Percent5Minutes = deploymentConfig('CodeDeployDefault.LambdaCanary10Percent5Minutes'); + public static readonly Canary10Percent10Minutes = deploymentConfig('CodeDeployDefault.LambdaCanary10Percent10Minutes'); + public static readonly Canary10Percent15Minutes = deploymentConfig('CodeDeployDefault.LambdaCanary10Percent15Minutes'); + public static readonly Linear10PercentEvery10Minutes = deploymentConfig('CodeDeployDefault.LambdaLinear10PercentEvery10Minutes'); + public static readonly Linear10PercentEvery1Minute = deploymentConfig('CodeDeployDefault.LambdaLinear10PercentEvery1Minute'); + public static readonly Linear10PercentEvery2Minutes = deploymentConfig('CodeDeployDefault.LambdaLinear10PercentEvery2Minutes'); + public static readonly Linear10PercentEvery3Minutes = deploymentConfig('CodeDeployDefault.LambdaLinear10PercentEvery3Minutes'); /** * Import a custom Deployment Configuration for a Lambda Deployment Group defined outside the CDK. * - * @param scope the parent Construct for this new Construct - * @param id the logical ID of this new Construct + * @param _scope the parent Construct for this new Construct + * @param _id the logical ID of this new Construct * @param props the properties of the referenced custom Deployment Configuration * @returns a Construct representing a reference to an existing custom Deployment Configuration */ - public static import(scope: cdk.Construct, id: string, props: LambdaDeploymentConfigImportProps): ILambdaDeploymentConfig { - return new ImportedLambdaDeploymentConfig(scope, id, props); + public static import(_scope: cdk.Construct, _id: string, props: LambdaDeploymentConfigImportProps): ILambdaDeploymentConfig { + return deploymentConfig(props.deploymentConfigName); } private constructor() { // nothing to do until CFN supports custom lambda deployment configurations } } + +function deploymentConfig(name: string): ILambdaDeploymentConfig { + return { + deploymentConfigName: name, + deploymentConfigArn: arnForDeploymentConfig(name) + }; +} diff --git a/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts b/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts index 6308a656a37c1..194c69cc9caf1 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts @@ -5,7 +5,7 @@ import cdk = require('@aws-cdk/cdk'); import { CfnDeploymentGroup } from '../codedeploy.generated'; import { AutoRollbackConfig } from '../rollback-config'; -import { deploymentGroupNameToArn, renderAlarmConfiguration, renderAutoRollbackConfiguration } from '../utils'; +import { arnForDeploymentGroup, renderAlarmConfiguration, renderAutoRollbackConfiguration } from '../utils'; import { ILambdaApplication, LambdaApplication } from './application'; import { ILambdaDeploymentConfig, LambdaDeploymentConfig } from './deployment-config'; @@ -156,7 +156,7 @@ export class LambdaDeploymentGroup extends cdk.Construct implements ILambdaDeplo }); this.deploymentGroupName = resource.deploymentGroupName; - this.deploymentGroupArn = deploymentGroupNameToArn(this.application.applicationName, this.deploymentGroupName, this); + this.deploymentGroupArn = arnForDeploymentGroup(this.application.applicationName, this.deploymentGroupName); if (props.preHook) { this.onPreHook(props.preHook); @@ -264,8 +264,7 @@ class ImportedLambdaDeploymentGroup extends cdk.Construct implements ILambdaDepl super(scope, id); this.application = props.application; this.deploymentGroupName = props.deploymentGroupName; - this.deploymentGroupArn = deploymentGroupNameToArn(props.application.applicationName, - props.deploymentGroupName, this); + this.deploymentGroupArn = arnForDeploymentGroup(props.application.applicationName, props.deploymentGroupName); } public export() { diff --git a/packages/@aws-cdk/aws-codedeploy/lib/server/application.ts b/packages/@aws-cdk/aws-codedeploy/lib/server/application.ts index faa78f4bb7f6b..08a0a9bae23ae 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/server/application.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/server/application.ts @@ -1,6 +1,6 @@ import cdk = require('@aws-cdk/cdk'); import { CfnApplication } from '../codedeploy.generated'; -import { applicationNameToArn } from '../utils'; +import { arnForApplication } from '../utils'; /** * Represents a reference to a CodeDeploy Application deploying to EC2/on-premise instances. @@ -41,7 +41,7 @@ class ImportedServerApplication extends cdk.Construct implements IServerApplicat super(scope, id); this.applicationName = props.applicationName; - this.applicationArn = applicationNameToArn(this.applicationName, this); + this.applicationArn = arnForApplication(this.applicationName); } public export(): ServerApplicationImportProps { @@ -90,7 +90,7 @@ export class ServerApplication extends cdk.Construct implements IServerApplicati }); this.applicationName = resource.ref; - this.applicationArn = applicationNameToArn(this.applicationName, this); + this.applicationArn = arnForApplication(this.applicationName); } public export(): ServerApplicationImportProps { diff --git a/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-config.ts b/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-config.ts index 87a4fbb259344..f59b1761251b9 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-config.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-config.ts @@ -1,6 +1,6 @@ import cdk = require('@aws-cdk/cdk'); import { CfnDeploymentConfig } from '../codedeploy.generated'; -import { arnForDeploymentConfigName } from '../utils'; +import { arnForDeploymentConfig } from '../utils'; /** * The Deployment Configuration of an EC2/on-premise Deployment Group. @@ -11,7 +11,7 @@ import { arnForDeploymentConfigName } from '../utils'; */ export interface IServerDeploymentConfig { readonly deploymentConfigName: string; - deploymentConfigArn(scope: cdk.IConstruct): string; + readonly deploymentConfigArn: string; export(): ServerDeploymentConfigImportProps; } @@ -29,42 +29,9 @@ export interface ServerDeploymentConfigImportProps { readonly deploymentConfigName: string; } -class ImportedServerDeploymentConfig extends cdk.Construct implements IServerDeploymentConfig { - public readonly deploymentConfigName: string; - - constructor(scope: cdk.Construct, id: string, private readonly props: ServerDeploymentConfigImportProps) { - super(scope, id); - - this.deploymentConfigName = props.deploymentConfigName; - } - - public deploymentConfigArn(scope: cdk.IConstruct): string { - return arnForDeploymentConfigName(this.deploymentConfigName, scope); - } - - public export() { - return this.props; - } -} - -class DefaultServerDeploymentConfig implements IServerDeploymentConfig { - public readonly deploymentConfigName: string; - - constructor(deploymentConfigName: string) { - this.deploymentConfigName = deploymentConfigName; - } - - public deploymentConfigArn(scope: cdk.IConstruct): string { - return arnForDeploymentConfigName(this.deploymentConfigName, scope); - } - - public export(): ServerDeploymentConfigImportProps { - return { - deploymentConfigName: this.deploymentConfigName - }; - } -} - +/** + * Minimum number of healthy hosts for a server deployment. + */ export class MinimumHealthyHosts { /** @@ -117,25 +84,26 @@ export interface ServerDeploymentConfigProps { /** * A custom Deployment Configuration for an EC2/on-premise Deployment Group. */ -export class ServerDeploymentConfig extends cdk.Construct implements IServerDeploymentConfig { - public static readonly OneAtATime: IServerDeploymentConfig = new DefaultServerDeploymentConfig('CodeDeployDefault.OneAtATime'); - public static readonly HalfAtATime: IServerDeploymentConfig = new DefaultServerDeploymentConfig('CodeDeployDefault.HalfAtATime'); - public static readonly AllAtOnce: IServerDeploymentConfig = new DefaultServerDeploymentConfig('CodeDeployDefault.AllAtOnce'); +export class ServerDeploymentConfig extends cdk.Resource implements IServerDeploymentConfig { + public static readonly OneAtATime = deploymentConfig('CodeDeployDefault.OneAtATime'); + public static readonly HalfAtATime = deploymentConfig('CodeDeployDefault.HalfAtATime'); + public static readonly AllAtOnce = deploymentConfig('CodeDeployDefault.AllAtOnce'); /** * Import a custom Deployment Configuration for an EC2/on-premise Deployment Group defined either outside the CDK, * or in a different CDK Stack and exported using the {@link #export} method. * - * @param scope the parent Construct for this new Construct - * @param id the logical ID of this new Construct + * @param _scope the parent Construct for this new Construct + * @param _id the logical ID of this new Construct * @param props the properties of the referenced custom Deployment Configuration * @returns a Construct representing a reference to an existing custom Deployment Configuration */ - public static import(scope: cdk.Construct, id: string, props: ServerDeploymentConfigImportProps): IServerDeploymentConfig { - return new ImportedServerDeploymentConfig(scope, id, props); + public static import(_scope: cdk.Construct, _id: string, props: ServerDeploymentConfigImportProps): IServerDeploymentConfig { + return deploymentConfig(props.deploymentConfigName); } public readonly deploymentConfigName: string; + public readonly deploymentConfigArn: string; constructor(scope: cdk.Construct, id: string, props: ServerDeploymentConfigProps) { super(scope, id); @@ -146,10 +114,7 @@ export class ServerDeploymentConfig extends cdk.Construct implements IServerDepl }); this.deploymentConfigName = resource.ref.toString(); - } - - public deploymentConfigArn(scope: cdk.IConstruct): string { - return arnForDeploymentConfigName(this.deploymentConfigName, scope); + this.deploymentConfigArn = arnForDeploymentConfig(this.deploymentConfigName); } public export(): ServerDeploymentConfigImportProps { @@ -160,3 +125,11 @@ export class ServerDeploymentConfig extends cdk.Construct implements IServerDepl }; } } + +function deploymentConfig(name: string): IServerDeploymentConfig { + return { + deploymentConfigName: name, + deploymentConfigArn: arnForDeploymentConfig(name), + export() { return { deploymentConfigName: name }; } + }; +} diff --git a/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts b/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts index 0b0200220aba2..c7046c89f6c7d 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts @@ -7,7 +7,7 @@ import s3 = require('@aws-cdk/aws-s3'); import cdk = require('@aws-cdk/cdk'); import { CfnDeploymentGroup } from '../codedeploy.generated'; import { AutoRollbackConfig } from '../rollback-config'; -import { deploymentGroupNameToArn, renderAlarmConfiguration, renderAutoRollbackConfiguration } from '../utils'; +import { arnForDeploymentGroup, renderAlarmConfiguration, renderAutoRollbackConfiguration } from '../utils'; import { IServerApplication, ServerApplication } from './application'; import { IServerDeploymentConfig, ServerDeploymentConfig } from './deployment-config'; @@ -86,8 +86,7 @@ class ImportedServerDeploymentGroup extends ServerDeploymentGroupBase { this.application = props.application; this.deploymentGroupName = props.deploymentGroupName; - this.deploymentGroupArn = deploymentGroupNameToArn(props.application.applicationName, - props.deploymentGroupName, this); + this.deploymentGroupArn = arnForDeploymentGroup(props.application.applicationName, props.deploymentGroupName); } public export() { @@ -297,8 +296,7 @@ export class ServerDeploymentGroup extends ServerDeploymentGroupBase { }); this.deploymentGroupName = resource.deploymentGroupName; - this.deploymentGroupArn = deploymentGroupNameToArn(this.application.applicationName, - this.deploymentGroupName, this); + this.deploymentGroupArn = arnForDeploymentGroup(this.application.applicationName, this.deploymentGroupName); } public export(): ServerDeploymentGroupImportProps { diff --git a/packages/@aws-cdk/aws-codedeploy/lib/utils.ts b/packages/@aws-cdk/aws-codedeploy/lib/utils.ts index 12fc92475c295..9eca2c51327ff 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/utils.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/utils.ts @@ -1,33 +1,18 @@ import cloudwatch = require('@aws-cdk/aws-cloudwatch'); -import cdk = require('@aws-cdk/cdk'); +import { Aws } from '@aws-cdk/cdk'; import { CfnDeploymentGroup } from './codedeploy.generated'; import { AutoRollbackConfig } from './rollback-config'; -export function applicationNameToArn(applicationName: string, scope: cdk.IConstruct): string { - return scope.node.stack.formatArn({ - service: 'codedeploy', - resource: 'application', - resourceName: applicationName, - sep: ':', - }); +export function arnForApplication(applicationName: string): string { + return `arn:${Aws.partition}:codedeploy:${Aws.region}:${Aws.accountId}:application:${applicationName}`; } -export function deploymentGroupNameToArn(applicationName: string, deploymentGroupName: string, scope: cdk.IConstruct): string { - return scope.node.stack.formatArn({ - service: 'codedeploy', - resource: 'deploymentgroup', - resourceName: `${applicationName}/${deploymentGroupName}`, - sep: ':', - }); +export function arnForDeploymentGroup(applicationName: string, deploymentGroupName: string): string { + return `arn:${Aws.partition}:codedeploy:${Aws.region}:${Aws.accountId}:deploymentgroup:${applicationName}/${deploymentGroupName}`; } -export function arnForDeploymentConfigName(name: string, scope: cdk.IConstruct): string { - return scope.node.stack.formatArn({ - service: 'codedeploy', - resource: 'deploymentconfig', - resourceName: name, - sep: ':', - }); +export function arnForDeploymentConfig(name: string): string { + return `arn:${Aws.partition}:codedeploy:${Aws.region}:${Aws.accountId}:deploymentconfig:${name}`; } export function renderAlarmConfiguration(alarms: cloudwatch.Alarm[], ignorePollAlarmFailure?: boolean): diff --git a/packages/@aws-cdk/aws-codedeploy/package.json b/packages/@aws-cdk/aws-codedeploy/package.json index 5d9f7cb3a3862..80ba385b48c00 100644 --- a/packages/@aws-cdk/aws-codedeploy/package.json +++ b/packages/@aws-cdk/aws-codedeploy/package.json @@ -95,7 +95,7 @@ }, "awslint": { "exclude": [ - "construct-ctor:@aws-cdk/aws-codedeploy.ServerDeploymentGroupBase..params[2]" + "construct-interface-extends-iconstruct:@aws-cdk/aws-codedeploy.IServerDeploymentConfig" ] } } diff --git a/packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/server-deploy-action.ts b/packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/server-deploy-action.ts index f576c40ed359e..2c36e4ba32735 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/server-deploy-action.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/server-deploy-action.ts @@ -54,7 +54,7 @@ export class CodeDeployServerDeployAction extends codepipeline.DeployAction { )); info.role.addToPolicy(new iam.PolicyStatement() - .addResource(this.deploymentGroup.deploymentConfig.deploymentConfigArn(info.scope)) + .addResource(this.deploymentGroup.deploymentConfig.deploymentConfigArn) .addActions( 'codedeploy:GetDeploymentConfig', )); diff --git a/packages/@aws-cdk/aws-ec2/lib/index.ts b/packages/@aws-cdk/aws-ec2/lib/index.ts index 32946b75d470c..d086304362bec 100644 --- a/packages/@aws-cdk/aws-ec2/lib/index.ts +++ b/packages/@aws-cdk/aws-ec2/lib/index.ts @@ -4,7 +4,6 @@ export * from './machine-image'; export * from './security-group'; export * from './security-group-rule'; export * from './vpc'; -export * from './vpc-ref'; export * from './vpc-network-provider'; export * from './vpn'; export * from './vpc-endpoint'; diff --git a/packages/@aws-cdk/aws-ec2/lib/security-group.ts b/packages/@aws-cdk/aws-ec2/lib/security-group.ts index 37df9218c1252..56b962fa0a48d 100644 --- a/packages/@aws-cdk/aws-ec2/lib/security-group.ts +++ b/packages/@aws-cdk/aws-ec2/lib/security-group.ts @@ -12,6 +12,11 @@ export interface ISecurityGroup extends IResource, ISecurityGroupRule, IConnecta */ readonly securityGroupId: string; + /** + * The ID of the VPC this security group is part of. + */ + readonly securityGroupVpcId: string; + /** * Add an ingress rule for the current security group * @@ -45,12 +50,18 @@ export interface SecurityGroupImportProps { * ID of security group */ readonly securityGroupId: string; + + /** + * The VPC ID this security group is part of. If not provided, the `securityGroupVpcId` property + * will throw an exception. + */ + readonly securityGroupVpcId?: string; } /** * A SecurityGroup that is not created in this template */ -export abstract class SecurityGroupBase extends Resource implements ISecurityGroup { +abstract class SecurityGroupBase extends Resource implements ISecurityGroup { /** * Return whether the indicated object is a security group */ @@ -59,6 +70,8 @@ export abstract class SecurityGroupBase extends Resource implements ISecurityGro } public abstract readonly securityGroupId: string; + public abstract readonly securityGroupVpcId: string; + public readonly canInlineRule = false; public readonly connections: Connections = new Connections({ securityGroups: [this] }); @@ -245,7 +258,20 @@ export class SecurityGroup extends SecurityGroupBase { * Import an existing SecurityGroup */ public static import(scope: Construct, id: string, props: SecurityGroupImportProps): ISecurityGroup { - return new ImportedSecurityGroup(scope, id, props); + class Import extends SecurityGroupBase { + public readonly securityGroupId = props.securityGroupId; + + public get securityGroupVpcId() { + if (!props.securityGroupVpcId) { throw new Error(`Imported security group did not specify 'securityGroupVpcId'`); } + return props.securityGroupVpcId; + } + + public export() { + return props; + } + } + + return new Import(scope, id); } /** @@ -263,6 +289,11 @@ export class SecurityGroup extends SecurityGroupBase { */ public readonly securityGroupId: string; + /** + * The VPC ID this security group is part of. + */ + public readonly securityGroupVpcId: string; + private readonly securityGroup: CfnSecurityGroup; private readonly directIngressRules: CfnSecurityGroup.IngressProperty[] = []; private readonly directEgressRules: CfnSecurityGroup.EgressProperty[] = []; @@ -285,6 +316,7 @@ export class SecurityGroup extends SecurityGroupBase { }); this.securityGroupId = this.securityGroup.securityGroupId; + this.securityGroupVpcId = this.securityGroup.securityGroupVpcId; this.groupName = this.securityGroup.securityGroupName; this.vpcId = this.securityGroup.securityGroupVpcId; @@ -489,23 +521,6 @@ export interface ConnectionRule { readonly description?: string; } -/** - * A SecurityGroup that hasn't been created here - */ -class ImportedSecurityGroup extends SecurityGroupBase { - public readonly securityGroupId: string; - - constructor(scope: Construct, id: string, private readonly props: SecurityGroupImportProps) { - super(scope, id); - - this.securityGroupId = props.securityGroupId; - } - - public export() { - return this.props; - } -} - /** * Compare two ingress rules for equality the same way CloudFormation would (discarding description) */ diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index 9a1ab1ff269ef..85fb2955ecc5b 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -1,13 +1,424 @@ import cdk = require('@aws-cdk/cdk'); -import { ConcreteDependable, IDependable } from '@aws-cdk/cdk'; +import { ConcreteDependable, Construct, IConstruct, IDependable } from '@aws-cdk/cdk'; import { CfnEIP, CfnInternetGateway, CfnNatGateway, CfnRoute, CfnVPNGateway, CfnVPNGatewayRoutePropagation } from './ec2.generated'; import { CfnRouteTable, CfnSubnet, CfnSubnetRouteTableAssociation, CfnVPC, CfnVPCGatewayAttachment } from './ec2.generated'; import { NetworkBuilder } from './network-util'; -import { DEFAULT_SUBNET_NAME, ExportSubnetGroup, ImportSubnetGroup, subnetId } from './util'; +import { DEFAULT_SUBNET_NAME, ExportSubnetGroup, ImportSubnetGroup, subnetId, subnetName } from './util'; import { GatewayVpcEndpoint, GatewayVpcEndpointAwsService, GatewayVpcEndpointOptions } from './vpc-endpoint'; +import { InterfaceVpcEndpoint, InterfaceVpcEndpointOptions } from './vpc-endpoint'; import { VpcNetworkProvider, VpcNetworkProviderProps } from './vpc-network-provider'; -import { IVpcNetwork, IVpcSubnet, SubnetSelection, SubnetType, VpcNetworkBase, VpcNetworkImportProps, VpcSubnetImportProps } from './vpc-ref'; -import { VpnConnectionOptions, VpnConnectionType } from './vpn'; +import { VpnConnection, VpnConnectionOptions, VpnConnectionType } from './vpn'; + +export interface IVpcSubnet extends IConstruct { + /** + * The Availability Zone the subnet is located in + */ + readonly availabilityZone: string; + + /** + * The subnetId for this particular subnet + */ + readonly subnetId: string; + + /** + * Dependable that can be depended upon to force internet connectivity established on the VPC + */ + readonly internetConnectivityEstablished: IDependable; + + /** + * Route table ID + */ + readonly routeTableId?: string; + + /** + * Exports this subnet to another stack. + */ + export(): VpcSubnetImportProps; +} + +export interface IVpcNetwork extends IConstruct { + /** + * Identifier for this VPC + */ + readonly vpcId: string; + + /** + * List of public subnets in this VPC + */ + readonly publicSubnets: IVpcSubnet[]; + + /** + * List of private subnets in this VPC + */ + readonly privateSubnets: IVpcSubnet[]; + + /** + * List of isolated subnets in this VPC + */ + readonly isolatedSubnets: IVpcSubnet[]; + + /** + * AZs for this VPC + */ + readonly availabilityZones: string[]; + + /** + * Region where this VPC is located + */ + readonly vpcRegion: string; + + /** + * Identifier for the VPN gateway + */ + readonly vpnGatewayId?: string; + + /** + * Return IDs of the subnets appropriate for the given selection strategy + * + * Requires that at least one subnet is matched, throws a descriptive + * error message otherwise. + * + * @deprecated Use selectSubnets() instead. + */ + selectSubnetIds(selection?: SubnetSelection): string[]; + + /** + * Return information on the subnets appropriate for the given selection strategy + * + * Requires that at least one subnet is matched, throws a descriptive + * error message otherwise. + */ + selectSubnets(selection?: SubnetSelection): SelectedSubnets; + + /** + * Return whether all of the given subnets are from the VPC's public subnets. + */ + isPublicSubnets(subnetIds: string[]): boolean; + + /** + * Adds a new VPN connection to this VPC + */ + addVpnConnection(id: string, options: VpnConnectionOptions): VpnConnection; + + /** + * Adds a new interface endpoint to this VPC + */ + addInterfaceEndpoint(id: string, options: InterfaceVpcEndpointOptions): InterfaceVpcEndpoint + + /** + * Exports this VPC so it can be consumed by another stack. + */ + export(): VpcNetworkImportProps; +} + +/** + * The type of Subnet + */ +export enum SubnetType { + /** + * Isolated Subnets do not route Outbound traffic + * + * This can be good for subnets with RDS or + * Elasticache endpoints + */ + Isolated = 1, + + /** + * Subnet that routes to the internet, but not vice versa. + * + * Instances in a private subnet can connect to the Internet, but will not + * allow connections to be initiated from the Internet. + * + * Outbound traffic will be routed via a NAT Gateway. Preference being in + * the same AZ, but if not available will use another AZ (control by + * specifing `maxGateways` on VpcNetwork). This might be used for + * experimental cost conscious accounts or accounts where HA outbound + * traffic is not needed. + */ + Private = 2, + + /** + * Subnet connected to the Internet + * + * Instances in a Public subnet can connect to the Internet and can be + * connected to from the Internet as long as they are launched with public + * IPs (controlled on the AutoScalingGroup or other constructs that launch + * instances). + * + * Public subnets route outbound traffic via an Internet Gateway. + */ + Public = 3 +} + +/** + * Customize subnets that are selected for placement of ENIs + * + * Constructs that allow customization of VPC placement use parameters of this + * type to provide placement settings. + * + * By default, the instances are placed in the private subnets. + */ +export interface SubnetSelection { + /** + * Place the instances in the subnets of the given type + * + * At most one of `subnetType` and `subnetName` can be supplied. + * + * @default SubnetType.Private + */ + readonly subnetType?: SubnetType; + + /** + * Place the instances in the subnets with the given name + * + * (This is the name supplied in subnetConfiguration). + * + * At most one of `subnetType` and `subnetName` can be supplied. + * + * @default name + */ + readonly subnetName?: string; + + /** + * If true, return at most one subnet per AZ + * + * @defautl false + */ + readonly onePerAz?: boolean; +} + +/** + * Result of selecting a subset of subnets from a VPC + */ +export interface SelectedSubnets { + /** + * The subnet IDs + */ + readonly subnetIds: string[]; + + /** + * The respective AZs of each subnet + */ + readonly availabilityZones: string[]; + + /** + * Route table IDs of each respective subnet + */ + readonly routeTableIds: string[]; + + /** + * Dependency representing internet connectivity for these subnets + */ + readonly internetConnectedDependency: IDependable; +} + +/** + * A new or imported VPC + */ +abstract class VpcNetworkBase extends Construct implements IVpcNetwork { + + /** + * Identifier for this VPC + */ + public abstract readonly vpcId: string; + + /** + * List of public subnets in this VPC + */ + public abstract readonly publicSubnets: IVpcSubnet[]; + + /** + * List of private subnets in this VPC + */ + public abstract readonly privateSubnets: IVpcSubnet[]; + + /** + * List of isolated subnets in this VPC + */ + public abstract readonly isolatedSubnets: IVpcSubnet[]; + + /** + * AZs for this VPC + */ + public abstract readonly availabilityZones: string[]; + + /** + * Identifier for the VPN gateway + */ + public abstract readonly vpnGatewayId?: string; + + /** + * Dependencies for internet connectivity + */ + public readonly internetDependencies = new Array(); + + /** + * Dependencies for NAT connectivity + */ + public readonly natDependencies = new Array(); + + public selectSubnetIds(selection?: SubnetSelection): string[] { + return this.selectSubnets(selection).subnetIds; + } + + /** + * Returns IDs of selected subnets + */ + public selectSubnets(selection: SubnetSelection = {}): SelectedSubnets { + const subnets = this.selectSubnetObjects(selection); + + return { + subnetIds: subnets.map(s => s.subnetId), + availabilityZones: subnets.map(s => s.availabilityZone), + routeTableIds: subnets.map(s => s.routeTableId).filter(notUndefined), // Possibly don't have this information + internetConnectedDependency: tap(new CompositeDependable(), d => subnets.forEach(s => d.add(s.internetConnectivityEstablished))), + }; + } + + /** + * Adds a new VPN connection to this VPC + */ + public addVpnConnection(id: string, options: VpnConnectionOptions): VpnConnection { + return new VpnConnection(this, id, { + vpc: this, + ...options + }); + } + + /** + * Adds a new interface endpoint to this VPC + */ + public addInterfaceEndpoint(id: string, options: InterfaceVpcEndpointOptions): InterfaceVpcEndpoint { + return new InterfaceVpcEndpoint(this, id, { + vpc: this, + ...options + }); + } + + /** + * Export this VPC from the stack + */ + public abstract export(): VpcNetworkImportProps; + + /** + * Return whether all of the given subnets are from the VPC's public subnets. + */ + public isPublicSubnets(subnetIds: string[]): boolean { + const pubIds = new Set(this.publicSubnets.map(n => n.subnetId)); + return subnetIds.every(pubIds.has.bind(pubIds)); + } + + /** + * The region where this VPC is defined + */ + public get vpcRegion(): string { + return this.node.stack.region; + } + + /** + * Return the subnets appropriate for the placement strategy + */ + protected selectSubnetObjects(selection: SubnetSelection = {}): IVpcSubnet[] { + selection = reifySelectionDefaults(selection); + let subnets: IVpcSubnet[] = []; + + if (selection.subnetName !== undefined) { // Select by name + const allSubnets = [...this.publicSubnets, ...this.privateSubnets, ...this.isolatedSubnets]; + subnets = allSubnets.filter(s => subnetName(s) === selection.subnetName); + } else { // Select by type + subnets = { + [SubnetType.Isolated]: this.isolatedSubnets, + [SubnetType.Private]: this.privateSubnets, + [SubnetType.Public]: this.publicSubnets, + }[selection.subnetType || SubnetType.Private]; + + if (selection.onePerAz && subnets.length > 0) { + // Restrict to at most one subnet group + subnets = subnets.filter(s => subnetName(s) === subnetName(subnets[0])); + } + } + + if (subnets.length === 0) { + throw new Error(`There are no ${describeSelection(selection)} in this VPC. Use a different VPC subnet selection.`); + } + + return subnets; + } +} + +/** + * Properties that reference an external VpcNetwork + */ +export interface VpcNetworkImportProps { + /** + * VPC's identifier + */ + readonly vpcId: string; + + /** + * List of availability zones for the subnets in this VPC. + */ + readonly availabilityZones: string[]; + + /** + * List of public subnet IDs + * + * Must be undefined or match the availability zones in length and order. + */ + readonly publicSubnetIds?: string[]; + + /** + * List of names for the public subnets + * + * Must be undefined or have a name for every public subnet group. + */ + readonly publicSubnetNames?: string[]; + + /** + * List of private subnet IDs + * + * Must be undefined or match the availability zones in length and order. + */ + readonly privateSubnetIds?: string[]; + + /** + * List of names for the private subnets + * + * Must be undefined or have a name for every private subnet group. + */ + readonly privateSubnetNames?: string[]; + + /** + * List of isolated subnet IDs + * + * Must be undefined or match the availability zones in length and order. + */ + readonly isolatedSubnetIds?: string[]; + + /** + * List of names for the isolated subnets + * + * Must be undefined or have a name for every isolated subnet group. + */ + readonly isolatedSubnetNames?: string[]; + + /** + * VPN gateway's identifier + */ + readonly vpnGatewayId?: string; +} + +export interface VpcSubnetImportProps { + /** + * The Availability Zone the subnet is located in + */ + readonly availabilityZone: string; + + /** + * The subnetId for this particular subnet + */ + readonly subnetId: string; +} /** * Name tag constant @@ -846,3 +1257,68 @@ class ImportedVpcSubnet extends cdk.Construct implements IVpcSubnet { return this.props; } } + +/** + * If the placement strategy is completely "default", reify the defaults so + * consuming code doesn't have to reimplement the same analysis every time. + * + * Returns "private subnets" by default. + */ +function reifySelectionDefaults(placement: SubnetSelection): SubnetSelection { + if (placement.subnetType !== undefined && placement.subnetName !== undefined) { + throw new Error('Only one of subnetType and subnetName can be supplied'); + } + + if (placement.subnetType === undefined && placement.subnetName === undefined) { + return { subnetType: SubnetType.Private, onePerAz: placement.onePerAz }; + } + + return placement; +} + +/** + * Describe the given placement strategy + */ +function describeSelection(placement: SubnetSelection): string { + if (placement.subnetType !== undefined) { + return `'${DEFAULT_SUBNET_NAME[placement.subnetType]}' subnets`; + } + if (placement.subnetName !== undefined) { + return `subnets named '${placement.subnetName}'`; + } + return JSON.stringify(placement); +} + +class CompositeDependable implements IDependable { + private readonly dependables = new Array(); + + /** + * Add a construct to the dependency roots + */ + public add(dep: IDependable) { + this.dependables.push(dep); + } + + /** + * Retrieve the current set of dependency roots + */ + public get dependencyRoots(): IConstruct[] { + const ret = []; + for (const dep of this.dependables) { + ret.push(...dep.dependencyRoots); + } + return ret; + } +} + +/** + * Invoke a function on a value (for its side effect) and return the value + */ +function tap(x: T, fn: (x: T) => void): T { + fn(x); + return x; +} + +function notUndefined(x: T | undefined): x is T { + return x !== undefined; +} diff --git a/packages/@aws-cdk/aws-kinesis/lib/stream.ts b/packages/@aws-cdk/aws-kinesis/lib/stream.ts index db4254ea545c6..737b7051ab451 100644 --- a/packages/@aws-cdk/aws-kinesis/lib/stream.ts +++ b/packages/@aws-cdk/aws-kinesis/lib/stream.ts @@ -87,7 +87,7 @@ export interface StreamImportProps { * Stream.import(this, 'MyImportedStream', ref); * */ -export abstract class StreamBase extends Resource implements IStream { +abstract class StreamBase extends Resource implements IStream { /** * The ARN of the stream. */ diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index cf4c7584c061c..7f7d4c859b620 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -58,7 +58,7 @@ export interface EncryptionKeyImportProps { readonly keyArn: string; } -export abstract class EncryptionKeyBase extends Construct implements IEncryptionKey { +abstract class EncryptionKeyBase extends Construct implements IEncryptionKey { /** * The ARN of the key. */ diff --git a/packages/@aws-cdk/aws-logs/lib/log-group.ts b/packages/@aws-cdk/aws-logs/lib/log-group.ts index 04dc73a1e7260..9637167da345a 100644 --- a/packages/@aws-cdk/aws-logs/lib/log-group.ts +++ b/packages/@aws-cdk/aws-logs/lib/log-group.ts @@ -87,7 +87,7 @@ export interface LogGroupImportProps { /** * An CloudWatch Log Group */ -export abstract class LogGroupBase extends Resource implements ILogGroup { +abstract class LogGroupBase extends Resource implements ILogGroup { /** * The ARN of this log group */ diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index a671d0ee43a26..6eb308b6f0037 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -23,9 +23,19 @@ export interface IBucket extends IResource { readonly bucketName: string; /** - * The domain of the bucket. + * The IPv4 DNS name of the specified bucket. */ - readonly domainName: string; + readonly bucketDomainName: string; + + /** + * The IPv6 DNS name of the specified bucket. + */ + readonly bucketDualStackDomainName: string; + + /** + * The regional domain name of the specified bucket. + */ + readonly bucketRegionalDomainName: string; /** * Optional KMS encryption key associated with this bucket. @@ -206,6 +216,16 @@ export interface BucketImportProps { */ readonly bucketWebsiteUrl?: string; + /** + * The regional domain name of the specified bucket. + */ + readonly bucketRegionalDomainName?: string; + + /** + * The IPv6 DNS name of the specified bucket. + */ + readonly bucketDualStackDomainName?: string; + /** * The format of the website URL of the bucket. This should be true for * regions launched since 2014. @@ -232,21 +252,12 @@ export interface BucketImportProps { * Bucket.import(this, 'MyImportedBucket', ref); * */ -export abstract class BucketBase extends Resource implements IBucket { - /** - * The ARN of the bucket. - */ +abstract class BucketBase extends Resource implements IBucket { public abstract readonly bucketArn: string; - - /** - * The name of the bucket. - */ public abstract readonly bucketName: string; - - /** - * The domain of the bucket. - */ - public abstract readonly domainName: string; + public abstract readonly bucketDomainName: string; + public abstract readonly bucketRegionalDomainName: string; + public abstract readonly bucketDualStackDomainName: string; /** * Optional KMS encryption key associated with this bucket. @@ -650,14 +661,53 @@ export class Bucket extends BucketBase { * `bucket.export()` or manually created. */ public static import(scope: Construct, id: string, props: BucketImportProps): IBucket { - return new ImportedBucket(scope, id, props); + const region = scope.node.stack.region; + const urlSuffix = scope.node.stack.urlSuffix; + + const bucketName = parseBucketName(scope, props); + if (!bucketName) { + throw new Error('Bucket name is required'); + } + + const newUrlFormat = props.bucketWebsiteNewUrlFormat === undefined + ? false + : props.bucketWebsiteNewUrlFormat; + + const websiteUrl = newUrlFormat + ? `${bucketName}.s3-website.${region}.${urlSuffix}` + : `${bucketName}.s3-website-${region}.${urlSuffix}`; + + class Import extends BucketBase { + public readonly bucketName = bucketName!; + public readonly bucketArn = parseBucketArn(scope, props); + public readonly bucketDomainName = props.bucketDomainName || `${bucketName}.s3.${urlSuffix}`; + public readonly bucketWebsiteUrl = props.bucketWebsiteUrl || websiteUrl; + public readonly bucketRegionalDomainName = props.bucketRegionalDomainName || `${bucketName}.s3.${region}.${urlSuffix}`; + public readonly bucketDualStackDomainName = props.bucketDualStackDomainName || `${bucketName}.s3.dualstack.${region}.${urlSuffix}`; + public readonly bucketWebsiteNewUrlFormat = newUrlFormat; + public readonly encryptionKey?: kms.EncryptionKey; + public policy?: BucketPolicy = undefined; + protected autoCreatePolicy = false; + protected disallowPublicAccess = false; + + /** + * Exports this bucket from the stack. + */ + public export() { + return props; + } + } + + return new Import(scope, id); } public readonly bucketArn: string; public readonly bucketName: string; - public readonly domainName: string; + public readonly bucketDomainName: string; public readonly bucketWebsiteUrl: string; - public readonly dualstackDomainName: string; + public readonly bucketDualStackDomainName: string; + public readonly bucketRegionalDomainName: string; + public readonly encryptionKey?: kms.IEncryptionKey; public policy?: BucketPolicy; protected autoCreatePolicy = true; @@ -687,11 +737,14 @@ export class Bucket extends BucketBase { this.versioned = props.versioned; this.encryptionKey = encryptionKey; + this.bucketArn = resource.bucketArn; this.bucketName = resource.bucketName; - this.domainName = resource.bucketDomainName; + this.bucketDomainName = resource.bucketDomainName; this.bucketWebsiteUrl = resource.bucketWebsiteUrl; - this.dualstackDomainName = resource.bucketDualStackDomainName; + this.bucketDualStackDomainName = resource.bucketDualStackDomainName; + this.bucketRegionalDomainName = resource.bucketRegionalDomainName; + this.disallowPublicAccess = props.blockPublicAccess && props.blockPublicAccess.blockPublicPolicy; // Add all lifecycle rules @@ -713,7 +766,7 @@ export class Bucket extends BucketBase { return { bucketArn: new CfnOutput(this, 'BucketArn', { value: this.bucketArn }).makeImportValue().toString(), bucketName: new CfnOutput(this, 'BucketName', { value: this.bucketName }).makeImportValue().toString(), - bucketDomainName: new CfnOutput(this, 'DomainName', { value: this.domainName }).makeImportValue().toString(), + bucketDomainName: new CfnOutput(this, 'DomainName', { value: this.bucketDomainName }).makeImportValue().toString(), bucketWebsiteUrl: new CfnOutput(this, 'WebsiteURL', { value: this.bucketWebsiteUrl }).makeImportValue().toString() }; } @@ -1079,54 +1132,3 @@ export interface NotificationKeyFilter { */ readonly suffix?: string; } - -class ImportedBucket extends BucketBase { - public readonly bucketArn: string; - public readonly bucketName: string; - public readonly domainName: string; - public readonly bucketWebsiteUrl: string; - public readonly bucketWebsiteNewUrlFormat: boolean; - public readonly encryptionKey?: kms.EncryptionKey; - - public policy?: BucketPolicy; - protected autoCreatePolicy: boolean; - - protected disallowPublicAccess?: boolean; - - constructor(scope: Construct, id: string, private readonly props: BucketImportProps) { - super(scope, id); - - const bucketName = parseBucketName(this, props); - if (!bucketName) { - throw new Error('Bucket name is required'); - } - - this.bucketArn = parseBucketArn(this, props); - this.bucketName = bucketName; - this.domainName = props.bucketDomainName || this.generateDomainName(); - this.bucketWebsiteUrl = props.bucketWebsiteUrl || this.generateBucketWebsiteUrl(); - this.autoCreatePolicy = false; - this.bucketWebsiteNewUrlFormat = props.bucketWebsiteNewUrlFormat === undefined - ? false - : props.bucketWebsiteNewUrlFormat; - this.policy = undefined; - this.disallowPublicAccess = false; - } - - /** - * Exports this bucket from the stack. - */ - public export() { - return this.props; - } - - private generateDomainName() { - return `${this.bucketName}.s3.amazonaws.com`; - } - - private generateBucketWebsiteUrl() { - return this.bucketWebsiteNewUrlFormat - ? `${this.bucketName}.s3-website.${this.node.stack.region}.${this.node.stack.urlSuffix}` - : `${this.bucketName}.s3-website-${this.node.stack.region}.${this.node.stack.urlSuffix}`; - } -} diff --git a/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.ts b/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.ts index ac0245e570543..f4c53d16039f4 100644 --- a/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.ts +++ b/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.ts @@ -13,8 +13,8 @@ class TestStack extends cdk.Stack { bucketArn: "arn:aws:s3:::my-bucket-test" }); - new cdk.CfnOutput(this, 'RealBucketDomain', { value: bucket.domainName }); - new cdk.CfnOutput(this, 'ImportedBucketDomain', { value: bucket2.domainName }); + new cdk.CfnOutput(this, 'RealBucketDomain', { value: bucket.bucketDomainName }); + new cdk.CfnOutput(this, 'ImportedBucketDomain', { value: bucket2.bucketDomainName }); /// !hide } } diff --git a/packages/@aws-cdk/aws-ssm/lib/parameter.ts b/packages/@aws-cdk/aws-ssm/lib/parameter.ts index 0138585c0b62f..27708f0fc4c2b 100644 --- a/packages/@aws-cdk/aws-ssm/lib/parameter.ts +++ b/packages/@aws-cdk/aws-ssm/lib/parameter.ts @@ -107,7 +107,7 @@ export interface StringListParameterProps extends ParameterOptions { /** * Basic features shared across all types of SSM Parameters. */ -export abstract class ParameterBase extends Resource implements IParameter { +abstract class ParameterBase extends Resource implements IParameter { public abstract readonly parameterName: string; public abstract readonly parameterType: string; diff --git a/tools/awslint/lib/rules/construct.ts b/tools/awslint/lib/rules/construct.ts index 5920b7d1dcb4c..52833f80be424 100644 --- a/tools/awslint/lib/rules/construct.ts +++ b/tools/awslint/lib/rules/construct.ts @@ -183,13 +183,3 @@ constructLinter.add({ e.assert(e.ctx.interfaceType.extends(interfaceBase), e.ctx.interfaceType.fqn); } }); - -constructLinter.add({ - code: 'construct-base-is-private', - message: 'Construct base class (FooBase) should not be public', - eval: e => { - const baseTypeFqn = `${e.ctx.fqn}Base`; - const found = e.ctx.sys.tryFindFqn(baseTypeFqn); - e.assert(!found, baseTypeFqn); - } -}); From 4e625b4ae9048d106daedce8715192d6e4ac4f94 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 22 Apr 2019 23:24:20 +0300 Subject: [PATCH 3/6] update expectation --- .../integ.bucket.domain-name.expected.json | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.expected.json b/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.expected.json index 8879907be8cc9..06ba9c9e7466d 100644 --- a/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.expected.json +++ b/packages/@aws-cdk/aws-s3/test/integ.bucket.domain-name.expected.json @@ -7,12 +7,24 @@ "Outputs": { "RealBucketDomain": { "Value": { - "Fn::GetAtt":["MyBucketF68F3FF0","DomainName"] + "Fn::GetAtt": [ + "MyBucketF68F3FF0", + "DomainName" + ] } }, "ImportedBucketDomain": { - "Value": "my-bucket-test.s3.amazonaws.com" + "Value": { + "Fn::Join": [ + "", + [ + "my-bucket-test.s3.", + { + "Ref": "AWS::URLSuffix" + } + ] + ] + } } } -} - +} \ No newline at end of file From 3269e0c7e5864275c7ce73a9f75ea83df9e57590 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 23 Apr 2019 00:13:01 +0300 Subject: [PATCH 4/6] Hide more XxxBase classes add awslint:construct-base-is-private - eks.ClusterBase - codebuild.ProjectBase - codecommit.RepositoryBase - codedeploy.ServerDeploymentGroupBase - ecr.RepositoryBase - eks.ClusterBase - lambda.LayerVersionBase - rds.DatabaseClusterBase - secretsmanager.SecretBase - ses.ReceiptRuleSetBase - update "foreach.sh" to ignore undefined npm scripts - update contribution guide with instructions on how to use foreach.sh --- CONTRIBUTING.md | 1 + .../@aws-cdk/aws-codebuild/lib/project.ts | 2 +- .../@aws-cdk/aws-codecommit/lib/repository.ts | 2 +- .../lib/server/deployment-group.ts | 2 +- packages/@aws-cdk/aws-ecr/lib/index.ts | 1 - packages/@aws-cdk/aws-ecr/lib/repository.ts | 273 +++++++++++++++++- packages/@aws-cdk/aws-eks/lib/cluster-base.ts | 98 ------- packages/@aws-cdk/aws-eks/lib/cluster.ts | 99 ++++++- packages/@aws-cdk/aws-eks/lib/index.ts | 1 - packages/@aws-cdk/aws-lambda/lib/layers.ts | 2 +- packages/@aws-cdk/aws-lambda/package.json | 3 +- packages/@aws-cdk/aws-rds/lib/cluster.ts | 2 +- .../@aws-cdk/aws-secretsmanager/lib/secret.ts | 2 +- .../@aws-cdk/aws-ses/lib/receipt-rule-set.ts | 2 +- packages/@aws-cdk/aws-sns/package.json | 5 + packages/@aws-cdk/aws-sqs/package.json | 2 +- scripts/foreach.sh | 12 + tools/awslint/lib/rules/construct.ts | 10 + 18 files changed, 406 insertions(+), 113 deletions(-) delete mode 100644 packages/@aws-cdk/aws-eks/lib/cluster-base.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 333f6589b28f4..1f4d83aa53d7c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -177,6 +177,7 @@ Here are a few useful commands: * `npm run awslint` in every module will run __awslint__ for that module. * `npm run awslint list` prints all rules (details and rationale in the guidelines doc) + * `scripts/foreach.sh npm run awslint` will start linting the entire repo, progressively. Rerun `scripts/foreach.sh` after fixing to continue. * `lerna run awslint --no-bail --stream 2> awslint.txt` will run __awslint__ in all modules and collect all results into awslint.txt * `lerna run awslint -- -i ` will run awslint throughout the repo and evaluate only the rule specified [awslint README](./tools/awslint/README.md) diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts index e042b6f724636..950c43c06a232 100644 --- a/packages/@aws-cdk/aws-codebuild/lib/project.ts +++ b/packages/@aws-cdk/aws-codebuild/lib/project.ts @@ -155,7 +155,7 @@ export interface ProjectImportProps { * (or one defined in a different CDK Stack), * use the {@link import} method. */ -export abstract class ProjectBase extends Resource implements IProject { +abstract class ProjectBase extends Resource implements IProject { public abstract readonly grantPrincipal: iam.IPrincipal; /** The ARN of this Project. */ diff --git a/packages/@aws-cdk/aws-codecommit/lib/repository.ts b/packages/@aws-cdk/aws-codecommit/lib/repository.ts index acb2d6207ba48..d053931af119a 100644 --- a/packages/@aws-cdk/aws-codecommit/lib/repository.ts +++ b/packages/@aws-cdk/aws-codecommit/lib/repository.ts @@ -95,7 +95,7 @@ export interface RepositoryImportProps { * If you want to reference an already existing Repository, * use the {@link Repository.import} method. */ -export abstract class RepositoryBase extends Resource implements IRepository { +abstract class RepositoryBase extends Resource implements IRepository { /** The ARN of this Repository. */ public abstract readonly repositoryArn: string; diff --git a/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts b/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts index c7046c89f6c7d..20006fbe7d9da 100644 --- a/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts +++ b/packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts @@ -58,7 +58,7 @@ export interface ServerDeploymentGroupImportProps { * or one defined in a different CDK Stack, * use the {@link #import} method. */ -export abstract class ServerDeploymentGroupBase extends cdk.Construct implements IServerDeploymentGroup { +abstract class ServerDeploymentGroupBase extends cdk.Construct implements IServerDeploymentGroup { public abstract readonly application: IServerApplication; public abstract readonly role?: iam.Role; public abstract readonly deploymentGroupName: string; diff --git a/packages/@aws-cdk/aws-ecr/lib/index.ts b/packages/@aws-cdk/aws-ecr/lib/index.ts index 21b453e140916..bfeae449ded7c 100644 --- a/packages/@aws-cdk/aws-ecr/lib/index.ts +++ b/packages/@aws-cdk/aws-ecr/lib/index.ts @@ -2,5 +2,4 @@ export * from './ecr.generated'; export * from './repository'; -export * from './repository-ref'; export * from './lifecycle'; diff --git a/packages/@aws-cdk/aws-ecr/lib/repository.ts b/packages/@aws-cdk/aws-ecr/lib/repository.ts index 7e445baf1ff13..e53425e111b4c 100644 --- a/packages/@aws-cdk/aws-ecr/lib/repository.ts +++ b/packages/@aws-cdk/aws-ecr/lib/repository.ts @@ -1,8 +1,277 @@ +import events = require('@aws-cdk/aws-events'); import iam = require('@aws-cdk/aws-iam'); -import { CfnOutput, Construct, DeletionPolicy, Token } from '@aws-cdk/cdk'; +import { CfnOutput, Construct, DeletionPolicy, IConstruct, IResource, Resource, Token } from '@aws-cdk/cdk'; import { CfnRepository } from './ecr.generated'; import { CountType, LifecycleRule, TagStatus } from './lifecycle'; -import { RepositoryBase, RepositoryImportProps } from "./repository-ref"; + +/** + * Represents an ECR repository. + */ +export interface IRepository extends IResource { + /** + * The name of the repository + */ + readonly repositoryName: string; + + /** + * The ARN of the repository + */ + readonly repositoryArn: string; + + /** + * The URI of this repository (represents the latest image): + * + * ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY + * + */ + readonly repositoryUri: string; + + /** + * Returns the URI of the repository for a certain tag. Can be used in `docker push/pull`. + * + * ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY[:TAG] + * + * @param tag Image tag to use (tools usually default to "latest" if omitted) + */ + repositoryUriForTag(tag?: string): string; + + /** + * Add a policy statement to the repository's resource policy + */ + addToResourcePolicy(statement: iam.PolicyStatement): void; + + /** + * Grant the given principal identity permissions to perform the actions on this repository + */ + grant(grantee: iam.IGrantable, ...actions: string[]): iam.Grant; + + /** + * Grant the given identity permissions to pull images in this repository. + */ + grantPull(grantee: iam.IGrantable): iam.Grant; + + /** + * Grant the given identity permissions to pull and push images to this repository. + */ + grantPullPush(grantee: iam.IGrantable): iam.Grant; + + /** + * Defines an AWS CloudWatch event rule that can trigger a target when an image is pushed to this + * repository. + * @param name The name of the rule + * @param target An IEventRuleTarget to invoke when this event happens (you can add more targets using `addTarget`) + * @param imageTag Only trigger on the specific image tag + */ + onImagePushed(name: string, target?: events.IEventRuleTarget, imageTag?: string): events.EventRule; + + /** + * Export this repository from the stack + */ + export(): RepositoryImportProps; +} + +export interface RepositoryImportProps { + /** + * The ARN of the repository to import. + * + * At least one of `repositoryArn` or `repositoryName` is required. + * + * @default If you only have a repository name and the repository is in the same + * account/region as the current stack, you can set `repositoryName` instead + * and the ARN will be formatted with the current region and account. + */ + readonly repositoryArn?: string; + + /** + * The full name of the repository to import. + * + * This is only needed if the repository ARN is not a concrete string, in which + * case it is impossible to safely parse the ARN and extract full repository + * names from it if it includes multiple components (e.g. `foo/bar/myrepo`). + * + * If the repository is in the same region/account as the stack, it is sufficient + * to only specify the repository name. + */ + readonly repositoryName?: string; +} + +/** + * Base class for ECR repository. Reused between imported repositories and owned repositories. + */ +abstract class RepositoryBase extends Resource implements IRepository { + /** + * Import a repository + */ + public static import(scope: Construct, id: string, props: RepositoryImportProps): IRepository { + return new ImportedRepository(scope, id, props); + } + + /** + * Returns an ECR ARN for a repository that resides in the same account/region + * as the current stack. + */ + public static arnForLocalRepository(repositoryName: string, scope: IConstruct): string { + return scope.node.stack.formatArn({ + service: 'ecr', + resource: 'repository', + resourceName: repositoryName + }); + } + + /** + * The name of the repository + */ + public abstract readonly repositoryName: string; + + /** + * The ARN of the repository + */ + public abstract readonly repositoryArn: string; + + /** + * Add a policy statement to the repository's resource policy + */ + public abstract addToResourcePolicy(statement: iam.PolicyStatement): void; + + /** + * The URI of this repository (represents the latest image): + * + * ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY + * + */ + public get repositoryUri() { + return this.repositoryUriForTag(); + } + + /** + * Returns the URL of the repository. Can be used in `docker push/pull`. + * + * ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY[:TAG] + * + * @param tag Optional image tag + */ + public repositoryUriForTag(tag?: string): string { + const tagSuffix = tag ? `:${tag}` : ''; + const parts = this.node.stack.parseArn(this.repositoryArn); + return `${parts.account}.dkr.ecr.${parts.region}.amazonaws.com/${this.repositoryName}${tagSuffix}`; + } + + /** + * Export this repository from the stack + */ + public abstract export(): RepositoryImportProps; + + /** + * Defines an AWS CloudWatch event rule that can trigger a target when an image is pushed to this + * repository. + * @param name The name of the rule + * @param target An IEventRuleTarget to invoke when this event happens (you can add more targets using `addTarget`) + * @param imageTag Only trigger on the specific image tag + */ + public onImagePushed(name: string, target?: events.IEventRuleTarget, imageTag?: string): events.EventRule { + return new events.EventRule(this, name, { + targets: target ? [target] : undefined, + eventPattern: { + source: ['aws.ecr'], + detail: { + eventName: [ + 'PutImage', + ], + requestParameters: { + repositoryName: [ + this.repositoryName, + ], + imageTag: imageTag ? [imageTag] : undefined, + }, + }, + }, + }); + } + + /** + * Grant the given principal identity permissions to perform the actions on this repository + */ + public grant(grantee: iam.IGrantable, ...actions: string[]) { + return iam.Grant.addToPrincipalOrResource({ + grantee, + actions, + resourceArns: [this.repositoryArn], + resource: this, + }); + } + + /** + * Grant the given identity permissions to use the images in this repository + */ + public grantPull(grantee: iam.IGrantable) { + const ret = this.grant(grantee, "ecr:BatchCheckLayerAvailability", "ecr:GetDownloadUrlForLayer", "ecr:BatchGetImage"); + + iam.Grant.addToPrincipal({ + grantee, + actions: ["ecr:GetAuthorizationToken"], + resourceArns: ['*'], + scope: this, + }); + + return ret; + } + + /** + * Grant the given identity permissions to pull and push images to this repository. + */ + public grantPullPush(grantee: iam.IGrantable) { + this.grantPull(grantee); + return this.grant(grantee, + "ecr:PutImage", + "ecr:InitiateLayerUpload", + "ecr:UploadLayerPart", + "ecr:CompleteLayerUpload"); + } +} + +/** + * An already existing repository + */ +class ImportedRepository extends RepositoryBase { + public readonly repositoryName: string; + public readonly repositoryArn: string; + + constructor(scope: Construct, id: string, private readonly props: RepositoryImportProps) { + super(scope, id); + + if (props.repositoryArn) { + this.repositoryArn = props.repositoryArn; + } else { + if (!props.repositoryName) { + throw new Error('If "repositoryArn" is not specified, you must specify "repositoryName", ' + + 'which also implies that the repository resides in the same region/account as this stack'); + } + + this.repositoryArn = RepositoryBase.arnForLocalRepository(props.repositoryName, this); + } + + if (props.repositoryName) { + this.repositoryName = props.repositoryName; + } else { + // if repositoryArn is a token, the repository name is also required. this is because + // repository names can include "/" (e.g. foo/bar/myrepo) and it is impossible to + // parse the name from an ARN using CloudFormation's split/select. + if (Token.unresolved(this.repositoryArn)) { + throw new Error('repositoryArn is a late-bound value, and therefore repositoryName is required'); + } + + this.repositoryName = this.repositoryArn.split('/').slice(1).join('/'); + } + } + + public export(): RepositoryImportProps { + return this.props; + } + + public addToResourcePolicy(_statement: iam.PolicyStatement) { + // FIXME: Add annotation about policy we dropped on the floor + } +} export interface RepositoryProps { /** diff --git a/packages/@aws-cdk/aws-eks/lib/cluster-base.ts b/packages/@aws-cdk/aws-eks/lib/cluster-base.ts deleted file mode 100644 index b2196e142a50d..0000000000000 --- a/packages/@aws-cdk/aws-eks/lib/cluster-base.ts +++ /dev/null @@ -1,98 +0,0 @@ -import ec2 = require('@aws-cdk/aws-ec2'); -import { CfnOutput, IResource, Resource } from '@aws-cdk/cdk'; - -/** - * An EKS cluster - */ -export interface ICluster extends IResource, ec2.IConnectable { - /** - * The VPC in which this Cluster was created - */ - readonly vpc: ec2.IVpcNetwork; - - /** - * The physical name of the Cluster - */ - readonly clusterName: string; - - /** - * The unique ARN assigned to the service by AWS - * in the form of arn:aws:eks: - */ - readonly clusterArn: string; - - /** - * The API Server endpoint URL - */ - readonly clusterEndpoint: string; - - /** - * The certificate-authority-data for your cluster. - */ - readonly clusterCertificateAuthorityData: string; - - /** - * Export cluster references to use in other stacks - */ - export(): ClusterImportProps; -} - -/** - * A SecurityGroup Reference, object not created with this template. - */ -export abstract class ClusterBase extends Resource implements ICluster { - public abstract readonly connections: ec2.Connections; - public abstract readonly vpc: ec2.IVpcNetwork; - public abstract readonly clusterName: string; - public abstract readonly clusterArn: string; - public abstract readonly clusterEndpoint: string; - public abstract readonly clusterCertificateAuthorityData: string; - - /** - * Export cluster references to use in other stacks - */ - public export(): ClusterImportProps { - return { - vpc: this.vpc.export(), - clusterName: this.makeOutput('ClusterNameExport', this.clusterName), - clusterArn: this.makeOutput('ClusterArn', this.clusterArn), - clusterEndpoint: this.makeOutput('ClusterEndpoint', this.clusterEndpoint), - clusterCertificateAuthorityData: this.makeOutput('ClusterCAData', this.clusterCertificateAuthorityData), - securityGroups: this.connections.securityGroups.map(sg => sg.export()), - }; - } - - private makeOutput(name: string, value: any): string { - return new CfnOutput(this, name, { value }).makeImportValue().toString(); - } -} - -export interface ClusterImportProps { - /** - * The VPC in which this Cluster was created - */ - readonly vpc: ec2.VpcNetworkImportProps; - - /** - * The physical name of the Cluster - */ - readonly clusterName: string; - - /** - * The unique ARN assigned to the service by AWS - * in the form of arn:aws:eks: - */ - readonly clusterArn: string; - - /** - * The API Server endpoint URL - */ - readonly clusterEndpoint: string; - - /** - * The certificate-authority-data for your cluster. - */ - readonly clusterCertificateAuthorityData: string; - - readonly securityGroups: ec2.SecurityGroupImportProps[]; -} diff --git a/packages/@aws-cdk/aws-eks/lib/cluster.ts b/packages/@aws-cdk/aws-eks/lib/cluster.ts index 53978f525fb8b..6f428a94426f0 100644 --- a/packages/@aws-cdk/aws-eks/lib/cluster.ts +++ b/packages/@aws-cdk/aws-eks/lib/cluster.ts @@ -1,12 +1,107 @@ import autoscaling = require('@aws-cdk/aws-autoscaling'); import ec2 = require('@aws-cdk/aws-ec2'); import iam = require('@aws-cdk/aws-iam'); -import { CfnOutput, Construct, Tag } from '@aws-cdk/cdk'; +import { CfnOutput, Construct, IResource, Resource, Tag } from '@aws-cdk/cdk'; import { EksOptimizedAmi, nodeTypeForInstanceType } from './ami'; -import { ClusterBase, ClusterImportProps, ICluster } from './cluster-base'; import { CfnCluster } from './eks.generated'; import { maxPodsForInstanceType } from './instance-data'; +/** + * An EKS cluster + */ +export interface ICluster extends IResource, ec2.IConnectable { + /** + * The VPC in which this Cluster was created + */ + readonly vpc: ec2.IVpcNetwork; + + /** + * The physical name of the Cluster + */ + readonly clusterName: string; + + /** + * The unique ARN assigned to the service by AWS + * in the form of arn:aws:eks: + */ + readonly clusterArn: string; + + /** + * The API Server endpoint URL + */ + readonly clusterEndpoint: string; + + /** + * The certificate-authority-data for your cluster. + */ + readonly clusterCertificateAuthorityData: string; + + /** + * Export cluster references to use in other stacks + */ + export(): ClusterImportProps; +} + +/** + * A SecurityGroup Reference, object not created with this template. + */ +abstract class ClusterBase extends Resource implements ICluster { + public abstract readonly connections: ec2.Connections; + public abstract readonly vpc: ec2.IVpcNetwork; + public abstract readonly clusterName: string; + public abstract readonly clusterArn: string; + public abstract readonly clusterEndpoint: string; + public abstract readonly clusterCertificateAuthorityData: string; + + /** + * Export cluster references to use in other stacks + */ + public export(): ClusterImportProps { + return { + vpc: this.vpc.export(), + clusterName: this.makeOutput('ClusterNameExport', this.clusterName), + clusterArn: this.makeOutput('ClusterArn', this.clusterArn), + clusterEndpoint: this.makeOutput('ClusterEndpoint', this.clusterEndpoint), + clusterCertificateAuthorityData: this.makeOutput('ClusterCAData', this.clusterCertificateAuthorityData), + securityGroups: this.connections.securityGroups.map(sg => sg.export()), + }; + } + + private makeOutput(name: string, value: any): string { + return new CfnOutput(this, name, { value }).makeImportValue().toString(); + } +} + +export interface ClusterImportProps { + /** + * The VPC in which this Cluster was created + */ + readonly vpc: ec2.VpcNetworkImportProps; + + /** + * The physical name of the Cluster + */ + readonly clusterName: string; + + /** + * The unique ARN assigned to the service by AWS + * in the form of arn:aws:eks: + */ + readonly clusterArn: string; + + /** + * The API Server endpoint URL + */ + readonly clusterEndpoint: string; + + /** + * The certificate-authority-data for your cluster. + */ + readonly clusterCertificateAuthorityData: string; + + readonly securityGroups: ec2.SecurityGroupImportProps[]; +} + /** * Properties to instantiate the Cluster */ diff --git a/packages/@aws-cdk/aws-eks/lib/index.ts b/packages/@aws-cdk/aws-eks/lib/index.ts index 07f3f07aa8fff..d6e9e82ec6a90 100644 --- a/packages/@aws-cdk/aws-eks/lib/index.ts +++ b/packages/@aws-cdk/aws-eks/lib/index.ts @@ -1,4 +1,3 @@ -export * from "./cluster-base"; export * from "./cluster"; export * from "./ami"; diff --git a/packages/@aws-cdk/aws-lambda/lib/layers.ts b/packages/@aws-cdk/aws-lambda/lib/layers.ts index c506668adf572..3c694736a937d 100644 --- a/packages/@aws-cdk/aws-lambda/lib/layers.ts +++ b/packages/@aws-cdk/aws-lambda/lib/layers.ts @@ -67,7 +67,7 @@ export interface ILayerVersion extends IResource { /** * A reference to a Lambda Layer version. */ -export abstract class LayerVersionBase extends Resource implements ILayerVersion { +abstract class LayerVersionBase extends Resource implements ILayerVersion { public abstract readonly layerVersionArn: string; public abstract readonly compatibleRuntimes?: Runtime[]; diff --git a/packages/@aws-cdk/aws-lambda/package.json b/packages/@aws-cdk/aws-lambda/package.json index 368c2ba886655..4bbabc813343e 100644 --- a/packages/@aws-cdk/aws-lambda/package.json +++ b/packages/@aws-cdk/aws-lambda/package.json @@ -110,7 +110,8 @@ }, "awslint": { "exclude": [ - "grant-result:@aws-cdk/aws-lambda.LayerVersionBase.grantUsage" + "construct-base-is-private:@aws-cdk/aws-lambda.FunctionBase", + "grant-result:@aws-cdk/aws-lambda.LayerVersion.grantUsage" ] } } diff --git a/packages/@aws-cdk/aws-rds/lib/cluster.ts b/packages/@aws-cdk/aws-rds/lib/cluster.ts index 13b81137ad220..a52d9f79c5b91 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster.ts @@ -115,7 +115,7 @@ export interface DatabaseClusterProps { /** * A new or imported clustered database. */ -export abstract class DatabaseClusterBase extends cdk.Construct implements IDatabaseCluster { +abstract class DatabaseClusterBase extends cdk.Construct implements IDatabaseCluster { /** * Import an existing DatabaseCluster from properties */ diff --git a/packages/@aws-cdk/aws-secretsmanager/lib/secret.ts b/packages/@aws-cdk/aws-secretsmanager/lib/secret.ts index 23abd936350b2..de35adc0a6554 100644 --- a/packages/@aws-cdk/aws-secretsmanager/lib/secret.ts +++ b/packages/@aws-cdk/aws-secretsmanager/lib/secret.ts @@ -102,7 +102,7 @@ export interface SecretImportProps { /** * The common behavior of Secrets. Users should not use this class directly, and instead use ``Secret``. */ -export abstract class SecretBase extends Resource implements ISecret { +abstract class SecretBase extends Resource implements ISecret { public abstract readonly encryptionKey?: kms.IEncryptionKey; public abstract readonly secretArn: string; diff --git a/packages/@aws-cdk/aws-ses/lib/receipt-rule-set.ts b/packages/@aws-cdk/aws-ses/lib/receipt-rule-set.ts index d79df3d6837bf..2af3f9683a816 100644 --- a/packages/@aws-cdk/aws-ses/lib/receipt-rule-set.ts +++ b/packages/@aws-cdk/aws-ses/lib/receipt-rule-set.ts @@ -52,7 +52,7 @@ export interface ReceiptRuleSetProps { /** * A new or imported receipt rule set. */ -export abstract class ReceiptRuleSetBase extends Resource implements IReceiptRuleSet { +abstract class ReceiptRuleSetBase extends Resource implements IReceiptRuleSet { public abstract readonly name: string; private lastAddedRule?: ReceiptRule; diff --git a/packages/@aws-cdk/aws-sns/package.json b/packages/@aws-cdk/aws-sns/package.json index b6712107a2add..91c32d93496b8 100644 --- a/packages/@aws-cdk/aws-sns/package.json +++ b/packages/@aws-cdk/aws-sns/package.json @@ -92,5 +92,10 @@ }, "engines": { "node": ">= 8.10.0" + }, + "awslint": { + "exclude": [ + "construct-base-is-private:@aws-cdk/aws-sns.TopicBase" + ] } } diff --git a/packages/@aws-cdk/aws-sqs/package.json b/packages/@aws-cdk/aws-sqs/package.json index 473051cbbf302..4fa08bf11c1ac 100644 --- a/packages/@aws-cdk/aws-sqs/package.json +++ b/packages/@aws-cdk/aws-sqs/package.json @@ -89,7 +89,7 @@ }, "awslint": { "exclude": [ - "resource-attribute:@aws-cdk/aws-sqs.IQueue.queueName" + "construct-base-is-private:@aws-cdk/aws-sqs.QueueBase" ] } } diff --git a/scripts/foreach.sh b/scripts/foreach.sh index 29829efd58171..25155e3ea52ff 100755 --- a/scripts/foreach.sh +++ b/scripts/foreach.sh @@ -79,6 +79,18 @@ heading "${next}: ${command} (${remaining} remaining)" ( cd ${next} + + # special-case "npm run" - skip any modules that simply don't have + # that script (similar to how "lerna run" behaves) + if [[ "${command}" == "npm run"* ]]; then + script="$(echo ${command} | cut -d" " -f3)" + exists=$(node -pe "require('./package.json').scripts.${script} || ''") + if [ -z "${exists}" ]; then + echo "skipping (no "${script}" script in package.json)" + exit 0 + fi + fi + sh -c "${command}" &> /tmp/foreach.stdio || { cd ${base} cat /tmp/foreach.stdio | ${scriptdir}/path-prefix ${next} diff --git a/tools/awslint/lib/rules/construct.ts b/tools/awslint/lib/rules/construct.ts index 52833f80be424..e6d18df31c6c4 100644 --- a/tools/awslint/lib/rules/construct.ts +++ b/tools/awslint/lib/rules/construct.ts @@ -183,3 +183,13 @@ constructLinter.add({ e.assert(e.ctx.interfaceType.extends(interfaceBase), e.ctx.interfaceType.fqn); } }); + +constructLinter.add({ + code: 'construct-base-is-private', + message: 'prefer that the construct base class is private', + eval: e => { + if (!e.ctx.interfaceType) { return; } + const baseFqn = `${e.ctx.classType.fqn}Base`; + e.assert(!e.ctx.sys.tryFindFqn(baseFqn), baseFqn); + } +}); \ No newline at end of file From bce8a2d00451e2d7e4293782f8395df30f11f50c Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 1 May 2019 15:25:26 +0300 Subject: [PATCH 5/6] export RepositoryBase so that it can be used by AdoptedRepository --- packages/@aws-cdk/aws-ecr/lib/repository.ts | 2 +- packages/@aws-cdk/aws-ecr/package.json | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-ecr/lib/repository.ts b/packages/@aws-cdk/aws-ecr/lib/repository.ts index e53425e111b4c..6c6e59e386fdc 100644 --- a/packages/@aws-cdk/aws-ecr/lib/repository.ts +++ b/packages/@aws-cdk/aws-ecr/lib/repository.ts @@ -98,7 +98,7 @@ export interface RepositoryImportProps { /** * Base class for ECR repository. Reused between imported repositories and owned repositories. */ -abstract class RepositoryBase extends Resource implements IRepository { +export abstract class RepositoryBase extends Resource implements IRepository { /** * Import a repository */ diff --git a/packages/@aws-cdk/aws-ecr/package.json b/packages/@aws-cdk/aws-ecr/package.json index 43e3d79bda691..31f7905c59d4d 100644 --- a/packages/@aws-cdk/aws-ecr/package.json +++ b/packages/@aws-cdk/aws-ecr/package.json @@ -85,7 +85,8 @@ }, "awslint": { "exclude": [ - "import:@aws-cdk/aws-ecr.Repository" + "import:@aws-cdk/aws-ecr.Repository", + "construct-base-is-private:@aws-cdk/aws-ecr.RepositoryBase" ] } } From d5012b1f10dc1cd3c2d711af5e1499470e3eadc9 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Wed, 1 May 2019 15:45:54 +0300 Subject: [PATCH 6/6] remove repository-ref.ts --- .../@aws-cdk/aws-ecr/lib/repository-ref.ts | 272 ------------------ 1 file changed, 272 deletions(-) delete mode 100644 packages/@aws-cdk/aws-ecr/lib/repository-ref.ts diff --git a/packages/@aws-cdk/aws-ecr/lib/repository-ref.ts b/packages/@aws-cdk/aws-ecr/lib/repository-ref.ts deleted file mode 100644 index ab0f99e647983..0000000000000 --- a/packages/@aws-cdk/aws-ecr/lib/repository-ref.ts +++ /dev/null @@ -1,272 +0,0 @@ -import events = require('@aws-cdk/aws-events'); -import iam = require('@aws-cdk/aws-iam'); -import { Construct, IConstruct, IResource, Resource, Token } from '@aws-cdk/cdk'; - -/** - * Represents an ECR repository. - */ -export interface IRepository extends IResource { - /** - * The name of the repository - */ - readonly repositoryName: string; - - /** - * The ARN of the repository - */ - readonly repositoryArn: string; - - /** - * The URI of this repository (represents the latest image): - * - * ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY - * - */ - readonly repositoryUri: string; - - /** - * Returns the URI of the repository for a certain tag. Can be used in `docker push/pull`. - * - * ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY[:TAG] - * - * @param tag Image tag to use (tools usually default to "latest" if omitted) - */ - repositoryUriForTag(tag?: string): string; - - /** - * Add a policy statement to the repository's resource policy - */ - addToResourcePolicy(statement: iam.PolicyStatement): void; - - /** - * Grant the given principal identity permissions to perform the actions on this repository - */ - grant(grantee: iam.IGrantable, ...actions: string[]): iam.Grant; - - /** - * Grant the given identity permissions to pull images in this repository. - */ - grantPull(grantee: iam.IGrantable): iam.Grant; - - /** - * Grant the given identity permissions to pull and push images to this repository. - */ - grantPullPush(grantee: iam.IGrantable): iam.Grant; - - /** - * Defines an AWS CloudWatch event rule that can trigger a target when an image is pushed to this - * repository. - * @param name The name of the rule - * @param target An IEventRuleTarget to invoke when this event happens (you can add more targets using `addTarget`) - * @param imageTag Only trigger on the specific image tag - */ - onImagePushed(name: string, target?: events.IEventRuleTarget, imageTag?: string): events.EventRule; - - /** - * Export this repository from the stack - */ - export(): RepositoryImportProps; -} - -export interface RepositoryImportProps { - /** - * The ARN of the repository to import. - * - * At least one of `repositoryArn` or `repositoryName` is required. - * - * @default If you only have a repository name and the repository is in the same - * account/region as the current stack, you can set `repositoryName` instead - * and the ARN will be formatted with the current region and account. - */ - readonly repositoryArn?: string; - - /** - * The full name of the repository to import. - * - * This is only needed if the repository ARN is not a concrete string, in which - * case it is impossible to safely parse the ARN and extract full repository - * names from it if it includes multiple components (e.g. `foo/bar/myrepo`). - * - * If the repository is in the same region/account as the stack, it is sufficient - * to only specify the repository name. - */ - readonly repositoryName?: string; -} - -/** - * Base class for ECR repository. Reused between imported repositories and owned repositories. - */ -export abstract class RepositoryBase extends Resource implements IRepository { - /** - * Import a repository - */ - public static import(scope: Construct, id: string, props: RepositoryImportProps): IRepository { - return new ImportedRepository(scope, id, props); - } - - /** - * Returns an ECR ARN for a repository that resides in the same account/region - * as the current stack. - */ - public static arnForLocalRepository(repositoryName: string, scope: IConstruct): string { - return scope.node.stack.formatArn({ - service: 'ecr', - resource: 'repository', - resourceName: repositoryName - }); - } - - /** - * The name of the repository - */ - public abstract readonly repositoryName: string; - - /** - * The ARN of the repository - */ - public abstract readonly repositoryArn: string; - - /** - * Add a policy statement to the repository's resource policy - */ - public abstract addToResourcePolicy(statement: iam.PolicyStatement): void; - - /** - * The URI of this repository (represents the latest image): - * - * ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY - * - */ - public get repositoryUri() { - return this.repositoryUriForTag(); - } - - /** - * Returns the URL of the repository. Can be used in `docker push/pull`. - * - * ACCOUNT.dkr.ecr.REGION.amazonaws.com/REPOSITORY[:TAG] - * - * @param tag Optional image tag - */ - public repositoryUriForTag(tag?: string): string { - const tagSuffix = tag ? `:${tag}` : ''; - const parts = this.node.stack.parseArn(this.repositoryArn); - return `${parts.account}.dkr.ecr.${parts.region}.amazonaws.com/${this.repositoryName}${tagSuffix}`; - } - - /** - * Export this repository from the stack - */ - public abstract export(): RepositoryImportProps; - - /** - * Defines an AWS CloudWatch event rule that can trigger a target when an image is pushed to this - * repository. - * @param name The name of the rule - * @param target An IEventRuleTarget to invoke when this event happens (you can add more targets using `addTarget`) - * @param imageTag Only trigger on the specific image tag - */ - public onImagePushed(name: string, target?: events.IEventRuleTarget, imageTag?: string): events.EventRule { - return new events.EventRule(this, name, { - targets: target ? [target] : undefined, - eventPattern: { - source: ['aws.ecr'], - detail: { - eventName: [ - 'PutImage', - ], - requestParameters: { - repositoryName: [ - this.repositoryName, - ], - imageTag: imageTag ? [imageTag] : undefined, - }, - }, - }, - }); - } - - /** - * Grant the given principal identity permissions to perform the actions on this repository - */ - public grant(grantee: iam.IGrantable, ...actions: string[]) { - return iam.Grant.addToPrincipalOrResource({ - grantee, - actions, - resourceArns: [this.repositoryArn], - resource: this, - }); - } - - /** - * Grant the given identity permissions to use the images in this repository - */ - public grantPull(grantee: iam.IGrantable) { - const ret = this.grant(grantee, "ecr:BatchCheckLayerAvailability", "ecr:GetDownloadUrlForLayer", "ecr:BatchGetImage"); - - iam.Grant.addToPrincipal({ - grantee, - actions: ["ecr:GetAuthorizationToken"], - resourceArns: ['*'], - scope: this, - }); - - return ret; - } - - /** - * Grant the given identity permissions to pull and push images to this repository. - */ - public grantPullPush(grantee: iam.IGrantable) { - this.grantPull(grantee); - return this.grant(grantee, - "ecr:PutImage", - "ecr:InitiateLayerUpload", - "ecr:UploadLayerPart", - "ecr:CompleteLayerUpload"); - } -} - -/** - * An already existing repository - */ -class ImportedRepository extends RepositoryBase { - public readonly repositoryName: string; - public readonly repositoryArn: string; - - constructor(scope: Construct, id: string, private readonly props: RepositoryImportProps) { - super(scope, id); - - if (props.repositoryArn) { - this.repositoryArn = props.repositoryArn; - } else { - if (!props.repositoryName) { - throw new Error('If "repositoryArn" is not specified, you must specify "repositoryName", ' + - 'which also implies that the repository resides in the same region/account as this stack'); - } - - this.repositoryArn = RepositoryBase.arnForLocalRepository(props.repositoryName, this); - } - - if (props.repositoryName) { - this.repositoryName = props.repositoryName; - } else { - // if repositoryArn is a token, the repository name is also required. this is because - // repository names can include "/" (e.g. foo/bar/myrepo) and it is impossible to - // parse the name from an ARN using CloudFormation's split/select. - if (Token.unresolved(this.repositoryArn)) { - throw new Error('repositoryArn is a late-bound value, and therefore repositoryName is required'); - } - - this.repositoryName = this.repositoryArn.split('/').slice(1).join('/'); - } - } - - public export(): RepositoryImportProps { - return this.props; - } - - public addToResourcePolicy(_statement: iam.PolicyStatement) { - // FIXME: Add annotation about policy we dropped on the floor - } -}