From 868952511f7666fd726554a504afb1089e7d6c9d Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Mon, 26 May 2025 16:43:17 -0500
Subject: [PATCH 01/15] Fix Complement not using `HSPortBindingIP`
 (`127.0.0.1`) for the homeserver `BaseURL`

Regressed in https://github.com/matrix-org/complement/pull/776
which started using the Docker default (`0.0.0.0`).

This caused downstream issues in some of our out-of-repo Complement tests
which stress some SSO/OIDC login flows and expect to be using the canonical
`public_baseurl` configured in Synapse.
---
 config/config.go            |   1 +
 internal/docker/builder.go  |  51 +++++++++++------
 internal/docker/deployer.go | 111 ++++++++++++++++++++++++++++++------
 3 files changed, 130 insertions(+), 33 deletions(-)

diff --git a/config/config.go b/config/config.go
index 04b0f821..9e3317a5 100644
--- a/config/config.go
+++ b/config/config.go
@@ -106,6 +106,7 @@ type Complement struct {
 	// disable this behaviour being added later, once this has stablised.
 	EnableDirtyRuns bool
 
+	// The hostname that will be used to bind the homeserver ports to, e.g. `127.0.0.1`
 	HSPortBindingIP string
 
 	// Name: COMPLEMENT_POST_TEST_SCRIPT
diff --git a/internal/docker/builder.go b/internal/docker/builder.go
index 721fdd9b..6b5e3720 100644
--- a/internal/docker/builder.go
+++ b/internal/docker/builder.go
@@ -539,27 +539,46 @@ func printPortBindingsOfAllComplementContainers(docker *client.Client, contextSt
 	log.Printf("=============== %s : END ALL COMPLEMENT DOCKER PORT BINDINGS ===============\n\n\n", contextStr)
 }
 
-func endpoints(p nat.PortMap, csPort, ssPort int) (baseURL, fedBaseURL string, err error) {
-	csapiPort := fmt.Sprintf("%d/tcp", csPort)
-	csapiPortInfo, ok := p[nat.Port(csapiPort)]
-	if !ok {
-		return "", "", fmt.Errorf("port %s not exposed - exposed ports: %v", csapiPort, p)
+func endpoints(p nat.PortMap, hsPortBindingIP string, csPort, ssPort int) (baseURL, fedBaseURL string, err error) {
+	csapiPortBinding, err := findPortBinding(p, hsPortBindingIP, csPort)
+	if err != nil {
+		return "", "", fmt.Errorf("Problem finding CS API port: %s", err)
 	}
-	if len(csapiPortInfo) == 0 {
-		return "", "", fmt.Errorf("port %s exposed with not mapped port: %+v", csapiPort, p)
+	baseURL = fmt.Sprintf("http://"+csapiPortBinding.HostIP+":%s", csapiPortBinding.HostPort)
+
+	ssapiPortBinding, err := findPortBinding(p, hsPortBindingIP, ssPort)
+	if err != nil {
+		return "", "", fmt.Errorf("Problem finding SS API port: %s", err)
 	}
-	baseURL = fmt.Sprintf("http://"+csapiPortInfo[0].HostIP+":%s", csapiPortInfo[0].HostPort)
+	fedBaseURL = fmt.Sprintf("https://"+ssapiPortBinding.HostIP+":%s", ssapiPortBinding.HostPort)
+	return
+}
 
-	ssapiPort := fmt.Sprintf("%d/tcp", ssPort)
-	ssapiPortInfo, ok := p[nat.Port(ssapiPort)]
+// Find a matching port binding for the given host/port in the nat.PortMap.
+func findPortBinding(p nat.PortMap, hsPortBindingIP string, port int) (portBinding nat.PortBinding, err error) {
+	portString := fmt.Sprintf("%d/tcp", port)
+	portBindings, ok := p[nat.Port(portString)]
 	if !ok {
-		return "", "", fmt.Errorf("port %s not exposed - exposed ports: %v", ssapiPort, p)
-	}
-	if len(ssapiPortInfo) == 0 {
-		return "", "", fmt.Errorf("port %s exposed with not mapped port: %+v", ssapiPort, p)
+		return nat.PortBinding{}, fmt.Errorf("port %s not exposed - exposed ports: %v", portString, p)
+	}
+	if len(portBindings) == 0 {
+		return nat.PortBinding{}, fmt.Errorf("port %s exposed with not mapped port: %+v", portString, p)
+	}
+
+	for _, pb := range portBindings {
+		if pb.HostIP == hsPortBindingIP {
+			return pb, nil
+		} else if pb.HostIP == "0.0.0.0" {
+			// `0.0.0.0` means "all interfaces", so we can assume that this will be listening
+			// for connections from `hsPortBindingIP` as well.
+			return nat.PortBinding{
+				HostIP:   hsPortBindingIP,
+				HostPort: pb.HostPort,
+			}, nil
+		}
 	}
-	fedBaseURL = fmt.Sprintf("https://"+csapiPortInfo[0].HostIP+":%s", ssapiPortInfo[0].HostPort)
-	return
+
+	return portBindings[0], nil
 }
 
 type result struct {
diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index a72d9bbf..1b94d24e 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -313,9 +313,13 @@ func (d *Deployer) StartServer(hsDep *HomeserverDeployment) error {
 	}
 
 	// Wait for the container to be ready.
-	baseURL, fedBaseURL, err := waitForPorts(ctx, d.Docker, hsDep.ContainerID)
+	err = waitForPorts(ctx, d.Docker, hsDep.ContainerID)
 	if err != nil {
-		return fmt.Errorf("failed to get ports for container %s: %s", hsDep.ContainerID, err)
+		return fmt.Errorf("failed to wait for ports on container %s: %s", hsDep.ContainerID, err)
+	}
+	baseURL, fedBaseURL, err := getHostAccessibleHomeserverUrls(ctx, d.Docker, hsDep.ContainerID, d.config.HSPortBindingIP)
+	if err != nil {
+		return fmt.Errorf("failed to get host accessible homeserver URL's from container %s: %s", hsDep.ContainerID, err)
 	}
 	hsDep.SetEndpoints(baseURL, fedBaseURL)
 
@@ -441,10 +445,19 @@ func deployImage(
 		log.Printf("%s: Started container %s", contextStr, containerID)
 	}
 
-	baseURL, fedBaseURL, err := waitForPorts(ctx, docker, containerID)
+	// Wait for the container to be ready.
+	err = waitForPorts(ctx, docker, containerID)
 	if err != nil {
-		return stubDeployment, fmt.Errorf("%s : image %s : %w", contextStr, imageID, err)
+		return stubDeployment, fmt.Errorf("%s: failed to wait for ports on container %s: %w", contextStr, containerID, err)
+	}
+	baseURL, fedBaseURL, err := getHostAccessibleHomeserverUrls(ctx, docker, containerID, cfg.HSPortBindingIP)
+	if err != nil {
+		return stubDeployment, fmt.Errorf(
+			"%s: failed to get host accessible homeserver URL's from container %s: %s",
+			contextStr, containerID, err,
+		)
 	}
+
 	inspect, err := docker.ContainerInspect(ctx, containerID)
 	if err != nil {
 		return stubDeployment, fmt.Errorf("ContainerInspect: %s", err)
@@ -504,26 +517,90 @@ func copyToContainer(docker *client.Client, containerID, path string, data []byt
 	return nil
 }
 
-// Waits until a homeserver container has NAT ports assigned and returns its clientside API URL and federation API URL.
-func waitForPorts(ctx context.Context, docker *client.Client, containerID string) (baseURL string, fedBaseURL string, err error) {
+func assertHostnameEqual(inputUrl string, expectedHostname string) error {
+	parsedUrl, err := url.Parse(inputUrl)
+	if err != nil {
+		return fmt.Errorf("failed to parse URL %s: %s", inputUrl, err)
+	}
+	if parsedUrl.Hostname() != expectedHostname {
+		return fmt.Errorf("expected hostname %s in URL %s, got %s", expectedHostname, inputUrl, parsedUrl.Hostname())
+	}
+
+	return nil
+}
+
+func getHostAccessibleHomeserverUrls(ctx context.Context, docker *client.Client, containerID string, hsPortBindingIP string) (baseURL string, fedBaseURL string, err error) {
+	inspectResponse, err := inspectPortsOnContainer(ctx, docker, containerID)
+	if err != nil {
+		return "", "", fmt.Errorf("failed to inspect ports: %w", err)
+	}
+
+	baseURL, fedBaseURL, err = endpoints(inspectResponse.NetworkSettings.Ports, hsPortBindingIP, 8008, 8448)
+
+	// Sanity check that the URL's match the expected binding hostname. It's important
+	// that we use the canonical publically accessible hostname for the homeserver as ...
+	// such as important cookies that are set during a SSO/OIDC login process (cookies are
+	// scoped to the domain).
+	err = assertHostnameEqual(baseURL, hsPortBindingIP)
+	if err != nil {
+		return "", "", fmt.Errorf("failed to assert baseURL has the correct hostname: %w", err)
+	}
+	err = assertHostnameEqual(fedBaseURL, hsPortBindingIP)
+	if err != nil {
+		return "", "", fmt.Errorf("failed to assert fedBaseURL has the correct hostname: %w", err)
+	}
+
+	return baseURL, fedBaseURL, nil
+}
+
+// Waits until a homeserver container has NAT ports assigned.
+func waitForPorts(ctx context.Context, docker *client.Client, containerID string) (err error) {
 	// We need to hammer the inspect endpoint until the ports show up, they don't appear immediately.
-	var inspect container.InspectResponse
 	inspectStartTime := time.Now()
 	for time.Since(inspectStartTime) < time.Second {
-		inspect, err = docker.ContainerInspect(ctx, containerID)
-		if err != nil {
-			return "", "", err
-		}
-		if inspect.State != nil && !inspect.State.Running {
-			// the container exited, bail out with a container ID for logs
-			return "", "", fmt.Errorf("container is not running, state=%v", inspect.State.Status)
-		}
-		baseURL, fedBaseURL, err = endpoints(inspect.NetworkSettings.Ports, 8008, 8448)
+		_, err = inspectPortsOnContainer(ctx, docker, containerID)
 		if err == nil {
 			break
 		}
+
+		if inspectionErr, ok := err.(*ContainerInspectionError); ok && inspectionErr.Fatal {
+			// If the error is fatal, we should not retry.
+			return fmt.Errorf("Fatal inspection error: %s", err)
+		}
 	}
-	return baseURL, fedBaseURL, nil
+	return nil
+}
+
+type ContainerInspectionError struct {
+	// Error message
+	msg string
+	// Whether this error should stop retrying to inspect the container.
+	Fatal bool
+}
+
+func (e *ContainerInspectionError) Error() string { return e.msg }
+
+func inspectPortsOnContainer(
+	ctx context.Context,
+	docker *client.Client,
+	containerID string,
+) (inspectResponse container.InspectResponse, err error) {
+	inspectResponse, err = docker.ContainerInspect(ctx, containerID)
+	if err != nil {
+		return container.InspectResponse{}, &ContainerInspectionError{
+			msg:   err.Error(),
+			Fatal: false,
+		}
+	}
+	if inspectResponse.State != nil && !inspectResponse.State.Running {
+		// the container exited, bail out with a container ID for logs
+		return container.InspectResponse{}, &ContainerInspectionError{
+			msg:   fmt.Sprintf("container (%s) is not running, state=%v", containerID, inspectResponse.State.Status),
+			Fatal: true,
+		}
+	}
+
+	return inspectResponse, nil
 }
 
 // Waits until a homeserver deployment is ready to serve requests.

From cdeb5ece673ee8bf9021c4e8ebc1f58a09c9b601 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Mon, 26 May 2025 17:28:36 -0500
Subject: [PATCH 02/15] Better docstrings

---
 internal/docker/builder.go  |  8 +++++++-
 internal/docker/deployer.go | 13 ++++++++-----
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/internal/docker/builder.go b/internal/docker/builder.go
index 6b5e3720..7582d220 100644
--- a/internal/docker/builder.go
+++ b/internal/docker/builder.go
@@ -539,6 +539,7 @@ func printPortBindingsOfAllComplementContainers(docker *client.Client, contextSt
 	log.Printf("=============== %s : END ALL COMPLEMENT DOCKER PORT BINDINGS ===============\n\n\n", contextStr)
 }
 
+// Transform the homeserver ports into the base URL and federation base URL.
 func endpoints(p nat.PortMap, hsPortBindingIP string, csPort, ssPort int) (baseURL, fedBaseURL string, err error) {
 	csapiPortBinding, err := findPortBinding(p, hsPortBindingIP, csPort)
 	if err != nil {
@@ -554,7 +555,12 @@ func endpoints(p nat.PortMap, hsPortBindingIP string, csPort, ssPort int) (baseU
 	return
 }
 
-// Find a matching port binding for the given host/port in the nat.PortMap.
+// Find a matching port binding for the given host/port in the `nat.PortMap`.
+//
+// This function will return the first port binding that matches the given host IP. If a
+// `0.0.0.0` binding is found, we will assume that it is listening on all interfaces,
+// including the `hsPortBindingIP`, and return a binding with the `hsPortBindingIP` as
+// the host IP.
 func findPortBinding(p nat.PortMap, hsPortBindingIP string, port int) (portBinding nat.PortBinding, err error) {
 	portString := fmt.Sprintf("%d/tcp", port)
 	portBindings, ok := p[nat.Port(portString)]
diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index 1b94d24e..8da88e30 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -529,6 +529,8 @@ func assertHostnameEqual(inputUrl string, expectedHostname string) error {
 	return nil
 }
 
+// Returns URL's that are accessible from the host machine (outside the container) for
+// the homeserver's client API and federation API.
 func getHostAccessibleHomeserverUrls(ctx context.Context, docker *client.Client, containerID string, hsPortBindingIP string) (baseURL string, fedBaseURL string, err error) {
 	inspectResponse, err := inspectPortsOnContainer(ctx, docker, containerID)
 	if err != nil {
@@ -537,10 +539,10 @@ func getHostAccessibleHomeserverUrls(ctx context.Context, docker *client.Client,
 
 	baseURL, fedBaseURL, err = endpoints(inspectResponse.NetworkSettings.Ports, hsPortBindingIP, 8008, 8448)
 
-	// Sanity check that the URL's match the expected binding hostname. It's important
-	// that we use the canonical publically accessible hostname for the homeserver as ...
-	// such as important cookies that are set during a SSO/OIDC login process (cookies are
-	// scoped to the domain).
+	// Sanity check that the URL's match the expected configured binding hostname. It's
+	// also important that we use the canonical publicly accessible hostname for the
+	// homeserver for some situations like SSO/OIDC login where important cookies are set
+	// for the domain.
 	err = assertHostnameEqual(baseURL, hsPortBindingIP)
 	if err != nil {
 		return "", "", fmt.Errorf("failed to assert baseURL has the correct hostname: %w", err)
@@ -574,7 +576,8 @@ func waitForPorts(ctx context.Context, docker *client.Client, containerID string
 type ContainerInspectionError struct {
 	// Error message
 	msg string
-	// Whether this error should stop retrying to inspect the container.
+	// Indicates whether the caller should stop retrying to inspect the container because
+	// it has already exited.
 	Fatal bool
 }
 

From 5077e33fd270816af2db47cd819856f75e2be481 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Tue, 27 May 2025 16:24:13 -0500
Subject: [PATCH 03/15] Incorporate empty `HostIP` podman edge case

As discovered by @richvdh

Spawning from

 - https://github.com/matrix-org/complement/pull/779#discussion_r2110166553
 - https://github.com/matrix-org/complement/pull/779#discussion_r2110169402
---
 internal/docker/builder.go | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/internal/docker/builder.go b/internal/docker/builder.go
index 7582d220..be368647 100644
--- a/internal/docker/builder.go
+++ b/internal/docker/builder.go
@@ -581,6 +581,13 @@ func findPortBinding(p nat.PortMap, hsPortBindingIP string, port int) (portBindi
 				HostIP:   hsPortBindingIP,
 				HostPort: pb.HostPort,
 			}, nil
+		} else if pb.HostIP == "" && hsPortBindingIP == "127.0.0.1" {
+			// `HostIP` can be empty in certain environments (observed with podman v4.3.1). We
+			// will assume this is only a binding for `127.0.0.1`.
+			return nat.PortBinding{
+				HostIP:   hsPortBindingIP,
+				HostPort: pb.HostPort,
+			}, nil
 		}
 	}
 

From 1bfe471d95d5582ece9628fd6e80905a8a7e3bf4 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 10:27:06 -0500
Subject: [PATCH 04/15] URL grammar

---
 internal/docker/deployer.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index 8da88e30..70775e71 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -317,7 +317,7 @@ func (d *Deployer) StartServer(hsDep *HomeserverDeployment) error {
 	if err != nil {
 		return fmt.Errorf("failed to wait for ports on container %s: %s", hsDep.ContainerID, err)
 	}
-	baseURL, fedBaseURL, err := getHostAccessibleHomeserverUrls(ctx, d.Docker, hsDep.ContainerID, d.config.HSPortBindingIP)
+	baseURL, fedBaseURL, err := getHostAccessibleHomeserverURLs(ctx, d.Docker, hsDep.ContainerID, d.config.HSPortBindingIP)
 	if err != nil {
 		return fmt.Errorf("failed to get host accessible homeserver URL's from container %s: %s", hsDep.ContainerID, err)
 	}
@@ -450,7 +450,7 @@ func deployImage(
 	if err != nil {
 		return stubDeployment, fmt.Errorf("%s: failed to wait for ports on container %s: %w", contextStr, containerID, err)
 	}
-	baseURL, fedBaseURL, err := getHostAccessibleHomeserverUrls(ctx, docker, containerID, cfg.HSPortBindingIP)
+	baseURL, fedBaseURL, err := getHostAccessibleHomeserverURLs(ctx, docker, containerID, cfg.HSPortBindingIP)
 	if err != nil {
 		return stubDeployment, fmt.Errorf(
 			"%s: failed to get host accessible homeserver URL's from container %s: %s",
@@ -529,9 +529,9 @@ func assertHostnameEqual(inputUrl string, expectedHostname string) error {
 	return nil
 }
 
-// Returns URL's that are accessible from the host machine (outside the container) for
+// Returns URLs that are accessible from the host machine (outside the container) for
 // the homeserver's client API and federation API.
-func getHostAccessibleHomeserverUrls(ctx context.Context, docker *client.Client, containerID string, hsPortBindingIP string) (baseURL string, fedBaseURL string, err error) {
+func getHostAccessibleHomeserverURLs(ctx context.Context, docker *client.Client, containerID string, hsPortBindingIP string) (baseURL string, fedBaseURL string, err error) {
 	inspectResponse, err := inspectPortsOnContainer(ctx, docker, containerID)
 	if err != nil {
 		return "", "", fmt.Errorf("failed to inspect ports: %w", err)
@@ -539,7 +539,7 @@ func getHostAccessibleHomeserverUrls(ctx context.Context, docker *client.Client,
 
 	baseURL, fedBaseURL, err = endpoints(inspectResponse.NetworkSettings.Ports, hsPortBindingIP, 8008, 8448)
 
-	// Sanity check that the URL's match the expected configured binding hostname. It's
+	// Sanity check that the URLs match the expected configured binding hostname. It's
 	// also important that we use the canonical publicly accessible hostname for the
 	// homeserver for some situations like SSO/OIDC login where important cookies are set
 	// for the domain.

From 64147f10984ca1bba28daf4c0b32d048effb2657 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 10:53:05 -0500
Subject: [PATCH 05/15] Clarify `HSPortBindingIP` better

---
 config/config.go            | 10 +++++++++-
 internal/docker/deployer.go | 13 +++++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/config/config.go b/config/config.go
index 9e3317a5..43db89d3 100644
--- a/config/config.go
+++ b/config/config.go
@@ -106,7 +106,15 @@ type Complement struct {
 	// disable this behaviour being added later, once this has stablised.
 	EnableDirtyRuns bool
 
-	// The hostname that will be used to bind the homeserver ports to, e.g. `127.0.0.1`
+	// The IP that will be used to bind the homeserver ports to, e.g. `127.0.0.1`  (only
+	// allow localhost access), `0.0.0.0` (bind to all available interfaces).
+	//
+	// For Complement tests, this is always configured as `127.0.0.1` but can be
+	// overridden by homerunner to allow binding to a different IP address
+	// (`HOMERUNNER_HS_PORTBINDING_IP`).
+	//
+	// This field is also used for the host-accessible homeserver URLs (as the hostname)
+	// so clients in your tests can access the homeserver.
 	HSPortBindingIP string
 
 	// Name: COMPLEMENT_POST_TEST_SCRIPT
diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index 70775e71..cf1e0664 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -385,7 +385,16 @@ func deployImage(
 			"complement_hs_name":   hsName,
 		},
 	}, &container.HostConfig{
-		CapAdd:          []string{"NET_ADMIN"}, // TODO : this should be some sort of option
+		CapAdd: []string{"NET_ADMIN"}, // TODO : this should be some sort of option
+		// We use `PublishAllPorts` because although Complement only requires the ports 8008
+		// and 8448 to be accessible in the image, other custom out-of-repo tests may use
+		// additional ports that are specific to their own application.
+		//
+		// Ideally, we would only bind to `cfg.HSPortBindingIP` but there isn't a way to
+		// specify the `HostIP` when using `PublishAllPorts`. And although, we could specify
+		// a manual port mapping, it's not compatible with also having `PublishAllPorts` set
+		// to true (we run into `address already in use` errors). Binding to all interfaces
+		// means we're also listening on `cfg.HSPortBindingIP` so it's good enough.
 		PublishAllPorts: true,
 		ExtraHosts:      extraHosts,
 		Mounts:          mounts,
@@ -539,7 +548,7 @@ func getHostAccessibleHomeserverURLs(ctx context.Context, docker *client.Client,
 
 	baseURL, fedBaseURL, err = endpoints(inspectResponse.NetworkSettings.Ports, hsPortBindingIP, 8008, 8448)
 
-	// Sanity check that the URLs match the expected configured binding hostname. It's
+	// Sanity check that the URLs match the expected configured binding IP. It's
 	// also important that we use the canonical publicly accessible hostname for the
 	// homeserver for some situations like SSO/OIDC login where important cookies are set
 	// for the domain.

From 39edab52bad8f7a9b3b109af7b0bca71ce867f7f Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 12:42:04 -0500
Subject: [PATCH 06/15] Add docs for `ContainerInspectionError` being returned

See https://github.com/matrix-org/complement/pull/781#discussion_r2111211642
---
 internal/docker/deployer.go | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index cf1e0664..88b1cf32 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -541,7 +541,7 @@ func assertHostnameEqual(inputUrl string, expectedHostname string) error {
 // Returns URLs that are accessible from the host machine (outside the container) for
 // the homeserver's client API and federation API.
 func getHostAccessibleHomeserverURLs(ctx context.Context, docker *client.Client, containerID string, hsPortBindingIP string) (baseURL string, fedBaseURL string, err error) {
-	inspectResponse, err := inspectPortsOnContainer(ctx, docker, containerID)
+	inspectResponse, err := inspectContainer(ctx, docker, containerID)
 	if err != nil {
 		return "", "", fmt.Errorf("failed to inspect ports: %w", err)
 	}
@@ -569,7 +569,7 @@ func waitForPorts(ctx context.Context, docker *client.Client, containerID string
 	// We need to hammer the inspect endpoint until the ports show up, they don't appear immediately.
 	inspectStartTime := time.Now()
 	for time.Since(inspectStartTime) < time.Second {
-		_, err = inspectPortsOnContainer(ctx, docker, containerID)
+		_, err = inspectContainer(ctx, docker, containerID)
 		if err == nil {
 			break
 		}
@@ -592,7 +592,11 @@ type ContainerInspectionError struct {
 
 func (e *ContainerInspectionError) Error() string { return e.msg }
 
-func inspectPortsOnContainer(
+// inspectContainer inspects the container with the given ID and returns response.
+//
+// Returns a `ContainerInspectionError` representing the underlying error and indicates
+// `err.Fatal: true` if the container is no longer running.
+func inspectContainer(
 	ctx context.Context,
 	docker *client.Client,
 	containerID string,

From e17f4db69d8073ce1be71cf953d8d65a412c0c77 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 12:44:11 -0500
Subject: [PATCH 07/15] Make `ContainerInspectionError` private

See https://github.com/matrix-org/complement/pull/781#discussion_r2111210493
---
 internal/docker/deployer.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index 88b1cf32..d46e56a0 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -574,7 +574,7 @@ func waitForPorts(ctx context.Context, docker *client.Client, containerID string
 			break
 		}
 
-		if inspectionErr, ok := err.(*ContainerInspectionError); ok && inspectionErr.Fatal {
+		if inspectionErr, ok := err.(*containerInspectionError); ok && inspectionErr.Fatal {
 			// If the error is fatal, we should not retry.
 			return fmt.Errorf("Fatal inspection error: %s", err)
 		}
@@ -582,7 +582,7 @@ func waitForPorts(ctx context.Context, docker *client.Client, containerID string
 	return nil
 }
 
-type ContainerInspectionError struct {
+type containerInspectionError struct {
 	// Error message
 	msg string
 	// Indicates whether the caller should stop retrying to inspect the container because
@@ -590,11 +590,11 @@ type ContainerInspectionError struct {
 	Fatal bool
 }
 
-func (e *ContainerInspectionError) Error() string { return e.msg }
+func (e *containerInspectionError) Error() string { return e.msg }
 
 // inspectContainer inspects the container with the given ID and returns response.
 //
-// Returns a `ContainerInspectionError` representing the underlying error and indicates
+// Returns a `containerInspectionError` representing the underlying error and indicates
 // `err.Fatal: true` if the container is no longer running.
 func inspectContainer(
 	ctx context.Context,
@@ -603,14 +603,14 @@ func inspectContainer(
 ) (inspectResponse container.InspectResponse, err error) {
 	inspectResponse, err = docker.ContainerInspect(ctx, containerID)
 	if err != nil {
-		return container.InspectResponse{}, &ContainerInspectionError{
+		return container.InspectResponse{}, &containerInspectionError{
 			msg:   err.Error(),
 			Fatal: false,
 		}
 	}
 	if inspectResponse.State != nil && !inspectResponse.State.Running {
 		// the container exited, bail out with a container ID for logs
-		return container.InspectResponse{}, &ContainerInspectionError{
+		return container.InspectResponse{}, &containerInspectionError{
 			msg:   fmt.Sprintf("container (%s) is not running, state=%v", containerID, inspectResponse.State.Status),
 			Fatal: true,
 		}

From 4eb1cb6526b46320d623cfbe3ecb9d5a2ac52e62 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 12:46:42 -0500
Subject: [PATCH 08/15] Use idiomatic Go doc comments

See https://github.com/matrix-org/complement/pull/781#discussion_r2111238664
---
 internal/docker/builder.go  | 4 ++--
 internal/docker/deployer.go | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/internal/docker/builder.go b/internal/docker/builder.go
index be368647..079a32be 100644
--- a/internal/docker/builder.go
+++ b/internal/docker/builder.go
@@ -539,7 +539,7 @@ func printPortBindingsOfAllComplementContainers(docker *client.Client, contextSt
 	log.Printf("=============== %s : END ALL COMPLEMENT DOCKER PORT BINDINGS ===============\n\n\n", contextStr)
 }
 
-// Transform the homeserver ports into the base URL and federation base URL.
+// endpoints transforms the homeserver ports into the base URL and federation base URL.
 func endpoints(p nat.PortMap, hsPortBindingIP string, csPort, ssPort int) (baseURL, fedBaseURL string, err error) {
 	csapiPortBinding, err := findPortBinding(p, hsPortBindingIP, csPort)
 	if err != nil {
@@ -555,7 +555,7 @@ func endpoints(p nat.PortMap, hsPortBindingIP string, csPort, ssPort int) (baseU
 	return
 }
 
-// Find a matching port binding for the given host/port in the `nat.PortMap`.
+// findPortBinding finds a matching port binding for the given host/port in the `nat.PortMap`.
 //
 // This function will return the first port binding that matches the given host IP. If a
 // `0.0.0.0` binding is found, we will assume that it is listening on all interfaces,
diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index d46e56a0..79fb2c3b 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -538,8 +538,8 @@ func assertHostnameEqual(inputUrl string, expectedHostname string) error {
 	return nil
 }
 
-// Returns URLs that are accessible from the host machine (outside the container) for
-// the homeserver's client API and federation API.
+// getHostAccessibleHomeserverURLs returns URLs that are accessible from the host
+// machine (outside the container) for the homeserver's client API and federation API.
 func getHostAccessibleHomeserverURLs(ctx context.Context, docker *client.Client, containerID string, hsPortBindingIP string) (baseURL string, fedBaseURL string, err error) {
 	inspectResponse, err := inspectContainer(ctx, docker, containerID)
 	if err != nil {
@@ -564,7 +564,7 @@ func getHostAccessibleHomeserverURLs(ctx context.Context, docker *client.Client,
 	return baseURL, fedBaseURL, nil
 }
 
-// Waits until a homeserver container has NAT ports assigned.
+// waitForPorts waits until a homeserver container has NAT ports assigned.
 func waitForPorts(ctx context.Context, docker *client.Client, containerID string) (err error) {
 	// We need to hammer the inspect endpoint until the ports show up, they don't appear immediately.
 	inspectStartTime := time.Now()
@@ -619,7 +619,7 @@ func inspectContainer(
 	return inspectResponse, nil
 }
 
-// Waits until a homeserver deployment is ready to serve requests.
+// waitForContainer waits until a homeserver deployment is ready to serve requests.
 func waitForContainer(ctx context.Context, docker *client.Client, hsDep *HomeserverDeployment, stopTime time.Time) (iterCount int, lastErr error) {
 	iterCount = 0
 

From 39c2ac298be5fee23967ab525a8bf417173b4401 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 12:48:54 -0500
Subject: [PATCH 09/15] Remove incorrect fallback when we can't find a matching
 port binding

See https://github.com/matrix-org/complement/pull/781#discussion_r2111233256
---
 internal/docker/builder.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/docker/builder.go b/internal/docker/builder.go
index 079a32be..b2bcc1cc 100644
--- a/internal/docker/builder.go
+++ b/internal/docker/builder.go
@@ -591,7 +591,7 @@ func findPortBinding(p nat.PortMap, hsPortBindingIP string, port int) (portBindi
 		}
 	}
 
-	return portBindings[0], nil
+	return nat.PortBinding{}, fmt.Errorf("unable to find matching port binding for %s %s: %+v", hsPortBindingIP, portString, p)
 }
 
 type result struct {

From da98b903e9abdbb08880382fda74b8468fcff851 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 12:51:33 -0500
Subject: [PATCH 10/15] Update to `waitForContainer`

See https://github.com/matrix-org/complement/pull/781#discussion_r2111220790
---
 internal/docker/deployer.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index 79fb2c3b..c240e06b 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -313,7 +313,7 @@ func (d *Deployer) StartServer(hsDep *HomeserverDeployment) error {
 	}
 
 	// Wait for the container to be ready.
-	err = waitForPorts(ctx, d.Docker, hsDep.ContainerID)
+	err = waitForContainer(ctx, d.Docker, hsDep.ContainerID)
 	if err != nil {
 		return fmt.Errorf("failed to wait for ports on container %s: %s", hsDep.ContainerID, err)
 	}
@@ -455,7 +455,7 @@ func deployImage(
 	}
 
 	// Wait for the container to be ready.
-	err = waitForPorts(ctx, docker, containerID)
+	err = waitForContainer(ctx, docker, containerID)
 	if err != nil {
 		return stubDeployment, fmt.Errorf("%s: failed to wait for ports on container %s: %w", contextStr, containerID, err)
 	}
@@ -564,8 +564,8 @@ func getHostAccessibleHomeserverURLs(ctx context.Context, docker *client.Client,
 	return baseURL, fedBaseURL, nil
 }
 
-// waitForPorts waits until a homeserver container has NAT ports assigned.
-func waitForPorts(ctx context.Context, docker *client.Client, containerID string) (err error) {
+// waitForContainer waits until a homeserver container has NAT ports assigned.
+func waitForContainer(ctx context.Context, docker *client.Client, containerID string) (err error) {
 	// We need to hammer the inspect endpoint until the ports show up, they don't appear immediately.
 	inspectStartTime := time.Now()
 	for time.Since(inspectStartTime) < time.Second {

From e33cbc8fe2ea3e14e1ed14616564e6a0d2e6c4f7 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 13:02:17 -0500
Subject: [PATCH 11/15] Revert "Update to `waitForContainer`"

This reverts commit da98b903e9abdbb08880382fda74b8468fcff851.
---
 internal/docker/deployer.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index c240e06b..79fb2c3b 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -313,7 +313,7 @@ func (d *Deployer) StartServer(hsDep *HomeserverDeployment) error {
 	}
 
 	// Wait for the container to be ready.
-	err = waitForContainer(ctx, d.Docker, hsDep.ContainerID)
+	err = waitForPorts(ctx, d.Docker, hsDep.ContainerID)
 	if err != nil {
 		return fmt.Errorf("failed to wait for ports on container %s: %s", hsDep.ContainerID, err)
 	}
@@ -455,7 +455,7 @@ func deployImage(
 	}
 
 	// Wait for the container to be ready.
-	err = waitForContainer(ctx, docker, containerID)
+	err = waitForPorts(ctx, docker, containerID)
 	if err != nil {
 		return stubDeployment, fmt.Errorf("%s: failed to wait for ports on container %s: %w", contextStr, containerID, err)
 	}
@@ -564,8 +564,8 @@ func getHostAccessibleHomeserverURLs(ctx context.Context, docker *client.Client,
 	return baseURL, fedBaseURL, nil
 }
 
-// waitForContainer waits until a homeserver container has NAT ports assigned.
-func waitForContainer(ctx context.Context, docker *client.Client, containerID string) (err error) {
+// waitForPorts waits until a homeserver container has NAT ports assigned.
+func waitForPorts(ctx context.Context, docker *client.Client, containerID string) (err error) {
 	// We need to hammer the inspect endpoint until the ports show up, they don't appear immediately.
 	inspectStartTime := time.Now()
 	for time.Since(inspectStartTime) < time.Second {

From cb2586f8be1fd5f93ec9339f8d55210c12cdc909 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 13:13:01 -0500
Subject: [PATCH 12/15] Actually look for ports in `waitForPorts`

See https://github.com/matrix-org/complement/pull/781
---
 internal/docker/deployer.go | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index 79fb2c3b..01a17f26 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -564,20 +564,24 @@ func getHostAccessibleHomeserverURLs(ctx context.Context, docker *client.Client,
 	return baseURL, fedBaseURL, nil
 }
 
-// waitForPorts waits until a homeserver container has NAT ports assigned.
-func waitForPorts(ctx context.Context, docker *client.Client, containerID string) (err error) {
+// waitForPorts waits until a homeserver container has NAT ports assigned (8008, 8448).
+func waitForPorts(ctx context.Context, docker *client.Client, containerID string, hsPortBindingIP string) (err error) {
 	// We need to hammer the inspect endpoint until the ports show up, they don't appear immediately.
 	inspectStartTime := time.Now()
 	for time.Since(inspectStartTime) < time.Second {
-		_, err = inspectContainer(ctx, docker, containerID)
-		if err == nil {
-			break
-		}
-
+		inspectResponse, err := inspectContainer(ctx, docker, containerID)
 		if inspectionErr, ok := err.(*containerInspectionError); ok && inspectionErr.Fatal {
 			// If the error is fatal, we should not retry.
 			return fmt.Errorf("Fatal inspection error: %s", err)
 		}
+
+		// Check to see if we can see the ports yet
+		_, csPortErr := findPortBinding(inspectResponse.NetworkSettings.Ports, hsPortBindingIP, 8008)
+		_, ssPortErr := findPortBinding(inspectResponse.NetworkSettings.Ports, hsPortBindingIP, 8448)
+		if csPortErr == nil && ssPortErr == nil {
+			break
+		}
+
 	}
 	return nil
 }

From b91e72316521f9a1b7ff1cb05ab11f1ef6b29905 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Wed, 28 May 2025 13:19:31 -0500
Subject: [PATCH 13/15] Fix new args missing

---
 internal/docker/deployer.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index 01a17f26..5c6c627c 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -313,7 +313,7 @@ func (d *Deployer) StartServer(hsDep *HomeserverDeployment) error {
 	}
 
 	// Wait for the container to be ready.
-	err = waitForPorts(ctx, d.Docker, hsDep.ContainerID)
+	err = waitForPorts(ctx, d.Docker, hsDep.ContainerID, d.config.HSPortBindingIP)
 	if err != nil {
 		return fmt.Errorf("failed to wait for ports on container %s: %s", hsDep.ContainerID, err)
 	}
@@ -455,7 +455,7 @@ func deployImage(
 	}
 
 	// Wait for the container to be ready.
-	err = waitForPorts(ctx, docker, containerID)
+	err = waitForPorts(ctx, docker, containerID, cfg.HSPortBindingIP)
 	if err != nil {
 		return stubDeployment, fmt.Errorf("%s: failed to wait for ports on container %s: %w", contextStr, containerID, err)
 	}

From 50ea1325ba242394b5e55e2e430b7b30178c8d6f Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Thu, 29 May 2025 09:30:30 -0500
Subject: [PATCH 14/15] Returns error on failure

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 internal/docker/deployer.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/docker/deployer.go b/internal/docker/deployer.go
index 5c6c627c..6ce9dc33 100644
--- a/internal/docker/deployer.go
+++ b/internal/docker/deployer.go
@@ -598,7 +598,7 @@ func (e *containerInspectionError) Error() string { return e.msg }
 
 // inspectContainer inspects the container with the given ID and returns response.
 //
-// Returns a `containerInspectionError` representing the underlying error and indicates
+// On failure, returns a `containerInspectionError` representing the underlying error and indicates
 // `err.Fatal: true` if the container is no longer running.
 func inspectContainer(
 	ctx context.Context,

From 616981f61114c15c87e2bc5eaa931019f56a85b7 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Thu, 29 May 2025 09:33:49 -0500
Subject: [PATCH 15/15] `HSPortBindingIP` doc-comment usage nuance

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
---
 config/config.go | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/config/config.go b/config/config.go
index 43db89d3..5260a62e 100644
--- a/config/config.go
+++ b/config/config.go
@@ -106,14 +106,13 @@ type Complement struct {
 	// disable this behaviour being added later, once this has stablised.
 	EnableDirtyRuns bool
 
-	// The IP that will be used to bind the homeserver ports to, e.g. `127.0.0.1`  (only
-	// allow localhost access), `0.0.0.0` (bind to all available interfaces).
+	// The IP that is used to connect to the running homeserver from the host.
 	//
 	// For Complement tests, this is always configured as `127.0.0.1` but can be
 	// overridden by homerunner to allow binding to a different IP address
 	// (`HOMERUNNER_HS_PORTBINDING_IP`).
 	//
-	// This field is also used for the host-accessible homeserver URLs (as the hostname)
+	// This field is used for the host-accessible homeserver URLs (as the hostname)
 	// so clients in your tests can access the homeserver.
 	HSPortBindingIP string