Skip to content

UNC-99 apigateway accept-encoding header wrong #13350

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
cloutierMat merged 2 commits into from
Nov 6, 2025
Merged

UNC-99 apigateway accept-encoding header wrong #13350

cloutierMat merged 2 commits into from
Nov 6, 2025
+24 −2

Conversation

@cloutierMat
Copy link
Member

@cloutierMat cloutierMat commented Nov 6, 2025 •
edited
Loading

Motivation

In upstream projects the Accept-Encoding can change due to the presence of the zstandard library. It seems safer for now to skip those.

Changes

Skip the header from snapshot when captured

Tests

Related

@cloutierMat cloutierMat added area: ci Running LocalStack in CI environments semver: patch Non-breaking changes which can be included in patch releases docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes labels Nov 6, 2025
@github-actions
Copy link

github-actions bot commented Nov 6, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 282 tests  +4   20 530 ✅ +4   16m 7s ⏱️ -44s
     1 suites ±0    1 752 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit dc23c1a. ± Comparison against base commit 4484dd1.

This pull request removes 13 and adds 17 tests. Note that renamed tests count towards both. 
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_build_cfn_catalog_resources
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_get_cfn_resource_status[AWS::Athena::CapacitiesReservation-athena-SUPPORTED_WITH_LICENSE_UPGRADE]
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_get_cfn_resource_status[AWS::NonExistentService::NonExistent-nonexistentservice-NOT_SUPPORTED]
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_get_cfn_resource_status[AWS::S3::Bucket-s3-AVAILABLE]
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_get_cfn_resource_status[AWS::S3::NonExistent-s3-NOT_SUPPORTED]
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_get_service_status[athena-SUPPORTED_WITH_LICENSE_UPGRADE]
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_get_service_status[nonexistent-NOT_SUPPORTED]
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_get_service_status[s3-SUPPORTED]
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_get_service_status_with_empty_operation
tests.unit.utils.test_catalog.TestAwsCatalog ‑ test_get_service_status_with_only_one_emulator_type
…

tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_build_cfn_catalog_resources
tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_get_cfn_resource_status[AWS::Athena::CapacitiesReservation-athena-SUPPORTED_WITH_LICENSE_UPGRADE]
tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_get_cfn_resource_status[AWS::NonExistentService::NonExistent-nonexistentservice-NOT_SUPPORTED]
tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_get_cfn_resource_status[AWS::S3::Bucket-s3-AVAILABLE]
tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_get_cfn_resource_status[AWS::S3::NonExistent-s3-NOT_SUPPORTED]
tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_get_service_status[athena-SUPPORTED_WITH_LICENSE_UPGRADE]
tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_get_service_status[nonexistent-NOT_SUPPORTED]
tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_get_service_status[s3-SUPPORTED]
tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_get_service_status_with_empty_operation
tests.unit.utils.catalog.test_catalog.TestAwsCatalog ‑ test_get_service_status_with_only_one_emulator_type
…

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Nov 6, 2025 •
edited
Loading

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 30s ⏱️ +9s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit dc23c1a. ± Comparison against base commit 4484dd1.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Nov 6, 2025 •
edited
Loading

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 4m 6s ⏱️
3 543 tests 3 286 ✅ 257 💤 0 ❌
3 549 runs  3 286 ✅ 263 💤 0 ❌

Results for commit dc23c1a.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Nov 6, 2025 •
edited
Loading

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 34m 22s ⏱️ - 27m 6s
3 519 tests  - 1 375  3 258 ✅  - 1 261  261 💤  - 108  0 ❌  - 6 
3 521 runs   - 1 375  3 258 ✅  - 1 261  263 💤  - 108  0 ❌  - 6 

Results for commit dc23c1a. ± Comparison against base commit 4484dd1.

This pull request removes 1375 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 marked this pull request as ready for review November 6, 2025 19:24
simonrw
simonrw approved these changes Nov 6, 2025
View reviewed changes
Copy link
Contributor

@simonrw simonrw left a comment

Choose a reason for hiding this comment

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

I guess the presence and value of this header is not critical - accept-encoding is so standard that presumably nobody worries about the parity of it with AWS. We are more lenient with what we accept so I guess it's not a problem.

LGTM

@cloutierMat
Copy link
Member Author

cloutierMat commented Nov 6, 2025

Thanks for the review @simonrw. I will add a bit of clarity as to the issue, since I realise my PR description might not be super explicit.

We do wnat to keep those in parity, as it can impact the response encoding from an integration if we modify headers that shouldn't be modified.

We are more lenient with what we accept so I guess it's not a problem.

This issue is actually a parity with AWS. The headers sent from our testing python client is impacted by the addition of the package in the upstream project. This is why we can't merely update the snapshot here. Since the same package is not installed in this project.

I will try to bring a follow up that will bring back an assertion that what is sent by the requests client is what is received by the integration to make sure we don't end up losing parity on that front either.

@cloutierMat cloutierMat merged commit 447dd8e into main Nov 6, 2025
43 checks passed
@cloutierMat cloutierMat deleted the unc-99-apigateway-accept-encoding-header-wrong branch November 6, 2025 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simonrw simonrw simonrw approved these changes

@joe4dev joe4dev Awaiting requested review from joe4dev joe4dev is a code owner

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

@dfangl dfangl Awaiting requested review from dfangl dfangl is a code owner

@gregfurman gregfurman Awaiting requested review from gregfurman gregfurman is a code owner

@bentsku bentsku Awaiting requested review from bentsku bentsku is a code owner

Assignees

No one assigned

Labels

area: ci Running LocalStack in CI environments docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes 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.

3 participants

@cloutierMat @simonrw