Make invocation URL dynamic and fix default function name

[skwashd]

Issue #, if available:

• 43

Description of changes:

The API endpoint in Amazon's implementation of Lambda used the path /2015-03-31/functions/[func]/invocations to invoke the function. [func] the name of the function and is set as the value of the AWS_LAMBDA_FUNCTION_NAME environment variable when the function is executing on AWS.

The emulator uses a hard coded endpoint of /2015-03-31/functions/function/invocations. This is even the case when the AWS_LAMBDA_FUNCTION_NAME environment variable is set to another value.

This patch changes the endpoint URL when the AWS_LAMBDA_FUNCTION_NAME environment variable is set. In this case the invocation URL will be /2015-03-31/functions/${AWS_LAMBDA_FUNCTION_NAME}/invocations. When the environment variable isn't set, the current behaviour persists and the function name is set to function.

The end result is the emulator behaviour is closer to the real environment the functions execute in.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[skwashd]

@valerena thanks. Fixed, but before merging this you should review my comment on https://github.com/aws/aws-lambda-runtime-interface-emulator/pull/49#issuecomment-933664875.

[skwashd]

@valerena bumping the thread as suggested in https://github.com/aws/aws-lambda-runtime-interface-emulator/pull/49#issuecomment-946872526

[valerena]

Hi @skwashd , we had a discussion with the team and there are concerns about backwards compatibility and breaking existing customers that had automated local calls for testing using RIE. The consensus is that this behavior could be optional, having to pass an extra parameter to have it like this.

To have this as the default behavior might come in the future, but it's something that we still have to confirm with more stakeholders and if it happens it could take several months, to make sure that we have all use cases covered and the right communication about that change.

I know that this has already taken more time than what I imagined you originally expected, but if you change this Pull Request to work as an optional behavior I would work to prioritize its revision and include it in a shorter amount of time that what has taken to review the rest of the changes so far. Let me know if this sounds okay for you.

[skwashd]

@valerena it's always "fun" trying to fix broken behaviour when people rely on the (broken) behaviour 😢

Before I spend more time on this, I'd like to make sure we're aligned on the details.

Is the proposal to keep the default function name fix as implemented in ebc933d as is? I hope this is a less controversial change as the default function name should match the path in the url. I hope many users aren't depending on the AWS_LAMBDA_FUNCTION_NAME environment variable being set to test_function.

For the invoke URL change, I propose that if AWS_LAMBDA_FUNCTION_NAME is set, we have the server listen on both the hardcoded function path and the one generated from the environment variable. It will use the same handler function for both. If the variable isn't set, we simply listen on the function path.

This approach keeps backwards compatibility and makes it easier to remove the broken behaviour down the road. I will ensure I minimise any duplicated logic. Does this work for you and the team? I'd like to see the old hardcoded function path url when the variable is set marked as deprecated in the docs, or is that too soon?

[valerena]

@skwashd The team's stance on this is that RIE wasn't launched to be an exact copy of Lambda's Control Plane, so the current behavior is not "broken", and it's just "the way that RIE works". So, we shouldn't make any changes to the "default behavior", even if we think not many people use that. Every change that could break existing customers should be activated through a specific flag passed, and we won't change the recommended path url for now, even if the other url is also available.

It's possible than during next year we will have a longer discussion about the future here and potentially add your recommendations as the default behavior, but at this moment we don't have the resources to start that conversation.

[skwashd]

@valerena lets agree to disagree on inconsistent behaviour is a bug or a feature. I've incorporated the discussions we've discussed here. All the existing tests pass and I added tests to exercise the new functionality.

[skwashd]

@valerena any update on this one?

[swantzter]

Hi, @valerena any updates on this? We're running into issues due to not being able to change the function name when trying to invoke the function from AWS step functions local this is making it hard for us to develop and run test locally

[valerena]

Hi. I'm really sorry this is taking so long. Our team is very very short on resources, and at the moment we only have this package in maintenance mode, only upgrading the version of Go because of security related findings.

The change overall looks good, but we don't have the bandwidth to review it thoroughly, merge it, release it, and monitor in the future if this causes any unintended consequences that would cause us to have to revert it back.

I can't promise any dates, but I can tell that we won't be able to take this during June either, but we'll see during July how are things by then and potentially have bandwidth to take a deeper look.

I'm sorry this is affecting your flows. Maybe you can compile the binary with these changes and use it manually instead of using the official version for now as a workaround (if that works for your use case), and I hope we can get enough resourcing in July to dedicate more time to this repository.

[swantzter]

Hi @valerena, just wanted to check in on this and see if there's any potential for progress, but given the commit history I assume the situation for your team hasn't gotten to a better state yet?

[skwashd]

@valerena Can I please get an update on this?

[cppntn]

Can you guys finally merge this please?

[timini]

hit the button, merge it!

[calh]

I'm getting into developing step functions, developing and testing locally along with executing local lambda functions using this. This feature really should have been in the RIE from the first release. I don't see how a hard coded function name is usable when doing test driven development.

Please merge this @valerena! I really want to give AWS my money, but my wallet is stuck because it only responds to the name Wallet, not function.

[skwashd]

Given the recent activity in the repo is appears the team has resources again. @valerena can I please get an update? It's been over 2 years since you last responded.

[calh]

Hey @skwashd, would you be willing to pull AWS's commits into your dynamic-url branch and publish a release of your code? You're the captain now!

[skwashd]

@calh I'm not going to expend any more energy on this until there is a clear commitment from AWS to accept it. Given the radio silence from @valerena I don't think it is going to be accepted.

[valerena]

Hi everyone. I apologize for not replying earlier and for the silence for so long. The last few months we've been able to work on this again, mostly on security-related improvements, but I'd say we are in a position today where we can actually review and try to bring this in.

I think last time I looked at this, I got stuck trying to suggest a better naming than AWS_LAMBDA_RIE_INCONSISTENT_BEHAVIOUR, because it's confusing and it's unclear what the inconsistency is. Also, I think a positive env var is more appropriate instead of having to set something to "false". I suggest changing it to AWS_LAMBDA_RIE_CONSISTENT_FUNCTION_NAME, that has to be set to TRUE (or to any value, really)

There are currently some conflicts (the tests have changed a lot), so if you can update the PR with those changes and the variable name, I can take a look and see how we can include this in an upcoming release (better late than never?).

Do you think you can do that, @skwashd ? And apologies again.

[skwashd]

I went with AWS_LAMBDA_RIE_DYNAMIC_FUNCTION_URL as it is more descriptive and is a positive name. Calling it AWS_LAMBDA_RIE_CONSISTENT_FUNCTION_NAME is less clear. I look forward to it being merged soon.

I'd love to see this fixed properly in v2. It shouldn't require two environment variables to make RIE handle URLs the same way as Lambda.

[skwashd]

I'll try to look at this again in the coming days