dherault / serverless-offline

Emulate AWS λ and API Gateway locally when developing your Serverless project
MIT License
5.2k stars 793 forks source link

solves an issue when using path parameters and policy based resource access limits #1633

Closed zoellner closed 5 months ago

zoellner commented 1 year ago

Description

This is a reintroduction of #1283 to fix bug introduced in https://github.com/dherault/serverless-offline/commit/5ff81fb318a741f7d58f83ae4e550717c4d3bbbc

Now based on latest master with tests passing.

It solves an issue when using path parameters and policy based resource access limits.

Motivation and Context

Fixes #1043 Fixes #1586

How Has This Been Tested?

used in our own application. working again as before the bug was introduced

Screenshots (if appropriate):

zoellner commented 1 year ago

@dherault there are two tests failing but I am not sure what the tests are supposed to test and have a feeling they might have been put in place based on previous responses without any real requirement for that outcome. e.g. this one:

    {
      description: 'event.resource should not contain wildcards',
      expected: '/{id}/test-resource-variable-handler',
      path: '/1/test-resource-variable-handler',
      status: 200,
    }

it's not clear to me why {id} would be expected in the path

dnalborczyk commented 1 year ago

thank you for the PR @zoellner

there are two tests failing but I am not sure what the tests are supposed to test and have a feeling they might have been put in place based on previous responses without any real requirement for that outcome. e.g. this one:

I looked at those failing tests briefly, incl. deployment, and it seems they behave as expected. only thing is that those don't belong in the handler tests section, as those are solely testing the different handler implementations (e.g. callback, context.done, Promises/async functions).

that said, I might be mistaken, what do you expect the outcome of those tests to be?

zoellner commented 1 year ago

I would expect the same as I'm getting when logging the event in AWS Lambda and there I see methodArn and path having path parameters replaced but resource not and in the requestContext: resroucePath without replacement, path with replacement

a logged request from AWS Lambda, adjusted to this example and sanitized

{
    "type": "REQUEST",
    "methodArn": "arn:aws:execute-api:region:accountId:gatewayid/STAGE/GET/1/test-resource-variable-handler",
    "resource": "/{id}/test-resource-variable-handler",
    "path": "/prefix/1/test-resource-variable-handler",
  ...
     "requestContext": {

        "resourcePath": "/{id}/test-resource-variable-handler",
        "httpMethod": "GET",
        "path": "/prefix/1/test-resource-variable-handler",
        ...
    }
}
dnalborczyk commented 1 year ago

I looked at this failing test as a start: https://github.com/dherault/serverless-offline/actions/runs/3785736032/jobs/6436014680

1) star routes
       when a catch all route is used in a rest api
         it should return a payload:

      AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:

{
  path: '/hello',
  resource: '/hello'
}

should loosely deep-equal

{
  path: '/hello',
  resource: '/{proxy+}'
}
      + expected - actual

       {
         "path": "/hello"
      -  "resource": "/hello"
      +  "resource": "/{proxy+}"
       }

      at Context.<anonymous> (file:///home/runner/work/serverless-offline/serverless-offline/tests/end-to-end/starRoutesRestApi/starRoutesRestApi.test.js:24:14)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

I went to: /tests/end-to-end/starRoutesRestApi/src

and deployed with npx serverless deploy

then fetched the endpoint: https://xxxxxxx.execute-api.us-east-1.amazonaws.com/dev/foo

the response:

{
  "path": "/foo",
  "resource": "/{proxy+}"
}

then I started the local process with npx serverless offline start

fetched http://localhost:3000/dev/foo

the response:

{
  "path": "/foo",
  "resource": "/{proxy+}"
}

there are two tests failing but I am not sure what the tests are supposed to test

in this case it seems just the path and the resource of the event object: https://github.com/dherault/serverless-offline/blob/606fab5ef340fe86166b71c63e6e87cf9024452f/tests/end-to-end/starRoutesRestApi/src/handler.js#L3