aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.67k stars 3.92k forks source link

aws_apigateway: LambdaRestApi does not respect the proxy: false property #21787

Open jglesner opened 2 years ago

jglesner commented 2 years ago

Describe the bug

Hello! I want to use the LambdaRestApi construct and use a non-proxy integration. Therefore, I set the proxy prop to false as shown in the LambdaRestApi documentation to create an API GW resource and POST method.

   const gateway = new apigateway.LambdaRestApi(
      this,
      "APIGW_MobileApiTestEndpoint",
      {
        handler: mobileApi_test_Lambda,
        endpointConfiguration: {
          types: [apigateway.EndpointType.REGIONAL],
        },
        proxy: false,
      }
    );

   const testResource = gateway.root.addResource("test");
   testResource.addMethod("POST"); 

Trouble is, CDK deploys the stack with the API Gateway Integration Request boolean 'Use Lambda Proxy Integration' set to True. Testing this produces the result you would expect for a Lambda that isn't expecting a proxy request ... it fails. When testing that Resource using the console, the API Gateway Test Log also shows that it's sending the full proxy payload. And you can plainly see it is configured as a LAMBDA_PROXY in the method’s Integration Request window in the console:

Screen Shot 2022-08-26 at 7 39 50 PM

This bug was reported back in February 2021 in issue #13026. At that time @nija-at tested it, and said it was working normally. However, in his test he actually demonstrated that this is a bug ... he just didn't seem to realize it. In his sample nodejs code, @nija-at is returning the status code in the response return { statusCode: 200, body: "Hello world!" } which is what you have to do when the lambda integration IS configured to be a proxy, even though he set Proxy to false in his test. In a non-proxy lambda integration, you don't return the status code. Others have reported this bug on ticket 13026 as well.

I would greatly appreciate it if we can get a fix for this!

Expected Behavior

I expect the 'proxy: false' property to be respected by the LambdaRestApi construct.

Current Behavior

The 'proxy: false' property of the LambdaRestApi construct is not respected.

Reproduction Steps

Shown in the description above.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.39.0

Framework Version

2.39.0

Node.js Version

14.17.0

OS

OS X Monterey 12.1

Language

Typescript

Language Version

3.9.7

Other information

No response

peterwoodworth commented 2 years ago

Synthing the code provided, it generates a Method resource with Integration type AWS_PROXY

The code for LambdaRestApi generates a LambdaIntegration, and doesn't change what gets passed in for integrationOptions.Proxy, which defaults to true no matter what you set for the proxy prop on LambdaRestApi.

https://github.com/aws/aws-cdk/blob/e36bfe5fe17d8e4e4a31903c95c9482346cf99ba/packages/%40aws-cdk/aws-apigateway/lib/lambda-api.ts#L51-L58

So in short, i think we will need to ensure the LambdaIntegration created is created with proxy: false as well. You can set this with options currently, all that setting integrationOptions.Proxy to false will do is change AWS_PROXY in the template to AWS. Does this give the desired behavior?

    const gateway = new apigateway.LambdaRestApi(
      this,
      "APIGW_MobileApiTestEndpoint",
      {
        handler: mobileApi_test_Lambda,
        endpointConfiguration: {
          types: [apigateway.EndpointType.REGIONAL],
        },
        proxy: false,
        integrationOptions: { // added
          proxy: false // added
        } // added
      }
    );
jglesner commented 2 years ago

Thank you for the quick reply @peterwoodworth. I just tested that out -- the lambda integration was successfully converted to a non-proxy lambda. So your suggestion was very helpful, and I agree that the LambdaRestApiProps proxy: false should be fixed.

However, after verifying the work around was successful at converting the API GW method to a non-proxy integration, my tests were still failing. When I test within the API GW console, I now see this error when testing the POST resource method I defined:

Execution failed due to configuration error: No match for output mapping and no default output mapping configured. Endpoint Response Status Code: 200

So even a success response from the lambda is converted to a 500 response from API Gateway. Researching this on the internet points to missing Method Responses and Integration Responses. And indeed when I look at the POST method in the API GW console, I can see that there are no method responses or integration responses configured:

Screen Shot 2022-08-26 at 10 52 07 PM

Not sure if this is a by-product of converting an API GW Integration from proxy to non-proxy, or if this would also happen when freshly deploying this non-proxy resource. The documentation doesn't mention having to define Method Responses or Integration Responses; this might need to be clarified.

Looking at the LambdaRestApiProps, it appears possible to set up defaultMethodOptions and integrationOptions to configure the IntegrationRequest, IntegrationResponse, and MethodResponse. Will give this a try.

peterwoodworth commented 2 years ago

Let me know if you can get something working with integrationOptions @jglesner!

jglesner commented 2 years ago

Hello @peterwoodworth. So I spent some serious time today trying to get the IntegrationRequest, IntegrationResponse, and MethodResponse all configured correctly to work for a non-proxy method. I finally found one way to make it all work as intended. But it is a little convoluted, and I think there may be another bug here as well.

First, what didn't work. As I said in my post above, I was hoping to use the LambdaRestApiProps to set up defaultMethodOptions and integrationOptions to configure the IntegrationRequest, IntegrationResponse, and MethodResponse. integrationOptions did work as intended, however, defaultMethodOptions seems to be partially ignored.

  const gateway = new apigateway.LambdaRestApi(
      this,
      "APIGW_MobileApiTestEndpoint",
      {
        handler: mobileApi_test_Lambda,
        endpointConfiguration: {
          types: [apigateway.EndpointType.REGIONAL],
        },
        proxy: false, // probably not needed until this gets fixed
        integrationOptions: {
           proxy: false,
           integrationResponses: [{ statusCode: "200" }],
           passthroughBehavior: PassthroughBehavior.WHEN_NO_TEMPLATES,
        },
        defaultMethodOptions: {
           methodResponses: [
           {
              statusCode: "200",
              responseModels: {
                 "application/json": apigateway.Model.EMPTY_MODEL,
              },
           },
        ],
      },
    }
  );

Despite defining the defaultMethodOptions methodResponses within the LambdaRestApi, the MethodResponse wasn't configured as expected, and API GW throws a 500 even with a successful response from the Lambda function. And, from the test page in the API GW console, the system reports the same error about missing mappings:

Execution failed due to configuration error: No match for output mapping and no default output mapping configured. Endpoint Response Status Code: 200

So I think defaultMethodOptions prop is buggy, and needs to be fixed. I would also recommend adding some documentation to explain that wiring up the request/response is needed to configure a non-proxy integration.

To get this to work as intended, I needed to move my attempts to configure the IntegrationRequest, IntegrationResponse, and MethodResponse out of the LambdaRestApi altogether. As you'll notice below, I needed to configure a LambdaIntegration, and then pass that to the resource POST addMethod, and configure the methodResponses there. Here is the code that finally worked.

    const gateway = new apigateway.LambdaRestApi(
      this,
      "APIGW_MobileApiTestEndpoint",
      {
        handler: mobileApi_test_Lambda,
        endpointConfiguration: {
          types: [apigateway.EndpointType.REGIONAL],
        },
        proxy: false, // probably not necessary until fixed
      }
   );

   const integration = new apigateway.LambdaIntegration(
      mobileApi_test_Lambda,
      {
        integrationResponses: [{ statusCode: "200" }],
        passthroughBehavior: PassthroughBehavior.WHEN_NO_TEMPLATES,
        proxy: false,
      }
    );

    const testResource = gateway.root.addResource("test");
    testResource.addMethod("POST", integration, {
      methodResponses: [
        {
          statusCode: "200",
          responseModels: {
            "application/json": apigateway.Model.EMPTY_MODEL,
          },
        },
      ],
    });

It might be possible to keep the LambdaIntegration / integrationOptions inside the LambdaRestApiProps and pass 'undefined' for the Integration on the addMethod -- I haven't tested that. Once I started going down this path, it seemed logical to set all configuration on addMethod to see if MethodOptions worked from this location. While this does work, it took quite a while to figure this out through trial and error, and feels more complicated than it should be to set up a non-proxy Lambda integration -- or at the very least not documented well.

Welcome your thoughts.

peterwoodworth commented 2 years ago

Thanks for the thorough investigation here, it's really helpful 🙂 It might take us a while to get to this though. If you have any ideas on how to improve this documentation and submit that in a PR, or even any redesigns, it would be super helpful and much appreciated! Check out our contributing guide if you're interested

miguelmartin-eb commented 7 months ago
const testResource = gateway.root.addResource("test");
testResource.addMethod("POST", integration, {
methodResponses: [
 {
     statusCode: "200",
    responseModels: {
      "application/json": apigateway.Model.EMPTY_MODEL,
   },
  },
],
});

HI there @jglesner ! Did you add any other config for this to work?

I'm currently using the exact same and, despite the method response showing correctly in the console, the api gateway still throws this error :/

Execution failed due to configuration error: No match for output mapping and no default output mapping configured. Endpoint Response Status Code: 202

The only difference I can spot is that my lambda is returning a 202 instead of a 200.

ChristianUlbrich commented 2 months ago

@miguelmartin-eb For me this is working now, many thanks to @jglesner code example. While the bug persists still today - proxy: false basically gets ignored, the problem seems to be more apparent, if you are using a custom Lambda runtime, such as provided.al2023 or Runtime.PROVIDED_AL2023 in CDK lingo.

If using a "normal" NodeJs runtime, the following:

// [...] in a construct...
 const myFunction = new lambda.Function(this, 'HelloWorldFunction', {
      runtime: lambda.Runtime.NODEJS_20_X, // Provide any supported Node.js runtime
      handler: 'index.handler',
      code: lambda.Code.fromInline(`
        exports.handler = async function(event) {
          return {
            statusCode: 200,
            body: JSON.stringify('Hello CDK!'),
          };
        };
      `),
    });

const functionApi = new apigateway.LambdaRestApi(this, 'SimpleHelloWorldApi', {
      handler: myFunction,
      proxy: false,
    });

    const helloResource = functionApi.root.addResource('hello');
    helloResource.addMethod('GET');

works. It seems, that for custom runtimes, you need additional configuration, that gets set by default if using the AWS console for API Gateway, whereas the default from the LambdaRestApi does not have the same defaults and on the other hand, internally non-custom runtimes seem to instructed differently in AWS, than custom ones, i.e. they do not need the additional stuff (integrationResponse, methodResponse) that custom ones need.

When setting up RestAPIs for non-custom runtimes in AWS, they get the same defaults and yet they work, so I think the core problem is, that LambdaRestApi does not set the same defaults as one might expect. I think a potential fix might be a new type, such as DefaultLambdaRestApi, that behaves exactly as the AWS console does and would thus allow to work with both custom and non-custom runtimes or at least match expectations better.