From 8d21eb0d75a510330dc47cf4304c2054dcf6eb80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristian=20Pallar=C3=A9s?= Date: Wed, 5 Aug 2026 09:57:34 +0200 Subject: [PATCH 1/3] Add expose_ports config option to publish extra container ports Co-Authored-By: Claude --- CLAUDE.md | 2 +- internal/config/CLAUDE.md | 13 +++ internal/config/containers.go | 114 ++++++++++++++++++++++ internal/config/expose_ports_test.go | 110 +++++++++++++++++++++ internal/container/CLAUDE.md | 9 ++ internal/container/expose.go | 64 +++++++++++++ internal/container/expose_test.go | 138 +++++++++++++++++++++++++++ internal/container/start.go | 54 +++++++++-- test/integration/start_test.go | 69 ++++++++++++++ 9 files changed, 564 insertions(+), 9 deletions(-) create mode 100644 internal/config/expose_ports_test.go create mode 100644 internal/container/expose.go create mode 100644 internal/container/expose_test.go diff --git a/CLAUDE.md b/CLAUDE.md index cf857f74..bd911a69 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -121,7 +121,7 @@ Created automatically on first run with defaults. Supports emulator types: `aws` Only one `[[containers]]` block may be enabled at a time. `container.Start` rejects a config with more than one block up front (before health/auth checks and image pulls), since running multiple emulators together (e.g. AWS + Snowflake) is unsupported and would otherwise fail later during startup with container-name conflicts or port collisions. The guard lives on the start path (not `config.Get()`) on purpose: recovery/reporting commands like `stop`, `status`, and `logout` must still enumerate multiple running emulators. -Each `[[containers]]` block may set an optional `container_name` (override the derived container name; also what the emulator reports as `MAIN_CONTAINER_NAME`), an optional `image` (override the default Docker Hub image), and a `volumes` list of Docker-style bind specs (persistence dir, init hooks, arbitrary mounts). Container-name derivation and its deliberate decoupling from the default persistence directory, image/tag precedence, `volume` vs `volumes` semantics, and path-resolution rules are documented in `internal/config/CLAUDE.md`. +Each `[[containers]]` block may set an optional `container_name` (override the derived container name; also what the emulator reports as `MAIN_CONTAINER_NAME`), an optional `image` (override the default Docker Hub image), a `volumes` list of Docker-style bind specs (persistence dir, init hooks, arbitrary mounts), and an `expose_ports` list publishing container ports the gateway/service ranges don't cover (e.g. `expose_ports = [53]` for the emulator's DNS server). Container-name derivation and its deliberate decoupling from the default persistence directory, image/tag precedence, `volume` vs `volumes` semantics, path-resolution rules, and the `expose_ports` grammar are documented in `internal/config/CLAUDE.md`. ## Selecting the emulator (`--type`) diff --git a/internal/config/CLAUDE.md b/internal/config/CLAUDE.md index 8dd5ce08..9dff8d78 100644 --- a/internal/config/CLAUDE.md +++ b/internal/config/CLAUDE.md @@ -16,6 +16,19 @@ Validation is `validate.ContainerName` (`internal/validate`), called from `Conta Each `[[containers]]` block may set an optional `image` to override the default Docker Hub image (e.g. an internal registry mirror or a locally loaded offline image). `ContainerConfig.Image()` returns `image` as-is when it already carries a tag (so the separately-configured `tag` is dropped in that case), otherwise it appends `tag` (or `latest`); the default `localstack/:` is used when `image` is unset. +## Exposing additional ports (`expose_ports`) + +Each `[[containers]]` block may set `expose_ports` to publish container ports beyond the ones lstk publishes on its own (the edge port, the extra `GATEWAY_LISTEN` ports, the 4510-4559 service range). The motivating case (DEVX-994) is the emulator's DNS server: `expose_ports = [53]` replaces the v1 CLI's `--host-dns` flag. There is deliberately no CLI flag — the ticket asked for a config setting, and every consumer of the value is the start path. + +Grammar per entry, parsed by `parseExposePort`/`ExposedPorts` (`internal/config/containers.go`): + +- Values may be TOML **integers or strings** in the same list (`expose_ports = [53, "5354:5353/udp"]`). Ints work because viper's decoder sets `WeaklyTypedInput`, so the `[]string` field accepts numbers; the mixed form is pinned by `TestGet_ExposePortsAcceptsIntsAndStrings`. +- A string is `[host:]container[/proto]`; a bare port publishes host-port == container-port. Ports are canonicalized through `strconv` (`"0053"` → `"53"`), so two spellings of the same port collide as expected. +- **An entry that names no protocol expands to both tcp and udp.** DNS serves queries over both, so `[53]` alone has to be enough; publishing a protocol nothing listens on costs nothing. An explicit `/tcp` or `/udp` publishes only that protocol. +- Two entries that would make Docker key the same binding twice — same container port + protocol with different host ports, or the same host port + protocol for different container ports — are a **validation error** (`Validate()` calls `ExposedPorts()`), because Docker would silently keep only one. Exact duplicates are de-duplicated instead. + +Consumption is `mergeExposePorts` (`internal/container/expose.go`), appending `runtime.PortMapping`s with `Optional: false` — an explicitly requested port is a demand, so a busy or unbindable host port fails the start (same rule as a user-supplied `GATEWAY_LISTEN`). Entries clashing with a port lstk already publishes are skipped: silently when they ask for exactly what lstk already does, with a warning when the automatic mapping wins over what the user wrote. See `internal/container/CLAUDE.md` for the preflight's UDP exclusion. + ## Volume Mounts Each `[[containers]]` block accepts a `volumes` list of Docker-style `"host:container[:ro]"` bind specs (e.g. for Snowflake init hooks mounted into `/etc/localstack/init/{boot,start,ready,shutdown}.d`). The persistence/cache mount to `/var/lib/localstack` is folded into this list: the entry whose container target is `/var/lib/localstack` (`persistenceTarget` in `internal/config/containers.go`) defines the host dir backing it, and that path is what `VolumeDir()`, `lstk volume path`, and `lstk volume clear` resolve. Resolution precedence in `VolumeDir()`: a `volumes` entry targeting `/var/lib/localstack` → the legacy singular `volume = "..."` field (still honored for backward compatibility) → the default OS cache dir. Setting the persistence dir via both `volume` and a `volumes` entry with differing sources is a validation error. diff --git a/internal/config/containers.go b/internal/config/containers.go index d7c7f799..6df48f6d 100644 --- a/internal/config/containers.go +++ b/internal/config/containers.go @@ -138,6 +138,12 @@ type ContainerConfig struct { // arbitrary mounts (e.g. Snowflake init hooks) and may also contain the persistence // mount (the entry targeting /var/lib/localstack). Volumes []string `mapstructure:"volumes"` + // ExposePorts publishes additional container ports on the host, beyond the gateway + // and service ports lstk publishes by default — e.g. port 53 so the emulator's DNS + // server can be used as the host's resolver. Each entry is a bare port number + // (published on the same host port) or a Docker-style "[host:]container[/proto]" + // string. See ExposedPorts for the protocol rules. + ExposePorts []string `mapstructure:"expose_ports"` // Env is a list of named environment references defined in the top-level [env.*] config sections. Env []string `mapstructure:"env"` // Snapshot is an optional snapshot REF (e.g. "pod:my-baseline" or a local path) @@ -307,6 +313,111 @@ func (c *ContainerConfig) VolumeDir() (string, error) { return filepath.Join(cacheDir, "lstk", "volume", c.defaultName()), nil } +// PortSpec is one parsed expose_ports publication: a host port bound to a container +// port for a single protocol. +type PortSpec struct { + HostPort string + ContainerPort string + Protocol string // "tcp" or "udp" +} + +func (p PortSpec) String() string { + return p.HostPort + ":" + p.ContainerPort + "/" + p.Protocol +} + +// ExposedPorts returns the publications requested via expose_ports, in config order +// and de-duplicated. An entry that names no protocol expands to both tcp and udp: +// the motivating case is the emulator's DNS server (port 53), which serves both, and +// publishing a protocol nothing listens on is harmless. +func (c *ContainerConfig) ExposedPorts() ([]PortSpec, error) { + var specs []PortSpec + seen := map[PortSpec]bool{} + hostForContainer := map[string]string{} + containerForHost := map[string]string{} + for _, entry := range c.ExposePorts { + parsed, err := parseExposePort(entry) + if err != nil { + return nil, err + } + for _, s := range parsed { + if seen[s] { + continue + } + // Docker keys published ports by container port and by host port, so either + // kind of clash means one of the two entries would be silently discarded. + containerKey := s.ContainerPort + "/" + s.Protocol + if host, ok := hostForContainer[containerKey]; ok { + return nil, fmt.Errorf("invalid expose_ports: container port %s is published on both host port %s and %s", containerKey, host, s.HostPort) + } + hostKey := s.HostPort + "/" + s.Protocol + if container, ok := containerForHost[hostKey]; ok { + return nil, fmt.Errorf("invalid expose_ports: host port %s is claimed by both container port %s and %s", hostKey, container, s.ContainerPort) + } + seen[s] = true + hostForContainer[containerKey] = s.HostPort + containerForHost[hostKey] = s.ContainerPort + specs = append(specs, s) + } + } + return specs, nil +} + +// parseExposePort parses a single expose_ports entry of the form +// "[host:]container[/proto]"; a bare "53" publishes container port 53 on host port +// 53. One PortSpec is returned per protocol, so an entry without a protocol yields +// two (tcp and udp). +func parseExposePort(entry string) ([]PortSpec, error) { + spec := strings.TrimSpace(entry) + if spec == "" { + return nil, errors.New("invalid expose_ports entry: entry is empty") + } + + portPart, proto, hasProto := strings.Cut(spec, "/") + protocols := []string{"tcp", "udp"} + if hasProto { + proto = strings.ToLower(strings.TrimSpace(proto)) + if proto != "tcp" && proto != "udp" { + return nil, fmt.Errorf("invalid expose_ports entry %q: protocol must be \"tcp\" or \"udp\"", entry) + } + protocols = []string{proto} + } + + hostPort, containerPort, hasHost := strings.Cut(portPart, ":") + if !hasHost { + containerPort = hostPort + } + if strings.Contains(containerPort, ":") { + return nil, fmt.Errorf("invalid expose_ports entry %q: expected \"port\", \"host:container\" or \"host:container/proto\"", entry) + } + hostPort, err := parsePortNumber(entry, hostPort) + if err != nil { + return nil, err + } + containerPort, err = parsePortNumber(entry, containerPort) + if err != nil { + return nil, err + } + + specs := make([]PortSpec, 0, len(protocols)) + for _, p := range protocols { + specs = append(specs, PortSpec{HostPort: hostPort, ContainerPort: containerPort, Protocol: p}) + } + return specs, nil +} + +// parsePortNumber validates a port from an expose_ports entry and returns it in +// canonical form (so "0053" and "53" produce the same mapping key). +func parsePortNumber(entry, value string) (string, error) { + port, err := strconv.Atoi(strings.TrimSpace(value)) + if err != nil { + return "", fmt.Errorf("invalid expose_ports entry %q: %q is not a valid port number", entry, value) + } + if port < 1 || port > 65535 { + return "", fmt.Errorf("invalid expose_ports entry %q: port %d is out of range (must be 1–65535)", entry, port) + } + return strconv.Itoa(port), nil +} + // TagSuggestion returns an actionable hint naming a recent calendar tag and // "latest", for messages about tags the license server cannot parse. func TagSuggestion() string { @@ -366,6 +477,9 @@ func (c *ContainerConfig) Validate() error { if port < 1 || port > 65535 { return fmt.Errorf("port %d is out of range (must be 1–65535)", port) } + if _, err := c.ExposedPorts(); err != nil { + return err + } return c.validateVolumes() } diff --git a/internal/config/expose_ports_test.go b/internal/config/expose_ports_test.go new file mode 100644 index 00000000..e90ee2fa --- /dev/null +++ b/internal/config/expose_ports_test.go @@ -0,0 +1,110 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestExposedPorts_BarePortCoversBothProtocols(t *testing.T) { + // A bare port expands to tcp and udp so `expose_ports = [53]` is enough for the + // emulator's DNS server, which serves queries over both. + c := &ContainerConfig{Type: EmulatorAWS, Port: "4566", ExposePorts: []string{"53"}} + + specs, err := c.ExposedPorts() + require.NoError(t, err) + assert.Equal(t, []PortSpec{ + {HostPort: "53", ContainerPort: "53", Protocol: "tcp"}, + {HostPort: "53", ContainerPort: "53", Protocol: "udp"}, + }, specs) +} + +func TestExposedPorts_Forms(t *testing.T) { + tests := []struct { + entry string + want []PortSpec + }{ + {"53/udp", []PortSpec{{HostPort: "53", ContainerPort: "53", Protocol: "udp"}}}, + {"5354:53/udp", []PortSpec{{HostPort: "5354", ContainerPort: "53", Protocol: "udp"}}}, + {"53/UDP", []PortSpec{{HostPort: "53", ContainerPort: "53", Protocol: "udp"}}}, + {"9000:9000/tcp", []PortSpec{{HostPort: "9000", ContainerPort: "9000", Protocol: "tcp"}}}, + {" 25 ", []PortSpec{ + {HostPort: "25", ContainerPort: "25", Protocol: "tcp"}, + {HostPort: "25", ContainerPort: "25", Protocol: "udp"}, + }}, + {"0053", []PortSpec{ + {HostPort: "53", ContainerPort: "53", Protocol: "tcp"}, + {HostPort: "53", ContainerPort: "53", Protocol: "udp"}, + }}, + } + for _, tt := range tests { + t.Run(tt.entry, func(t *testing.T) { + c := &ContainerConfig{Type: EmulatorAWS, Port: "4566", ExposePorts: []string{tt.entry}} + specs, err := c.ExposedPorts() + require.NoError(t, err) + assert.Equal(t, tt.want, specs) + }) + } +} + +func TestExposedPorts_DeduplicatesIdenticalPublications(t *testing.T) { + c := &ContainerConfig{Type: EmulatorAWS, Port: "4566", ExposePorts: []string{"53", "53/udp", "53/tcp"}} + + specs, err := c.ExposedPorts() + require.NoError(t, err) + assert.Len(t, specs, 2) +} + +func TestExposedPorts_InvalidEntries(t *testing.T) { + tests := []struct { + name string + entries []string + wantErr string + }{ + {"empty", []string{""}, "entry is empty"}, + {"non-numeric", []string{"dns"}, "not a valid port number"}, + {"non-numeric host", []string{"dns:53"}, "not a valid port number"}, + {"zero", []string{"0"}, "out of range"}, + {"too high", []string{"65536"}, "out of range"}, + {"unknown protocol", []string{"53/sctp"}, `protocol must be "tcp" or "udp"`}, + {"too many colons", []string{"1:2:3"}, "expected"}, + {"same container port, two host ports", []string{"53/udp", "5354:53/udp"}, "container port 53/udp is published on both"}, + {"same host port, two container ports", []string{"1053:53/udp", "1053:54/udp"}, "host port 1053/udp is claimed by both"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &ContainerConfig{Type: EmulatorAWS, Port: "4566", ExposePorts: tt.entries} + _, err := c.ExposedPorts() + assert.ErrorContains(t, err, tt.wantErr) + // A bad entry must fail at config load, not at container creation. + assert.ErrorContains(t, c.Validate(), tt.wantErr) + }) + } +} + +// TestGet_ExposePortsAcceptsIntsAndStrings pins the TOML surface promised in the +// docs: numbers and strings may be mixed in the same list. +func TestGet_ExposePortsAcceptsIntsAndStrings(t *testing.T) { + // Cannot run in parallel: mutates process-wide viper state. + configFile := filepath.Join(t.TempDir(), configFileName) + require.NoError(t, os.WriteFile(configFile, []byte(` +[[containers]] +type = "aws" +port = "4566" +expose_ports = [53, "5354:5353/udp", 9000] +`), 0600)) + + viper.Reset() + t.Cleanup(viper.Reset) + viper.SetConfigFile(configFile) + require.NoError(t, viper.ReadInConfig()) + + cfg, err := Get() + require.NoError(t, err) + require.Len(t, cfg.Containers, 1) + assert.Equal(t, []string{"53", "5354:5353/udp", "9000"}, cfg.Containers[0].ExposePorts) +} diff --git a/internal/container/CLAUDE.md b/internal/container/CLAUDE.md index eeb9df51..1d4fd359 100644 --- a/internal/container/CLAUDE.md +++ b/internal/container/CLAUDE.md @@ -13,6 +13,15 @@ Detail moved out of the root CLAUDE.md. - **Leftover self-heal, label-guarded**: every container lstk creates is stamped with the label `cloud.localstack.lstk=true` (`internal/runtime/docker.go`). When the name lstk wants is held by an existing non-running container, `healLeftoverContainer` removes it only when it is positively ours or positively on its way out: (a) labeled + `created` state = a create-succeeded-start-failed leftover — removed with a warning; (b) labeled + any other state, or unlabeled-but-AutoRemove and not `created` = a container mid-self-removal — removed/waited out **silently**, because `lstk restart` races the AutoRemove of the container it just stopped on every single run (this raced CI's restart tests when the guard was created-state-only), and the unlabeled variant covers containers from pre-label lstk versions. A foreign parked container (unlabeled, no AutoRemove, or unlabeled in `created` state) is never removed; the user gets a clear "name is already taken (image X), not created by lstk" error instead of the daemon's raw name-conflict. If removal fails, the error names the container and the manual `docker rm -f` command. - **Bind-denied optional ports retry at start time**: the preflight *dials*, so it only sees ports someone is listening on — it cannot see that the daemon lacks permission to bind a port (e.g. 443 under Rancher Desktop without Administrative Access, or rootless Podman machine; daemon rejects at `ContainerStart` with `listen tcp …:443: bind: permission denied`). `startWithOptionalPortFallback` wraps `rt.Start`: when the error names an *optional* port's bind (`failedOptionalPortBind`), it warns (same drop warning, `portBindDenied` cause), removes the created-but-never-started container (AutoRemove only fires on exit, so the name would otherwise collide), and retries without that mapping. Non-bind errors and required-port binds pass through unchanged. +## User-requested ports (`expose_ports`) + +`mergeExposePorts` (`internal/container/expose.go`) appends the config's `expose_ports` publications to the mappings derived above. Grammar and validation live in `internal/config/CLAUDE.md`; the start-path rules are: + +- Mappings are **never `Optional`** — unlike the 443 lstk adds on its own, these were asked for, so a busy or unbindable host port fails the start rather than degrading silently. +- **UDP is excluded from the port preflight** (`requiredHostPorts`): `ports.CheckAvailable` dials TCP, so checking a UDP publication that way would report a phantom conflict whenever an unrelated TCP listener holds the same number. A real UDP conflict surfaces at `ContainerStart`. +- A required port whose bind the daemon refuses cannot be dropped, so `annotateRequiredPortBindError` appends the runtime-specific remedy (`tailoredPortDropHint`) to the daemon error instead. This is the common path for the sub-1024 ports `expose_ports` makes reachable: 53 under rootless Podman or Rancher Desktop without Administrative Access fails exactly like 443 does. +- Ports lstk already publishes cannot be re-declared (Docker keys bindings by container port *and* by host port, so one of the two declarations would vanish). Redundant entries are skipped silently; an entry the automatic mapping overrides is skipped with a warning. + ## Offline / Enterprise degradation There is no `--offline` flag. Instead `container.Start` degrades gracefully when internet requests fail (the common enterprise blockers: Docker Hub unreachable, proxy/TLS interception, license server unreachable): diff --git a/internal/container/expose.go b/internal/container/expose.go new file mode 100644 index 00000000..e28e3bbd --- /dev/null +++ b/internal/container/expose.go @@ -0,0 +1,64 @@ +package container + +import ( + "fmt" + + "github.com/localstack/lstk/internal/config" + "github.com/localstack/lstk/internal/output" + "github.com/localstack/lstk/internal/runtime" +) + +// mergeExposePorts appends the publications requested via the config's expose_ports +// to mappings. Ports lstk already publishes on its own (the edge port, the extra +// gateway ports, the 4510-4559 service range) cannot be re-declared: Docker keys +// bindings by container port and by host port, so a second declaration of either +// side would silently discard one of the two. Such an entry is skipped, and warned +// about whenever skipping it means the user's mapping does not happen — a redundant +// entry that asks for exactly what lstk already does is dropped quietly. +// +// Mappings are never Optional: expose_ports is an explicit request, so a busy or +// unbindable host port is a hard failure rather than a silent degradation (unlike +// the 443 lstk adds on its own). +func mergeExposePorts(sink output.Sink, mappings []runtime.PortMapping, primaryContainerPort, primaryHostPort string, exposed []config.PortSpec) []runtime.PortMapping { + hostForContainer := map[string]string{primaryContainerPort + "/tcp": primaryHostPort} + containerForHost := map[string]string{primaryHostPort + "/tcp": primaryContainerPort} + for _, m := range mappings { + proto := m.Protocol + if proto == "" { + proto = "tcp" + } + hostForContainer[m.ContainerPort+"/"+proto] = m.HostPort + containerForHost[m.HostPort+"/"+proto] = m.ContainerPort + } + + warn := func(text string) { + sink.Emit(output.MessageEvent{Severity: output.SeverityWarning, Text: text}) + } + + for _, e := range exposed { + containerKey := e.ContainerPort + "/" + e.Protocol + hostKey := e.HostPort + "/" + e.Protocol + if host, ok := hostForContainer[containerKey]; ok { + if host != e.HostPort { + warn(fmt.Sprintf( + "Ignoring expose_ports entry %s — lstk already publishes container port %s on host port %s.", + e, containerKey, host)) + } + continue + } + if container, ok := containerForHost[hostKey]; ok { + warn(fmt.Sprintf( + "Ignoring expose_ports entry %s — host port %s is already used to publish container port %s.", + e, hostKey, container)) + continue + } + hostForContainer[containerKey] = e.HostPort + containerForHost[hostKey] = e.ContainerPort + mappings = append(mappings, runtime.PortMapping{ + ContainerPort: e.ContainerPort, + HostPort: e.HostPort, + Protocol: e.Protocol, + }) + } + return mappings +} diff --git a/internal/container/expose_test.go b/internal/container/expose_test.go new file mode 100644 index 00000000..b4411d29 --- /dev/null +++ b/internal/container/expose_test.go @@ -0,0 +1,138 @@ +package container + +import ( + "context" + "errors" + "testing" + + "github.com/localstack/lstk/internal/config" + "github.com/localstack/lstk/internal/runtime" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +// autoPublished is what lstk publishes on its own for an AWS emulator on the +// default port: the gateway's extra 443 plus a couple of service-range ports +// (the full 4510-4559 range is irrelevant to these cases). +func autoPublished() []runtime.PortMapping { + return []runtime.PortMapping{ + {ContainerPort: "443", HostPort: "443", Optional: true}, + {ContainerPort: "4510", HostPort: "4510"}, + {ContainerPort: "4511", HostPort: "4511"}, + } +} + +func exposedFor(t *testing.T, entries ...string) []config.PortSpec { + t.Helper() + c := &config.ContainerConfig{Type: config.EmulatorAWS, Port: "4566", ExposePorts: entries} + specs, err := c.ExposedPorts() + require.NoError(t, err) + return specs +} + +func TestMergeExposePortsPublishesRequestedPorts(t *testing.T) { + sink := &recordingSink{} + + merged := mergeExposePorts(sink, autoPublished(), "4566", "4566", exposedFor(t, "53")) + + // expose_ports is an explicit request, so the mappings are never Optional: + // a busy or unbindable host port must fail the start, not be dropped. + assert.Equal(t, []runtime.PortMapping{ + {ContainerPort: "53", HostPort: "53", Protocol: "tcp"}, + {ContainerPort: "53", HostPort: "53", Protocol: "udp"}, + }, merged[len(autoPublished()):]) + assert.Empty(t, sink.messageTexts()) +} + +func TestMergeExposePortsRemapsHostPort(t *testing.T) { + sink := &recordingSink{} + + merged := mergeExposePorts(sink, nil, "4566", "4566", exposedFor(t, "5354:53/udp")) + + assert.Equal(t, []runtime.PortMapping{ + {ContainerPort: "53", HostPort: "5354", Protocol: "udp"}, + }, merged) + assert.Empty(t, sink.messageTexts()) +} + +func TestMergeExposePortsSkipsRedundantEntriesSilently(t *testing.T) { + sink := &recordingSink{} + + // Asking for exactly what lstk already publishes changes nothing, so there is + // nothing to warn about. + merged := mergeExposePorts(sink, autoPublished(), "4566", "4566", exposedFor(t, "443/tcp", "4510:4510/tcp")) + + assert.Equal(t, autoPublished(), merged) + assert.Empty(t, sink.messageTexts()) +} + +func TestMergeExposePortsWarnsWhenLstkAlreadyPublishesTheContainerPort(t *testing.T) { + sink := &recordingSink{} + + // The automatic mapping wins, so the requested host port would never take + // effect — say so instead of dropping it silently. + merged := mergeExposePorts(sink, autoPublished(), "4566", "4566", exposedFor(t, "8443:443/tcp")) + + assert.Equal(t, autoPublished(), merged) + require.Len(t, sink.messageTexts(), 1) + assert.Contains(t, sink.messageTexts()[0], "already publishes container port 443/tcp on host port 443") +} + +func TestMergeExposePortsWarnsWhenHostPortIsTakenByTheEdgePort(t *testing.T) { + sink := &recordingSink{} + + merged := mergeExposePorts(sink, autoPublished(), "4566", "4566", exposedFor(t, "4566:53/tcp")) + + assert.Equal(t, autoPublished(), merged) + require.Len(t, sink.messageTexts(), 1) + assert.Contains(t, sink.messageTexts()[0], "host port 4566/tcp is already used to publish container port 4566") +} + +func TestRequiredHostPortsExcludesUDP(t *testing.T) { + // The preflight dials TCP, so a UDP publication cannot be checked that way — + // including it would report a phantom conflict whenever the same port number is + // held by an unrelated TCP listener. + required := requiredHostPorts([]runtime.PortMapping{ + {ContainerPort: "443", HostPort: "443", Optional: true}, + {ContainerPort: "4510", HostPort: "4510"}, + {ContainerPort: "53", HostPort: "53", Protocol: "tcp"}, + {ContainerPort: "53", HostPort: "53", Protocol: "udp"}, + }) + + assert.Equal(t, []string{"4510", "53"}, required) +} + +// TestStartFailureOnRequiredPortBindCarriesRuntimeHint covers the sub-1024 case +// expose_ports makes reachable: a required port the daemon may refuse to bind. +func TestStartFailureOnRequiredPortBindCarriesRuntimeHint(t *testing.T) { + ctrl := gomock.NewController(t) + mockRT := runtime.NewMockRuntime(ctrl) + + c := runtime.ContainerConfig{ + Name: "localstack-aws", + Port: "4566", + ExtraPorts: []runtime.PortMapping{{ContainerPort: "53", HostPort: "53", Protocol: "udp"}}, + } + bindErr := errors.New(`Error response from daemon: driver failed programming external connectivity: listen tcp 127.0.0.1:53: bind: permission denied`) + + mockRT.EXPECT().Start(gomock.Any(), c).Return("", nil, bindErr) + mockRT.EXPECT().Flavor().Return(runtime.FlavorPodman) + + _, _, err := startWithOptionalPortFallback(context.Background(), mockRT, &recordingSink{}, c) + require.Error(t, err) + assert.ErrorIs(t, err, bindErr, "the daemon error must stay the cause — a required port is never dropped") + assert.Contains(t, err.Error(), "podman machine set --rootful") +} + +func TestMergeExposePortsAllowsUDPOnAnAlreadyPublishedTCPPort(t *testing.T) { + sink := &recordingSink{} + + // 4510/tcp being published says nothing about 4510/udp. + merged := mergeExposePorts(sink, autoPublished(), "4566", "4566", exposedFor(t, "4510/udp")) + + assert.Equal(t, []runtime.PortMapping{ + {ContainerPort: "4510", HostPort: "4510", Protocol: "udp"}, + }, merged[len(autoPublished()):]) + assert.Empty(t, sink.messageTexts()) +} diff --git a/internal/container/start.go b/internal/container/start.go index 2bc43e09..41ec77f2 100644 --- a/internal/container/start.go +++ b/internal/container/start.go @@ -265,6 +265,13 @@ func startOnce(ctx context.Context, rt runtime.Runtime, sink output.Sink, opts S primaryPort, _, _ := strings.Cut(containerPort, "/") extraPorts := append(gateway.extraGatewayPorts(primaryPort, gatewayDefaulted), servicePortRange()...) + // Ports the user asked for explicitly (e.g. 53 for DNS) are published on top. + exposed, err := c.ExposedPorts() + if err != nil { + return "", err + } + extraPorts = mergeExposePorts(sink, extraPorts, primaryPort, c.Port, exposed) + containers[i] = runtime.ContainerConfig{ Image: image, Name: containerName, @@ -896,13 +903,7 @@ func selectContainersToStart(ctx context.Context, rt runtime.Runtime, sink outpu // one. Optional ports (443 from the default GATEWAY_LISTEN) are commonly // squatted by other software — e.g. Rancher Desktop's Traefik ingress — so // a busy one is dropped with a warning instead of blocking the start. - var requiredSpecs []string - for _, ep := range c.ExtraPorts { - if !ep.Optional { - requiredSpecs = append(requiredSpecs, ep.HostPort) - } - } - if conflictPort, err := ports.CheckAvailable(requiredSpecs...); err != nil { + if conflictPort, err := ports.CheckAvailable(requiredHostPorts(c.ExtraPorts)...); err != nil { sink.Emit(output.ErrorEvent{ Title: fmt.Sprintf("Port %s is already in use", conflictPort), Summary: "LocalStack requires this port. Free it before starting.", @@ -993,6 +994,20 @@ func healLeftoverContainer(ctx context.Context, rt runtime.Runtime, sink output. return nil } +// requiredHostPorts returns the host ports whose availability must be verified +// before the start. UDP publications (only reachable via expose_ports) are left +// out: the check dials, and a TCP dial says nothing about whether a UDP port is +// free — a UDP conflict surfaces at container start instead. +func requiredHostPorts(mappings []runtime.PortMapping) []string { + var required []string + for _, ep := range mappings { + if !ep.Optional && ep.Protocol != "udp" { + required = append(required, ep.HostPort) + } + } + return required +} + // dropBusyOptionalPorts removes optional port mappings whose host port is // already taken, warning about each one, and returns the mappings to publish. // Required mappings are passed through untouched — the caller has already @@ -1113,7 +1128,7 @@ func startWithOptionalPortFallback(ctx context.Context, rt runtime.Runtime, sink } i := failedOptionalPortBind(err, c.ExtraPorts) if i < 0 { - return "", nil, err + return "", nil, annotateRequiredPortBindError(rt, err, c.ExtraPorts) } cause := portBusy if strings.Contains(err.Error(), "permission denied") { @@ -1135,6 +1150,29 @@ func startWithOptionalPortFallback(ctx context.Context, rt runtime.Runtime, sink } } +// annotateRequiredPortBindError appends the runtime-specific remedy when the daemon +// refuses to bind a *required* published port, which the optional-port fallback above +// cannot rescue. A port below 1024 requested via expose_ports (e.g. 53 for DNS) is +// the common case: some runtimes need admin/rootful mode to publish those at all. +func annotateRequiredPortBindError(rt runtime.Runtime, err error, mappings []runtime.PortMapping) error { + if err == nil || !strings.Contains(err.Error(), "bind:") { + return err + } + cause := portBusy + if strings.Contains(err.Error(), "permission denied") { + cause = portBindDenied + } + for _, ep := range mappings { + if ep.Optional || !strings.Contains(err.Error(), ":"+ep.HostPort+": bind:") { + continue + } + if hint := tailoredPortDropHint(rt.Flavor(), runtime.DetectInstalledFlavor(), ep.HostPort, cause); hint != "" { + return fmt.Errorf("%w — %s", err, hint) + } + } + return err +} + // portConflictActions builds the next-step actions for a fatal extra-port // conflict, tailored to the runtime where the squatter is known. Rancher merely // being installed counts (see tailoredPortDropHint): Traefik holds 443 no diff --git a/test/integration/start_test.go b/test/integration/start_test.go index 74ec85be..1c422cda 100644 --- a/test/integration/start_test.go +++ b/test/integration/start_test.go @@ -594,6 +594,75 @@ GATEWAY_LISTEN = "0.0.0.0:4566,0.0.0.0:443,0.0.0.0:8443" } } +// TestStartCommandExposesConfiguredPorts covers the expose_ports config option +// (DEVX-994): the motivating case is publishing port 53 so the emulator's DNS +// server can serve the host. Ports 53 and 5353 are avoided here because they are +// privileged/commonly held on CI hosts; the mechanism is identical. +func TestStartCommandExposesConfiguredPorts(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + configContent := ` +[[containers]] +type = "aws" +tag = "latest" +port = "4566" +expose_ports = [15353, "15354:15355/udp"] +` + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configFile, []byte(configContent), 0644)) + + ctx := testContext(t) + _, stderr, err := runLstk(t, ctx, "", env.With(env.APIEndpoint, mockServer.URL), "--config", configFile, "start") + require.NoError(t, err, "lstk start failed: %s", stderr) + + inspect, err := dockerClient.ContainerInspect(ctx, containerName, client.ContainerInspectOptions{}) + require.NoError(t, err, "failed to inspect container") + + // A bare port is published on the same host port for both protocols (DNS needs + // both); an entry with an explicit host port and protocol is published as written. + expected := map[string]string{ + "15353/tcp": "15353", + "15353/udp": "15353", + "15355/udp": "15354", + } + for port, hostPort := range expected { + bindings := inspect.Container.HostConfig.PortBindings[network.MustParsePort(port)] + if assert.NotEmpty(t, bindings, "port %s should be bound", port) { + assert.Equal(t, hostPort, bindings[0].HostPort) + assert.Equal(t, "127.0.0.1", bindings[0].HostIP.String()) + } + } + assert.Empty(t, inspect.Container.HostConfig.PortBindings[network.MustParsePort("15355/tcp")], + "an entry that names udp must not also publish tcp") +} + +// TestStartCommandRejectsInvalidExposePorts checks the config surface fails at load +// time with a clear message rather than at container creation. +func TestStartCommandRejectsInvalidExposePorts(t *testing.T) { + t.Parallel() + + configContent := ` +[[containers]] +type = "aws" +tag = "latest" +port = "4566" +expose_ports = ["53/sctp"] +` + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configFile, []byte(configContent), 0644)) + + _, stderr, err := runLstk(t, testContext(t), "", testEnvWithHome(t.TempDir(), ""), "--config", configFile, "status") + require.Error(t, err) + assert.Contains(t, stderr, `protocol must be "tcp" or "udp"`) +} + func TestStartCommandSetsUpContainerCorrectly(t *testing.T) { requireDocker(t) _ = env.Require(t, env.AuthToken) From d991db27551e333820945ab9d0724bc3dc5454b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristian=20Pallar=C3=A9s?= Date: Wed, 5 Aug 2026 10:16:12 +0200 Subject: [PATCH 2/3] Add integration test for default expose_ports (no extra ports) Co-Authored-By: Claude --- test/integration/start_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/integration/start_test.go b/test/integration/start_test.go index 1c422cda..49489c0c 100644 --- a/test/integration/start_test.go +++ b/test/integration/start_test.go @@ -643,6 +643,40 @@ expose_ports = [15353, "15354:15355/udp"] "an entry that names udp must not also publish tcp") } +// TestStartCommandPublishesOnlyDefaultPortsWhenExposePortsUnset pins the no-op case: +// with expose_ports absent, the published ports are exactly the default set (edge +// port, gateway 443, service range 4510-4559) — nothing extra sneaks in. +func TestStartCommandPublishesOnlyDefaultPortsWhenExposePortsUnset(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + ctx := testContext(t) + _, stderr, err := runLstk(t, ctx, "", env.With(env.APIEndpoint, mockServer.URL), "start") + require.NoError(t, err, "lstk start failed: %s", stderr) + + inspect, err := dockerClient.ContainerInspect(ctx, containerName, client.ContainerInspectOptions{}) + require.NoError(t, err, "failed to inspect container") + + wantPublished := map[string]bool{"4566/tcp": true, "443/tcp": true} + for p := 4510; p <= 4559; p++ { + wantPublished[strconv.Itoa(p)+"/tcp"] = true + } + + gotPublished := map[string]bool{} + for port, bindings := range inspect.Container.HostConfig.PortBindings { + if len(bindings) > 0 { + gotPublished[port.String()] = true + } + } + assert.Equal(t, wantPublished, gotPublished, "no expose_ports configured should publish exactly the default port set") +} + // TestStartCommandRejectsInvalidExposePorts checks the config surface fails at load // time with a clear message rather than at container creation. func TestStartCommandRejectsInvalidExposePorts(t *testing.T) { From 52c3b6a4649651c46eac790a984958c72c943912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristian=20Pallar=C3=A9s?= Date: Wed, 5 Aug 2026 10:22:37 +0200 Subject: [PATCH 3/3] Add integration tests for expose_ports edge cases Co-Authored-By: Claude --- test/integration/start_test.go | 114 +++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/test/integration/start_test.go b/test/integration/start_test.go index 49489c0c..ef67b466 100644 --- a/test/integration/start_test.go +++ b/test/integration/start_test.go @@ -643,6 +643,120 @@ expose_ports = [15353, "15354:15355/udp"] "an entry that names udp must not also publish tcp") } +// TestStartCommandExposePortsCannotRemapPrimaryEdgePort covers the delicate case +// where an expose_ports entry names the primary edge port's container side (4566) +// but a different host port: lstk already owns that container-port/protocol +// mapping (bound to the configured host port, 4566), so the conflicting entry is +// dropped with a warning rather than silently rebinding the edge port. +func TestStartCommandExposePortsCannotRemapPrimaryEdgePort(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + configContent := ` +[[containers]] +type = "aws" +tag = "latest" +port = "4566" +expose_ports = ["8080:4566"] +` + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configFile, []byte(configContent), 0644)) + + ctx := testContext(t) + stdout, stderr, err := runLstk(t, ctx, "", env.With(env.APIEndpoint, mockServer.URL), "--config", configFile, "start") + require.NoError(t, err, "lstk start failed: %s", stderr) + assert.Contains(t, stdout+stderr, "Ignoring expose_ports entry 8080:4566/tcp", + "expected a warning explaining why the entry was dropped") + assert.Contains(t, stdout+stderr, "already publishes container port 4566/tcp on host port 4566") + + inspect, err := dockerClient.ContainerInspect(ctx, containerName, client.ContainerInspectOptions{}) + require.NoError(t, err, "failed to inspect container") + + edgeBindings := inspect.Container.HostConfig.PortBindings[network.MustParsePort("4566/tcp")] + require.NotEmpty(t, edgeBindings, "the edge port must still be published") + assert.Equal(t, "4566", edgeBindings[0].HostPort, "the edge port's host binding must be unchanged") + assert.Empty(t, inspect.Container.HostConfig.PortBindings[network.MustParsePort("8080/tcp")], + "the conflicting host port must not be published") +} + +// TestStartCommandExposePortsAddsMissingProtocolForPrimaryEdgePort covers the other +// delicate case: an expose_ports entry names the primary edge port itself but a +// protocol lstk does not already publish (4566 is tcp-only by default). Since it +// doesn't collide with the existing tcp/4566 mapping, it is added — no warning. +func TestStartCommandExposePortsAddsMissingProtocolForPrimaryEdgePort(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + configContent := ` +[[containers]] +type = "aws" +tag = "latest" +port = "4566" +expose_ports = ["4566/udp"] +` + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configFile, []byte(configContent), 0644)) + + ctx := testContext(t) + stdout, stderr, err := runLstk(t, ctx, "", env.With(env.APIEndpoint, mockServer.URL), "--config", configFile, "start") + require.NoError(t, err, "lstk start failed: %s", stderr) + assert.NotContains(t, stdout+stderr, "Ignoring expose_ports entry", + "a new protocol on the edge port does not collide with anything and needs no warning") + + inspect, err := dockerClient.ContainerInspect(ctx, containerName, client.ContainerInspectOptions{}) + require.NoError(t, err, "failed to inspect container") + + udpBindings := inspect.Container.HostConfig.PortBindings[network.MustParsePort("4566/udp")] + if assert.NotEmpty(t, udpBindings, "4566/udp should now be published") { + assert.Equal(t, "4566", udpBindings[0].HostPort) + } + tcpBindings := inspect.Container.HostConfig.PortBindings[network.MustParsePort("4566/tcp")] + require.NotEmpty(t, tcpBindings, "4566/tcp must remain published") + assert.Equal(t, "4566", tcpBindings[0].HostPort) +} + +// TestStartCommandFailsWhenExposedPortIsTaken covers a busy host port requested via +// expose_ports: unlike lstk's own optional ports (e.g. 443), an expose_ports entry +// is an explicit request, so a conflict must fail the start rather than degrade +// silently — the same rule as a user-supplied GATEWAY_LISTEN port. +func TestStartCommandFailsWhenExposedPortIsTaken(t *testing.T) { + requireDocker(t) + cleanup() + t.Cleanup(cleanup) + + ln, err := net.Listen("tcp", ":15360") + require.NoError(t, err, "failed to bind port 15360 for test") + defer func() { _ = ln.Close() }() + + configContent := ` +[[containers]] +type = "aws" +tag = "latest" +port = "4566" +expose_ports = [15360] +` + configFile := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(configFile, []byte(configContent), 0644)) + + stdout, _, err := runLstk(t, testContext(t), "", env.With(env.AuthToken, "fake-token"), "--config", configFile, "start") + require.Error(t, err, "expected lstk start to fail when a requested expose_ports port is in use") + requireExitCode(t, 1, err) + assert.Contains(t, stdout, "Port 15360 is already in use") + assert.Contains(t, stdout, "LocalStack requires this port. Free it before starting.") +} + // TestStartCommandPublishesOnlyDefaultPortsWhenExposePortsUnset pins the no-op case: // with expose_ports absent, the published ports are exactly the default set (edge // port, gateway 443, service range 4510-4559) — nothing extra sneaks in.