Skip to content

fix: stop publishing ~420 service ports to the host by default - #7

Merged
hectorvent merged 3 commits into
floci-io:mainfrom
okinaka:fix/go-test-failures
Sep 3, 2026
Merged

hectorvent merged 3 commits into
floci-io:mainfrom
okinaka:fix/go-test-failures

Conversation

@okinaka

@okinaka okinaka commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

go test ./... was failing intermittently — container startup would hang until the 120s health-check timeout ("wait until ready: context deadline exceeded"), especially when the six test packages ran in parallel, but sometimes even for a single container.

The root cause is that every Floci container published ~500 host ports by default: the edge port (4566) plus the full proxy ranges of ECR (100), EKS (100), ElastiCache (21), OpenSearch (100), and RDS (99), unconditionally. Under this load Docker Desktop intermittently starts the container (it even reports healthy) but never materializes the host port mappings — NetworkSettings.Ports stays empty — so the wait strategy can never reach the health endpoint. With go test ./... running six containers in parallel (~3,000 port forwards requested at once), failure was near-certain.

Changes

  • Gate each port range behind an opt-in flag (default false), mirroring the existing LambdaConfig.ExposeRuntimePorts pattern:
    • EcrConfig.ExposeRegistryPorts
    • EksConfig.ExposeApiServerPorts
    • ElastiCacheConfig.ExposeProxyPorts
    • OpenSearchConfig.ExposeProxyPorts
    • RdsConfig.ExposeProxyPorts
  • Make the flags declarative: refreshExposedPorts now rebuilds the exposed-port set from the current configs instead of only adding to it, so re-applying a config with a flag turned off also removes the previously added range (addresses review feedback).
  • Add white-box unit tests (ports_internal_test.go) asserting that each of the six Expose flags publishes exactly its service range plus the edge port, and that opt-out removes previously added ports.

By default only port 4566 is published now. Container startup drops from 15–20s to under a second.

Breaking change

Code that connects to ECR/EKS/ElastiCache/OpenSearch/RDS proxy ports from the host (e.g. via GetMappedPort) must now set the corresponding Expose…Ports flag. In-container access (Lambda handlers, service-to-service) is unaffected.

Test plan

  • Reproduced the hang before the fix (container healthy but no port mappings; single-container hangs >300s).
  • After the fix: four consecutive go test ./... runs, all packages green, 1–3s per package.
  • New unit tests cover every Expose flag (range published exactly) and the enable-then-disable case.
  • go build ./... and go vet ./... pass.

Every container published the full ECR/EKS/ElastiCache/OpenSearch/RDS
proxy port ranges (~500 host ports with 4566), which made startup slow
and intermittently hung it forever: Docker Desktop would start the
container but never materialize the port mappings, so the health-check
wait timed out. Running `go test ./...` (six packages in parallel, each
starting its own container) made the failure near-certain.

Gate each range behind an opt-in flag, mirroring the existing
LambdaConfig.ExposeRuntimePorts pattern:

- EcrConfig.ExposeRegistryPorts
- EksConfig.ExposeApiServerPorts
- ElastiCacheConfig.ExposeProxyPorts
- OpenSearchConfig.ExposeProxyPorts
- RdsConfig.ExposeProxyPorts

By default only the edge port (4566) is published now; container startup
drops from 15-20s to under a second, and the full parallel test suite
passes consistently (verified with four consecutive `go test ./...`
runs).
@greptile-apps

greptile-apps Bot commented Aug 28, 2026

Copy link
Copy Markdown

Greptile Summary

The PR makes large service-specific host-port ranges opt-in and now rebuilds the exposed-port set whenever configuration changes, ensuring a later opt-out removes previously selected ports.

  • Adds exposure flags for ECR, EKS, ElastiCache, OpenSearch, and RDS port ranges.
  • Preserves only the edge port by default.
  • Adds coverage for each exposure flag and for enable-then-disable configuration.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
floci.go Rebuilds exposed ports from current configuration state, resolving the previously reported additive opt-out behavior.
services.go Makes the five large service-specific host-port ranges explicitly opt-in.
ports_internal_test.go Verifies exact ranges for every exposure flag and confirms that disabling exposure removes previously added ports.

Reviews (2): Last reviewed commit: "test: cover each Expose flag publishing ..." | Re-trigger Greptile

Comment thread services.go
The port set was additive, so applying a config with an Expose flag
enabled and then re-applying it with the flag disabled left the range
published. refreshExposedPorts now resets the set to the edge port and
re-derives it from the current configs, making the flags declarative.
Table-driven test asserting that enabling each of the six Expose flags
(ECR, EKS, ElastiCache, Lambda, OpenSearch, RDS) publishes exactly the
service range plus the edge port.
@okinaka

okinaka commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

@hectorvent Gentle ping, this has been open for about a week. Would appreciate a review when you have a chance 🙏

@hectorvent
hectorvent merged commit a1bf81d into floci-io:main Sep 3, 2026
1 check passed
@okinaka
okinaka deleted the fix/go-test-failures branch September 3, 2026 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants