Skip to content

Conversation

@whummer
Copy link
Member

@whummer whummer commented Oct 26, 2025

This PR adds a couple of refactorings, to prepare the aws-proxy project for upcoming changes and additional testing:

  • restructure project to use pyproject.toml, as well as modern build tools (ruff instead of black/flake8, etc)
  • refactor the integration tests into separate modules, as the single test_proxy_requests.py module is starting to become too large
  • add a simple first test for proxying SecretsManager requests
  • format the codebase with ruff (which causes lots of changes in this PR)

Mostly refactoring changes in this PR, no functional changes per se - apologies in advance for the large size of the changeset! 🙌

@whummer whummer marked this pull request as draft October 26, 2025 13:42
@whummer whummer force-pushed the test-secretsmanager branch 19 times, most recently from 02fbfd3 to a36c2b0 Compare October 26, 2025 19:57
@whummer whummer changed the title Restructure project to use pyproject.toml Restructure project to use pyproject.toml; add secretsmanager test Oct 26, 2025
@whummer whummer force-pushed the test-secretsmanager branch from a36c2b0 to e94c0d4 Compare October 26, 2025 20:13
@whummer whummer force-pushed the test-secretsmanager branch from e94c0d4 to f0f5daa Compare October 26, 2025 20:39
@whummer whummer force-pushed the test-secretsmanager branch 2 times, most recently from 1de8ba6 to 492168e Compare October 26, 2025 21:57
@whummer whummer force-pushed the test-secretsmanager branch from 492168e to da49f57 Compare October 26, 2025 23:15
):
for bucket in buckets_aws + buckets_proxied:
# TODO: tmp fix - seems like this attribute is missing for certain boto3 versions. To be investigated!
bucket.pop("BucketArn", None)
Copy link
Member Author

@whummer whummer Oct 27, 2025

Choose a reason for hiding this comment

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

Note: this is a pretty ugly workaround :/ - I didn't have the time to investigate in-depth, but it seems that installing different boto3 versions can result in different attributes being returned here (sometimes BucketArn is present, sometimes missing). Afaict, the S3 tests would currently also fail against latest main branch. Again, not a great solution - but I'd prefer to merge the PR as-is, and then investigate and properly fix this issue separately in a follow-up.. 👍

Copy link
Member Author

@whummer whummer Oct 27, 2025

Choose a reason for hiding this comment

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

Update: Just re-ran the builds on main, and confirmed that the tests are also currently failing on main: https://github.com/localstack/localstack-extensions/actions/runs/18838765454/job/53745841266

        buckets_proxied = s3_client.list_buckets()["Buckets"]
        buckets_aws = s3_client_aws.list_buckets()["Buckets"]
>       assert buckets_proxied and buckets_proxied == buckets_aws
E       AssertionError: assert ([{'BucketArn': 'arn:aws:s3:::test-bucket-c9996004', 'CreationDate': datetime.datetime(2025, 10, 27, 11, 10, 4, tzinfo=tzlocal()), 'Name': 'test-bucket-c9996004'}] and [{'BucketArn'...et-c9996004'}] == [{'CreationDa...et-c9996004'}]
E         
E         At index 0 diff: {'Name': 'test-bucket-c9996004', 'CreationDate': datetime.datetime(2025, 10, 27, 11, 10, 4, tzinfo=tzlocal()), 'BucketArn': 'arn:aws:s3:::test-bucket-c9996004'} != {'Name': 'test-bucket-c9996004', 'CreationDate': datetime.datetime(2025, 10, 27, 11, 10, 4, tzinfo=tzlocal())}
E         
E         Full diff:
E           [
E               {
E         +         'BucketArn': 'arn:aws:s3:::test-bucket-c9996004',
E                   'CreationDate': datetime.datetime(2025, 10, 27, 11, 10, 4, tzinfo=tzlocal()),
E                   'Name': 'test-bucket-c9996004',
E               },
E           ])

tests/test_proxy_requests.py:92: AssertionError

(again, something to be investigated in a follow-up iteration..)

@whummer whummer marked this pull request as ready for review October 27, 2025 09:05
@whummer whummer changed the title Restructure project to use pyproject.toml; add secretsmanager test Restructure project to use pyproject.toml; add secretsmanager proxy test Oct 27, 2025
Copy link
Contributor

@nik-localstack nik-localstack left a comment

Choose a reason for hiding this comment

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

LGTM (apart from a minor change)
Thanks for the improvements 🙌
I agree with tackling the test failure in a separate step

Co-authored-by: Nikos <nikos.michas@localstack.cloud>
@whummer whummer merged commit a25f189 into main Oct 27, 2025
1 check passed
@whummer whummer deleted the test-secretsmanager branch October 27, 2025 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants