fix: handle docker network host

[GuillaumeDesforges]

Which issue(s) does this change fix?

Fixes #4236.

Why is this change necessary?

PR #669 allowed usage of the host network for Docker containers, but that feature was removed in PR #5279.

How does it address the issue?

This change reintroduces setting network_mode to host when docker_network is host, and adds some additional debug logging to ease debugging.

What side effects does this change have?

None AFAIK.

Mandatory Checklist

PRs will only be reviewed after checklist is complete

• Add input/output type hints to new functions/methods
• Write design document if needed (Do I need to write a design document?)
• Write/update unit tests
• Write/update integration tests
• Write/update functional tests if needed
• make pr passes
• make update-reproducible-reqs if dependencies were changed
• Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[GuillaumeDesforges]

My bad, seems like black was not applied properly. I'll fix it.

[GuillaumeDesforges]

Tests should be fixed now

[hnnasit]

Hi @GuillaumeDesforges, thanks for opening the PR. We will review if these changes have any side effects and get back to you.

[mndeveci]

Thanks for raising this PR @GuillaumeDesforges!

I've gave a quick test, however I saw that with --docker-network host is failing with sam local invoke command with these changes. It always times out for simple execution.

SAM_CLI_CONTAINER_CONNECTION_TIMEOUT environment variable. The current timeout is 20.0 (seconds).

The other concern we have with this change is exposing RIE port to the host network interface. RIE was never designed to expose its ports to the host system, it might have some issues when an attacker gets access to these exposed ports.

Using host network mode will bind to port 8080

We would rather recommend running your services and lambda function in a network that you manage, which is not the host network.

[GuillaumeDesforges]

Hi @mndeveci, thanks for testing the changes.

It always times out for simple execution.

I am surprised you get a timeout, it works fine on my end. 👀
Could you please run with --debug and share logs?

RIE was never designed to expose its ports to the host system, it might have some issues when an attacker gets access to these exposed ports.

I do not know waht RIE is, and the risks from having it bound to a port accessible on host.
I would argue that it is up to users to make sure they run with proper firewall and networking configuration, especially given that running with network_mode="host" would be for advanced users.
I will add a recommendation in the help. 👍

[mndeveci]

I do not know waht RIE is, and the risks from having it bound to a port accessible on host.
I would argue that it is up to users to make sure they run with proper firewall and networking configuration, especially given that running with network_mode="host" would be for advanced users.

RIE is the Lambda Runtime Interface Emulator SAM CLI uses under the hood to invoke the lambda function locally. It is supposed to work between SAM CLI and the execution itself. However, when we set the network mode to "host" it is exposing RIE to the host system, which raises the above concerns.

For your use case, are there any blockers that prevents you set up a custom network between your container and use it with sam local commands?

[GuillaumeDesforges]

are there any blockers that prevents you set up a custom network between your container and use it with sam local commands

I have documented my use case in the issue mentioned: #4236 (comment)
To summarize, I want to test my lambda against a local database that runs in Docker.

[GuillaumeDesforges]

However, when we set the network mode to "host" it is exposing RIE to the host system, which raises the above concerns.

You have said previously

RIE was never designed to expose its ports to the host system, it might have some issues when an attacker gets access to these exposed ports.

A way to improve this would be to prevent accessing RIE from outside the host machine. IIRC binding RIE on 127.0.0.1:8080 (loopback) and not 0.0.0.0:8080 should suffice on Linux.

[mndeveci]

are there any blockers that prevents you set up a custom network between your container and use it with sam local commands

I have documented my use case in the issue mentioned: #4236 (comment) To summarize, I want to test my lambda against a local database that runs in Docker.

Yeap I saw your use case, but are there any blockers for your to use another (custom) network, rather than the host one in your compose file? Something like this;

services:
  networks:
    custom_nework
  database:
    image: postgres
    networks: 
      - custom_network
    environment:
      POSTGRES_USER: postgres
      POSTGRES_PASSWORD: password
    volumes:
     - ./tmp/compose/database/data:/var/lib/postgresql/data

[GuillaumeDesforges]

Indeed, I can remove binding the postgres service to the host network, and specify to SAM CLI's local invoke command to use the network (custom or not) created by Docker.

Example:

services:
  database:
    image: postgres
    environment:
      POSTGRES_USER: postgres
      POSTGRES_PASSWORD: password
    volumes:
      - ./tmp/compose/database/data:/var/lib/postgresql/data

Then docker compose up, read the name of the network from stdout (in my case my-project_default).

Then

sam build && sam local invoke MyHandler -e src/test/resources/events/event.json --docker-network my-project_default

So for my use case, I am fine.

However, that means one would have to always have the database running/bound to a docker network, which may not always be easy nor desirable.

---

In case anyone in the future needs these changes, you can use Nix to get it with the following overlay (tested on nixos-24.05).

final: prev: {
  aws-sam-cli = prev.aws-sam-cli.overrideAttrs (oldAttrs: {
    patches = (oldAttrs.patches or [ ]) ++ [
      # Apply fix for https://github.com/aws/aws-sam-cli/issues/4236
      ./aws-sam-cli.7278.patch
    ];
  });
}

and patch

aws-sam-cli.7278.patch

From 9cf44b4822dea2c742166efe0000690676c7644e Mon Sep 17 00:00:00 2001
From: Guillaume Desforges <guillaume.desforges.pro@gmail.com>
Date: Wed, 24 Jul 2024 19:37:41 +0200
Subject: [PATCH] fix: handle docker network host

Fixes #4236.

PR #669 allowed usage of the host network for Docker containers,
but that feature was removed in PR #5279.

This change reintroduces setting `network_mode` to `host` when `docker_network`
is `host`, and adds some additional debug logging to ease debugging.
---
 samcli/local/docker/container.py          | 32 ++++++++++++++++-------
 tests/unit/local/docker/test_container.py |  2 +-
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/samcli/local/docker/container.py b/samcli/local/docker/container.py
index a34e96c7c1..ef40a60e83 100644
--- a/samcli/local/docker/container.py
+++ b/samcli/local/docker/container.py
@@ -154,6 +154,7 @@ def __init__(
             self.rapid_port_host = find_free_port(
                 network_interface=self._container_host_interface, start=self._start_port_range, end=self._end_port_range
             )
+
         except NoFreePortsError as ex:
             raise ContainerNotStartableException(str(ex)) from ex
 
@@ -216,15 +217,25 @@ def create(self):
         if self._env_vars:
             kwargs["environment"] = self._env_vars
 
-        kwargs["ports"] = {self.RAPID_PORT_CONTAINER: (self._container_host_interface, self.rapid_port_host)}
-
-        if self._exposed_ports:
-            kwargs["ports"].update(
-                {
-                    container_port: (self._container_host_interface, host_port)
-                    for container_port, host_port in self._exposed_ports.items()
-                }
-            )
+        if self.network_id == "host":
+            kwargs["network_mode"] = self.network_id
+            # When using host network, aws-lambda-rie will bind to 8080
+            self.rapid_port_host = int(self.RAPID_PORT_CONTAINER)
+            LOG.warning("Using host network mode will bind to port %s", self.RAPID_PORT_CONTAINER)
+
+        # It is not possible to both use host network and expose ports
+        if "network_mode" in kwargs and kwargs["network_mode"] == "host":
+            LOG.warning("Using host network mode, ignoring any port mappings")
+        else:
+            kwargs["ports"] = {self.RAPID_PORT_CONTAINER: (self._container_host_interface, self.rapid_port_host)}
+
+            if self._exposed_ports:
+                kwargs["ports"].update(
+                    {
+                        container_port: (self._container_host_interface, host_port)
+                        for container_port, host_port in self._exposed_ports.items()
+                    }
+                )
 
         if self._entrypoint:
             kwargs["entrypoint"] = self._entrypoint
@@ -233,9 +244,12 @@ def create(self):
             # Ex: 128m => 128MB
             kwargs["mem_limit"] = "{}m".format(self._memory_limit_mb)
 
+
         if self._extra_hosts:
             kwargs["extra_hosts"] = self._extra_hosts
 
+        LOG.debug("Docker will start with the following arguments: %s", kwargs)
+
         try:
             real_container = self.docker_client.containers.create(self._image, **kwargs)
         except DockerAPIError as ex:
diff --git a/tests/unit/local/docker/test_container.py b/tests/unit/local/docker/test_container.py
index b181ce33fa..2cab6cbfc8 100644
--- a/tests/unit/local/docker/test_container.py
+++ b/tests/unit/local/docker/test_container.py
@@ -307,7 +307,7 @@ def test_must_connect_to_host_network_on_create(self, mock_resolve_symlinks):
             self.image,
             command=self.cmd,
             working_dir=self.working_dir,
-            ports=self.always_exposed_ports,
+            network_mode='host',
             tty=False,
             use_config_proxy=True,
             volumes=expected_volumes,

[hawflau]

After a discussion and careful consideration within our Team, we believe the right approach is to create a network instead of using the "host" network mode. Enabling "host" network mode will likely bring out other issues as @mndeveci suggested above. With that, we have decided not to merge this PR.

Thank you for your contribution. We appreciate the effort and time you spent in this PR. Your input is valuable to us, and we encourage you to continue contributing. If you have any questions or need further clarification, please feel free to reach out.

Thank you again for your understanding and for being a part of our community!