8000 add IPv6 support to the runtime gateway by labaneilers · Pull Request #11601 · localstack/localstack · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

labaneilers
Copy link
Contributor
@labaneilers labaneilers commented Sep 28, 2024

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:

GATEWAY_LISTEN="[::]"

Continue supporting the host + port format:

GATEWAY_LISTEN="[::]:4566"

Addresses #11600

Note on the IPv6 address format: GATEWAY_LISTEN expects IPv6 address to be enclosed in square brackets, since the existing format 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.

Changes

Updated:

  • The HostAndPort.parse() method, which can now interpret an IPv6 address correctly. Added unit tests for this.
  • The HostAndPort.host_and_port() method (used by .to_str()) to output IPv6 addresses in square brackets (to match the input format)
  • The 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.
  • The Twisted gateway, which will now use an IPv6 endpoint for an IPv6 host.

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:

  1. Build the LocalStack Docker image
  2. Deploy it to an IPv6-only environment (such as a pod in an EKS cluster in IPv6-only mode)
  3. Set the environment variable GATEWAY_LISTEN="[::]"
  4. Use curl/wget to make a request to http://[ipv6 address]]:4566 (e.g. curl -v http://[2602:f95d:0:12:75cc::4]:4566)
  5. Validate that you got a 200 response, and the LocalStack logs show the request succeeded:
2024-09-28T17:50:29.073  INFO --- [et.reactor-0] localstack.request.http    : GET / => 200

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:

  • 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.
  • There may be other services (e.g. emulated AWS interfaces) that may require additional changes to support IPv6, though I don't know enough about the LocalStack architecture to be sure.
  • Modify documentation here: https://docs.localstack.cloud/references/network-troubleshooting/ (the admonition box should be replaced with instructions on setting GATEWAY_LISTEN="::" in IPv6 environments.
  • Consider in the future using [::] as the default host instead of 0.0.0.0, since it will bind all IPv6 and IPv4 addresses.

@labaneilers labaneilers requested a review from thrau as a code owner September 28, 2024 18:37
Copy link
Collaborator
@localstack-bot localstack-bot left a 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.

@localstack-bot
Copy link
Collaborator
localstack-bot commented Sep 28, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@thrau thrau requested review from simonrw and removed request for thrau September 29, 2024 19:30
@thrau
Copy link
Member
thrau commented Sep 29, 2024

thanks for the contribution @labaneilers . handing over this review to @simonrw

@labaneilers
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@labaneilers
Copy link
Contributor Author

FYI: I don't think OSS contributors can add labels to a PR, but I would suggest this be labeled semver:minor.

@simonrw simonrw added the semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases label Sep 30, 2024
@labaneilers
Copy link
Contributor Author
labaneilers commented Oct 1, 2024

@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 0.0.0.0, and they're on the same port. I'm working on a fix now, along with some new unit tests.

@labaneilers
Copy link
Contributor Author

@localstack-bot recheck

@labaneilers
Copy link
Contributor Author

@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 0.0.0.0, and they're on the same port. I'm working on a fix now, along with some new unit tests.

@simonrw I've fixed this, ready for review! Also took the opportunity to centralize the is_ipv6_address() function in the config module.

@labaneilers labaneilers force-pushed the feat/ipv6-gateway branch 2 times, most recently from 011c79d to f76de43 Compare October 1, 2024 18:02
@labaneilers
Copy link
Contributor Author
labaneilers commented Oct 1, 2024

and.... I've learned that in Twisted, you can't bind to both 0.0.0.0 and ::. Binding to :: appears to be dualstack (works for both IPv4 and IPv6). I'm going to rework my changes to UniqueHostAndPortList to make this work correctly.

@labaneilers
Copy link
Contributor Author

and.... I've learned that in Twisted, you can't bind to both 0.0.0.0 and ::. Binding to :: appears to be dualstack (works for both IPv4 and IPv6). I'm going to rework my changes to UniqueHostAndPortList to make this work correctly.

This is now fixed. The de-duping rules in UniqueHostAndPortList now treat :: as dualstack, and take priority over any IPv6 or IPv4 addresses, while 0.0.0.0 continues to take priority over any IPv4 addresses.

super().__init__()
for item in iterable or []:
self.append(item)
def __init__(self, iterable: List[HostAndPort] | None = None):
Copy link
Contributor Author

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).

@labaneilers labaneilers force-pushed the feat/ipv6-gateway branch 2 times, most recently from 3df4813 to 5c26d32 Compare October 2, 2024 18:15
localstack-bot added a commit that referenced this pull request Oct 2, 2024
@labaneilers
Copy link
Contributor Author

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.

@labaneilers
Copy link
Contributor Author

@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 GATEWAY_LISTEN=[::] in an IPv4 environment, it still works- it just binds to all IPv4 interfaces.

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.

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

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
@labaneilers
Copy link
Contributor Author

@simonrw I've updated everything with your feedback, and 6D40 changed that union type to an old-school Python3.8-style Union type.

I'd be happy to look at that DNS change for my next contribution :)

@labaneilers labaneilers requested a review from simonrw October 18, 2024 19:31
@labaneilers
Copy link
Contributor Author

@simonrw oh, to answer this question:

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.

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.

@simonrw
Copy link
Contributor
simonrw commented Oct 18, 2024

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?

On non-k8s we typically run Lambda functions as separate Docker containers.

... 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.

So long as the inter-container networking is ipv4 then I think everything will work.

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.

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.

@labaneilers
Copy link
Contributor Author

I take it your workloads don't involve lambda?

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.

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.

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.

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.

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!

@simonrw simonrw merged commit 241bfbb into localstack:master Oct 31, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0