Skip to content
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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove use of http2_server in replicator aws proxy #62

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

thrau
Copy link
Member

@thrau thrau commented Jun 4, 2024

Motivation

馃毀

Fixes #45

This is an attempt to move away from the http2_server implementation, by replacing the current auth proxy implementation with a handler chain.

Changes

  • ...

@thrau thrau requested a review from whummer as a code owner June 4, 2024 20:50
@thrau thrau marked this pull request as draft June 4, 2024 23:36
@thrau thrau marked this pull request as draft June 4, 2024 23:36
@thrau thrau marked this pull request as ready for review June 4, 2024 23:36
@thrau thrau marked this pull request as draft June 4, 2024 23:36
@thrau
Copy link
Member Author

thrau commented Jun 5, 2024

May not have been that easy after all. Will investigate further..

@thrau
Copy link
Member Author

thrau commented Jun 5, 2024

I tried to re-write the entire proxying mechanism by re-using more from our handler chain, but there still seem to be several issues related to headers and streaming data, which I think is a result of the proxy mechanism on both sides doing too many things to the request.

I need to sit down with @whummer to better understand what the components should be doing exactly before moving forward, because I'm shotgun debugging at this point.

@whummer
Copy link
Member

whummer commented Jun 8, 2024

Thanks for digging into this @thrau and getting the extension upgraded to the new gateway logic! 馃檶 Happy to do some pairing on this - or maybe I'll find some time this weekend to dig into it a bit.. Let's compare notes soon!

@whummer
Copy link
Member

whummer commented Jun 20, 2024

@thrau Sorry for the delay, finally got a chance to look into this. Great job on the refactoring and removing the use of http2_server! 馃殌 Pushed a small change to drop Transfer-Encoding headers from the proxied response, see: https://github.com/localstack/localstack-extensions/pull/62/files#diff-ee721dbb335a260769d713b880c29dc772e825e268aedea2a0b3d67fd7f99bc3R46-R47

Was running some quick tests locally, seems to be working. 馃帀 At least for S3 requests - these buckets are proxied from a real AWS account:

$ awslocal s3 ls
2024-06-20 22:25:33 ls-test-whummer-123
2024-06-20 22:51:04 test-bucket-a8c53ada

SQS tests still seem to be failing in the CI pipeline, we can look into those next.. 馃憖

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.

Remove usages of deprecated LocalStack code features in AWS replicator
2 participants