Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial API Gateway Construct Library #665

Merged
merged 16 commits into from
Sep 17, 2018
Merged

Initial API Gateway Construct Library #665

merged 16 commits into from
Sep 17, 2018

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Sep 4, 2018

Introduce an initial construct library for API Gateway. By all means it does not cover the entire surface area of API Gateway, but provides basic support for defining API gateway configurations (resources/methods hierarchy).

Integration support includes LambdaIntegration which accepts a lambda.Function, sets up the appropriate permissions and binds it to an API method.

Includes many "smart defaults" at every level such as:

  • Automatically define a Deployment and a Stage for the API, which will be recreated every time the API configuration changes (by generating the logical ID of the AWS::ApiGateway::Deployment resource from a hash of the API configuration, similarly to SAM).
  • Automatically configure a role for API Gateway to allow writing CloudWatch logs and alarms.
  • Specify defaultIntegration at the API level which will apply to all methods without integration.

Supports defining APIs like this:

const integ = new apigw.LambdaIntegration(toysLambdaHandler);
const api = new apigw.RestApi(this, 'my-awesome-api');

api.root.onMethod('GET', new apigw.HttpIntegration('https://amazon.com/toys'));

const toys = api.root.addResource('toys', { defaultIntegration: integ });
toys.addMethod('ANY');

const toy = toys.addResource('{toy}');
toy.addMethod('GET'); // get a toy
toy.addMethod('DELETE'); // remove a toy

See README for more details.

Framework Changes

  • resolve: allow object keys to include tokens, as long as they resolvable to strings.
  • assert: invoke stack.validateTree() by default for expect(stack) to simulate synthesis.

Not Supported

See #727
See #723


Fixes #602, copy: @hassankhan

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

@eladb eladb requested review from mindstorms6 and rix0rrr September 4, 2018 19:50
@eladb
Copy link
Contributor Author

eladb commented Sep 5, 2018

This is blocked by aws/jsii#175

@hassankhan
Copy link

Hi @eladb, had a quick look at the PR and I really like the programmatic way of creating an API 👍

That said, I feel like its a teensy bit verbose as compared to what I proposed in on the issue itself. For example, is there any way to mass-define the API and assign Lambda, or should this be done in user code instead? I personally would prefer it being in the CDK instead of user code, because with API Gateway defining similar options for each API endpoint quickly becomes quite tedious (things such as authorization, CORS options etc.) and mistakes can be made.

I couldn't really tell from the code, but I was wondering if in a scenario where you have multiple API Gateways and enable logging for both, will both APIs use the same Account resource and logging IAM role? Also, might it be possible to add 'Endpoint Types' to the checklist above? (i.e. EDGE/REGIONAL).

@eladb
Copy link
Contributor Author

eladb commented Sep 6, 2018

@hassankhan

Is there any way to mass-define the API and assign Lambda, or should this be done in user code instead?

You can define a defaultIntegration and then you don't have to define it for each method. At the moment you can only define it at the API level, but I am thinking that perhaps it will be valuable to support this at any level of the resource tree:

const api = new apigw.RestApi(this, 'my-api', { defaultIntegration: lambdaInteg });
const toys = api.addResource('toys');
toys.onMethod('GET'); // will implicitly integrate with `lambdaIteg`

From your comment though I think you are also looking for ways to define other defaults for methods such as authentication, etc. Is that correct?

Do you also think that it would be useful to add support for a declarative way to define the entire API in RestApi props?

I couldn't really tell from the code, but I was wondering if in a scenario where you have multiple API Gateways and enable logging for both, will both APIs use the same Account resource and logging IAM role?

There's actually no association between Account and RestApi. Technically, you will have a role/account per API, but API Gateway will only use the last one created I believe, which I think should be fine because the roles will have similar permissions. It's a bit of a hack. Happy to hear if you have a suggestion on a better approach that will not require deploying a separate stack just for this singleton role.

Also, might it be possible to add 'Endpoint Types' to the checklist above? (i.e. EDGE/REGIONAL).

Endpoint types are supported via the endpointTypes prop. For example:

endpointTypes: [ apigw.EndpointType.Edge, apigw.EndpointType.Regional ]

@hassankhan
Copy link

You can define a defaultIntegration and then you don't have to define it for each method. At the moment you can only define it at the API level, but I am thinking that perhaps it will be valuable to support this at any level of the resource tree:

I agree, we should be able to define it at any level. One major reason why this is useful is because you may have several lambda.Functions, for which you'd create correspondent apigw.LambdaIntegration instances. Some people (like me) use a single Lambda for their API, but others prefer to do things differently and it is a completely valid use-case.

From your comment though I think you are also looking for ways to define other defaults for methods such as authentication, etc. Is that correct?

That is correct 😄

Do you also think that it would be useful to add support for a declarative way to define the entire API in RestApi props?

Oh my god, that would be mindblowingly amazing 🤯

Happy to hear if you have a suggestion on a better approach that will not require deploying a separate stack just for this singleton role.

I didn't actually, I was just curious to see how you'd handled it, thanks for the explanation 👍

Endpoint types are supported via the endpointTypes prop. For example:

Ahh awesome, perhaps it could be mentioned on the README?

I was thinking, could we potentially have a apigw.Function construct that handles the integration, method and resource configuration?

Elad Ben-Israel added 3 commits September 16, 2018 11:47
@eladb
Copy link
Contributor Author

eladb commented Sep 16, 2018

@hassankhan in the meantime, I've just added support for specifying default integration and method options at all levels of the resource tree. As for a declarative API for describing the tree, I wonder how this interplays with swagger support. At any rate, I will address at a subsequent commit

@eladb eladb mentioned this pull request Sep 16, 2018
Elad Ben-Israel added 3 commits September 16, 2018 15:50
* "ANY" represented as "*" method in Lambda permissions
* Use `api.root` to represent the root resource ("/") instead of the API itself

Update package-lock.json files
@eladb
Copy link
Contributor Author

eladb commented Sep 16, 2018

@rix0rrr can you take a look at the latest version? I think I we are ready for an initial merge

Copy link
Contributor

@RomainMuller RomainMuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also kind of asks for a way to make cdk diff gracefully handle the deployment magic (but that's also not an absolute necessity). I don't think any of my comments are blockers (should be considered for future improvements, though).

packages/@aws-cdk/aws-apigateway/README.md Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/README.md Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/aws-apigateway/lib/integration.ts Outdated Show resolved Hide resolved
Elad Ben-Israel added 2 commits September 17, 2018 12:51
* Rename `onMethod` to `addMethod`.
* Fixed some docs
@eladb eladb merged commit b0f3857 into master Sep 17, 2018
eladb pushed a commit that referenced this pull request Sep 20, 2018
__NOTICE__: This release includes a framework-wide [__breaking
change__](#712) which changes the type
of all the string resource attributes across the framework. Instead of using
strong-types that extend `cdk.Token` (such as `QueueArn`, `TopicName`, etc), we
now represent all these attributes as normal `string`s, and codify the tokens
into the string (using the feature introduced in [#168](#168)).

Furthermore, the `cdk.Arn` type has been removed. In order to format/parse ARNs,
use the static methods on `cdk.ArnUtils`.

See motivation and discussion in [#695](#695).

* **cfn2ts:** use stringified tokens for resource attributes instead of strong types ([#712](#712)) ([6508f78](6508f78)), closes [#518](#518) [#695](#695) [#744](#744)
* **aws-dynamodb:** Attribute type for keys, changes the signature of the `addPartitionKey` and `addSortKey` methods to be consistent across the board. ([#720](#720)) ([e6cc189](e6cc189))
* **aws-codebuild:** fix typo "priviledged" -> "privileged

* **assets:** cab't use multiple assets in the same stack ([#725](#725)) ([bba2e5b](bba2e5b)), closes [#706](#706)
* **aws-codebuild:** typo in BuildEnvironment "priviledged" -> "privileged     ([#734](#734)) ([72fec36](72fec36))
* **aws-ecr:** fix addToResourcePolicy ([#737](#737)) ([eadbda5](eadbda5))
* **aws-events:** ruleName can now be specified ([#726](#726)) ([a7bc5ee](a7bc5ee)), closes [#708](#708)
* **aws-lambda:** jsii use no long requires 'sourceAccount' ([#728](#728)) ([9e7d311](9e7d311)), closes [#714](#714)
* **aws-s3:** remove `policy` argument ([#730](#730)) ([a79190c](a79190c)), closes [#672](#672)
* **cdk:** "cdk init" java template is broken ([#732](#732)) ([281c083](281c083)), closes [#711](#711) [aws/jsii#233](aws/jsii#233)

* **aws-apigateway:** new API Gateway Construct Library ([#665](#665)) ([b0f3857](b0f3857))
* **aws-cdk:** detect presence of EC2 credentials ([#724](#724)) ([8e8c295](8e8c295)), closes [#702](#702) [#130](#130)
* **aws-codepipeline:** make the Stage insertion API in CodePipeline more flexible ([#460](#460)) ([d182818](d182818))
* **aws-codepipeline:** new "Pipeline#addStage" convenience method ([#647](#647)) ([25c9fa0](25c9fa0))
* **aws-rds:** add support for parameter groups ([#729](#729)) ([2541508](2541508)), closes [#719](#719)
* **docs:** add documentation for CDK toolkit plugings ([#733](#733)) ([965b918](965b918))
* **dependencies:** upgrade to [jsii 0.7.6](https://github.com/awslabs/jsii/releases/tag/v0.7.6)
eladb pushed a commit that referenced this pull request Sep 20, 2018
* v0.9.2

__NOTICE__: This release includes a framework-wide [__breaking
change__](#712) which changes the type
of all the string resource attributes across the framework. Instead of using
strong-types that extend `cdk.Token` (such as `QueueArn`, `TopicName`, etc), we
now represent all these attributes as normal `string`s, and codify the tokens
into the string (using the feature introduced in [#168](#168)).

Furthermore, the `cdk.Arn` type has been removed. In order to format/parse ARNs,
use the static methods on `cdk.ArnUtils`.

See motivation and discussion in [#695](#695).

* **cfn2ts:** use stringified tokens for resource attributes instead of strong types ([#712](#712)) ([6508f78](6508f78)), closes [#518](#518) [#695](#695) [#744](#744)
* **aws-dynamodb:** Attribute type for keys, changes the signature of the `addPartitionKey` and `addSortKey` methods to be consistent across the board. ([#720](#720)) ([e6cc189](e6cc189))
* **aws-codebuild:** fix typo "priviledged" -> "privileged

* **assets:** cab't use multiple assets in the same stack ([#725](#725)) ([bba2e5b](bba2e5b)), closes [#706](#706)
* **aws-codebuild:** typo in BuildEnvironment "priviledged" -> "privileged     ([#734](#734)) ([72fec36](72fec36))
* **aws-ecr:** fix addToResourcePolicy ([#737](#737)) ([eadbda5](eadbda5))
* **aws-events:** ruleName can now be specified ([#726](#726)) ([a7bc5ee](a7bc5ee)), closes [#708](#708)
* **aws-lambda:** jsii use no long requires 'sourceAccount' ([#728](#728)) ([9e7d311](9e7d311)), closes [#714](#714)
* **aws-s3:** remove `policy` argument ([#730](#730)) ([a79190c](a79190c)), closes [#672](#672)
* **cdk:** "cdk init" java template is broken ([#732](#732)) ([281c083](281c083)), closes [#711](#711) [aws/jsii#233](aws/jsii#233)

* **aws-apigateway:** new API Gateway Construct Library ([#665](#665)) ([b0f3857](b0f3857))
* **aws-cdk:** detect presence of EC2 credentials ([#724](#724)) ([8e8c295](8e8c295)), closes [#702](#702) [#130](#130)
* **aws-codepipeline:** make the Stage insertion API in CodePipeline more flexible ([#460](#460)) ([d182818](d182818))
* **aws-codepipeline:** new "Pipeline#addStage" convenience method ([#647](#647)) ([25c9fa0](25c9fa0))
* **aws-rds:** add support for parameter groups ([#729](#729)) ([2541508](2541508)), closes [#719](#719)
* **docs:** add documentation for CDK toolkit plugings ([#733](#733)) ([965b918](965b918))
* **dependencies:** upgrade to [jsii 0.7.6](https://github.com/awslabs/jsii/releases/tag/v0.7.6)
@eladb eladb deleted the benisrae/apigw branch November 19, 2018 12:31
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Examples with API Gateway
5 participants