Skip to content

APIGW: fix MOCK integration with no URI and path parameters #11784

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 1 commit into from
Nov 5, 2024
Merged

APIGW: fix MOCK integration with no URI and path parameters #11784

merged 1 commit into from
Nov 5, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Nov 5, 2024

Motivation

We got a report in our Slack Community channel about the legacy implementation. While switching to NextGen, another issue happened. This setup is from an exported OpenAPI spec from an AWS REST API.

2024-11-05T10:59:03.146  WARN --- [et.reactor-0] localstack.deprecations    : /restapis/<api_id>/<stage>/_user_request_ is deprecated (since 3.8.0) and will be removed in upcoming releases of LocalStack! Use /_aws/execute-api/<api_id>/<stage> instead.
2024-11-05T10:59:03.146935385Z 2024-11-05T10:59:03.146 DEBUG --- [et.reactor-0] l.s.a.n.execute_api.router : APIGW v1 Endpoint called
[...removed]
2024-11-05T10:59:03.165331385Z 2024-11-05T10:59:03.162  WARN --- [et.reactor-0] l.s.a.n.e.h.gateway_except : Non Gateway Exception raised: 'NoneType' object has no attribute 'replace'
2024-11-05T10:59:03.165356385Z Traceback (most recent call last):
2024-11-05T10:59:03.165381385Z   File "/opt/code/localstack/.venv/lib/python3.11/site-packages/rolo/gateway/chain.py", line 166, in handle
2024-11-05T10:59:03.165403385Z     handler(self, self.context, response)
2024-11-05T10:59:03.165425385Z   File "/opt/code/localstack/localstack-core/localstack/services/apigateway/next_gen/execute_api/handlers/integration_request.py", line 140, in __call__
2024-11-05T10:59:03.165447385Z     rendered_integration_uri = render_integration_uri(
2024-11-05T10:59:03.165468385Z                                ^^^^^^^^^^^^^^^^^^^^^^^
2024-11-05T10:59:03.165492385Z   File "/opt/code/localstack/localstack-core/localstack/services/apigateway/next_gen/execute_api/helpers.py", line 86, in render_integration_uri
2024-11-05T10:59:03.165512385Z     uri_with_path = render_uri_with_path_parameters(uri, path_parameters)
2024-11-05T10:59:03.165531385Z                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2024-11-05T10:59:03.165553385Z   File "/opt/code/localstack/localstack-core/localstack/services/apigateway/next_gen/execute_api/helpers.py", line 63, in render_uri_with_path_parameters
2024-11-05T10:59:03.165575385Z     uri = uri.replace(f"{{{key}}}", value)
2024-11-05T10:59:03.165596385Z           ^^^^^^^^^^^
2024-11-05T10:59:03.165618385Z AttributeError: 'NoneType' object has no attribute 'replace'
2024-11-05T10:59:03.166060385Z 2024-11-05T10:59:03.165  INFO --- [et.reactor-0] localstack.request.http    : POST /restapis/<api-id>/localstack/_user_request_/v1/journeys/<redacted>/applications => 500

Trying to understand what is happening, it seems this issue happens when a MOCK integration does not have a URI (normal behavior) and also has integration path parameters.

The issue is fixed by skipping the logic and returning early. Those helper functions can be used alone or in combination, so the logic is in all of them.

Changes

  • fixed the logic when the URI is None
  • added a test to validate the fix and prevent regression

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Nov 5, 2024
@bentsku bentsku self-assigned this Nov 5, 2024
@bentsku bentsku requested a review from cloutierMat as a code owner November 5, 2024 13:19
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 5, 2024

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   25m 50s ⏱️ - 1h 16m 49s
819 tests  - 2 713  759 ✅  - 2 359  60 💤  - 354  0 ❌ ±0 
821 runs   - 2 713  759 ✅  - 2 359  62 💤  - 354  0 ❌ ±0 

Results for commit f0207ec. ± Comparison against base commit 17156d0.

This pull request removes 2714 and adds 1 tests. Note that renamed tests count towards both. 
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]
…

tests.aws.services.apigateway.test_apigateway_integrations ‑ test_integration_mock_with_path_param
This pull request removes 355 skipped tests and adds 1 skipped test. Note that renamed tests count towards both. 
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input4-FAILED]
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_deployed_infra_state
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_populate_data
tests.aws.scenario.mythical_mysfits.test_mythical_misfits.TestMythicalMisfitsScenario ‑ test_user_clicks_are_stored
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_api_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_exceptions
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_create_invalid_desiredstate
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_double_create_with_client_token
tests.aws.services.cloudcontrol.test_cloudcontrol_api.TestCloudControlResourceApi ‑ test_lifecycle
…

tests.aws.services.apigateway.test_apigateway_integrations ‑ test_integration_mock_with_path_param

cloutierMat
cloutierMat approved these changes Nov 5, 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 dig! 👍

tests/aws/services/apigateway/test_apigateway_integrations.py
Comment on lines -412 to -413
# TODO: Aws does not return the uri when creating a MOCK integration
@markers.snapshot.skip_snapshot_verify(paths=["$..not-required-integration-method-MOCK.uri"])
Copy link
Contributor

Choose a reason for hiding this comment

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

🥳

tests/aws/services/apigateway/test_apigateway_integrations.py
Comment on lines +588 to +590
requestParameters={
"integration.request.path.integrationPath": "method.request.path.testPath",
},
Copy link
Contributor

Choose a reason for hiding this comment

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

What a useful feature on a MOCK integration! 🤣

@bentsku bentsku merged commit 0b2850e into master Nov 5, 2024
37 checks passed
@bentsku bentsku deleted the fix-apigw-mock-uri-none branch November 5, 2024 18:52
zaingz pushed a commit that referenced this pull request Nov 11, 2024
@bentsku @zaingz
APIGW: fix MOCK integration with no URI and path parameters (#11784)
37962a8
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