From a6d7e8e45a9beb734748addb10c20fccc2a74e72 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 18 Jun 2019 14:48:18 +0300 Subject: [PATCH 1/2] refactor(apigateway): API cleanups * Fixed formatting * Add `vcpLink.addTargets` and made `targets` optional, with a validation error. BREAKING CHANGE: `MethodOptions.authorizerId` is now called `authorizer` and accepts an `IAuthorizer` which is a placeholder interface for the authorizer resource. * **apigateway:** `restapi.executeApiArn` renamed to `arnForExecuteApi`. * **apigateway:** `restapi.latestDeployment` and `deploymentStage` are now read-only. --- .../@aws-cdk/aws-apigateway/lib/authorizer.ts | 9 ++++ packages/@aws-cdk/aws-apigateway/lib/index.ts | 1 + .../@aws-cdk/aws-apigateway/lib/method.ts | 13 +++-- .../@aws-cdk/aws-apigateway/lib/restapi.ts | 42 ++++++++++------- .../@aws-cdk/aws-apigateway/lib/usage-plan.ts | 22 ++++----- .../@aws-cdk/aws-apigateway/lib/vpc-link.ts | 37 ++++++++++++--- .../aws-apigateway/test/test.restapi.ts | 8 ++-- .../aws-apigateway/test/test.vpc-link.ts | 47 ++++++++++++++++++- 8 files changed, 130 insertions(+), 49 deletions(-) create mode 100644 packages/@aws-cdk/aws-apigateway/lib/authorizer.ts diff --git a/packages/@aws-cdk/aws-apigateway/lib/authorizer.ts b/packages/@aws-cdk/aws-apigateway/lib/authorizer.ts new file mode 100644 index 0000000000000..0e9f6e0bb1a86 --- /dev/null +++ b/packages/@aws-cdk/aws-apigateway/lib/authorizer.ts @@ -0,0 +1,9 @@ +/** + * Represents an API Gateway authorizer. + */ +export interface IAuthorizer { + /** + * The authorizer ID. + */ + readonly authorizerId: string; +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-apigateway/lib/index.ts b/packages/@aws-cdk/aws-apigateway/lib/index.ts index e4d77bb5bf238..cb8e7ffde866f 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/index.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/index.ts @@ -11,6 +11,7 @@ export * from './usage-plan'; export * from './vpc-link'; export * from './methodresponse'; export * from './model'; +export * from './authorizer'; // AWS::ApiGateway CloudFormation Resources: export * from './apigateway.generated'; diff --git a/packages/@aws-cdk/aws-apigateway/lib/method.ts b/packages/@aws-cdk/aws-apigateway/lib/method.ts index d357423f00ccc..4950b4afa8692 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/method.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/method.ts @@ -1,5 +1,6 @@ import { Construct, Resource, Stack } from '@aws-cdk/cdk'; import { CfnMethod, CfnMethodProps } from './apigateway.generated'; +import { IAuthorizer } from './authorizer'; import { ConnectionType, Integration } from './integration'; import { MockIntegration } from './integrations/mock'; import { MethodResponse } from './methodresponse'; @@ -23,11 +24,8 @@ export interface MethodOptions { /** * If `authorizationType` is `Custom`, this specifies the ID of the method * authorizer resource. - * - * NOTE: in the future this will be replaced with an `IAuthorizer` - * construct. */ - readonly authorizerId?: string; + readonly authorizer?: IAuthorizer; /** * Indicates whether the method requires clients to submit a valid API key. @@ -108,6 +106,7 @@ export class Method extends Resource { const options = props.options || {}; const defaultMethodOptions = props.resource.defaultMethodOptions || {}; + const authorizer = options.authorizer || defaultMethodOptions.authorizer; const methodProps: CfnMethodProps = { resourceId: props.resource.resourceId, @@ -116,7 +115,7 @@ export class Method extends Resource { operationName: options.operationName || defaultMethodOptions.operationName, apiKeyRequired: options.apiKeyRequired || defaultMethodOptions.apiKeyRequired, authorizationType: options.authorizationType || defaultMethodOptions.authorizationType || AuthorizationType.None, - authorizerId: options.authorizerId || defaultMethodOptions.authorizerId, + authorizerId: authorizer && authorizer.authorizerId, requestParameters: options.requestParameters, integration: this.renderIntegration(props.integration), methodResponses: this.renderMethodResponses(options.methodResponses), @@ -153,7 +152,7 @@ export class Method extends Resource { } const stage = this.restApi.deploymentStage.stageName.toString(); - return this.restApi.executeApiArn(this.httpMethod, this.resource.path, stage); + return this.restApi.arnForExecuteApi(this.httpMethod, this.resource.path, stage); } /** @@ -161,7 +160,7 @@ export class Method extends Resource { * 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.path, 'test-invoke-stage'); + return this.restApi.arnForExecuteApi(this.httpMethod, this.resource.path, 'test-invoke-stage'); } private renderIntegration(integration?: Integration): CfnMethod.IntegrationProperty { diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts index baba37991b047..7cf77b58ba04b 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts @@ -177,21 +177,6 @@ export class RestApi extends Resource implements IRestApi { */ 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. - * This will be undefined if `deploy` is false. - */ - public latestDeployment?: Deployment; - - /** - * API Gateway stage that points to the latest deployment (if defined). - * - * If `deploy` is disabled, you will need to explicitly assign this value in order to - * set up integrations. - */ - public deploymentStage?: Stage; - /** * Represents the root resource ("/") of this API. Use it to define the API model: * @@ -202,6 +187,8 @@ export class RestApi extends Resource implements IRestApi { public readonly root: IResource; private readonly methods = new Array(); + private _latestDeployment: Deployment; + private _deploymentStage: Stage; constructor(scope: Construct, id: string, props: RestApiProps = { }) { super(scope, id); @@ -231,6 +218,25 @@ export class RestApi extends Resource implements IRestApi { this.root = new RootResource(this, props, resource.restApiRootResourceId); } + /** + * API Gateway deployment that represents the latest changes of the API. + * This resource will be automatically updated every time the REST API model changes. + * This will be undefined if `deploy` is false. + */ + public get latestDeployment() { + return this._latestDeployment; + } + + /** + * API Gateway stage that points to the latest deployment (if defined). + * + * If `deploy` is disabled, you will need to explicitly assign this value in order to + * set up integrations. + */ + public get deploymentStage(): Stage { + return this._deploymentStage; + } + /** * The deployed root URL of this REST API. */ @@ -275,7 +281,7 @@ export class RestApi extends Resource implements IRestApi { * @param path The resource path. Must start with '/' (default `*`) * @param stage The stage (default `*`) */ - public executeApiArn(method: string = '*', path: string = '/*', stage: string = '*') { + public arnForExecuteApi(method: string = '*', path: string = '/*', stage: string = '*') { if (!path.startsWith('/')) { throw new Error(`"path" must begin with a "/": '${path}'`); } @@ -317,7 +323,7 @@ export class RestApi extends Resource implements IRestApi { const deploy = props.deploy === undefined ? true : props.deploy; if (deploy) { - this.latestDeployment = new Deployment(this, 'Deployment', { + this._latestDeployment = new Deployment(this, 'Deployment', { description: 'Automatically created by the RestApi construct', api: this, retainDeployments: props.retainDeployments @@ -327,7 +333,7 @@ export class RestApi extends Resource implements IRestApi { // stage name is part of the endpoint, so that makes sense. const stageName = (props.deployOptions && props.deployOptions.stageName) || 'prod'; - this.deploymentStage = new Stage(this, `DeploymentStage.${stageName}`, { + this._deploymentStage = new Stage(this, `DeploymentStage.${stageName}`, { deployment: this.latestDeployment, ...props.deployOptions }); diff --git a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts index 29051a3a8711b..7bd58584ebb6f 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts @@ -66,13 +66,13 @@ export interface ThrottlingPerMethod { * The method for which you specify the throttling settings. * @default none */ - readonly method: Method, + readonly method: Method; /** * Specifies the overall request rate (average requests per second) and burst capacity. * @default none */ - readonly throttle: ThrottleSettings + readonly throttle: ThrottleSettings; } /** @@ -90,19 +90,19 @@ export interface UsagePlanPerApiStage { /** * @default none */ - readonly api?: IRestApi, + readonly api?: IRestApi; /** * * [disable-awslint:ref-via-interface] * @default none */ - readonly stage?: Stage, + readonly stage?: Stage; /** * @default none */ - readonly throttle?: ThrottlingPerMethod[] + readonly throttle?: ThrottlingPerMethod[]; } export interface UsagePlanProps { @@ -110,37 +110,37 @@ export interface UsagePlanProps { * API Stages to be associated which the usage plan. * @default none */ - readonly apiStages?: UsagePlanPerApiStage[], + readonly apiStages?: UsagePlanPerApiStage[]; /** * Represents usage plan purpose. * @default none */ - readonly description?: string, + readonly description?: string; /** * Number of requests clients can make in a given time period. * @default none */ - readonly quota?: QuotaSettings + readonly quota?: QuotaSettings; /** * Overall throttle settings for the API. * @default none */ - readonly throttle?: ThrottleSettings, + readonly throttle?: ThrottleSettings; /** * Name for this usage plan. * @default none */ - readonly name?: string + readonly name?: string; /** * ApiKey to be associated with the usage plan. * @default none */ - readonly apiKey?: IApiKey + readonly apiKey?: IApiKey; } export class UsagePlan extends Resource { diff --git a/packages/@aws-cdk/aws-apigateway/lib/vpc-link.ts b/packages/@aws-cdk/aws-apigateway/lib/vpc-link.ts index 00a5fd3ded451..2ed6e7690a997 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/vpc-link.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/vpc-link.ts @@ -1,5 +1,5 @@ import elbv2 = require('@aws-cdk/aws-elasticloadbalancingv2'); -import { Construct, Resource } from '@aws-cdk/cdk'; +import { Construct, Lazy, Resource } from '@aws-cdk/cdk'; import { CfnVpcLink } from './apigateway.generated'; /** @@ -8,9 +8,9 @@ import { CfnVpcLink } from './apigateway.generated'; export interface VpcLinkProps { /** * The name used to label and identify the VPC link. - * @default automatically generated name + * @default - automatically generated name */ - readonly name?: string; + readonly vpcLinkName?: string; /** * The description of the VPC link. @@ -21,8 +21,10 @@ export interface VpcLinkProps { /** * The network load balancers of the VPC targeted by the VPC link. * The network load balancers must be owned by the same AWS account of the API owner. + * + * @default - no targets. Use `addTargets` to add targets */ - readonly targets: elbv2.INetworkLoadBalancer[]; + readonly targets?: elbv2.INetworkLoadBalancer[]; } /** @@ -36,15 +38,36 @@ export class VpcLink extends Resource { */ public readonly vpcLinkId: string; - constructor(scope: Construct, id: string, props: VpcLinkProps) { + private readonly targets = new Array(); + + constructor(scope: Construct, id: string, props: VpcLinkProps = {}) { super(scope, id); const cfnResource = new CfnVpcLink(this, 'Resource', { - name: props.name || this.node.uniqueId, + name: props.vpcLinkName || this.node.uniqueId, description: props.description, - targetArns: props.targets.map(nlb => nlb.loadBalancerArn) + targetArns: Lazy.listValue({ produce: () => this.renderTargets() }) }); this.vpcLinkId = cfnResource.vpcLinkId; + + if (props.targets) { + this.addTargets(...props.targets); + } + } + + public addTargets(...targets: elbv2.INetworkLoadBalancer[]) { + this.targets.push(...targets); + } + + protected validate(): string[] { + if (this.targets.length === 0) { + return [ `No targets added to vpc link` ]; + } + return []; + } + + private renderTargets() { + return this.targets.map(nlb => nlb.loadBalancerArn); } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-apigateway/test/test.restapi.ts b/packages/@aws-cdk/aws-apigateway/test/test.restapi.ts index 7b3b7f462cacc..146a51bb82274 100644 --- a/packages/@aws-cdk/aws-apigateway/test/test.restapi.ts +++ b/packages/@aws-cdk/aws-apigateway/test/test.restapi.ts @@ -402,7 +402,7 @@ export = { api.root.addMethod('GET'); // WHEN - const arn = api.executeApiArn('method', '/path', 'stage'); + const arn = api.arnForExecuteApi('method', '/path', 'stage'); // THEN test.deepEqual(stack.resolve(arn), { 'Fn::Join': @@ -426,7 +426,7 @@ export = { api.root.addMethod('GET'); // THEN - test.throws(() => api.executeApiArn('method', 'hey-path', 'stage'), /"path" must begin with a "\/": 'hey-path'/); + test.throws(() => api.arnForExecuteApi('method', 'hey-path', 'stage'), /"path" must begin with a "\/": 'hey-path'/); test.done(); }, @@ -534,7 +534,7 @@ export = { const api = new apigateway.RestApi(stack, 'myapi', { defaultIntegration: rootInteg, defaultMethodOptions: { - authorizerId: 'AUTHID', + authorizer: { authorizerId: 'AUTHID' }, authorizationType: apigateway.AuthorizationType.IAM, } }); @@ -553,7 +553,7 @@ export = { const child2 = api.root.addResource('child2', { defaultIntegration: new apigateway.MockIntegration(), defaultMethodOptions: { - authorizerId: 'AUTHID2', + authorizer: { authorizerId: 'AUTHID2' }, } }); diff --git a/packages/@aws-cdk/aws-apigateway/test/test.vpc-link.ts b/packages/@aws-cdk/aws-apigateway/test/test.vpc-link.ts index 585dff09030c2..bfd68d16edfff 100644 --- a/packages/@aws-cdk/aws-apigateway/test/test.vpc-link.ts +++ b/packages/@aws-cdk/aws-apigateway/test/test.vpc-link.ts @@ -16,16 +16,59 @@ export = { // WHEN new apigateway.VpcLink(stack, 'VpcLink', { - name: 'MyLink', + vpcLinkName: 'MyLink', targets: [nlb] }); // THEN expect(stack).to(haveResourceLike('AWS::ApiGateway::VpcLink', { Name: "MyLink", - TargetArns: [ { Ref: "NLB55158F82" } ] + TargetArns: [{ Ref: "NLB55158F82" }] })); test.done(); }, + + 'targets can be added using addTargets'(test: Test) { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'VPC'); + const nlb0 = new elbv2.NetworkLoadBalancer(stack, 'NLB0', { vpc }); + const nlb1 = new elbv2.NetworkLoadBalancer(stack, 'NLB1', { vpc }); + const nlb2 = new elbv2.NetworkLoadBalancer(stack, 'NLB2', { vpc }); + const nlb3 = new elbv2.NetworkLoadBalancer(stack, 'NLB3', { vpc }); + + // WHEN + const link = new apigateway.VpcLink(stack, 'VpcLink', { + targets: [nlb0] + }); + link.addTargets(nlb1, nlb2); + link.addTargets(nlb3); + + // THEN + expect(stack).to(haveResourceLike('AWS::ApiGateway::VpcLink', { + Name: "VpcLink", + TargetArns: [ + { Ref: "NLB03D178991" }, + { Ref: "NLB13224D47C" }, + { Ref: "NLB2BEBACE62" }, + { Ref: "NLB372DB3895" } + ] + })); + + test.done(); + }, + + 'validation error if vpc link is created and no targets are added'(test: Test) { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app, 'stack'); + + // WHEN + new apigateway.VpcLink(stack, 'vpclink'); + + // TEST + test.throws(() => app.synth(), /No targets added to vpc link/); + test.done(); + } }; \ No newline at end of file From 8b15c701efd5a98190782bf0cf9f4fc230d8c436 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Tue, 18 Jun 2019 15:32:54 +0300 Subject: [PATCH 2/2] CR fixes --- .../@aws-cdk/aws-apigateway/lib/restapi.ts | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts index 170056681ecf4..b946e5a753ce8 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts @@ -186,9 +186,16 @@ export class RestApi extends Resource implements IRestApi { */ public readonly root: IResource; + /** + * API Gateway stage that points to the latest deployment (if defined). + * + * If `deploy` is disabled, you will need to explicitly assign this value in order to + * set up integrations. + */ + public deploymentStage: Stage; + private readonly methods = new Array(); - private _latestDeployment: Deployment; - private _deploymentStage: Stage; + private _latestDeployment: Deployment | undefined; constructor(scope: Construct, id: string, props: RestApiProps = { }) { super(scope, id); @@ -227,16 +234,6 @@ export class RestApi extends Resource implements IRestApi { return this._latestDeployment; } - /** - * API Gateway stage that points to the latest deployment (if defined). - * - * If `deploy` is disabled, you will need to explicitly assign this value in order to - * set up integrations. - */ - public get deploymentStage(): Stage { - return this._deploymentStage; - } - /** * The deployed root URL of this REST API. */ @@ -333,8 +330,8 @@ export class RestApi extends Resource implements IRestApi { // stage name is part of the endpoint, so that makes sense. const stageName = (props.deployOptions && props.deployOptions.stageName) || 'prod'; - this._deploymentStage = new Stage(this, `DeploymentStage.${stageName}`, { - deployment: this.latestDeployment, + this.deploymentStage = new Stage(this, `DeploymentStage.${stageName}`, { + deployment: this._latestDeployment, ...props.deployOptions });