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

feat(aws-apigateway-kinesisstreams): New aws-apigateway-kinesisstreams pattern implementation #56

Merged
merged 6 commits into from
Sep 9, 2020

Conversation

dscpinheiro
Copy link
Contributor

Issue #, if available: #51

Description of changes: This PR introduces a new pattern, where data can be ingested to a Kinesis Data Streams via API Gateway.

API Gateway acts as a proxy, providing a layer of abstraction from the streaming storage. This enables customers to implement custom authentication approaches for data producers, control quotas for specific producers, and change the target Kinesis stream.

The pattern only includes the PutRecord and PutRecords actions in Kinesis, as there are other alternatives (such as the aws-kinesisstreams-lambda construct) for reading data from a stream.

In addition to the pattern, other changes also included in this PR are:

  • Update kinesis-streams-helper to allow using an existingStreamObj
  • Update apigateway-helper to specify optional RequestValidator and Model when adding a proxy method to an API resource.

The request templates used for the API methods follow the structures described in this documentation page (https://docs.aws.amazon.com/apigateway/latest/developerguide/integrating-api-with-aws-services-kinesis.html).

  • PutRecord:
{
    "StreamName": "$input.params('stream-name')",
    "Data": "$util.base64Encode($input.json('$.data'))",
    "PartitionKey": "$input.path('$.partitionKey')"
}
  • PutRecords:
{
    "StreamName": "$input.params('stream-name')",
    "Records": [
        #foreach($elem in $input.path('$.records'))
          {
            "Data": "$util.base64Encode($elem.data)",
            "PartitionKey": "$elem.partitionKey"
          }#if($foreach.hasNext),#end
        #end
    ]
}

Since these templates require a specific format, the aws-apigateway-kinesisstreams construct will also create the corresponding models, which will then be used for request validation. Without the models and validation, invalid request payloads would return a 200 response, even though the integration could not be invoked successfully:

Request: POST /record
Request body: { "foo": "bar" }

Response status: 200
Response body:
{
  "__type": "ValidationException",
  "message": "1 validation error detected: Value '' at 'partitionKey' failed to satisfy constraint: Member must have length greater than or equal to 1"
}

Model creation can be disabled by setting the createRequestModels property of ApiGatewayToKinesisStreamsProps to false, but it's set to true by default.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: codebuildgithubautobuildPro-rp7jMDIK1wBN
  • Commit ID: 0a5f1eb
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@dscpinheiro
Copy link
Contributor Author

I'm looking into the build error. It's failing because of this section on integ.apigateway-kinesis-overwrite.ts:

    kinesisStreamProps: {
        shardCount: 1,
        streamName: 'my-stream'
    },

cfn_nag then warns that "Resource found with an explicit name, this disallows updates that require replacement of this resource".

I'll remove the streamName property.

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: codebuildgithubautobuildPro-rp7jMDIK1wBN
  • Commit ID: 2dcbae0
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Contributor

@hnishar hnishar left a comment

Choose a reason for hiding this comment

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

Overall looks good, apart from minor comments, main feedback is around how to handle override for RequestTemplates and RequestModels and name of the pattern which I think should be called aws-apigateway-kinesisstream (with a singular stream instead of streams)

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: codebuildgithubautobuildPro-rp7jMDIK1wBN
  • Commit ID: a1552c3
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@dscpinheiro
Copy link
Contributor Author

Thanks for the feedback @hnishar, I've addressed your comments in my latest commit (e3b3f8f).

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: codebuildgithubautobuildPro-rp7jMDIK1wBN
  • Commit ID: e3b3f8f
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@dscpinheiro dscpinheiro marked this pull request as draft September 9, 2020 12:43
@dscpinheiro
Copy link
Contributor Author

I found a bug with my latest changes: the addModel method was not being invoked for user provided models. I've converted the PR to draft while I fix it.

@@ -55,9 +55,9 @@ _Parameters_
|:-------------|:----------------|-----------------|
|apiGatewayProps?|[`api.RestApiProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-apigateway.RestApiProps.html)|Optional user-provided props to override the default props for the API Gateway.|
|putRecordRequestTemplate?|`string`|API Gateway request template for the PutRecord action. If not provided, a default one will be used.|
|putRecordRequestModel?|[`api.Model`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-apigateway.Model.html)|API Gateway request model for the PutRecord action. If not provided, a default one will be created.|
|putRecordRequestModel?|[`api.ModelOptions`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-apigateway.ModelOptions.html)|API Gateway request model for the PutRecord action. If not provided, a default one will be created.|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I changed this property to ModelOptions is because the RestApi.addModel method takes a ModelOptions interface as a parameter (https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-apigateway.RestApi.html#add-wbr-modelid-props)

@dscpinheiro dscpinheiro marked this pull request as ready for review September 9, 2020 15:06
@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: codebuildgithubautobuildPro-rp7jMDIK1wBN
  • Commit ID: 8963808
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Contributor

@hnishar hnishar left a comment

Choose a reason for hiding this comment

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

Ship it! Thanks for you contribution !

@hnishar hnishar merged commit abddeed into awslabs:master Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants