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

ApiGateway LambdaRestApi doesn't allow overriding default integration properties #3269

Closed
1 of 5 tasks
mb-dev opened this issue Jul 10, 2019 · 11 comments · Fixed by #17065
Closed
1 of 5 tasks

ApiGateway LambdaRestApi doesn't allow overriding default integration properties #3269

mb-dev opened this issue Jul 10, 2019 · 11 comments · Fixed by #17065
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@mb-dev
Copy link

mb-dev commented Jul 10, 2019

Note: for support questions, please first reference our documentation, then use Stackoverflow. This repository's issues are intended for feature requests and bug reports.

  • I'm submitting a ...

    • 🪲 bug report
    • 🚀 feature request
    • 📚 construct library gap
    • ☎️ security issue or vulnerability => Please see policy
    • ❓ support request => Please see note at the top of this template.
  • What is the current behavior?
    If the current behavior is a 🪲bug🪲: Please provide the steps to reproduce

When defining LambdaRestApi I am unable to assign credentials role to the lambda. Only handler can be passed: https://github.com/awslabs/aws-cdk/blob/master/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts#L51

  • What is the expected behavior (or behavior of feature suggested)?

I should be able to assign credentialsRole or other integration properties.

  • What is the motivation / use case for changing the behavior or adding this feature?

When users are introduced to apigateway constructs are going to choose lambda rest api naturally when having a lambda handler. However they will soon find they can't override any props on the integration (https://github.com/awslabs/aws-cdk/blob/master/packages/@aws-cdk/aws-apigateway/lib/lambda-api.ts#L46) and will need to switch to a regular RestApi.

  • Please tell us about your environment:

    • CDK CLI Version: 0.38.0
    • Module Version: 0.38.0
    • OS: All
    • Language: All
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. associated pull-request, stackoverflow, gitter, etc)

@mb-dev mb-dev added the needs-triage This issue or PR still needs to be triaged. label Jul 10, 2019
@NGL321
Copy link
Contributor

NGL321 commented Jul 10, 2019

Hey @mb-dev,

If I understand correctly, you want to assign a credentials role to the lambda handling the requests from the API.
The LambdaRestApiProps has a handler member that is an IFunction, which itself has a member of type IRole. You should be able to assign a role to that sub-member.

Please let me know if this solves your problem!

@NGL321 NGL321 added @aws-cdk/aws-apigateway Related to Amazon API Gateway @aws-cdk/aws-lambda Related to AWS Lambda response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jul 10, 2019
@mb-dev
Copy link
Author

mb-dev commented Jul 10, 2019

@NGL321 A handler role and integration credentials role are different. I have a role assigned to the handler and that is correctly assigned to the lambda itself. However the credentials role defined in the integration allows api gateway to launch the lambda, and needs to have service principal of "apigateway.amazonaws.com" with invoke lambda permissions.

So that doesn't solve the problem.

@mb-dev
Copy link
Author

mb-dev commented Jul 10, 2019

@i-avalos
Copy link

Hey! I'm having the same issue but with a twist. I was trying to pass an integration to the default integration because I really need cache to be based on the query parameters (using cacheKeyParameters on the options of this integration, which is not available on LamdaRestApi options).
I agree this should be changed, because it seems counter-intuitive that you're allowed to pass an Integration only to be received by a "you can't do that" message after using cdk deploy.

@mb-dev
Copy link
Author

mb-dev commented Jul 29, 2019

i ended up using the plain RestApi. You can look at the code to see what LambdaRestApi does and apply it manually.

This seems to be the issue on many constructs that provide specialized behavior. For example ECSCluster addCapacity doesn't allow to specify iam role, or key name for the auto scaling group. Want a role? define auto scaling group yourself then use addAutoScalingGroup.

@eladb eladb assigned eladb and unassigned eladb Aug 12, 2019
@eladb eladb assigned nija-at and unassigned eladb Sep 3, 2019
@SomayaB SomayaB removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 29, 2019
@SomayaB SomayaB added the feature-request A feature should be added or improved. label Nov 11, 2019
@timurista
Copy link

This is really an issue for me too, we want to use a custom lambda integration that has a VpcLink to our own Vpc but I get this warning Cannot specify "defaultIntegration" since Lambda integration is automatically defined. So there seems to be no way to add this integration or override the existing one.

@nija-at nija-at added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md and removed @aws-cdk/aws-lambda Related to AWS Lambda labels Feb 5, 2020
@nija-at
Copy link
Contributor

nija-at commented Feb 5, 2020

This should be easy to add. It would require modifying LambdaRestApiProps to have another property - integrationOptions - of type IntegrationOptions.

@gustakasn0v
Copy link

@nija-at This sounds like a good first issue for me as I've used CDK with ApiGateway heavily. I'm happy to take a look

@nija-at
Copy link
Contributor

nija-at commented Apr 28, 2020

All yours @gustakasn0v

@kylelaker
Copy link
Contributor

#17065 now exists to try to resolve this after #12004 seems to have been closed. The PR takes in @nija-at's recommendation to add an integrationOptions field to the LambdaRestApiProps struct as well as the review comments left on #12004.

@mergify mergify bot closed this as completed in #17065 Jun 22, 2022
mergify bot pushed a commit that referenced this issue Jun 22, 2022
This adds an `integrationOptions` field to the `LambdaRestApiProps` and ensures that `proxy` cannot be set differently from the base `proxy` flag (but discourages setting it at all). This will allow setting the `cacheKeyParameters`, `allowTestInvoke`, and `vpcLink` configuration fields without having to fall back to using a regular `RestApi`.

Some of this is inspired by #12004 and its review comments, which was closed earlier this year due to inactivity.

Resolves: #3269

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

daschaa pushed a commit to daschaa/aws-cdk that referenced this issue Jul 9, 2022
…17065)

This adds an `integrationOptions` field to the `LambdaRestApiProps` and ensures that `proxy` cannot be set differently from the base `proxy` flag (but discourages setting it at all). This will allow setting the `cacheKeyParameters`, `allowTestInvoke`, and `vpcLink` configuration fields without having to fall back to using a regular `RestApi`.

Some of this is inspired by aws#12004 and its review comments, which was closed earlier this year due to inactivity.

Resolves: aws#3269

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
9 participants