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(apigatewayv2): AWS type websocket api integration in http api #28718

Merged
merged 8 commits into from
Jan 18, 2024

Conversation

GavinZZ
Copy link
Contributor

@GavinZZ GavinZZ commented Jan 15, 2024

Currently, Amazon.CDK.AWS.Apigatewayv2 lacks support for AWS option as the IntegrationType for WebSocket Apigateway.

Added the capability that allows user to create a WebSocket Apigateway that calls directly other AWS services without a Lambda function middleware.

Closes #27164.


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

@github-actions github-actions bot added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1 labels Jan 15, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team January 15, 2024 22:08
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jan 15, 2024
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@aws-cdk-automation aws-cdk-automation dismissed their stale review January 15, 2024 22:18

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@JobaDiniz
Copy link

I'd love to see a facilitator to configure the permission of the Websockets to invoke the service.

@GavinZZ
Copy link
Contributor Author

GavinZZ commented Jan 15, 2024

I'd love to see a facilitator to configure the permission of the Websockets to invoke the service.

I'm curious how that would be possible. For lambda integration, it's clear that this would be a connection between http api and lambda function so we can easily add a lambda permission to allow api to invoke lambda. For AWS integration, depending on the AWS service you want to integrate, the IAM role/policy varies. I couldn't think of a way to configure the permission automatically.

@GavinZZ GavinZZ marked this pull request as ready for review January 16, 2024 20:11
@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 16, 2024
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

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

Nice @GavinZZ! Just a few comments. In addition,

a) our standard is to have our PR title be mostly lowercase. see contributing guide

b) this is mostly me, i dislike using the word 'support' in the title. Should just be the feature added, i.e. 'AWS websocket integration in HTTP API'

@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 16, 2024
@GavinZZ GavinZZ changed the title feat(apigatewayv2): Support AWS type websocket API integration in HTTP API feat(apigatewayv2): AWS type websocket api integration in http api Jan 17, 2024
@GavinZZ GavinZZ requested a review from kaizencc January 17, 2024 20:16
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

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

Few more comments @GavinZZ

@GavinZZ GavinZZ requested a review from kaizencc January 17, 2024 23:16
@GavinZZ
Copy link
Contributor Author

GavinZZ commented Jan 17, 2024

Thanks @kaizencc for the reviews. I've addressed all the feedbacks. Feel free to give it another pass at your convenience.

Copy link
Contributor

mergify bot commented Jan 18, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 8d05a55
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify mergify bot merged commit 4d7374e into main Jan 18, 2024
12 checks passed
@mergify mergify bot deleted the yuanhaoz/websocket_aws_integration branch January 18, 2024 18:04
Copy link
Contributor

mergify bot commented Jan 18, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@georgechristman
Copy link
Contributor

georgechristman commented Jan 25, 2024

Hi @GavinZZ and @kaizencc, I was testing the code this evening and I'm noticing RequestParameters is not being added to the websocket integration. After reviewing the code, the WebSocketRouteIntegrationConfig within packages/aws-cdk-lib/aws-apigatewayv2-integrations/lib/websocket/aws.ts appears to be missing the requestParameters bind. I believe if "requestParameters: this.props.requestParameters" was added to the bind, it would resolve the issue.

@FayezX
Copy link

FayezX commented Jan 25, 2024

@georgechristman thanks for the find, I have added it and waiting for a review. @kaizencc #28859

@JobaDiniz
Copy link

has this been released to some version yet?

@georgechristman
Copy link
Contributor

I think I was able to resolve FayezX's issue. #28905

mergify bot pushed a commit that referenced this pull request Jan 30, 2024
…and integrationPassThrough behaviors (#28921)

#28718 missed adding requestParameters to the WebSocketAwsIntegration. Also added integrationPassThrough behaviors.
SankyRed pushed a commit that referenced this pull request Feb 8, 2024
…and integrationPassThrough behaviors (#28921)

#28718 missed adding requestParameters to the WebSocketAwsIntegration. Also added integrationPassThrough behaviors.
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. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Amazon.CDK.AWS.Apigatewayv2.Alpha: Support "AWS" as the IntegrationType for WebSockets
6 participants