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

AWS Lambda Integration #1005

Closed
sonicaghi opened this Issue Feb 18, 2016 · 30 comments

Comments

Projects
None yet
10 participants
@sonicaghi
Member

sonicaghi commented Feb 18, 2016

Support the ability to invoke & manage Lambda functions from Kong.

@ajayk

This comment has been minimized.

Show comment
Hide comment
@ajayk

ajayk Feb 20, 2016

Contributor

👍

Contributor

ajayk commented Feb 20, 2016

👍

@dag24

This comment has been minimized.

Show comment
Hide comment
@dag24

dag24 commented Feb 29, 2016

👍

@thekiwi

This comment has been minimized.

Show comment
Hide comment
@thekiwi

thekiwi commented Mar 10, 2016

This might be a good starting point:
https://github.com/adobe-apiplatform/api-gateway-aws

@aaronshaf

This comment has been minimized.

Show comment
Hide comment
@aaronshaf

aaronshaf Apr 28, 2016

This would be huge.

aaronshaf commented Apr 28, 2016

This would be huge.

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson Apr 29, 2016

I am working on this using the project @thekiwi mentioned. I have a working POC but am working out an integration design. I hope to post something very soon for comment.

timerickson commented Apr 29, 2016

I am working on this using the project @thekiwi mentioned. I have a working POC but am working out an integration design. I hope to post something very soon for comment.

@thefosk

This comment has been minimized.

Show comment
Hide comment
@thefosk

thefosk Apr 29, 2016

Member

@timerickson are you building a plugin for it?

Member

thefosk commented Apr 29, 2016

@timerickson are you building a plugin for it?

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson Apr 29, 2016

Yes. Based on the Adobe plug in.

timerickson commented Apr 29, 2016

Yes. Based on the Adobe plug in.

@thefosk

This comment has been minimized.

Show comment
Hide comment
@thefosk

thefosk Apr 29, 2016

Member

@timerickson nice - I was thinking to start working on it, but I guess I am going to wait for your PR at this point and then review it together.

Member

thefosk commented Apr 29, 2016

@timerickson nice - I was thinking to start working on it, but I guess I am going to wait for your PR at this point and then review it together.

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson Apr 29, 2016

Look forward to feedback. There will be some significant design decisions.

timerickson commented Apr 29, 2016

Look forward to feedback. There will be some significant design decisions.

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson Apr 29, 2016

Might not see a PR tonight - having some bumps getting a dev env set up. Had previously done an ad hoc POC in a regular installation...

timerickson commented Apr 29, 2016

Might not see a PR tonight - having some bumps getting a dev env set up. Had previously done an ad hoc POC in a regular installation...

@dag24

This comment has been minimized.

Show comment
Hide comment
@dag24

dag24 Apr 29, 2016

@timerickson I'm very keen to see this & test it out also.

dag24 commented Apr 29, 2016

@timerickson I'm very keen to see this & test it out also.

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson Apr 30, 2016

Does anyone know of a way to access the api to which a plugin is being attached in the self_check ? Seems like this would be appropriate to do and it would be helpful to my effort, but for the life of me I can't figure it out. Got stuck on this otherwise probably would have gotten a preliminary PR out tonight.

timerickson commented Apr 30, 2016

Does anyone know of a way to access the api to which a plugin is being attached in the self_check ? Seems like this would be appropriate to do and it would be helpful to my effort, but for the life of me I can't figure it out. Got stuck on this otherwise probably would have gotten a preliminary PR out tonight.

timerickson added a commit to timerickson/kong that referenced this issue May 1, 2016

feat(plugin): Create AWS Lambda Integration plugin
Implement #1005 - AWS Lamba Integration.
For design consideration/review only - not for merge consideration as yet.
Test cases to follow.
Create MVP Lambda plugin. See README.md for details.
@thefosk

This comment has been minimized.

Show comment
Hide comment
@thefosk

thefosk May 4, 2016

Member

Does anyone know of a way to access the api to which a plugin is being attached in the self_check

That is currently not possible. What kind of check are you trying to implement?

Member

thefosk commented May 4, 2016

Does anyone know of a way to access the api to which a plugin is being attached in the self_check

That is currently not possible. What kind of check are you trying to implement?

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson May 4, 2016

I would like to know the upstream_url in some context where a plugin's config values can be defaulted. E.g. I create an api foo-api with upstream_url aws-lambda://us-west-2/my-lambda-func and now I have to create a plugin for that api and define again the region and function name, among other things. Certainly the plugin doesn't need that upstream_url to make the call to the lambda, so long as those parameter values are in the plugin config. But if it has an upstream_url from which this data can be derived, then the plugin can be properly initialized by something like convention as opposed to (what should be superfluous) configuration.

I thought the self_check might be a place for that. I originally tried the config value check functions, thinking it would be great to set the default function_name and aws_region values from a parsing of the elements of the upstream_url, as there are examples of setting conditional/sensible default values in those functions - just not from any information in the api.

It also seems natural that you should be able to access the api from the plugin, which is not really the plugin in general, but an instance of that plugin that is explicitly attached to that particular api.

I don't think it should be made optional, however I don't like having a "throwaway" upstream_url, which is currently a required parameter for adding an api. The plugin needs this information, and so it needs to either be configured with it or be able to correctly and reliably infer that from somewhere else, and the upstream_url seems like an appropriate place for that.

I feel like the lambda target should be expressed in the api definition, and the aws-lambda scheme seems like a relatively good way of expressing that.

timerickson commented May 4, 2016

I would like to know the upstream_url in some context where a plugin's config values can be defaulted. E.g. I create an api foo-api with upstream_url aws-lambda://us-west-2/my-lambda-func and now I have to create a plugin for that api and define again the region and function name, among other things. Certainly the plugin doesn't need that upstream_url to make the call to the lambda, so long as those parameter values are in the plugin config. But if it has an upstream_url from which this data can be derived, then the plugin can be properly initialized by something like convention as opposed to (what should be superfluous) configuration.

I thought the self_check might be a place for that. I originally tried the config value check functions, thinking it would be great to set the default function_name and aws_region values from a parsing of the elements of the upstream_url, as there are examples of setting conditional/sensible default values in those functions - just not from any information in the api.

It also seems natural that you should be able to access the api from the plugin, which is not really the plugin in general, but an instance of that plugin that is explicitly attached to that particular api.

I don't think it should be made optional, however I don't like having a "throwaway" upstream_url, which is currently a required parameter for adding an api. The plugin needs this information, and so it needs to either be configured with it or be able to correctly and reliably infer that from somewhere else, and the upstream_url seems like an appropriate place for that.

I feel like the lambda target should be expressed in the api definition, and the aws-lambda scheme seems like a relatively good way of expressing that.

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson May 4, 2016

And I am just putting together a new PR that uses https://github.com/chatid/laws which is non-adobe-owned MIT licensed and available via luarocks (though only the dev channel as yet), to hopefully be more palatable to folk. I was hoping to get comment on the overall design strategy rather than simply on the particular dependencies.

timerickson commented May 4, 2016

And I am just putting together a new PR that uses https://github.com/chatid/laws which is non-adobe-owned MIT licensed and available via luarocks (though only the dev channel as yet), to hopefully be more palatable to folk. I was hoping to get comment on the overall design strategy rather than simply on the particular dependencies.

timerickson added a commit to timerickson/kong that referenced this issue May 5, 2016

feat(plugin): Create AWS Lambda Integration plugin
Implement #1005 - AWS Lamba Integration.
For design consideration/review only - not for merge consideration as yet.
Test cases to follow.
Create MVP Lambda plugin. See README.md for details.
(now with less entanglement)

timerickson added a commit to timerickson/kong that referenced this issue May 5, 2016

feat(plugin): Create AWS Lambda Integration plugin
Implement #1005 - AWS Lamba Integration.
For design consideration/review only - not for merge consideration as yet.
Test cases to follow.
Create MVP Lambda plugin. See README.md for details.
Add luaossl to rockspec
(now with less entanglement)
@thefosk

This comment has been minimized.

Show comment
Hide comment
@thefosk

thefosk May 5, 2016

Member

@timerickson I see what you are trying to do. Yes currently it's not possible to access the API - the only possible way is patching Kong core to make it possible.

Overall the design looks good. It makes sense to use the existing upstream_url rather than having a dummy one.

Member

thefosk commented May 5, 2016

@timerickson I see what you are trying to do. Yes currently it's not possible to access the API - the only possible way is patching Kong core to make it possible.

Overall the design looks good. It makes sense to use the existing upstream_url rather than having a dummy one.

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson May 6, 2016

@thefosk Thanks for the feedback. I'm working to add some specs around this and get the PR in shape for actual merge. I may look at patching core to get access to the api for additional validation and achieving something like what I'm describing, but that should probably be in another PR, neh? ;-)

timerickson commented May 6, 2016

@thefosk Thanks for the feedback. I'm working to add some specs around this and get the PR in shape for actual merge. I may look at patching core to get access to the api for additional validation and achieving something like what I'm describing, but that should probably be in another PR, neh? ;-)

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson May 6, 2016

@sinzone Can you say any more what you envision by "manage" lambda functions from Kong? Is it anything beyond or different from what I'm outlining in my PR #1190 ?

timerickson commented May 6, 2016

@sinzone Can you say any more what you envision by "manage" lambda functions from Kong? Is it anything beyond or different from what I'm outlining in my PR #1190 ?

@thefosk

This comment has been minimized.

Show comment
Hide comment
@thefosk

thefosk May 13, 2016

Member

@timerickson let me know if you need any help in patching core

Member

thefosk commented May 13, 2016

@timerickson let me know if you need any help in patching core

@ahmadnassri ahmadnassri added the BC label May 13, 2016

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson May 15, 2016

@thefosk will do - but I'm going to do that in a separate effort in a separate PR. I just want to get this first one in and be able to iterate from there.

timerickson commented May 15, 2016

@thefosk will do - but I'm going to do that in a separate effort in a separate PR. I just want to get this first one in and be able to iterate from there.

timerickson added a commit to timerickson/kong that referenced this issue May 15, 2016

feat(plugin): Create AWS Lambda Integration plugin
Implement #1005 - AWS Lamba Integration.
For design consideration/review only - not for merge consideration as yet.
Test cases to follow.
Create MVP Lambda plugin. See README.md for details.
Add luaossl to rockspec
(now with less entanglement)
@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson May 15, 2016

PR #1190 is now submitted for merge.

timerickson commented May 15, 2016

PR #1190 is now submitted for merge.

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson May 16, 2016

@thefosk I'd like to take you up on that core-patching help. Not for this PR, of course, but I've tried a couple times now and I think I could eventually get it, but I'm having trouble figuring out the best place. I feel like it could be passed as an additional arg to self_check from schemas_validation.validate_entity. There's getting to be too many args there already, though, but I don't see any of the existing args that could sensibly carry it, and that self_check really doesn't run in any informative context. Otherwise maybe an additional delegate/hook that can be defined on the plugin schema itself?

timerickson commented May 16, 2016

@thefosk I'd like to take you up on that core-patching help. Not for this PR, of course, but I've tried a couple times now and I think I could eventually get it, but I'm having trouble figuring out the best place. I feel like it could be passed as an additional arg to self_check from schemas_validation.validate_entity. There's getting to be too many args there already, though, but I don't see any of the existing args that could sensibly carry it, and that self_check really doesn't run in any informative context. Otherwise maybe an additional delegate/hook that can be defined on the plugin schema itself?

@sonicaghi sonicaghi modified the milestone: 0.9 May 16, 2016

@thefosk

This comment has been minimized.

Show comment
Hide comment
@thefosk

thefosk May 18, 2016

Member

@timerickson I was thinking about it, and I am still not 100% sold on the core patch. While it's true that the plugin could check the upstream_url to verify it contains a proper lambda endpoint, it's also true that the API could be later updated to change that property to anything else without any way for the plugin to possibly know it.

I think it would make more sense to retrieve and check the upstream_url at runtime (at that point you can access the api using ngx.ctx.api) and return an error if the URL is not a valid Lambda address.

So if for whatever reasons the upstream_url is not a valid Lambda endpoint, you can return a 500 error with a message like The Lambda plugin has been installed on an API that does not point to a Lambda function.

Could you update your PR to include this runtime check?

Member

thefosk commented May 18, 2016

@timerickson I was thinking about it, and I am still not 100% sold on the core patch. While it's true that the plugin could check the upstream_url to verify it contains a proper lambda endpoint, it's also true that the API could be later updated to change that property to anything else without any way for the plugin to possibly know it.

I think it would make more sense to retrieve and check the upstream_url at runtime (at that point you can access the api using ngx.ctx.api) and return an error if the URL is not a valid Lambda address.

So if for whatever reasons the upstream_url is not a valid Lambda endpoint, you can return a 500 error with a message like The Lambda plugin has been installed on an API that does not point to a Lambda function.

Could you update your PR to include this runtime check?

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson May 18, 2016

I am not 100% sold on the patch either, esp. considering what it looks like would be involved. But that doesn't change that I is appropriate to be able to validate the upstream_url or the api in general at plugin edit (insert and update) time. I thought there was another issue (which I cannot currently locate) that described wanting the plugin validation to occur on plugin change in addition to creation.

But unless or until it's decided for whatever other reason the api should be available in plugin validation context, then it makes sense to do what you are suggesting. The request-time check is not incongruent with this, either. I will put that together.

timerickson commented May 18, 2016

I am not 100% sold on the patch either, esp. considering what it looks like would be involved. But that doesn't change that I is appropriate to be able to validate the upstream_url or the api in general at plugin edit (insert and update) time. I thought there was another issue (which I cannot currently locate) that described wanting the plugin validation to occur on plugin change in addition to creation.

But unless or until it's decided for whatever other reason the api should be available in plugin validation context, then it makes sense to do what you are suggesting. The request-time check is not incongruent with this, either. I will put that together.

@thefosk

This comment has been minimized.

Show comment
Hide comment
@thefosk

thefosk May 18, 2016

Member

Since the check at run-time should be done anyways, I would start with that until I figure out a nice way of passing the API object to the plugin check.

Member

thefosk commented May 18, 2016

Since the check at run-time should be done anyways, I would start with that until I figure out a nice way of passing the API object to the plugin check.

@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson May 18, 2016

While I'm still thinking of it... it had occurred to me this association between an api or more specifically it's upstream_url's schema and a plugin might be made explicit. I.e. If the upstream_url scheme is not on a whitelist (e.g. http, https), then by enforced convention it would have to match an existing plugin that is marked as capable of such origin provision. In the case of lamba, then, the upstream_url is aws-lambda://something/somefunc and so for adding this api, kong would require a plugin called aws-lambda to exist and for it's schema to include some flag indicating it does indeed handle this scheme - something like {handles_schemes = ['aws-lambda']}

timerickson commented May 18, 2016

While I'm still thinking of it... it had occurred to me this association between an api or more specifically it's upstream_url's schema and a plugin might be made explicit. I.e. If the upstream_url scheme is not on a whitelist (e.g. http, https), then by enforced convention it would have to match an existing plugin that is marked as capable of such origin provision. In the case of lamba, then, the upstream_url is aws-lambda://something/somefunc and so for adding this api, kong would require a plugin called aws-lambda to exist and for it's schema to include some flag indicating it does indeed handle this scheme - something like {handles_schemes = ['aws-lambda']}

@thefosk

This comment has been minimized.

Show comment
Hide comment
@thefosk

thefosk May 18, 2016

Member

Yep, we would need to extend core to allow this too, since currently we patched core with plugin-specific logic (by allowing an additional protocol).

Member

thefosk commented May 18, 2016

Yep, we would need to extend core to allow this too, since currently we patched core with plugin-specific logic (by allowing an additional protocol).

timerickson added a commit to timerickson/kong that referenced this issue May 26, 2016

feat(plugin): Create AWS Lambda Integration plugin
Implement #1005 - AWS Lamba Integration.
For design consideration/review only - not for merge consideration as yet.
Test cases to follow.
Create MVP Lambda plugin. See README.md for details.
Add luaossl to rockspec
(now with less entanglement)

timerickson added a commit to timerickson/kong that referenced this issue Jun 8, 2016

feat(plugin): Create AWS Lambda Integration plugin
Implement #1005 - AWS Lamba Integration.
For design consideration/review only - not for merge consideration as yet.
Test cases to follow.
Create MVP Lambda plugin. See README.md for details.
Add luaossl to rockspec
(now with less entanglement)
@timerickson

This comment has been minimized.

Show comment
Hide comment
@timerickson

timerickson Jun 15, 2016

@thefosk I am going to halt my work on this for at least a bit. I hope it is acceptable to be merged, with whatever modifications you may feel necessary. I really do want to see it merged, but I need to focus on some other things at least for a while.

Overall, I have added qualifier (version parameter) and IAM Role authentication support, beyond what is currently in the spec/tests. The tests for these others are there as well, but commented out, as there is no versioned test lambda or IAM role context in which to run these tests with the Kong/Mashape provided credential. I have tested these with my personal account but do not want to put those credentials in the code.

timerickson commented Jun 15, 2016

@thefosk I am going to halt my work on this for at least a bit. I hope it is acceptable to be merged, with whatever modifications you may feel necessary. I really do want to see it merged, but I need to focus on some other things at least for a while.

Overall, I have added qualifier (version parameter) and IAM Role authentication support, beyond what is currently in the spec/tests. The tests for these others are there as well, but commented out, as there is no versioned test lambda or IAM role context in which to run these tests with the Kong/Mashape provided credential. I have tested these with my personal account but do not want to put those credentials in the code.

@swhear

This comment has been minimized.

Show comment
Hide comment
@swhear

swhear commented Jun 30, 2016

+1

@thibaultcha

This comment has been minimized.

Show comment
Hide comment
@thibaultcha

thibaultcha Mar 9, 2017

Member

Part of the recently released 0.10.0, so closing this. The documentation is not yet uploaded on the website but should follow-up shortly.

Member

thibaultcha commented Mar 9, 2017

Part of the recently released 0.10.0, so closing this. The documentation is not yet uploaded on the website but should follow-up shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment