Skip to content

fix: validate service port conflicts on same host#326

Merged
rshoemaker merged 2 commits intomainfrom
fix/PLAT-531/port-conflicts
Apr 9, 2026
Merged

fix: validate service port conflicts on same host#326
rshoemaker merged 2 commits intomainfrom
fix/PLAT-531/port-conflicts

Conversation

@rshoemaker
Copy link
Copy Markdown
Contributor

Summary

  • Add cross-service port conflict validation to both create-database and update-database paths
  • Track (hostID, port) pairs across all services and reject duplicates where port > 0
  • Nil ports (no publish) and port 0 (Docker random assignment) are excluded from checking

Test plan

  • make test passes
  • Create database with two services using same port on same host — should return validation error
  • Create database with two services using same port on different hosts — should succeed
  • Create database with services using nil/zero ports on same host — should succeed
  • Update database adding a service that conflicts with existing service port — should return validation error

PLAT-531

Reject database specs where two services bind the same explicit port
on the same host. Previously the conflict was only caught at Docker
deploy time, resulting in an opaque context deadline exceeded error.

Nil ports and port 0 (random assignment) are excluded from checking.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b70c150d-8636-4f04-8f12-1fe89f361287

📥 Commits

Reviewing files that changed from the base of the PR and between f5075a1 and 6595bc3.

📒 Files selected for processing (2)
  • server/internal/api/apiv1/validate.go
  • server/internal/api/apiv1/validate_test.go

📝 Walkthrough

Walkthrough

Added explicit cross-service and service-vs-database port conflict validation to database spec create and update paths: services’ explicit ports are checked against a host-port ownership map (pre-seeded with Postgres ports) and conflicts produce field-path validation errors.

Changes

Cohort / File(s) Summary
Port conflict validation
server/internal/api/apiv1/validate.go
Added servicePortOwnerMap tracking and validateServicePortConflicts. In validateDatabaseSpec and validateDatabaseUpdate the map is initialized and pre-seeded with each node’s effective Postgres port; each service is checked for explicit Port conflicts before existing service-spec validation. Conflicts append field-path errors.
Tests for port conflict behavior
server/internal/api/apiv1/validate_test.go
Expanded TestValidateDatabaseSpec with cases covering same-port-on-same-host (error), same-port-on-different-hosts (allowed), single service across multiple hosts (allowed), nil/zero ports (allowed), and conflicts with DB/Postgres ports at spec and node levels. Extended TestValidateDatabaseUpdate_ServiceBootstrapFields with an update case asserting detection of service port conflicts.

Poem

🐇 I hop through specs with keen delight,
Mapping hosts and ports by lantern light.
If two services claim the very same key,
I thump and I warn — “That port can’t be!”
Happy builds, conflict-free and bright ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is missing several required template sections (Changes, Testing, Checklist) and lacks sufficient detail in the provided sections. Add the missing 'Changes', 'Testing', and 'Checklist' sections from the template; expand the summary and test plan details.
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: validate service port conflicts on same host' directly and clearly summarizes the main change—adding validation for service port conflicts on the same host.

✏️ 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 fix/PLAT-531/port-conflicts

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

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

@codacy-production
Copy link
Copy Markdown

codacy-production bot commented Apr 3, 2026

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 11 complexity · 0 duplication

Metric Results
Complexity 11
Duplication 0

View in Codacy

TIP This summary will be updated as you push new changes. Give us feedback

This ensures all instances (database and service) avoid
port conflicts.
@rshoemaker rshoemaker requested a review from jason-lynch April 8, 2026 20:05
@rshoemaker rshoemaker merged commit 93f273f into main Apr 9, 2026
3 checks passed
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