Bind port 443 for HTTPS

[carole-lavillonniere]

Motivation

Port 443 was exposed by the LocalStack container but never mapped to the host, making HTTPS endpoints unreachable (e.g. `https://localhost.localstack.cloud/_localstack/health\` returned connection refused).

Tests

New sub-tests in `TestStartCommandSetsUpContainerCorrectly` verify that both `http://localhost.localstack.cloud:4566/_localstack/health\` and `https://localhost.localstack.cloud/_localstack/health\` return 200 after `lstk start`. The HTTPS test was failing before this fix.

Closes DRG-661

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add HTTPS support to the container gateway by configuring port 443 as an additional listen endpoint alongside port 4566, updating port mappings accordingly, and adding integration tests to verify both HTTP and HTTPS health endpoints function correctly.

Changes

Cohort / File(s)	Summary
Gateway Configuration internal/container/start.go	Added port 443 to GATEWAY_LISTEN environment variable (now :4566,:443) and updated servicePortRange() to include explicit port 443 mapping followed by existing 4510–4559 range.
Unit Tests internal/container/start_test.go	Updated servicePortRange() test assertions to reflect 51-entry slice (up from 50) with port 443 at index 0, followed by ports 4510–4559 at indices 1–50.
Integration Tests test/integration/start_test.go	Updated GATEWAY_LISTEN assertion to :4566,:443, added Docker port binding verification for 443/tcp, and added health endpoint checks for both HTTP (http://localhost.localstack.cloud:4566/_localstack/health) and HTTPS (https://localhost.localstack.cloud/_localstack/health) with insecure TLS verification.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Bind port 443 for HTTPS' directly and clearly summarizes the main change—adding port 443 binding for HTTPS support to the container port mappings.
Description check	✅ Passed	The description is directly related to the changeset, explaining the motivation (port 443 not mapped to host), the fix applied, and the tests added to verify the solution works.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch port-443

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/container/start.go (1)

427-431: ⚠️ Potential issue | 🟠 Major

Preflight port checks now miss a required host port (443).

Adding 443 to ExtraPorts can fail startup later with a Docker bind error, but current preflight validation only checks the main port. Please validate ExtraPorts host ports (at least 443) before rt.Start so users get deterministic Port ... already in use feedback.

🔧 Suggested fix

diff --git a/internal/container/start.go b/internal/container/start.go
@@
 func selectContainersToStart(ctx context.Context, rt runtime.Runtime, sink output.Sink, tel *telemetry.Client, containers []runtime.ContainerConfig) ([]runtime.ContainerConfig, error) {
 	var filtered []runtime.ContainerConfig
 	for _, c := range containers {
@@
 		if err := ports.CheckAvailable(c.Port); err != nil {
 			emitPortInUseError(sink, c.Port)
 			emitEmulatorStartError(ctx, tel, c, telemetry.ErrCodePortConflict, err.Error())
 			return nil, output.NewSilentError(err)
 		}
+		seen := map[string]struct{}{c.Port: {}}
+		for _, ep := range c.ExtraPorts {
+			if ep.HostPort == "" {
+				continue
+			}
+			if _, ok := seen[ep.HostPort]; ok {
+				continue
+			}
+			if err := ports.CheckAvailable(ep.HostPort); err != nil {
+				emitPortInUseError(sink, ep.HostPort)
+				emitEmulatorStartError(ctx, tel, c, telemetry.ErrCodePortConflict, err.Error())
+				return nil, output.NewSilentError(err)
+			}
+			seen[ep.HostPort] = struct{}{}
+		}
 		filtered = append(filtered, c)
 	}
 	return filtered, nil
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/container/start.go` around lines 427 - 431, Preflight validation
only checks the main port and omits host ports added to the ports slice
(including the hardcoded "443"), so a bind error can occur later during
rt.Start; update the preflight check to validate all host ports from the ports
slice (and from ExtraPorts) before calling rt.Start by iterating over ports (the
runtime.PortMapping entries constructed with ContainerPort/HostPort and the
ExtraPorts collection) and ensuring each HostPort is available, returning a
deterministic "Port X already in use" error if any are occupied.

🧹 Nitpick comments (1)

test/integration/start_test.go (1)

207-226: Add explicit client timeouts to prevent hanging integration runs.

The new health checks perform live network I/O without a request timeout, which can stall tests indefinitely on transport/DNS issues.

⏱️ Suggested hardening

diff --git a/test/integration/start_test.go b/test/integration/start_test.go
@@
 import (
 	"context"
 	"crypto/tls"
 	"fmt"
 	"net"
 	"net/http"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
+	"time"
@@
 	t.Run("http health endpoint", func(t *testing.T) {
-		resp, err := http.Get("http://localhost.localstack.cloud:4566/_localstack/health")
+		client := &http.Client{Timeout: 10 * time.Second}
+		resp, err := client.Get("http://localhost.localstack.cloud:4566/_localstack/health")
 		require.NoError(t, err)
 		defer resp.Body.Close()
 		assert.Equal(t, http.StatusOK, resp.StatusCode)
 	})
@@
 		client := &http.Client{
+			Timeout: 10 * time.Second,
 			Transport: &http.Transport{
 				TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
 			},
 		}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/integration/start_test.go` around lines 207 - 226, The health-check
tests lack request timeouts and can hang; update both t.Run blocks to use
explicit timeouts: for the "http health endpoint" replace http.Get with an
http.Client that has a Timeout (e.g., 5s) and call client.Get, and for the
"https health endpoint" set the same Timeout on the existing client (the one
with Transport and TLSClientConfig) before calling client.Get; alternatively use
context.WithTimeout and http.NewRequestWithContext to bound the request. Ensure
you update the code paths that reference resp, err, client.Get and the test
names so the tests fail fast on network/DNS issues.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@internal/container/start.go`:
- Around line 427-431: Preflight validation only checks the main port and omits
host ports added to the ports slice (including the hardcoded "443"), so a bind
error can occur later during rt.Start; update the preflight check to validate
all host ports from the ports slice (and from ExtraPorts) before calling
rt.Start by iterating over ports (the runtime.PortMapping entries constructed
with ContainerPort/HostPort and the ExtraPorts collection) and ensuring each
HostPort is available, returning a deterministic "Port X already in use" error
if any are occupied.

---

Nitpick comments:
In `@test/integration/start_test.go`:
- Around line 207-226: The health-check tests lack request timeouts and can
hang; update both t.Run blocks to use explicit timeouts: for the "http health
endpoint" replace http.Get with an http.Client that has a Timeout (e.g., 5s) and
call client.Get, and for the "https health endpoint" set the same Timeout on the
existing client (the one with Transport and TLSClientConfig) before calling
client.Get; alternatively use context.WithTimeout and http.NewRequestWithContext
to bound the request. Ensure you update the code paths that reference resp, err,
client.Get and the test names so the tests fail fast on network/DNS issues.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 6986ff80-6bae-4b32-bf69-cf35c88e0bb9

📥 Commits

Reviewing files that changed from the base of the PR and between a7f6804 and bdef5e9.

📒 Files selected for processing (3)

• internal/container/start.go
• internal/container/start_test.go
• test/integration/start_test.go

[anisaoshafi]

👏🏼