Skip to content

APIGW: ParameterMapping skip if invalid input #12023

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
Dec 12, 2024
Merged

APIGW: ParameterMapping skip if invalid input #12023

merged 1 commit into from
Dec 12, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Dec 12, 2024 •
edited
Loading

Motivation

We've got a report from a user with this stack trace:

2024-12-11 15:56:52   File "/opt/code/localstack/.venv/lib/python3.11/site-packages/localstack/services/apigateway/next_gen/execute_api/parameters_mapping.py", line 59, in map_integration_request

2024-12-11 15:56:52     if request_mapping.startswith("method.request."):

2024-12-11 15:56:52        ^^^^^^^^^^^^^^^^^^^^^^^^^^

2024-12-11 15:56:52 AttributeError: 'bool' object has no attribute 'startswith'

However, it seems the only way we can get to those conditions is by specifying an invalid requestParameters object with boolean values. It seems AWS does not accept that, but as we are lacking way to reproduce and to be sure we are not going too far with provider validation, this PR only skips the error for now.

We need as a follow up to implement proper Parameter Mapping validation like we have done for HTTP APIs.

I've already added a test for the AWS validation, but we need to implement the validation in a follow up.

Changes

  • add a skipped test to verify that we cannot have any else than strings in the parameter mappings
  • skip the parameter mapping if the values aren't string, to avoid non-caught exceptions

TODO

What's left to do:

  • in a follow-up, add the provider validation so that it's not possible to get in those conditions in the invocation layer

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Dec 12, 2024
@bentsku bentsku added this to the 4.1 milestone Dec 12, 2024
@bentsku bentsku self-assigned this Dec 12, 2024
@bentsku bentsku requested a review from cloutierMat as a code owner December 12, 2024 11:06
@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0    2 suites  ±0   31m 14s ⏱️ - 1h 19m 58s
1 013 tests  - 2 879  956 ✅  - 2 627  57 💤  - 252  0 ❌ ±0 
1 015 runs   - 2 879  956 ✅  - 2 627  59 💤  - 252  0 ❌ ±0 

Results for commit cf54a88. ± Comparison against base commit 5949987.

This pull request removes 2880 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_api.TestApigatewayIntegration ‑ test_put_integration_request_parameter_bool_type
This pull request removes 253 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_api.TestApigatewayIntegration ‑ test_put_integration_request_parameter_bool_type

cloutierMat
cloutierMat approved these changes Dec 12, 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.

Thanks for adding the fix and a good test for the provider.

That should unblock the user for now while we implement proper validation in the provider! 🙏

@cloutierMat cloutierMat merged commit 52df10a into master Dec 12, 2024
42 checks passed
@cloutierMat cloutierMat deleted the fix-apigw-param-mapping branch December 12, 2024 17:07
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
4.1
Development

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