Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

support multiple proxy sources at the same time ...? #334

Closed
breathe opened this issue Aug 6, 2017 · 6 comments
Closed

support multiple proxy sources at the same time ...? #334

breathe opened this issue Aug 6, 2017 · 6 comments

Comments

@breathe
Copy link
Contributor

breathe commented Aug 6, 2017

I'm working on adding automatic lambda-passthru deployment (https://github.com/bespoken/lambda-passthru/) to serverless-plugin-bespoken.

I'm adding a command like this:

serverless deploy-passthru -s dev -f someHandler

Which will deploy a passthru lambda pointing at the bespoken proxy url.

Its common to have multiple lambda's defined in the same serverless config. Ideally I could support multiple lambda passthru's at the same time:

serverless deploy-passthru -s dev => deploy's passthru versions of all lambda's defined in serverless.yml

And then allow serverless proxy to fire up a single process with correct set of handlers wired up appropriately (single process for best debugger ergonomics).

I can see a couple of ways to do this -- maybe this is the simplest way?

create a new subclass of LambdaServer with constructor something like:

export interface ILambdaReference  {
    file: string,
    functionName: string;
}

export class RoutedLambdaServer extends LambdaServer {

    public constructor(lambda: ILambdaReference[], port: number, verbose?: boolean) {...}
    private invoke (request: IncomingMessage, body: Buffer, response: ServerResponse): void {...}
}

Reuse the start and stop method implementations from superclass, and add an invoke implementation that uses a property from the request which is expected to be set inside the passthru (perhaps correctly populating context.invokedFunctionArn) to select the correct lambda handler for the request.

And then would need to change BSTProxy to create an instance of RoutedLambdaServer instead of LambdaServer.

I think this could be done in backwards compatible way. Does this sound like a reasonable feature/implementation strategy?

Another option might be to just change LambdaServer to above -- LambdaServer is currently part of the public exports though so the change wouldn't be backwards compatible ...

@breathe breathe changed the title support multiple proxying url's at the same time ...? support multiple proxy sources at the same time ...? Aug 7, 2017
@jkelvie
Copy link
Member

jkelvie commented Aug 7, 2017

These are really good ideas...I need to think about this a bit more, but I really appreciate the thought and consideration behind them.

@jkelvie
Copy link
Member

jkelvie commented Aug 14, 2017

With regard to the passthrough lambda, it sounds like you are proposing deploying multiple lambdas to handle passthrough for each local lambda.

Am I understanding that correctly? I believe that makes sense, given that we cannot control how the Lambda is invoked (i.e., it will be called by Lex or Alexa Smart Home Skills directly, and we cannot easily force another parameter onto it for routing).

The result is a one-to-one correlation between proxy lambdas and local lambdas.

With regard to the LambdaServer, I believe we should add a small enhancement to it that will look at the path of the HTTP request. If set, it will use that to lookup a Lambda function to execute. So, a call such as:
http://localhost:10000/lambda.handler

Will cause the function handler on lambda.js to be invoked.

If no path is provided, the server will continue to work as it normally does.

With regard to the constructor, the parameters should simply be changed to optional:
public constructor(private file?: string, private port?: number, private verbose?: boolean, private functionName?: string) {}

This approach has greater security implications versus the one you propose, but these can be addressed by limiting which files/functions can called. It should be:

  1. Only relative ones (with no "..")
  2. Nothing in node_modules

And it has the benefit of working for any use-case, not just serverless ones, and in a way that is intuitive and simple to work with.

@breathe
Copy link
Contributor Author

breathe commented Aug 14, 2017

With regard to the passthrough lambda, it sounds like you are proposing deploying multiple lambdas to handle passthrough for each local lambda.
Am I understanding that correctly? I believe that makes sense, given that we cannot control how the Lambda is invoked (i.e., it will be called by Lex or Alexa Smart Home Skills directly, and we cannot easily force another parameter onto it for routing).
The result is a one-to-one correlation between proxy lambdas and local lambdas.

Yeah that's what I'm thinking.

With regard to the LambdaServer, I believe we should add a small enhancement to it that will look at the path of the HTTP request. If set, it will use that to lookup a Lambda function to execute. So, a call such as:
http://localhost:10000/lambda.handler
Will cause the function handler on lambda.js to be invoked.
If no path is provided, the server will continue to work as it normally does.

Ok I see -- so the passthru should include the path to the lambda which should be invoked in the url used to connect to the bespoken proxy. Then in LambdaServer.invoke, if this.file is null should use information from request.url to load the correct module with which to handle the request?

If that sounds like I'm on the right path, I could take a stab at this tonight I think.

@jkelvie
Copy link
Member

jkelvie commented Aug 14, 2017

Yes, we are on the same page, except I think the file should only be used if there is no path in the HTTP request.

So, if the there is a path on the URL, it will always try to use that. If it cannot find a file with that path, it should be an error message.

If there is no path in the request, then it should go ahead and use the file set in the constructor. If no file is set and no path is provided, that should also be an error.

@breathe
Copy link
Contributor Author

breathe commented Aug 23, 2017

@jkelvie sorry about the delay -- I wasn't able to get to this last week. I just got to it now and submitted a PR toward adding the functionality to LambdaServer :)

@breathe
Copy link
Contributor Author

breathe commented Aug 24, 2017

Closed by #366

@breathe breathe closed this as completed Aug 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants