Skip to content

Commit 24b3185

Browse files
committed
Merge #299: refactor: [#298] Phase 3 Port Topology Domain Model - Foundation (P3.1-P3.3)
dd3ab12 docs: [#298] update epic and spec to reflect PR split strategy (Jose Celano) 943a274 refactor: [#298] add PortBinding domain type and cross-service port validation (Jose Celano) Pull request description: ## Summary This PR implements the domain layer foundation for Phase 3 of the Docker Compose Topology refactoring (Epic #287). It creates type-safe port binding types and cross-service port conflict validation, following the same pattern established for networks in Phase 1-2. ## Changes ### P3.1: Create Port Domain Types ✅ - **Reused `Protocol` enum** from `domain/tracker/protocol.rs` (no duplication) - **Created `PortBinding` struct** with: - Private fields: `host_port`, `container_port`, `protocol`, `host_ip`, `description` - Convenience constructors: `tcp()`, `udp()`, `localhost_tcp()` - `docker_compose_binding()` method for YAML rendering (e.g., `"6969:6969/udp"`) - Description support for inline YAML comments - **10 unit tests** covering all constructors and binding formats ### P3.2: Extend ServiceTopology with Ports ✅ - Added `ports: Vec<PortBinding>` field to `ServiceTopology` - Added `new()` constructor with ports parameter - Added `with_networks()` for backward compatibility - Made all fields private with getters (DDD compliance) - **4 new tests** for port functionality ### P3.3: Add Cross-Service Port Validation ✅ - Created `PortConflict` error type with full context (both services, both bindings) - Created `TopologyError` enum with `help()` method (DDD error conventions) - Implemented `validate_port_uniqueness()` on `DockerComposeTopology` - **6 tests** covering conflict scenarios ## DDD Compliance All code follows the project's DDD practices ([docs/contributing/ddd-practices.md](docs/contributing/ddd-practices.md)): | Requirement | Status | |-------------|--------| | Private fields with getters | ✅ All domain types | | `#[must_use]` on getters | ✅ Applied | | Expressive errors with `help()` | ✅ `TopologyError::help()` | | Single point of validation | ✅ Aggregate method | ## What's NOT in this PR **P3.4 (Template Integration)** will be a follow-up PR: - Update `DockerComposeContext` to use derived ports - Simplify `docker-compose.yml.tera` port sections - Remove conditional port logic from template This split allows the domain foundation to be reviewed and merged independently. ## Testing - ✅ 20 new unit tests for port topology - ✅ All 2021 existing tests pass - ✅ All 393 doc tests pass - ✅ E2E infrastructure lifecycle tests pass - ✅ E2E deployment workflow tests pass - ✅ All linters pass (clippy, rustfmt, markdown, yaml, toml, cspell, shellcheck) ## Files Changed | File | Change | |------|--------| | `src/domain/topology/port.rs` | NEW - `PortBinding` struct | | `src/domain/topology/error.rs` | NEW - `PortConflict`, `TopologyError` | | `src/domain/topology/aggregate.rs` | Extended `ServiceTopology` with ports | | `src/domain/topology/mod.rs` | Added module exports | | `docs/issues/287-*.md` | Updated epic with PR split | | `docs/issues/298-*.md` | Updated spec with completed tasks | ## Related - Epic: #287 - Issue: #298 - Spec: [docs/issues/298-phase-3-port-topology-domain-model.md](docs/issues/298-phase-3-port-topology-domain-model.md) ACKs for top commit: josecelano: ACK dd3ab12 Tree-SHA512: 408a7610a75fcf7ab82128c693f57b6cadc79ad6dbfc7de4073aca790c7719c6c62c4459a8093a0a9cab8a46e3b96e46be0caacd25169ea7079705961dd60cff
2 parents b7dec7d + dd3ab12 commit 24b3185

File tree

6 files changed

+879
-81
lines changed

6 files changed

+879
-81
lines changed

docs/issues/287-docker-compose-topology-refactoring-epic.md

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,14 @@ Implementation follows a 5-PR strategy (original scope), with Phase 3 added as a
3838
### Phase 3: Port Topology (Added Extension)
3939

4040
> **Note**: Phase 3 was part of the original refactoring plan analysis (see PORT-01 through PORT-11 rules in [docker-compose-topology-domain-model.md](../refactors/plans/docker-compose-topology-domain-model.md#port-exposure-rules)) but was not included in the initial 5-PR strategy. It follows the same pattern as networks and completes the topology domain model.
41-
42-
- [ ] [#298](https://github.com/torrust/torrust-tracker-deployer/issues/298) - [Refactor] Phase 3: Create Port Topology Domain Model (P3.1, P3.2, P3.3, P3.4) → [Spec](298-phase-3-port-topology-domain-model.md)
43-
- P3.1: Create Port domain types (`PortBinding`, `Protocol`)
44-
- P3.2: Extend `ServiceTopology` with port derivation
45-
- P3.3: Add cross-service port conflict validation
41+
>
42+
> **Implementation Note**: Phase 3 is split into two PRs for better reviewability. PR #298 delivers the domain layer foundation (P3.1-P3.3), while a follow-up PR will integrate ports into the template (P3.4).
43+
44+
- [ ] [#298](https://github.com/torrust/torrust-tracker-deployer/issues/298) - [Refactor] Phase 3: Port Topology Domain Model - Foundation (P3.1, P3.2, P3.3) → [Spec](298-phase-3-port-topology-domain-model.md)
45+
- P3.1: Create Port domain types (`PortBinding`, reuse `Protocol`)
46+
- P3.2: Extend `ServiceTopology` with ports field
47+
- P3.3: Add cross-service port conflict validation with `help()` method
48+
- [ ] TBD - [Refactor] Phase 3: Port Topology Template Integration (P3.4)
4649
- P3.4: Update template to use derived ports with descriptions
4750

4851
## PR Dependencies
@@ -60,7 +63,10 @@ PR 4 (Phase 1)
6063
PR 5 (Phase 2)
6164
6265
63-
PR 6 (Phase 3) ◄─── Extension: Port topology (added post-completion)
66+
PR 6 (Phase 3 Foundation) ◄─── Domain types: PortBinding, validation, help()
67+
68+
69+
PR 7 (Phase 3 Template) ◄─── Template integration (P3.4) - follow-up
6470
```
6571

6672
## Key Decisions
@@ -71,6 +77,8 @@ PR 6 (Phase 3) ◄─── Extension: Port topology (added post-completion)
7177
- ~47 domain rules identified with test specifications
7278
- Network descriptions rendered as YAML comments for sysadmin documentation
7379
- Port topology follows same pattern as networks (Phase 3)
80+
- Phase 3 split into foundation (domain) and integration (template) PRs for reviewability
81+
- `PortConflict` error includes `help()` method per DDD error handling conventions
7482

7583
## Related
7684

docs/issues/298-phase-3-port-topology-domain-model.md

Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,16 @@
22

33
**Epic**: [#287](https://github.com/torrust/torrust-tracker-deployer/issues/287) - Docker Compose Topology Domain Model Refactoring
44
**Related Plan**: [docs/refactors/plans/docker-compose-topology-domain-model.md](../../refactors/plans/docker-compose-topology-domain-model.md)
5-
**Status**: Draft
5+
**Status**: In Progress (P3.1-P3.3 Complete)
6+
7+
## Implementation Notes
8+
9+
> **PR Strategy**: This phase is split into two PRs for better reviewability:
10+
>
11+
> - **PR #298 (this PR)**: Domain layer foundation - P3.1, P3.2, P3.3
12+
> - **Follow-up PR**: Template integration - P3.4
13+
>
14+
> The domain types are stable and well-tested. Template integration is a larger change that benefits from being a separate, focused PR.
615
716
## Overview
817

@@ -177,46 +186,60 @@ services:
177186

178187
## Tasks
179188

180-
### P3.1: Create Port Domain Types
189+
### P3.1: Create Port Domain Types
181190

182-
- [ ] Create `Protocol` enum (or reuse from `domain/tracker`)
183-
- [ ] Create `PortBinding` struct with description support
184-
- [ ] Add `PortDefinition` context type for template rendering
185-
- [ ] Add unit tests for port binding creation
191+
- [x] Reuse `Protocol` enum from `domain/tracker/protocol.rs`
192+
- [x] Create `PortBinding` struct with description support
193+
- [x] Add convenience constructors: `tcp()`, `udp()`, `localhost_tcp()`
194+
- [x] Add `docker_compose_binding()` method for YAML rendering
195+
- [x] Add unit tests for port binding creation (10 tests)
186196

187-
### P3.2: Extend ServiceTopology with Ports
197+
### P3.2: Extend ServiceTopology with Ports
188198

189-
- [ ] Add `ports: Vec<PortBinding>` to `ServiceTopology`
190-
- [ ] Implement port derivation for each service type:
191-
- Tracker: UDP ports, HTTP ports (TLS-dependent), API (TLS-dependent)
192-
- Caddy: 80, 443, 443/udp (fixed)
193-
- Prometheus: 9090 (localhost)
194-
- Grafana: 3000 (TLS-dependent)
195-
- MySQL: none
196-
- [ ] Add unit tests for each service's port derivation
199+
- [x] Add `ports: Vec<PortBinding>` to `ServiceTopology`
200+
- [x] Add `new()` constructor with ports parameter
201+
- [x] Add `with_networks()` for backward compatibility
202+
- [x] Add `ports()` and `has_ports()` getters with `#[must_use]`
203+
- [x] Make all fields private with getters (DDD compliance)
204+
- [x] Add unit tests for port field (4 tests)
197205

198-
### P3.3: Add Cross-Service Port Validation
206+
### P3.3: Add Cross-Service Port Validation
199207

200-
- [ ] Implement `DockerComposeTopology::validate_port_uniqueness()`
201-
- [ ] Create `TopologyError::PortConflict` variant
202-
- [ ] Add tests for conflict detection scenarios
208+
- [x] Implement `DockerComposeTopology::validate_port_uniqueness()`
209+
- [x] Create `PortConflict` error type with full context
210+
- [x] Create `TopologyError` enum with `help()` method (DDD error conventions)
211+
- [x] Add tests for conflict detection scenarios (6 tests)
203212

204-
### P3.4: Update Template and Context
213+
### P3.4: Update Template and Context (Follow-up PR)
205214

206215
- [ ] Create `PortDefinition` with `binding()` and `description()` for template
207216
- [ ] Update `DockerComposeContext` to use derived ports
208217
- [ ] Simplify `docker-compose.yml.tera` port sections
209218
- [ ] Remove conditional port logic from template
219+
- [ ] Implement port derivation for each service type:
220+
- Tracker: UDP ports, HTTP ports (TLS-dependent), API (TLS-dependent)
221+
- Caddy: 80, 443, 443/udp (fixed)
222+
- Prometheus: 9090 (localhost)
223+
- Grafana: 3000 (TLS-dependent)
224+
- MySQL: none
210225

211226
## Acceptance Criteria
212227

228+
### PR #298 (Domain Foundation) ✅
229+
230+
- [x] `PortBinding` type with description support created
231+
- [x] `ServiceTopology` extended with ports field
232+
- [x] Cross-service port conflicts detected with actionable `help()` message
233+
- [x] All fields private with getters (DDD compliance)
234+
- [x] No duplication with `TrackerConfig` validation (different scopes)
235+
- [x] All existing E2E tests pass
236+
- [x] Pre-commit checks pass
237+
238+
### Follow-up PR (Template Integration)
239+
213240
- [ ] All PORT-\* rules from refactoring plan are implemented in domain
214241
- [ ] Port descriptions render as YAML comments in output
215-
- [ ] Cross-service port conflicts are detected and reported with actionable error
216242
- [ ] Template has no conditional port logic (just loops)
217-
- [ ] No duplication with `TrackerConfig` validation (different scopes)
218-
- [ ] All existing E2E tests pass
219-
- [ ] Pre-commit checks pass
220243

221244
## Testing Strategy
222245

0 commit comments

Comments
 (0)