Skip to content

APIGW NG: fix leftover REST issues #11518

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 3 commits into from
Sep 16, 2024
Merged

APIGW NG: fix leftover REST issues #11518

merged 3 commits into from
Sep 16, 2024

Conversation

bentsku
Copy link
Contributor

@bentsku bentsku commented Sep 14, 2024 •
edited
Loading

Motivation

Fixing leftover APIGW NextGen tasks. Realized special case for execute-api style AWS integration uri, where we don't need to pass an authorization header, as those requests aren't signed by default and would fail with a bad service name.

Also fixed some error handling related to Lambda, seemingly the AWS Lambda integration follows specific rules (as it already does with the selectionPattern specific behavior.

Changes

  • only add mocked headers if the request targets a service operation
  • remove the default exception message for AuthorizerConfigurationError
  • add specific error handling for AWS lambda integration that is very specific to this one integration, not returning the AccessDeniedError unlike all other like dynamodb, kinesis and sns
  • remove the asynchronous logic for AWS_PROXY, this is only possible for AWS Lambda (logical, as headers are not used to create the lambda invocation)
  • added logic for AWS_PROXY which changes the status code raised if the error is AccessDenied
  • fix AWS to properly add the Action query string parameter

@bentsku bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Sep 14, 2024
@bentsku bentsku self-assigned this Sep 14, 2024
@bentsku bentsku changed the title Apigw fix rest tests APIGW NG: fix leftover REST issues Sep 14, 2024
@bentsku bentsku mentioned this pull request Sep 14, 2024
Merged
2 tasks
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 14, 2024 •
edited
Loading

LocalStack Community integration with Pro

  2 files  ±    0    2 suites  ±0   25m 8s ⏱️ - 1h 10m 47s
787 tests  - 2 644  734 ✅  - 2 298  53 💤  - 346  0 ❌ ±0 
789 runs   - 2 644  734 ✅  - 2 298  55 💤  - 346  0 ❌ ±0 

Results for commit 3a85284. ± Comparison against base commit d121e5d.

This pull request removes 2644 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.

@bentsku bentsku marked this pull request as ready for review September 15, 2024 00:39
cloutierMat
cloutierMat approved these changes Sep 16, 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.

All good fixes thanks for tackling these! 🙏

localstack-core/localstack/services/apigateway/next_gen/execute_api/integrations/aws.py
Comment on lines -226 to +231
"params": integration_req["query_string_parameters"],
"params": query_params,
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! 👀

@cloutierMat cloutierMat merged commit dadf664 into master Sep 16, 2024
38 checks passed
@cloutierMat cloutierMat deleted the apigw-fix-rest-tests branch September 16, 2024 16:26
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