-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
add IPv6 support to the runtime gateway #11601
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.
All contributors have signed the CLA ✍️ ✅ |
thanks for the contribution @labaneilers . handing over this review to @simonrw |
I have read the CLA Document and I hereby sign the CLA |
FYI: I don't think OSS contributors can add labels to a PR, but I would suggest this be labeled |
@simonrw I think I've found a bug: UniqueHostAndPortsList will exclude an IPv6 address if GATEWAY_LISTEN includes two entries, and one of them is |
8f0f64f
to
3ac788a
Compare
@localstack-bot recheck |
@simonrw I've fixed this, ready for review! Also took the opportunity to centralize the |
011c79d
to
f76de43
Compare
and.... I've learned that in Twisted, you can't bind to both |
f76de43
to
9dc38f5
Compare
This is now fixed. The de-duping rules in |
localstack-core/localstack/config.py
Outdated
super().__init__() | ||
for item in iterable or []: | ||
self.append(item) | ||
def __init__(self, iterable: List[HostAndPort] | None = None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a new algorithm for doing the dedupe, where the entire list is reconciled each time the list is modified, instead of having two separate sets of rules depending on the order items are added.
This also handles some additional corner cases, such as if the list contains multiple different IPs on the same port, and then a single ::
or 0.0.0.0
is added with the same port (which would previously result in a duplicate entry).
3df4813
to
5c26d32
Compare
I just figured out why the CLA assistant wasn't working- I had inadvertently signed the commit with my work identity instead of my personal identity, which I used to open the PR. I've just re-pushed with the correct identity. |
@simonrw FWIW: We've had a couple of weeks now of running LocakStack with this patch (patched versions of both OSS and Pro) in IPv6 and IPv4 environments concurrently, and it is working perfectly. A cool thing I hadn't anticipated, when we run this with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this change! Your test changes are comprehensive and this adds a good feature.
Unfortunately I don't have an environment to test this thoroughly, but since the feature is additive and you are finding benefit in using this then I'm happy to trust your tests.
Please can you let us know if you encounter any issues with this feature in the future!
Regarding your questions: the big unanswered one for me is whether services continue to work in an ipv6 only environment when we explicitly use ipv4. For example, lambda functions report back to the LocalStack container using the ipv4 address of the LS container.
One area that would be good to follow up with is the DNS server. For ipv4 we use subnet matching to return a potentially different IP address for the LS container for the domain name localhost.localstack.cloud
. See my blog post for more information.
However we don't do this for IPv6. This would be a great follow up if you are interested?
Unfortunately this PR contains invalid syntax for Python 3.8 and 3.9, see the failing test report. Could you fix this so we have a green test suite please?
Once this has been done I will approve
5c26d32
to
ec254b4
Compare
Adds support for configuring the gateway listener to listen on IPv6 addresses. For example, you can now set: ``` GATEWAY_LISTEN="[::]" ``` NOTE: `GATEWAY_LISTEN` now expects IPv6 address to be enclosed in square brackets, since the existing interface expected for `GATEWAY_LISTEN` is a subset of a URL (host and port), and square brackets are required for IPv6 addresses in URLs (to distinguish between the colons in an IPv6 address and the delimiter with the port number). This format is defined in https://www.rfc-editor.org/rfc/rfc6874. This required modifications to: * The HostAndPort.parse() method, which can now interpret an IPv6 address correctly. * The UniqueHostAndPortList class, which now distinguishes between IPv6 and IPv4 addresses when deduping. * The Twisted gateway, which will now use an IPv6 endpoint for an IPv6 host. I didn't make any changes to the two other gateway implementations (hypercorn or werkzeug) as I'm not sure if they're still considered supported. I also noticed a bug in the existing twisted implementation, in which it ignores any IPv4 host passed in via GATEWAY_LISTEN, and simply binds to all interfaces ("", which is interpreted as 0.0.0.0). I didn't fix this (given users might be depending on the current behavior), but the new IPv6 implementation respects the specified host. Addresses localstack#11600
ec254b4
to
0d4326c
Compare
@simonrw I've updated everything with your feedback, and
6D40
changed that union type to an old-school Python3.8-style I'd be happy to look at that DNS change for my next contribution :) |
@simonrw oh, to answer this question:
I'm not sure I can answer this well without knowing much more about the relationship of the Lambda is to the main LocalStack process. Is it a separate process running in the same container? How does the Lambda get the LocalStack process's IP? Taking my best guess, that they're two processes in the same container: I think that in a true IPv6 only environment, the lambda wouldn't be able to connect to the main container if it's using an IPv4 address. However, in the environment I'm running in (EKS in IPv6 mode) containers still get an IPv4 address, but its just only routable from the same node (not part of the VPC). So I think the lambda callback would likely work in that case. If you have any time, and are interested in helping me set up a test case, I can run it in an IPv6 EKS cluster and see if it works. |
On non-k8s we typically run Lambda functions as separate Docker containers.
So long as the inter-container networking is ipv4 then I think everything will work.
Definitely. I can send you an example application which uses inter-container networking (more than just Lambda). I take it your workloads don't involve lambda? I'm also interested in how you are using LocalStack. Particularly: how are you running LocalStack in your EKS environment? Are you using our helm charts? Have you found difficulties in running LocalStack on k8s? My team and I recently improved support for running LocalStack on k8s and I'd love to know any pain points or missing features. |
Yeah- currently we aren't using LocalStack for Lambda. We use Lambda for other stuff in our infra, but don't have a need to emulate the API with LocalStack.
We're not using the helm chart. As part of integration test setup, we spin up a single replica k8s deployment with a LocalStack container, along with a Service pointing at it. We build a custom LocalStack image, taking the base OSS image and adding some initialization scripts to it (e.g. adding Dynamo tables, SQS queues, etc). We then run other dependencies, the test subject app, or the test harness, in the same namespaces, using the service names to wire them all together. The same tool that orchestrates all this in Kubernetes also can generate a docker-compose file and run the exact same stuff in Docker Desktop locally. As far as missing features: we're generally pretty happy with the feature set, though I don't think we're using a lot of variety- mostly Dynamo, SQS, SNS, S3, and a few other APIs. IPv6 is the first feature we've really needed that was missing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @labaneilers, thanks for making this contribution. From what I can see from my testing, LocalStack continues to work with IPv4 networking internally. Since this code change is additive and does not impact IPv4 networks, I am happy to merge this.
Thanks for this contribution, it's great to see wider support for IPv6 in general!
Motivation
Add support for configuring the gateway listener to listen on IPv6 addresses, so that LocalStack can be used in IPv6-only networks or environments (e.g. such as EKS in IPv6 mode).
For example, allow listening on all IPv6 interfaces:
Continue supporting the host + port format:
Addresses #11600
Note on the IPv6 address format:
GATEWAY_LISTEN
expects IPv6 address to be enclosed in square brackets, since the existing format expected forGATEWAY_LISTEN
is a subset of a URL (host and port), and square brackets are required for IPv6 addresses in URLs (to distinguish between the colons in an IPv6 address and the delimiter with the port number). This format is defined in https://www.rfc-editor.org/rfc/rfc6874.Changes
Updated:
HostAndPort.parse()
method, which can now interpret an IPv6 address correctly. Added unit tests for this.HostAndPort.host_and_port()
method (used by.to_str()
) to output IPv6 addresses in square brackets (to match the input format)UniqueHostAndPortList.append()
method, to distinguish between IPv6/IPv4 when deduping the "all interfaces" host, with::
taking priority over all IPv6 and IPv4 addresses. Added and updated unit tests for this.I also noticed a bug in the existing twisted implementation, in which it ignores any IPv4 host passed in via GATEWAY_LISTEN, and simply binds to all interfaces ("", which is interpreted as 0.0.0.0). I didn't fix this (given users might be depending on the current behavior), but the new IPv6 implementation respects the specified host.
Testing
To test manually:
GATEWAY_LISTEN="[::]"
http://[ipv6 address]]:4566
(e.g.curl -v http://[2602:f95d:0:12:75cc::4]:4566
)To validate that using the dualstack address (
::
) will also bind all IPv4 interfaces, repeat the same test in an IPv4-only environment, and in a dualstack environment, and curl the IPv4 address of localstack.TODO
What's left to do:
GATEWAY_LISTEN="::"
in IPv6 environments.[::]
as the default host instead of0.0.0.0
, since it will bind all IPv6 and IPv4 addresses.