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.

Sign up for GitHub

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

Jump to bottom

feat(apigatewayv2-integrations): full API for API Gateway V2 AWS_PROXY integrations #16287

Closed
wants to merge 20 commits into from
Closed

feat(apigatewayv2-integrations): full API for API Gateway V2 AWS_PROXY integrations #16287

wants to merge 20 commits into from

Conversation

alanraison
Copy link
Contributor

@alanraison alanraison commented Aug 30, 2021 •
edited
Loading

I propose this addition for AWS_PROXY integrations with API Gateway V2. The API seems usable from the integration tests, so I would appreciate a review.

This has been mentioned in a few issues, for example #11947 #10497

Your comments will be most welcome.

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

@alanraison
Getting started with eventbridge integration
c9f7d3b
@alanraison
Extending HttpIntegration will full set of properties for AWS_PROXY i…
8bc1f35 
…ntegrations
@alanraison
EventBridge-PutEvents and SQS-SendMessage integrations
420437a
@alanraison
SQS ReceiveMessage integration
b0d80ea
@alanraison
SQS DeleteMessage integration
087f83d
@alanraison
SQS-PurgeQueue integration
4dcd2d0
@alanraison
refactoring test
1f35e0c
@alanraison
formatting
216fd12
@alanraison
Kinesis PutRecord integration
06a90b2
@alanraison
Step Functions integrations
d92cf9d
@alanraison
Role is mandatory for all AWS Service integrations
7637ded
@alanraison
Fix EventBridge schema
69643ea
@alanraison
add permissions
39824c6
@alanraison
making role mandatory
dea5f56
@alanraison
Starting to support MappingExpressions
83fbd9b
@alanraison
Adding mapping expressions for more complete integrations
3aace30
@alanraison
Mapping Expressions for all AWS integrations
b4680e5
@alanraison
kinesis integration test
fbe3fe7
@alanraison
Step function tests
3da6a9f
@gitpod-io
Copy link

gitpod-io bot commented Aug 30, 2021

@mergify
Copy link
Contributor

mergify bot commented Aug 30, 2021

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@alanraison alanraison changed the title (apigatewayv2-integrations) Full API for API Gateway V2 AWS_PROXY integrations feat(apigatewayv2-integrations): full API for API Gateway V2 AWS_PROXY integrations Aug 30, 2021
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 1a9efab
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@peterwoodworth peterwoodworth added effort/large Large work item – several weeks of effort @aws-cdk/aws-apigatewayv2-integrations Related to AWS APIGatewayv2 Integrations labels Sep 1, 2021
@nija-at nija-at added the p2 label Sep 14, 2021
@nija-at
Copy link
Contributor

nija-at commented Sep 14, 2021

Thanks so much for submitting this pull request. I am marking this pull request as p2, which means that we are unable to work on it immediately, but it's definitely still on our radar.

We use +1s to help prioritize our work, and are happy to revaluate this pull request based on community feedback.

@nija-at nija-at removed their assignment Sep 14, 2021
@fwang fwang mentioned this pull request Sep 19, 2021
Closed
@fwang
Copy link

fwang commented Sep 23, 2021 •
edited
Loading

We use +1s to help prioritize our work, and are happy to revaluate this pull request based on community feedback.

+1 (Representing 10+ folks from the SST community :)

@amouly
Copy link

amouly commented Sep 23, 2021

+1

1 similar comment
@iDVB
Copy link

iDVB commented Sep 23, 2021

+1

jogold
jogold reviewed Dec 9, 2021
View reviewed changes
packages/@aws-cdk/aws-apigatewayv2-integrations/README.md
});
```

Integrations should always specify a role, with appropriate permissions to allow the actions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't the integration automatically add the correct permissions to the role?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, thanks for the review. I thought about that, but since the queue name, for example, could come from the payload, it wouldn't be possible for the cdk to know what permissions may be required.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. But we could maybe identify when the queue name is known at synth time and in this case give the right permissions.

@jogold
Copy link
Contributor

jogold commented Dec 11, 2021 •
edited
Loading

Thanks so much for submitting this pull request. I am marking this pull request as p2, which means that we are unable to work on it immediately, but it's definitely still on our radar.

We use +1s to help prioritize our work, and are happy to revaluate this pull request based on community feedback.

@nija-at how about extracting from this PR only the changes to the HttpIntegration class and HttpRouteIntegration class (adds support for integrationSubtype, credentialsArn and requestParameters in CfnIntegration)? This would allow users to implement their own class extending HttpRouteIntegration, unblocking them from using the AWS_PROXY integrations. Would you have time to review such a PR?

@jogold jogold mentioned this pull request Dec 23, 2021
Merged
@jogold
Copy link
Contributor

jogold commented Dec 23, 2021

@nija-at how about extracting from this PR only the changes to the HttpIntegration class and HttpRouteIntegration class (adds support for integrationSubtype, credentialsArn and requestParameters in CfnIntegration)? This would allow users to implement their own class extending HttpRouteIntegration, unblocking them from using the AWS_PROXY integrations. Would you have time to review such a PR?

#18154

mergify bot pushed a commit that referenced this pull request Jan 12, 2022
@jogold
feat(apigatewayv2): HttpRouteIntegration supports AWS services integr…
a8094c7 
…ations (#18154)

Add support for integration subtype and credentials allowing to extend
`HttpRouteIntegration` to create integrations for AWS services.

See https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-aws-services.html

Extracted part of #16287 to make it more reviewer friendly.

BREAKING CHANGE: `HttpIntegrationType.LAMBDA_PROXY` has been renamed to `HttpIntegrationType.AWS_PROXY`


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
@jogold @TikiTDO
feat(apigatewayv2): HttpRouteIntegration supports AWS services integr…
34ae776 
…ations (aws#18154)

Add support for integration subtype and credentials allowing to extend
`HttpRouteIntegration` to create integrations for AWS services.

See https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-aws-services.html

Extracted part of aws#16287 to make it more reviewer friendly.

BREAKING CHANGE: `HttpIntegrationType.LAMBDA_PROXY` has been renamed to `HttpIntegrationType.AWS_PROXY`


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@rix0rrr rix0rrr added feature-request A feature should be added or improved. p2 and removed p2 effort/large Large work item – several weeks of effort @aws-cdk/aws-apigatewayv2-integrations Related to AWS APIGatewayv2 Integrations feature-request A feature should be added or improved. labels Mar 4, 2022
@github-actions
Copy link

This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@github-actions
Copy link

github-actions bot commented May 5, 2022

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

@github-actions github-actions bot added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label May 5, 2022
@github-actions github-actions bot closed this May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jogold jogold jogold left review comments

Assignees
No one assigned
Labels
closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@alanraison @aws-cdk-automation @nija-at @fwang @amouly @iDVB @jogold @rix0rrr @peterwoodworth