Skip to content

Commit

Permalink
refactor: convert "import" to "from" methods (#2456)
Browse files Browse the repository at this point in the history
Implement and apply the following awslint rules:

- `awslint:from-method`: resources should have at least one static "from" method
- `awslint:from-signature`: enforce method signature
- `awslint:from-attributes`: a `fromAttributes` static method can be used to import from more than a single attribute
- `awslint:from-attributes-struct`: `fromFooAttributes` should accept a `FooAttributes` struct as input
- `awslint:no-static-import`: forbids a static `import` (deprecation helper rule)
- `awslint:attribute-tag`: all resource attributes should have an "@Attribute" doc tag
- `awslint:attribute-readonly`: all attributes must be readonly properties

Many resources now have an additional `fromFooArn` or `fromFooName` for importing from the intrinsic attribute.

Misc:
- Deprecate `Token.unresolved` in favor of `Token.isToken` (more idiomatic).
- Added eager resolution of `Fn.select` and `Fn.split` in case they receive concrete values
- Refactoring of awslint (decouple "resource" from "cfn-resource").
- Added `iam.Grant.drop` which allows "dropping" a grant on the floor for imported resources

NOTE: many of the new methods do not have inline documentation. We will address this in a subsequent pass that will be focused on docs.

Fixes #2450 
Fixes #2428 
Fixes #2424
Fixes #2429 
Fixes #2425
Fixes #2422
Fixes #2423
Fixes #89

BREAKING CHANGE: all `Foo.import` static methods are now `Foo.fromFooAttributes`
* All `FooImportProps` structs are now called `FooAttributes`
* `stepfunctions.StateMachine.export` has been removed.
* `ses.ReceiptRule.name` is now `ses.ReceiptRule.receiptRuleName`
* `ses.ReceiptRuleSet.name` is now `ses.ReceiptRuleSet.receiptRuleSetName`
* `secretsmanager.AttachedSecret` is now called `secretsmanager.SecretTargetAttachment` to match service semantics
* `ecr.Repository.export` has been removed
* `s3.Bucket.bucketUrl` is now called `s3.Bucket.bucketWebsiteUrl`
* `lambda.Version.functionVersion` is now called `lambda.Version.version`
* `ec2.SecurityGroup.groupName` is now `ec2.SecurityGroup.securityGroupName`
* `cognito.UserPoolClient.clientId` is now `cognito.UserPoolClient.userPoolClientId`
* `apigateway.IRestApiResource` is now `apigateway.IResource`
* `apigateway.IResource.resourcePath` is now `apigateway.IResource.path`
* `apigateway.IResource.resourceApi` is now `apigateway.IResource.restApi`
  • Loading branch information
Elad Ben-Israel authored May 6, 2019
1 parent e4ec30a commit 862ed7b
Show file tree
Hide file tree
Showing 148 changed files with 2,807 additions and 2,130 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ function createSelfUpdatingStack(pipelineStack: cdk.Stack): SelfUpdatingPipeline
});

// simple source
const bucket = s3.Bucket.import( pipeline, 'PatternBucket', { bucketArn: 'arn:aws:s3:::totally-fake-bucket' });
const bucket = s3.Bucket.fromBucketArn( pipeline, 'PatternBucket', 'arn:aws:s3:::totally-fake-bucket');
const sourceOutput = new codepipeline.Artifact('SourceOutput');
const sourceAction = new cpactions.S3SourceAction({
actionName: 'S3Source',
Expand Down
4 changes: 1 addition & 3 deletions packages/@aws-cdk/assets/lib/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,7 @@ export class Asset extends cdk.Construct {
const s3Filename = cdk.Fn.select(1, cdk.Fn.split(cxapi.ASSET_PREFIX_SEPARATOR, keyParam.stringValue)).toString();
this.s3ObjectKey = `${this.s3Prefix}${s3Filename}`;

this.bucket = s3.Bucket.import(this, 'AssetBucket', {
bucketName: this.s3BucketName
});
this.bucket = s3.Bucket.fromBucketName(this, 'AssetBucket', this.s3BucketName);

// form the s3 URL of the object key
this.s3Url = this.bucket.urlForObject(this.s3ObjectKey);
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-apigateway/lib/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export interface DeploymentProps {
* automatically for the `restApi.latestDeployment` deployment.
*/
export class Deployment extends Resource {
/** @attribute */
public readonly deploymentId: string;
public readonly api: IRestApi;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export class LambdaIntegration extends AwsIntegration {
super.bind(method);
const principal = new iam.ServicePrincipal('apigateway.amazonaws.com');

const desc = `${method.httpMethod}.${method.resource.resourcePath.replace(/\//g, '.')}`;
const desc = `${method.httpMethod}.${method.resource.path.replace(/\//g, '.')}`;

this.handler.addPermission(`ApiPermission.${desc}`, {
principal,
Expand Down
20 changes: 12 additions & 8 deletions packages/@aws-cdk/aws-apigateway/lib/method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { CfnMethod, CfnMethodProps } from './apigateway.generated';
import { ConnectionType, Integration } from './integration';
import { MockIntegration } from './integrations/mock';
import { MethodResponse } from './methodresponse';
import { IRestApiResource } from './resource';
import { IResource } from './resource';
import { RestApi } from './restapi';
import { validateHttpMethod } from './util';

Expand Down Expand Up @@ -66,7 +66,7 @@ export interface MethodProps {
* The resource this method is associated with. For root resource methods,
* specify the `RestApi` object.
*/
readonly resource: IRestApiResource;
readonly resource: IResource;

/**
* The HTTP method ("GET", "POST", "PUT", ...) that clients use to call this method.
Expand All @@ -85,16 +85,18 @@ export interface MethodProps {
}

export class Method extends Resource {
/** @attribute */
public readonly methodId: string;

public readonly httpMethod: string;
public readonly resource: IRestApiResource;
public readonly resource: IResource;
public readonly restApi: RestApi;

constructor(scope: Construct, id: string, props: MethodProps) {
super(scope, id);

this.resource = props.resource;
this.restApi = props.resource.resourceApi;
this.restApi = props.resource.restApi;
this.httpMethod = props.httpMethod.toUpperCase();

validateHttpMethod(this.httpMethod);
Expand All @@ -120,9 +122,9 @@ export class Method extends Resource {

this.methodId = resource.ref;

props.resource.resourceApi._attachMethod(this);
props.resource.restApi._attachMethod(this);

const deployment = props.resource.resourceApi.latestDeployment;
const deployment = props.resource.restApi.latestDeployment;
if (deployment) {
deployment.node.addDependency(resource);
deployment.addToLogicalId({ method: methodProps });
Expand All @@ -136,6 +138,8 @@ export class Method extends Resource {
*
* NOTE: {stage} will refer to the `restApi.deploymentStage`, which will
* automatically set if auto-deploy is enabled.
*
* @attribute
*/
public get methodArn(): string {
if (!this.restApi.deploymentStage) {
Expand All @@ -145,15 +149,15 @@ export class Method extends Resource {
}

const stage = this.restApi.deploymentStage.stageName.toString();
return this.restApi.executeApiArn(this.httpMethod, this.resource.resourcePath, stage);
return this.restApi.executeApiArn(this.httpMethod, this.resource.path, stage);
}

/**
* Returns an execute-api ARN for this method's "test-invoke-stage" stage.
* This stage is used by the AWS Console UI when testing the method.
*/
public get testMethodArn(): string {
return this.restApi.executeApiArn(this.httpMethod, this.resource.resourcePath, 'test-invoke-stage');
return this.restApi.executeApiArn(this.httpMethod, this.resource.path, 'test-invoke-stage');
}

private renderIntegration(integration?: Integration): CfnMethod.IntegrationProperty {
Expand Down
52 changes: 27 additions & 25 deletions packages/@aws-cdk/aws-apigateway/lib/resource.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { Construct, IResource, Resource as ResourceConstruct } from '@aws-cdk/cdk';
import { Construct, IResource as IResourceBase, Resource as ResourceConstruct } from '@aws-cdk/cdk';
import { CfnResource, CfnResourceProps } from './apigateway.generated';
import { Integration } from './integration';
import { Method, MethodOptions } from './method';
import { RestApi } from './restapi';

export interface IRestApiResource extends IResource {
export interface IResource extends IResourceBase {
/**
* The parent of this resource or undefined for the root resource.
*/
readonly parentResource?: IRestApiResource;
readonly parentResource?: IResource;

/**
* The rest API that this resource is part of.
Expand All @@ -18,17 +18,18 @@ export interface IRestApiResource extends IResource {
* hash to determine the ID of the deployment. This allows us to automatically update
* the deployment when the model of the REST API changes.
*/
readonly resourceApi: RestApi;
readonly restApi: RestApi;

/**
* The ID of the resource.
* @attribute
*/
readonly resourceId: string;

/**
* The full path of this resuorce.
*/
readonly resourcePath: string;
readonly path: string;

/**
* An integration to use as a default for all methods created within this
Expand Down Expand Up @@ -67,7 +68,7 @@ export interface IRestApiResource extends IResource {
* @param pathPart The path part of the child resource
* @returns the child resource or undefined if not found
*/
getResource(pathPart: string): IRestApiResource | undefined;
getResource(pathPart: string): IResource | undefined;

/**
* Adds a greedy proxy resource ("{proxy+}") and an ANY method to this route.
Expand Down Expand Up @@ -105,19 +106,19 @@ export interface ResourceProps extends ResourceOptions {
* The parent resource of this resource. You can either pass another
* `Resource` object or a `RestApi` object here.
*/
readonly parent: IRestApiResource;
readonly parent: IResource;

/**
* A path name for the resource.
*/
readonly pathPart: string;
}

export abstract class ResourceBase extends ResourceConstruct implements IRestApiResource {
public abstract readonly parentResource?: IRestApiResource;
public abstract readonly resourceApi: RestApi;
export abstract class ResourceBase extends ResourceConstruct implements IResource {
public abstract readonly parentResource?: IResource;
public abstract readonly restApi: RestApi;
public abstract readonly resourceId: string;
public abstract readonly resourcePath: string;
public abstract readonly path: string;
public abstract readonly defaultIntegration?: Integration;
public abstract readonly defaultMethodOptions?: MethodOptions;

Expand All @@ -139,7 +140,7 @@ export abstract class ResourceBase extends ResourceConstruct implements IRestApi
return new ProxyResource(this, '{proxy+}', { parent: this, ...options });
}

public getResource(pathPart: string): IRestApiResource | undefined {
public getResource(pathPart: string): IResource | undefined {
return this.children[pathPart];
}

Expand All @@ -153,8 +154,8 @@ export abstract class ResourceBase extends ResourceConstruct implements IRestApi
}

if (path.startsWith('/')) {
if (this.resourcePath !== '/') {
throw new Error(`Path may start with "/" only for the resource, but we are at: ${this.resourcePath}`);
if (this.path !== '/') {
throw new Error(`Path may start with "/" only for the resource, but we are at: ${this.path}`);
}

// trim trailing "/"
Expand All @@ -177,10 +178,11 @@ export abstract class ResourceBase extends ResourceConstruct implements IRestApi
}

export class Resource extends ResourceBase {
public readonly parentResource?: IRestApiResource;
public readonly resourceApi: RestApi;
public readonly parentResource?: IResource;
public readonly restApi: RestApi;
public readonly resourceId: string;
public readonly resourcePath: string;
public readonly path: string;

public readonly defaultIntegration?: Integration;
public readonly defaultMethodOptions?: MethodOptions;

Expand All @@ -196,21 +198,21 @@ export class Resource extends ResourceBase {
}

const resourceProps: CfnResourceProps = {
restApiId: props.parent.resourceApi.restApiId,
restApiId: props.parent.restApi.restApiId,
parentId: props.parent.resourceId,
pathPart: props.pathPart
};
const resource = new CfnResource(this, 'Resource', resourceProps);

this.resourceId = resource.resourceId;
this.resourceApi = props.parent.resourceApi;
this.restApi = props.parent.restApi;

// render resource path (special case for root)
this.resourcePath = props.parent.resourcePath;
if (!this.resourcePath.endsWith('/')) { this.resourcePath += '/'; }
this.resourcePath += props.pathPart;
this.path = props.parent.path;
if (!this.path.endsWith('/')) { this.path += '/'; }
this.path += props.pathPart;

const deployment = props.parent.resourceApi.latestDeployment;
const deployment = props.parent.restApi.latestDeployment;
if (deployment) {
deployment.node.addDependency(resource);
deployment.addToLogicalId({ resource: resourceProps });
Expand All @@ -231,7 +233,7 @@ export interface ProxyResourceProps extends ResourceOptions {
* The parent resource of this resource. You can either pass another
* `Resource` object or a `RestApi` object here.
*/
readonly parent: IRestApiResource;
readonly parent: IResource;

/**
* Adds an "ANY" method to this resource. If set to `false`, you will have to explicitly
Expand Down Expand Up @@ -270,7 +272,7 @@ export class ProxyResource extends Resource {
public addMethod(httpMethod: string, integration?: Integration, options?: MethodOptions): Method {
// In case this proxy is mounted under the root, also add this method to
// the root so that empty paths are proxied as well.
if (this.parentResource && this.parentResource.resourcePath === '/') {
if (this.parentResource && this.parentResource.path === '/') {
this.parentResource.addMethod(httpMethod);
}
return super.addMethod(httpMethod, integration, options);
Expand Down
51 changes: 20 additions & 31 deletions packages/@aws-cdk/aws-apigateway/lib/restapi.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import iam = require('@aws-cdk/aws-iam');
import { CfnOutput, Construct, IResource, Resource } from '@aws-cdk/cdk';
import { CfnOutput, Construct, IResource as IResourceBase, Resource } from '@aws-cdk/cdk';
import { CfnAccount, CfnRestApi } from './apigateway.generated';
import { Deployment } from './deployment';
import { Integration } from './integration';
import { Method, MethodOptions } from './method';
import { IRestApiResource, ResourceBase, ResourceOptions } from './resource';
import { IResource, ResourceBase, ResourceOptions } from './resource';
import { Stage, StageOptions } from './stage';

export interface RestApiImportProps {
export interface RestApiAttributes {
/**
* The REST API ID of an existing REST API resource.
*/
Expand All @@ -19,22 +19,18 @@ export interface RestApiImportProps {
readonly restApiRootResourceId?: string;
}

export interface IRestApi extends IResource {
export interface IRestApi extends IResourceBase {
/**
* The ID of this API Gateway RestApi.
* @attribute
*/
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.
*/
export(): RestApiImportProps;
export(): RestApiAttributes;
}

export interface RestApiProps extends ResourceOptions {
Expand Down Expand Up @@ -165,20 +161,11 @@ export interface RestApiProps extends ResourceOptions {
* public endpoint.
*/
export class RestApi extends Resource implements IRestApi {
/**
* Imports an existing REST API resource.
* @param scope Parent construct
* @param id Construct ID
* @param props Imported rest API properties
*/
public static import(scope: Construct, id: string, props: RestApiImportProps): IRestApi {
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; }

public static fromRestApiId(scope: Construct, id: string, restApiId: string): IRestApi {
class Import extends Resource implements IRestApi {
public readonly restApiId = restApiId;
public export(): RestApiAttributes { return { restApiId }; }
}

return new Import(scope, id);
Expand All @@ -191,6 +178,8 @@ export class RestApi extends Resource implements IRestApi {

/**
* The resource ID of the root resource.
*
* @attribute
*/
public readonly restApiRootResourceId: string;

Expand All @@ -216,7 +205,7 @@ export class RestApi extends Resource implements IRestApi {
* api.root.addResource('friends').addMethod('GET', getFriendsHandler); // "GET /friends"
*
*/
public readonly root: IRestApiResource;
public readonly root: IResource;

private readonly methods = new Array<Method>();

Expand Down Expand Up @@ -252,7 +241,7 @@ export class RestApi extends Resource implements IRestApi {
* Exports a REST API resource from this stack.
* @returns REST API props that can be imported to another stack.
*/
public export(): RestApiImportProps {
public export(): RestApiAttributes {
return {
restApiId: new CfnOutput(this, 'RestApiId', { value: this.restApiId }).makeImportValue().toString()
};
Expand Down Expand Up @@ -402,10 +391,10 @@ export enum EndpointType {
}

class RootResource extends ResourceBase {
public readonly parentResource?: IRestApiResource;
public readonly resourceApi: RestApi;
public readonly parentResource?: IResource;
public readonly restApi: RestApi;
public readonly resourceId: string;
public readonly resourcePath: string;
public readonly path: string;
public readonly defaultIntegration?: Integration | undefined;
public readonly defaultMethodOptions?: MethodOptions | undefined;

Expand All @@ -415,8 +404,8 @@ class RootResource extends ResourceBase {
this.parentResource = undefined;
this.defaultIntegration = props.defaultIntegration;
this.defaultMethodOptions = props.defaultMethodOptions;
this.resourceApi = api;
this.restApi = api;
this.resourceId = resourceId;
this.resourcePath = '/';
this.path = '/';
}
}
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-apigateway/lib/stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ export interface MethodDeploymentOptions {
}

export class Stage extends Resource {
/**
* @attribute
*/
public readonly stageName: string;

private readonly restApi: IRestApi;
Expand Down
Loading

0 comments on commit 862ed7b

Please sign in to comment.