Skip to content

fix: setup ExposeHostPorts forward on container start hook #3200

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.

Sign up for GitHub

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

Open
mdelapenya wants to merge 13 commits into
base: main
Choose a base branch
from
+27 −7
Open

fix: setup ExposeHostPorts forward on container start hook #3200

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ad3f698
setup ExposeHostPorts forwards on container start
hathvi Oct 5, 2024
90c865b
Merge branch 'main' into expose-host-ports-start
mdelapenya Jun 3, 2025
1d7b668
Merge branch 'main' into expose-host-ports-start
mdelapenya Jun 4, 2025
ab3dc11
Merge branch 'main' into expose-host-ports-start
mdelapenya Jun 5, 2025
2ff625b
Merge branch 'main' into expose-host-ports-start
mdelapenya Jun 10, 2025
fc9f012
Merge branch 'main' into expose-host-ports-start
mdelapenya Jun 30, 2025
dc4d61f
Merge branch 'main' into expose-host-ports-start
mdelapenya Sep 5, 2025
0d2c28d
chore(port-forwarding): create sshd container before the container is…
mdelapenya Sep 5, 2025
1f48580
Merge branch 'main' into expose-host-ports-start
mdelapenya Nov 17, 2025
bb3219c
Merge branch 'main' into expose-host-ports-start
mdelapenya Nov 18, 2025
5f56647
chore: simplify
mdelapenya Nov 19, 2025
327ed8e
fix(solace): increase wait timeout
mdelapenya Nov 20, 2025
092e175
chore(ci): bump setup docker action
mdelapenya Nov 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions port_forwarding.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,11 @@ func exposeHostPorts(ctx context.Context, req *ContainerRequest, ports ...int) (
},
}

// after the container is ready, create the SSH tunnel
// before the container is created, create the SSH tunnel
// for each exposed port from the host.
sshdConnectHook = ContainerLifecycleHooks{
PostReadies: []ContainerHook{
func(ctx context.Context, _ Container) error {
PreCreates: []ContainerRequestHook{
func(ctx context.Context, req ContainerRequest) error {
return sshdContainer.exposeHostPort(ctx, req.HostAccessPorts...)
},
},
Expand Down
24 changes: 23 additions & 1 deletion port_forwarding_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,41 @@ import (
"github.com/testcontainers/testcontainers-go"
tcexec "github.com/testcontainers/testcontainers-go/exec"
"github.com/testcontainers/testcontainers-go/network"
"github.com/testcontainers/testcontainers-go/wait"
)

const (
expectedResponse = "Hello, World!"
)

func TestExposeHostPorts(t *testing.T) {
hostPorts := make([]int, 3)
const numberOfPorts = 3

servers := make([]*httptest.Server, numberOfPorts)
hostPorts := make([]int, numberOfPorts)
waitStrategies := make([]wait.Strategy, numberOfPorts)

Comment on lines +18 to +31
Copy link

@coderabbitai coderabbitai bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

New waitStrategies are never used; clarify intent or remove to avoid dead code

The waitStrategies slice is fully populated with wait.ForExec(...).WithExitCodeMatcher(...).WithResponseMatcher(...), but it’s never read or passed into testExposeHostPorts / container options. As written, the new wait logic has no effect, and the tests still only validate host access via the explicit containerHasHostAccess / containerHasNoHostAccess calls after the container is started.

Similarly, the servers slice is only written to and never read, so it’s functionally redundant right now.

Depending on your intent:

  • If these strategies are meant to assert that host ports are reachable during container startup (per the PR description), you likely want to plumb them into the container’s waiting strategy (e.g., via a WaitingFor / wait customization) and possibly thread them into testExposeHostPorts.
  • If they’re not required, consider removing waitStrategies (and the wait import) and possibly the unused servers slice to keep the test simple and avoid confusion.

Also applies to: 37-49

🤖 Prompt for AI Agents 
In port_forwarding_test.go around lines 18 to 31 (and similarly 37 to 49), the
slices waitStrategies and servers are built but never used; either wire them
into the container waiting logic or remove them — to fix quickly, delete the
wait import, the waitStrategies and servers declarations and all code that
populates them (including the per-port wait.ForExec(...) setup), and remove any
related unused variables so the test only keeps hostPorts and the actual
assertions; if you prefer to use them instead, pass the corresponding
wait.Strategy into the container creation (e.g., via
WaitingFor/WithStartupTimeout or by extending testExposeHostPorts to accept and
apply a per-port wait strategy) and ensure servers is read where its
httptest.Servers are needed.

for i := range hostPorts {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
fmt.Fprint(w, expectedResponse)
}))
hostPorts[i] = server.Listener.Addr().(*net.TCPAddr).Port

servers[i] = server
freePort := server.Listener.Addr().(*net.TCPAddr).Port
hostPorts[i] = freePort

waitStrategies[i] = wait.
ForExec([]string{"wget", "-q", "-O", "-", fmt.Sprintf("http://%s:%d", testcontainers.HostInternal, freePort)}).
WithExitCodeMatcher(func(code int) bool {
return code == 0
}).
WithResponseMatcher(func(body io.Reader) bool {
bs, err := io.ReadAll(body)
require.NoError(t, err)
return string(bs) == expectedResponse
})

t.Cleanup(server.Close)
}

Expand Down
Loading