Skip to content

APIGW NG implement URI rendering #11140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jul 5, 2024
Merged

APIGW NG implement URI rendering #11140

merged 4 commits into from
Jul 5, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Jul 4, 2024 •
edited
Loading

Motivation

Throughout APIGW invocation, there are several places where you can use stageVariables inside some URI and ARN.
We also need to interpolate path parameters inside the Integration URI

This PR implements 2 utils, to render stage variables following the VTL syntax, and another one to render path parameters, only in curly braces.
The Integration Request handler combines those 2 utils to render the Integration URI.

More informations about stage variables here:
https://docs.aws.amazon.com/apigateway/latest/developerguide/aws-api-gateway-stage-variables-reference.html#stage-variables-in-integration-HTTP-uris

We will also use the utils inside the Integration for the credentials, and in the cognito authorizer to render the UserPool.

Changes

  • implement stage variables rendering util
  • implement path parameters rendering util
  • add unit tests for the URI rendering
  • renamed a variable inside the template mappings due to a PyCharm linting, as we were overriding the builtin vars

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Jul 4, 2024
@bentsku bentsku self-assigned this Jul 4, 2024
@bentsku bentsku requested a review from cloutierMat as a code owner July 4, 2024 14:27
@bentsku bentsku mentioned this pull request Jul 4, 2024
Merged
2 tasks
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 4, 2024 •
edited
Loading

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   23m 55s ⏱️ - 1h 11m 20s
655 tests  - 2 515  610 ✅  - 2 165  45 💤  - 350  0 ❌ ±0 
657 runs   - 2 515  610 ✅  - 2 165  47 💤  - 350  0 ❌ ±0 

Results for commit f99f8b2. ± Comparison against base commit a358ab8.

This pull request removes 2515 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
cloutierMat reviewed Jul 4, 2024
View reviewed changes
Copy link
Contributor

@cloutierMat cloutierMat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice PR! Well tested!

I just left a question about how to fail when a path still contains {path}, but nothing blocking. We can always get back to it when we do the integration.

...ack-core/localstack/services/apigateway/next_gen/execute_api/handlers/integration_request.py
@@ -66,12 +67,16 @@ def __call__(
body, request_override = self.render_request_template_mapping(
context=context, template=request_template
)
# TODO: log every override that happens afterwards (in a loop on `request_override`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I missed that one!

tests/unit/services/apigateway/test_uri_interpolation.py


class TestUriInterpolationStageVariables:
@pytest.mark.parametrize(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of parametrize, really quick and easy to see what is going on!

tests/unit/services/apigateway/test_uri_interpolation.py Outdated
Comment on lines 84 to 90
def test_uri_interpolating_with_no_variable(self):
uri_with_only_braces = "https://test.domain.example/root/${path}"
stage_variables = {}
rendered = render_uri_with_stage_variables(
uri=uri_with_only_braces, stage_variables=stage_variables
)
assert rendered == "https://test.domain.example/root/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice I was wondering if that was the result we would get! 👍
I assume the same happens if you use ${stageVariables.path}?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Good catch! This is actually wrong, this should be the result when using ${stageVariables.path}, I believe if we were to use ${path} it might just try to interpolate it as a path param, so if no path param named path it will leave it to ${path}, otherwise might render $<path-param>, I need to try it... oops 😬

@bentsku bentsku force-pushed the apigw-ng-http-integration branch from f3f7376 to f02f622 Compare July 5, 2024 00:26
@bentsku bentsku merged commit d3aff2d into master Jul 5, 2024
32 checks passed
@bentsku bentsku deleted the apigw-ng-http-integration branch July 5, 2024 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cloutierMat cloutierMat cloutierMat approved these changes

Assignees

@bentsku bentsku

Labels
aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bentsku @cloutierMat