add context variables and stage variables to API NG context #11111

bentsku · 2024-06-28T00:00:55Z

Motivation

We need a separate context, representing the $context variables used to render VTL templates, and which is also passed in the payload to AWS_PROXY integrations.

More informations about $context here.
https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-mapping-template-reference.html#context-variable-reference

It is a bit of duplicated information from the original RestApiInvocationContext object and the InvocationRequest, but they serve different purposes.

If we need to update different contexts at the same time, then maybe we could simplify. But we learned that creating those context when needed, collecting informations from different sources, can be pretty hard.

The InvocationRequest goal is simply to represent the incoming request "as is", with no transformation.

The idea would be to go from InvocationRequest, get the Resource. Use the original InvocationRequest, the Resource,the ContextVariables and StageVariables to construct the IntegrationRequest and send it down.
The InvocationRequest will not be mutated. The ContextVariables will be mutated and populated along the way of the different handlers.

Changes

add the ContextVariables and populate it
add the not yet used LoggingContextVariables which can add more data to the logging
add IntegrationRequest for future work in integration request handler
add the stage variables and fetch them from the stage
add small tests to validate we're populate the contexts properly

TODO

What's left to do:

add unit tests

github-actions · 2024-06-28T00:19:47Z

LocalStack Community integration with Pro

2 files ± 0 2 suites ±0 22m 58s ⏱️ - 1h 14m 50s
643 tests - 2 517 598 ✅ - 2 162 45 💤 - 351 0 ❌ ±0
645 runs - 2 517 598 ✅ - 2 162 47 💤 - 351 0 ❌ ±0

Results for commit 6a1308d. ± Comparison against base commit adc580c.

This pull request removes 2517 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! I can see so much extra code and repeated functionality we will be able to remove by having the context variables built and maintained together like that!

The split in integration request already makes it very clear as to what is meant to happen and this clear display of the logic's order will help maintaining the service 🚀

Nice additions to the tests too 😉

cloutierMat · 2024-06-28T16:30:33Z

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

+class IntegrationRequest(TypedDict, total=False):
+    http_method: Optional[HTTPMethod]
+    """HTTP Method of the incoming request"""
+    uri: Optional[str]
+    """URI of the integration"""
+    query_string_parameters: Optional[dict[str, str]]
+    """Query string parameters of the request"""
+    headers: Optional[dict[str, str]]
+    """Headers of the request"""
+    multi_value_query_string_parameters: Optional[dict[str, list[str]]]
+    """Multi value query string parameters of the request"""
+    multi_value_headers: Optional[dict[str, list[str]]]
+    """Multi value headers of the request"""
+    body: Optional[bytes]
+    """Body content of the request"""


Having that clear separation from the invocation request will be key in maintaining transparency over the requests.

cloutierMat · 2024-06-28T16:47:08Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/parse.py

+            requestTimeEpoch=int(now.timestamp() * 1000),
+            stage=context.stage,
+        )
+        return context_variables


Should we log this dict immediately?

Done! Good catch! We can revisit the overall logging later on when we have a clear picture, to make it a consistent experience, but for now simply logging it is really good. I believe we will also realize later on some values needs default values, maybe $context.identity for example?

cloutierMat · 2024-06-28T16:49:50Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py

+    def update_context_variables_with_resource(
+        context_variables: ContextVariables, resource: Resource
+    ):
+        # TODO: log updating the context_variables?


It is fine to leave as a TODO, but I think it would really useful to log them in debug mode.

cloutierMat · 2024-06-28T16:51:30Z

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

+    # this is merged with the Context returned by the Authorizer, which can attach any property to this dict in string
+    # format


Good catch!

bentsku

Thanks a lot for the review! 🙏 good comments about the logging, I've looked into #11114 and took the same format as you, it was very good 😄

bentsku · 2024-07-01T14:07:31Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/parse.py

+            requestTimeEpoch=int(now.timestamp() * 1000),
+            stage=context.stage,
+        )
+        return context_variables


Done! Good catch! We can revisit the overall logging later on when we have a clear picture, to make it a consistent experience, but for now simply logging it is really good. I believe we will also realize later on some values needs default values, maybe $context.identity for example?

bentsku · 2024-07-01T14:07:37Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/resource_router.py

+    def update_context_variables_with_resource(
+        context_variables: ContextVariables, resource: Resource
+    ):
+        # TODO: log updating the context_variables?


bentsku added 2 commits June 28, 2024 01:59

add contexts

01e00e3

update contexts

0fb41de

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

bentsku self-assigned this Jun 28, 2024

populate contexts

1216a74

bentsku force-pushed the apigw-add-contexts branch from 8ee5867 to 1216a74 Compare June 28, 2024 00:01

add todo

7e15dac

bentsku mentioned this pull request Jun 28, 2024

make next-gen APIGW provider default #11035

Merged

2 tasks

bentsku added 2 commits June 28, 2024 15:03

add tests

f105057

update tests

f96581c

bentsku marked this pull request as ready for review June 28, 2024 13:21

bentsku requested a review from cloutierMat as a code owner June 28, 2024 13:21

cloutierMat approved these changes Jun 28, 2024

View reviewed changes

cloutierMat mentioned this pull request Jun 28, 2024

APIGW NG implement api key validation handler #11114

Merged

1 task

bentsku commented Jul 1, 2024

View reviewed changes

address PR comments about logging

6a1308d

bentsku merged commit be9149c into master Jul 1, 2024
31 checks passed

bentsku deleted the apigw-add-contexts branch July 1, 2024 14:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

add context variables and stage variables to API NG context #11111

add context variables and stage variables to API NG context #11111

Uh oh!

bentsku commented Jun 28, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Jun 28, 2024 •

edited

Loading

Uh oh!

cloutierMat left a comment

Uh oh!

cloutierMat Jun 28, 2024

Uh oh!

cloutierMat Jun 28, 2024

Uh oh!

bentsku Jul 1, 2024

Uh oh!

cloutierMat Jun 28, 2024

Uh oh!

bentsku Jul 1, 2024

Uh oh!

cloutierMat Jun 28, 2024

Uh oh!

bentsku left a comment

Uh oh!

bentsku Jul 1, 2024

Uh oh!

bentsku Jul 1, 2024

Uh oh!

Uh oh!

Uh oh!

		# this is merged with the Context returned by the Authorizer, which can attach any property to this dict in string
		# format

Uh oh!

add context variables and stage variables to API NG context #11111

add context variables and stage variables to API NG context #11111

Uh oh!

Conversation

bentsku commented Jun 28, 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 Jun 28, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bentsku left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bentsku commented Jun 28, 2024 •

edited

Loading

github-actions bot commented Jun 28, 2024 •

edited

Loading