Skip to content

Commit

Permalink
refactor(awslint): construct-base-is-private, resource-attribute (#2349)
Browse files Browse the repository at this point in the history
Refactor the rule implementation of awslint such that the context of each linter is implemented as a `XxxReflection` class (i.e. `ConstructReflection`, `ResourceReflection`). This increases our ability to reuse the model extracted from the type system in other linters.

- `awslint:construct-base-is-private` will recommend that XxxBase are internal (required merging some files in some places)
- `awslint:resource-attribute`: implement missing attributes from several modules
- In some places, move the `ImportedXxx` class into the import method as a local class (makes the implementation much nicer because it's closed by the function call).

Misc:
- update "foreach.sh" to ignore undefined npm scripts
- update contribution guide with instructions on how to use foreach.sh

Fixes #2426 
Fixes #2409 

BREAKING CHANGE: `s3.Bucket.domainName` renamed to `s3.Bucket.bucketDomainName`.
* `codedeploy.IXxxDeploymentConfig.deploymentConfigArn` is now a property and not a method.
* `ec2.SecurityGroupBase` is now private
* `ec2.VpcNetworkBase` is now private
* `kinesis.StreamBase` is now private
* `kms.EncryptionKeyBase` is now private
* `logs.LogGroupBase` is now private
* `ssm.ParameterBase` is now private
* `eks.ClusterBase` is now private
* `codebuild.ProjectBase` is now private
* `codecommit.RepositoryBase` is now private
* `codedeploy.ServerDeploymentGroupBase` is now private
* `eks.ClusterBase` is now private
* `lambda.LayerVersionBase` is now private
* `rds.DatabaseClusterBase` is now private
* `secretsmanager.SecretBase` is now private
* `ses.ReceiptRuleSetBase` is now private
  • Loading branch information
Elad Ben-Israel authored May 1, 2019
1 parent 09d66a0 commit 973b506
Show file tree
Hide file tree
Showing 47 changed files with 1,374 additions and 927 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <RULE>` will run awslint throughout the repo and
evaluate only the rule specified [awslint README](./tools/awslint/README.md)
Expand Down
40 changes: 25 additions & 15 deletions packages/@aws-cdk/aws-apigateway/lib/restapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -162,14 +172,28 @@ 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);
}

/**
* The ID of this API Gateway RestApi.
*/
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.
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-cloudfront/lib/web_distribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codecommit/lib/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-codedeploy/lib/lambda/application.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
67 changes: 22 additions & 45 deletions packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-config.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
}

/**
Expand All @@ -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.
*
Expand All @@ -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)
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-codedeploy/lib/server/application.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 973b506

Please sign in to comment.