(apigatewayv2) Support abstraction for responseParameters

gitrealname commented 2 years ago

Describe the bug

Having the following cdk code: this._webLambdaProxyIntegration = new CfnIntegration(this,, { payloadFormatVersion: apiGateway.PayloadFormatVersion.VERSION_1_0.version, apiId: this.webAPI.apiId, integrationType: apiGateway.HttpIntegrationType.AWS_PROXY, integrationMethod: apiGateway.HttpMethod.ANY, integrationUri: cfnAlias.ref, responseParameters: {"403" : {"overwrite:statuscode" : "401"}} });

which produces the following template: "<skipped>Integration": { "Type": "AWS::ApiGatewayV2::Integration", "Properties": { "ApiId": { "Ref": "<skipped>API327D3D35" }, "IntegrationType": "AWS_PROXY", "IntegrationMethod": "ANY", "IntegrationUri": { "Ref": "<skipped>AliasBC47D6CA" }, "PayloadFormatVersion": "1.0", "ResponseParameters": { "403": { "overwrite:statuscode": "401" } } }, "Metadata": { "aws:cdk:path": "<skipped>" } },

fails during deployment with the following error: Property validation failure: [Encountered unsupported properties in {/ResponseParameters/403}: [overwrite:statuscode]]

Stack trace: failed: Error: The stack named failed to deploy: UPDATE_ROLLBACK_COMPLETE: Property validation ailure: [Encountered unsupported properties in {/ResponseParameters/403}: [overwrite:statuscode]] failure: [Encountered unsupported properties in {/ResponseParameters/403}: [overwrite:statuscode]] at prepareAndExecuteChangeSet (\node_modules\aws-cdk\lib\api\deploy-stack.ts:385:13) at processTicksAndRejections (internal/process/task_queues.js:95:5) at CdkToolkit.deploy (\node_modules\aws-cdk\lib\cdk-toolkit.ts:209:24) at initCommandLine (\node_modules\aws-cdk\lib\cli.ts:341:12)

Cdk version used "@aws-cdk/aws-apigatewayv2-alpha": "2.23.0-alpha.0", "@aws-cdk/aws-apigatewayv2-authorizers-alpha": "2.23.0-alpha.0", "@aws-cdk/aws-apigatewayv2-integrations-alpha": "2.23.0-alpha.0", "aws-cdk": "2.23.0", "aws-cdk-lib": "2.23.0", "constructs": "10.1.6",

Expected Behavior

ResponseParameters mapping to deploy.

Current Behavior

fails to deploy

Reproduction Steps

deploy integration with responseParameters mapping

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.23.0 (build 50444aa)

Framework Version

na

Node.js Version

14.19.0

OS

windows

Language

Typescript

Language Version

4.5.5

Other information

No response

peterwoodworth commented 2 years ago

Hey @gitrealname,

I see you're using CfnIntegration, which since it's an L1 resource (prefixed with Cfn) is an exact copy of CloudFormation's ApiGatewayV2::Integration, which means you should take a look at CloudFormation docs for full explanations on how to format this property

There's an example down below on this page which shows how to properly format ResponseParameters

Here's another full example of how to do this https://github.com/awsdocs/amazon-api-gateway-developer-guide/blob/main/cloudformation-templates/HTTP/http-api-parameter-mapping.yaml

github-actions[bot] commented 2 years ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

gitrealname commented 2 years ago

Thank you for you response @peterwoodworth. Could you please provide with exact code snippet that will replace 403 response with 401. thank you. Is it: ` ResponseParameters: "403": ResponseParameters:

Source: "401" Destination: "overwrite:statuscode" ` it is not very clear from the doc. At the same time when I try exporting "integration" from AWS console or try using aws cli the structure of this is similar to what I have tried and no "synth"/"build" errors were ever reported. I would be glad if I don't have to go "Cfn" level. Shell high level construct support this as well? https://docs.aws.amazon.com/cdk/api/v2/docs/@aws-cdk_aws-apigatewayv2-integrations-alpha.HttpLambdaIntegrationProps.html. It seems to me as a very fundamental integration issue. For example, without such a mapping no EvenBus scheduled events/rules, that call an API will ever work. Thank you.

erothvt commented 2 years ago

The appropriate snippet needs to look like this: responseParameters: { "403": { ResponseParameters: [{ Destination: 'overwrite:statuscode', Source: '401' }] } }

peterwoodworth commented 2 years ago

I would be glad if I don't have to go "Cfn" level. Shell high level construct support this as well?

I don't think we offer any abstractions for this currently - I agree, this would be a nice feature to have on our higher level Integration constructs so that you don't need to worry about this formatting! Marking this issue as a feature request for this

I am marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

codyfrisch commented 1 year ago

@peterwoodworth I've dug into this, and while its outside my area of expertise (I don't do much library work, and have never contributed to the CDK), seeing how the original abstraction in aws-apigatewayv2-alpha for requestParameters was done, I do not think this incredibly complex (technically), just tedious. (I could probably accomplish the actually functionality, but I honestly fail at writing tests, and with lack of CDK contribution experience I would definitely need mentoring)

If anything the hardest part is that the original abstraction was parameterMapping using the ParameterMapping class and that maps to requestParameters on CfnIntegration. The ParameterMapping class is not sufficiently abstracted to support both request and response without breaking changes (or requiring seemingly redundant methods that will cause confusion). Seems like the original implementation may have been a bit short-sighted.

It appears that the existing ParameterMapping class will need to be deprecated, as even though it is alpha, I don't think anyone wants breaking changes at this point.

peterwoodworth commented 1 year ago

Thanks @codyfrisch, I don't think we have the bandwidth to help with the design right now, but in the future we'll certainly take a look at this when we push to graduate this module. If you have a design and all you need is support with contributing, let me know and I can help with that

codyfrisch commented 1 year ago

@peterwoodworth I have design plans, but like everyone, its about the time to implement. The support with contributing would definitely be appreciated though, esp. with tests. I also don't want to waste anyone's time in case I come up short. Looking at the problem of responses, I can see why it was skipped originally - it raises some additional complexities not applicable to requests.

I will take a stab at it and see what I can accomplish. All this just to be able to forego putting strict-transport-security headers in every response.

aws / aws-cdk