Skip to content

Commit

Permalink
refactor: use construct interfaces in public api (awslint:ref-via-int…
Browse files Browse the repository at this point in the history
…erface) (#2499)

Adds a new awslint:ref-via-interface rule which validates that all input APIs
(e.g. props, method arguments) use construct interface (IBucket) and not concrete
classes (Bucket). This is in order to enable passing in unowned resources and
in accordance with the aws construct library guidelines.

There are situations where an owned resource is required. In those, the rule
can be disabled by adding [disable-awslint:ref-via-interface] to the element's
inline documentation.

To enable this, the following new construct interfaces were added, along with `fromXxx` import methods:
* `applicationautoscaling.IScalableTarget`
* `cloudwatch.IAlarm`
* `ecs.IService`
* `ecs.IEc2Service`
* `ec2.IFargateService`
* `ecs.ITaskDefinition`
* `iam.IGroup`
* `iam.IUser`
* `iam.IPolicy`
* `lambda.IVersion`

Fixes #2412 

BREAKING CHANGE: `apigateway.ResourceBase.trackChild` is now internal.
* `cloudfront.S3OriginConfig.originAccessIdentity` is now `originAccessIdentityId`
* `codedeploy.LambdaDeploymentGroup.alarms` is now `cloudwatch.IAlarm[]` (previously `cloudwatch.Alarm[]`)
* `codepipeline.crossRegionScaffoldingStacks` renamed to `crossRegionScaffolding`
* `codepipeline.CrossRegionScaffoldingStack` renamed to `codepipeline.CrossRegionScaffolding` and cannot be instantiated (abstract)
* `ec2.VpcSubnet.addDefaultRouteToNAT` renamed to `addDefaultNatRoute` and made public
* `ec2.VpcSubnet.addDefaultRouteToIGW` renamed to `addDefaultInternetRoute`, made public and first argument is the gateway ID (string) and not the CFN L1 class
* `ecs.Ec2EventRuleTarget.taskDefinition` is now `ITaskDefinition` (previously `TaskDefinition`)
* `lambda.IEventSource.bind` now accepts `IFunction` instead of `FunctionBase`. Use `IFunction.addEventSourceMapping` to add an event source mapping under the function.
* `lambda.Layer.grantUsage` renamed to `lambda.layer.addPermission` and returns void
* `stepfunctions.StateMachine.role` is now `iam.IRole` (previously `iam.Role`)
  • Loading branch information
Elad Ben-Israel committed May 8, 2019
1 parent e4ac767 commit f9c6ad6
Show file tree
Hide file tree
Showing 70 changed files with 1,068 additions and 605 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
*.tsbuildinfo
.cdk.staging

.vscode
# VSCode extension
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/assets/lib/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ export class Asset extends cdk.Construct {
*
* @see https://github.com/awslabs/aws-cdk/issues/1432
*
* @param resource The CloudFormation resource which is using this asset.
* @param resource The CloudFormation resource which is using this asset [disable-awslint:ref-via-interface]
* @param resourceProperty The property name where this asset is referenced
* (e.g. "Code" for AWS::Lambda::Function)
*/
Expand Down
7 changes: 5 additions & 2 deletions packages/@aws-cdk/aws-apigateway/lib/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ export abstract class ResourceBase extends ResourceConstruct implements IResourc
return this.children[pathPart];
}

public trackChild(pathPart: string, resource: Resource) {
/**
* @internal
*/
public _trackChild(pathPart: string, resource: Resource) {
this.children[pathPart] = resource;
}

Expand Down Expand Up @@ -194,7 +197,7 @@ export class Resource extends ResourceBase {
this.parentResource = props.parent;

if (props.parent instanceof ResourceBase) {
props.parent.trackChild(props.pathPart, this);
props.parent._trackChild(props.pathPart, this);
}

const resourceProps: CfnResourceProps = {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/lib/stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export interface StageOptions extends MethodDeploymentOptions {

export interface StageProps extends StageOptions {
/**
* The deployment that this stage points to.
* The deployment that this stage points to [disable-awslint:ref-via-interface].
*/
readonly deployment: Deployment;
}
Expand Down
4 changes: 1 addition & 3 deletions packages/@aws-cdk/aws-apigateway/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@
},
"awslint": {
"exclude": [
"resource-attribute:@aws-cdk/aws-apigateway.IRestApi.restApiRootResourceId",
"from-method:@aws-cdk/aws-apigateway.Resource",
"construct-base-is-private:@aws-cdk/aws-apigateway.ResourceBase"
"from-method:@aws-cdk/aws-apigateway.Resource"
]
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
import iam = require('@aws-cdk/aws-iam');
import { Construct, Resource } from '@aws-cdk/cdk';
import { Construct, IResource, Resource } from '@aws-cdk/cdk';
import { CfnScalableTarget } from './applicationautoscaling.generated';
import { BasicStepScalingPolicyProps, StepScalingPolicy } from './step-scaling-policy';
import { BasicTargetTrackingScalingPolicyProps, TargetTrackingScalingPolicy } from './target-tracking-scaling-policy';

export interface IScalableTarget extends IResource {
/**
* @attribute
*/
readonly scalableTargetId: string;
}

/**
* Properties for a scalable target
*/
Expand Down Expand Up @@ -61,7 +68,15 @@ export interface ScalableTargetProps {
/**
* Define a scalable target
*/
export class ScalableTarget extends Resource {
export class ScalableTarget extends Resource implements IScalableTarget {

public static fromScalableTargetId(scope: Construct, id: string, scalableTargetId: string): IScalableTarget {
class Import extends Resource implements IScalableTarget {
public readonly scalableTargetId = scalableTargetId;
}
return new Import(scope, id);
}

/**
* ID of the Scalable Target
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import cloudwatch = require('@aws-cdk/aws-cloudwatch');
import cdk = require('@aws-cdk/cdk');
import { CfnScalingPolicy } from './applicationautoscaling.generated';
import { ScalableTarget } from './scalable-target';
import { IScalableTarget } from './scalable-target';

/**
* Properties for a scaling policy
Expand All @@ -10,7 +10,7 @@ export interface StepScalingActionProps {
/**
* The scalable target
*/
readonly scalingTarget: ScalableTarget;
readonly scalingTarget: IScalableTarget;

/**
* A name for the scaling policy
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { findAlarmThresholds, normalizeIntervals } from '@aws-cdk/aws-autoscaling-common';
import cloudwatch = require('@aws-cdk/aws-cloudwatch');
import cdk = require('@aws-cdk/cdk');
import { ScalableTarget } from './scalable-target';
import { IScalableTarget } from './scalable-target';
import { AdjustmentType, MetricAggregationType, StepScalingAction } from './step-scaling-action';

export interface BasicStepScalingPolicyProps {
Expand Down Expand Up @@ -52,7 +52,7 @@ export interface StepScalingPolicyProps extends BasicStepScalingPolicyProps {
/**
* The scaling target
*/
readonly scalingTarget: ScalableTarget;
readonly scalingTarget: IScalableTarget;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import cloudwatch = require('@aws-cdk/aws-cloudwatch');
import cdk = require('@aws-cdk/cdk');
import { CfnScalingPolicy } from './applicationautoscaling.generated';
import { ScalableTarget } from './scalable-target';
import { IScalableTarget } from './scalable-target';

/**
* Base interface for target tracking props
Expand Down Expand Up @@ -95,7 +95,7 @@ export interface TargetTrackingScalingPolicyProps extends BasicTargetTrackingSca
/*
* The scalable target
*/
readonly scalingTarget: ScalableTarget;
readonly scalingTarget: IScalableTarget;
}

export class TargetTrackingScalingPolicy extends cdk.Construct {
Expand Down
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import route53 = require('@aws-cdk/aws-route53');
import s3 = require('@aws-cdk/aws-s3');
import cdk = require('@aws-cdk/cdk');
import { CfnCloudFrontOriginAccessIdentity, CfnDistribution } from './cloudfront.generated';
import { CfnDistribution } from './cloudfront.generated';

export enum HttpVersion {
HTTP1_1 = "http1.1",
Expand Down Expand Up @@ -237,12 +237,12 @@ export interface S3OriginConfig {
/**
* The source bucket to serve content from
*/
readonly s3BucketSource: s3.IBucket,
readonly s3BucketSource: s3.IBucket;

/**
* The optional origin identity cloudfront will use when calling your s3 bucket.
* The optional ID of the origin identity cloudfront will use when calling your s3 bucket.
*/
readonly originAccessIdentity?: CfnCloudFrontOriginAccessIdentity
readonly originAccessIdentityId?: string;
}

/**
Expand Down Expand Up @@ -570,8 +570,8 @@ export class CloudFrontWebDistribution extends cdk.Construct implements route53.
: originConfig.customOriginSource!.domainName,
originPath: originConfig.originPath,
originCustomHeaders: originHeaders.length > 0 ? originHeaders : undefined,
s3OriginConfig: originConfig.s3OriginSource && originConfig.s3OriginSource.originAccessIdentity
? { originAccessIdentity: `origin-access-identity/cloudfront/${originConfig.s3OriginSource.originAccessIdentity.ref}` }
s3OriginConfig: originConfig.s3OriginSource && originConfig.s3OriginSource.originAccessIdentityId
? { originAccessIdentity: `origin-access-identity/cloudfront/${originConfig.s3OriginSource.originAccessIdentityId}` }
: originConfig.s3OriginSource
? { }
: undefined,
Expand Down
25 changes: 23 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,21 @@
import { Construct, Resource, Token } from '@aws-cdk/cdk';
import { Construct, IResource, Resource, Token } from '@aws-cdk/cdk';
import { CfnAlarm } from './cloudwatch.generated';
import { HorizontalAnnotation } from './graph';
import { Dimension, Metric, MetricAlarmProps, Statistic, Unit } from './metric';
import { parseStatistic } from './util.statistic';

export interface IAlarm extends IResource {
/**
* @attribute
*/
readonly alarmArn: string;

/**
* @attribute
*/
readonly alarmName: string;
}

/**
* Properties for Alarms
*/
Expand Down Expand Up @@ -62,7 +74,16 @@ export enum TreatMissingData {
/**
* An alarm on a CloudWatch metric
*/
export class Alarm extends Resource {
export class Alarm extends Resource implements IAlarm {

public static fromAlarmArn(scope: Construct, id: string, alarmArn: string): IAlarm {
class Import extends Resource implements IAlarm {
public readonly alarmArn = alarmArn;
public readonly alarmName = scope.node.stack.parseArn(alarmArn, ':').resourceName!;
}
return new Import(scope, id);
}

/**
* ARN of this alarm
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export interface LambdaDeploymentGroupProps {
* @default []
* @see https://docs.aws.amazon.com/codedeploy/latest/userguide/monitoring-create-alarms.html
*/
readonly alarms?: cloudwatch.Alarm[];
readonly alarms?: cloudwatch.IAlarm[];

/**
* The service Role of this Deployment Group.
Expand All @@ -83,6 +83,8 @@ export interface LambdaDeploymentGroupProps {
/**
* Lambda Alias to shift traffic. Updating the version
* of the alias will trigger a CodeDeploy deployment.
*
* [disable-awslint:ref-via-interface] since we need to modify the alias CFN resource update policy
*/
readonly alias: lambda.Alias;

Expand Down Expand Up @@ -134,7 +136,7 @@ export class LambdaDeploymentGroup extends cdk.Resource implements ILambdaDeploy
public readonly deploymentGroupArn: string;
public readonly role: iam.IRole;

private readonly alarms: cloudwatch.Alarm[];
private readonly alarms: cloudwatch.IAlarm[];
private preHook?: lambda.IFunction;
private postHook?: lambda.IFunction;

Expand Down Expand Up @@ -188,7 +190,7 @@ export class LambdaDeploymentGroup extends cdk.Resource implements ILambdaDeploy
*
* @param alarm the alarm to associate with this Deployment Group
*/
public addAlarm(alarm: cloudwatch.Alarm): void {
public addAlarm(alarm: cloudwatch.IAlarm): void {
this.alarms.push(alarm);
}

Expand Down
24 changes: 15 additions & 9 deletions packages/@aws-cdk/aws-codedeploy/lib/server/deployment-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { IServerDeploymentConfig, ServerDeploymentConfig } from './deployment-co

export interface IServerDeploymentGroup extends cdk.IResource {
readonly application: IServerApplication;
readonly role?: iam.Role;
readonly role?: iam.IRole;
/**
* @attribute
*/
Expand Down Expand Up @@ -67,7 +67,7 @@ export interface ServerDeploymentGroupAttributes {
*/
abstract class ServerDeploymentGroupBase extends cdk.Resource implements IServerDeploymentGroup {
public abstract readonly application: IServerApplication;
public abstract readonly role?: iam.Role;
public abstract readonly role?: iam.IRole;
public abstract readonly deploymentGroupName: string;
public abstract readonly deploymentGroupArn: string;
public readonly deploymentConfig: IServerDeploymentConfig;
Expand Down Expand Up @@ -150,7 +150,7 @@ export interface ServerDeploymentGroupProps {
* The service Role of this Deployment Group.
* If you don't provide one, a new Role will be created.
*/
readonly role?: iam.Role;
readonly role?: iam.IRole;

/**
* The physical, human-readable name of the CodeDeploy Deployment Group.
Expand All @@ -169,7 +169,11 @@ export interface ServerDeploymentGroupProps {
/**
* The auto-scaling groups belonging to this Deployment Group.
*
* Auto-scaling groups can also be added after the Deployment Group is created using the {@link #addAutoScalingGroup} method.
* Auto-scaling groups can also be added after the Deployment Group is created
* using the {@link #addAutoScalingGroup} method.
*
* [disable-awslint:ref-via-interface] is needed because we update userdata
* for ASGs to install the codedeploy agent.
*
* @default []
*/
Expand Down Expand Up @@ -217,7 +221,7 @@ export interface ServerDeploymentGroupProps {
* @default []
* @see https://docs.aws.amazon.com/codedeploy/latest/userguide/monitoring-create-alarms.html
*/
readonly alarms?: cloudwatch.Alarm[];
readonly alarms?: cloudwatch.IAlarm[];

/**
* Whether to continue a deployment even if fetching the alarm status from CloudWatch failed.
Expand Down Expand Up @@ -254,14 +258,14 @@ export class ServerDeploymentGroup extends ServerDeploymentGroupBase {
}

public readonly application: IServerApplication;
public readonly role?: iam.Role;
public readonly role?: iam.IRole;
public readonly deploymentGroupArn: string;
public readonly deploymentGroupName: string;

private readonly _autoScalingGroups: autoscaling.AutoScalingGroup[];
private readonly installAgent: boolean;
private readonly codeDeployBucket: s3.IBucket;
private readonly alarms: cloudwatch.Alarm[];
private readonly alarms: cloudwatch.IAlarm[];

constructor(scope: cdk.Construct, id: string, props: ServerDeploymentGroupProps = {}) {
super(scope, id, props.deploymentConfig);
Expand Down Expand Up @@ -321,7 +325,9 @@ export class ServerDeploymentGroup extends ServerDeploymentGroupBase {
/**
* Adds an additional auto-scaling group to this Deployment Group.
*
* @param asg the auto-scaling group to add to this Deployment Group
* @param asg the auto-scaling group to add to this Deployment Group.
* [disable-awslint:ref-via-interface] is needed in order to install the code
* deploy agent by updating the ASGs user data.
*/
public addAutoScalingGroup(asg: autoscaling.AutoScalingGroup): void {
this._autoScalingGroups.push(asg);
Expand All @@ -333,7 +339,7 @@ export class ServerDeploymentGroup extends ServerDeploymentGroupBase {
*
* @param alarm the alarm to associate with this Deployment Group
*/
public addAlarm(alarm: cloudwatch.Alarm): void {
public addAlarm(alarm: cloudwatch.IAlarm): void {
this.alarms.push(alarm);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codedeploy/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ 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):
export function renderAlarmConfiguration(alarms: cloudwatch.IAlarm[], ignorePollAlarmFailure?: boolean):
CfnDeploymentGroup.AlarmConfigurationProperty | undefined {
return alarms.length === 0
? undefined
Expand All @@ -32,7 +32,7 @@ enum AutoRollbackEvent {
DeploymentStopOnRequest = 'DEPLOYMENT_STOP_ON_REQUEST'
}

export function renderAutoRollbackConfiguration(alarms: cloudwatch.Alarm[], autoRollbackConfig: AutoRollbackConfig = {}):
export function renderAutoRollbackConfiguration(alarms: cloudwatch.IAlarm[], autoRollbackConfig: AutoRollbackConfig = {}):
CfnDeploymentGroup.AutoRollbackConfigurationProperty | undefined {
const events = new Array<string>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -692,10 +692,10 @@ export = {
]
}));

test.equal(pipeline.crossRegionScaffoldStacks[pipelineRegion], undefined);
test.equal(pipeline.crossRegionScaffoldStacks['us-west-1'], undefined);
test.equal(pipeline.crossRegionScaffolding[pipelineRegion], undefined);
test.equal(pipeline.crossRegionScaffolding['us-west-1'], undefined);

const usEast1ScaffoldStack = pipeline.crossRegionScaffoldStacks['us-east-1'];
const usEast1ScaffoldStack = pipeline.crossRegionScaffolding['us-east-1'];
test.notEqual(usEast1ScaffoldStack, undefined);
test.equal(usEast1ScaffoldStack.env.region, 'us-east-1');
test.equal(usEast1ScaffoldStack.env.account, pipelineAccount);
Expand Down
Loading

0 comments on commit f9c6ad6

Please sign in to comment.