APIGW NG add Integration Response selection and parameter mapping #11135

bentsku · 2024-07-03T16:51:59Z

Motivation

This PR starts implementing the Integration Response handler, who's responsible for selecting the right IntegrationResponse based on the response status code (or an errorMessage field very specific to the AWS integration with Lambda, which we will tackle later).

The handler should then do the response parameters mapping, then the VTL rendering of the body.
We do not test the handler yet, same as the Integration Request handler, as the behavior is incomplete yet.

Instead, we can focus on the integration response selection, and the response parameters mapping. All of the unit tests have been manually tested against AWS, with the console and a SSH tunnel with localhost.run to my own small flask server to be able to return arbitrary payload from my HTTP payload.

More informations about the different subjects of the PR:

Changes

implement the base of the Integration Response handler, still very WIP, with comments outlining what I currently think is happening
ported the current selectionPattern util to select the right integration response (thanks @dfangl for implementing it!) and unit test it
refactor the parameter mapping a bit to share code between request and response parameter mappings (code is still pretty similar, but has a few distinct difference, we could always refactor later on)
add the response parameter mapping logic
add unit tests for response parameter mapping

TODO

What's left to do:

add unit tests for Response Parameters by validating with AWS
add unit tests for selecting the integration response

github-actions · 2024-07-03T17:09:02Z

LocalStack Community integration with Pro

2 files ± 0 2 suites ±0 23m 31s ⏱️ - 1h 16m 14s
643 tests - 2 513 598 ✅ - 2 159 45 💤 - 350 0 ❌ ±0
645 runs - 2 513 598 ✅ - 2 159 47 💤 - 350 0 ❌ ±0

Results for commit b8e979d. ± Comparison against base commit 6ea7da3.

This pull request removes 2513 tests.

tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

♻️ This comment has been updated with latest results.

cloutierMat

🚀 Awesome PR. Nice structure to the handler.

I still have some doubt/questions around the AWS lambda integration and the differences in how it handles success/error, but I guess we will discover more as we complete it's handler. For now, it seems logic to proceed this way, as the error end up in the body for the mapping, so we need it here.

Really good and clear tests. Congrats!

cloutierMat · 2024-07-04T05:14:34Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/parameters_mapping.py

+
+        return response_data_mapping
+
+    def _retrieve_parameter_from_variables_and_static(


Nice! That will be ready for reuse in the gateway responses as well when we get there!

cloutierMat · 2024-07-04T05:17:39Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/parameters_mapping.py

+            try:
+                decoded_body = self._json_load(body)
+            except ValueError:
+                # x-amzn-errortype: InternalFailureException


cloutierMat · 2024-07-04T05:26:17Z

tests/unit/services/apigateway/test_parameters_mapping.py

+
+        # this was validated against AWS
+        assert mapping == {
+            "header": {"body_value": "{}"},


It's back 👻

bentsku · 2024-07-04T11:12:08Z

I still have some doubt/questions around the AWS lambda integration and the differences in how it handles success/error, but I guess we will discover more as we complete it's handler. For now, it seems logic to proceed this way, as the error end up in the body for the mapping, so we need it here.

@cloutierMat yes, it's a bit specific, but it seems it's well documented in the documentation, and it seems it really happens at the Integration Response level, as they talk about the fact that the Integration will always return 200, success or failure. It seems like a small hacky solution on AWS side too! But we can see better later, yes. It might be that the AWS lambda integration might have some specificity. Surprise later 😄

bentsku · 2024-07-04T11:20:02Z

...ck-core/localstack/services/apigateway/next_gen/execute_api/handlers/integration_response.py

+        # override them if there are overrides.
+        # for the body, it will maybe depend on configuration?
+
+        response.headers.clear()


@cloutierMat this might have been a bit too aggressive 😄 I think we might need to keep a few, HTTP related headers, like Content-Type, Content-Length, etc. Might need to keep a list, and selectively remove all other headers. We can quickly try it with an integration with no mappings I suppose!

bentsku added 2 commits July 3, 2024 17:54

first draft

5efe67f

add select logic

ba08d95

bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Jul 3, 2024

bentsku self-assigned this Jul 3, 2024

bentsku mentioned this pull request Jul 3, 2024

make next-gen APIGW provider default #11035

Merged

2 tasks

add tests and validation assumptions

d4e624d

bentsku force-pushed the apigw-ng-param-mapping-int-response branch from 5b79f77 to d4e624d Compare July 4, 2024 01:59

bentsku marked this pull request as ready for review July 4, 2024 02:01

bentsku requested a review from cloutierMat as a code owner July 4, 2024 02:01

fix type annotation that leads to exception

b8e979d

cloutierMat approved these changes Jul 4, 2024

View reviewed changes

bentsku merged commit a409563 into master Jul 4, 2024
32 checks passed

bentsku deleted the apigw-ng-param-mapping-int-response branch July 4, 2024 11:13

bentsku commented Jul 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

APIGW NG add Integration Response selection and parameter mapping #11135

APIGW NG add Integration Response selection and parameter mapping #11135

Uh oh!

bentsku commented Jul 3, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Jul 3, 2024 •

edited

Loading

Uh oh!

cloutierMat left a comment

Uh oh!

cloutierMat Jul 4, 2024

Uh oh!

cloutierMat Jul 4, 2024

Uh oh!

cloutierMat Jul 4, 2024

Uh oh!

bentsku commented Jul 4, 2024

Uh oh!

Uh oh!

bentsku Jul 4, 2024 •

edited

Loading

Uh oh!

Uh oh!


		return response_data_mapping

		def _retrieve_parameter_from_variables_and_static(

Uh oh!

APIGW NG add Integration Response selection and parameter mapping #11135

APIGW NG add Integration Response selection and parameter mapping #11135

Uh oh!

Conversation

bentsku commented Jul 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

TODO

Uh oh!

github-actions bot commented Jul 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LocalStack Community integration with Pro

Uh oh!

cloutierMat left a comment

Choose a reason for hiding this comment

Uh oh!

cloutierMat Jul 4, 2024

Choose a reason for hiding this comment

Uh oh!

cloutierMat Jul 4, 2024

Choose a reason for hiding this comment

Uh oh!

cloutierMat Jul 4, 2024

Choose a reason for hiding this comment

Uh oh!

bentsku commented Jul 4, 2024

Uh oh!

Uh oh!

bentsku Jul 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bentsku commented Jul 3, 2024 •

edited

Loading

github-actions bot commented Jul 3, 2024 •

edited

Loading

bentsku Jul 4, 2024 •

edited

Loading