Skip to content

Commit 3bb2bf3

Browse files
josecelanojosecelano
committed
Merge #384: fix: [#382] guard all service networks blocks in docker-compose template
d75b7cd docs: [#382] mark all acceptance criteria complete after E2E verification (Jose Celano) 33bd2ef docs: add ADR for docker compose local validation placement (Jose Celano) ed2b06a feat: [#382] add docker compose config --quiet validation after rendering (Jose Celano) efc7297 docs: [#382] update issue spec progress for phases 1, 2 and 4 (Jose Celano) 4f7510c fix: guard all service networks blocks in docker-compose template (Jose Celano) Pull request description: ## Summary Fixes #382 — the `docker-compose.yml.tera` template rendered an invalid empty `networks:` key for the tracker service (and other services) when no optional services were enabled, causing `docker compose up` to fail with: ```text services.tracker.networks must be a list ``` This failure only surfaced at `run` time on the remote VM — after the full `create → provision → configure → release` workflow had already completed. ## Changes ### Fix (Phase 1) Guard all service `networks:` blocks with `{%- if <service>.networks | length > 0 %}`, matching the pre-existing `backup` service pattern. Affected services: `tracker`, `caddy`, `prometheus`, `grafana`, `mysql`. ### Tests (Phase 2) Added unit tests in `template.rs` and `local_validator.rs`: - `it_should_not_render_empty_networks_key_for_tracker_when_no_optional_services_are_configured` - `it_should_render_networks_key_for_tracker_when_prometheus_is_enabled` - 4 tests for `validate_docker_compose_file()` (valid file passes, empty `networks:` key fails) ### Local validation (Phase 3) New module `src/infrastructure/templating/docker_compose/local_validator.rs` — runs `docker compose config --quiet` on the build directory immediately after both `docker-compose.yml` and `.env` are rendered. Any structural error in the generated file now fails fast at `configure` time with a clear, actionable error message. The validator is called inside `DockerComposeProjectGenerator::render()` — the earliest point where the complete artifact exists, and still within the infrastructure layer (keeping the application layer free of process-spawning logic). This decision is documented in a new ADR. ### Documentation (Phase 4) - Issue spec fully updated with progress and E2E verification results - New ADR: `docs/decisions/docker-compose-local-validation-placement.md` ## Verification Full E2E workflow run with a minimal config (SQLite, no domains, no Prometheus — the exact reproduction case from the issue): ``` create → provision → configure → release → run → test ``` All steps passed. The `run` command no longer fails with the previously broken configuration. Pre-commit checks pass: `./scripts/pre-commit.sh` ✅ All linters pass: `cargo run --bin linter all` ✅ 2353 unit tests pass ✅ ACKs for top commit: josecelano: ACK d75b7cd Tree-SHA512: a10f428f344f4d3d7be5a37ccdaa83d3ac767e3e429592ba710a363c7996c79ed996959a1f73446306ddd76895557d0ee626443c28c98f408a5ad86da70341a0
2 parents 8e6a912 + d75b7cd commit 3bb2bf3

8 files changed

Lines changed: 574 additions & 39 deletions

File tree

‎docs/decisions/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ This directory contains architectural decision records for the Torrust Tracker D
66

77
| Status | Date | Decision | Summary |
88
| ------------- | ---------- | --------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------ |
9+
| ✅ Accepted | 2026-02-24 | [Docker Compose Local Validation Placement](./docker-compose-local-validation-placement.md) | Validate rendered docker-compose.yml in the infrastructure generator, not the app layer |
910
| ✅ Accepted | 2026-02-17 | [Application-Layer Progress Reporting Trait](./application-layer-progress-reporting-trait.md) | Use trait-based progress reporting with Dependency Inversion for testable, reusable design |
1011
| ✅ Accepted | 2026-02-13 | [Concurrent Docker Image Builds in Tests](./concurrent-docker-image-builds-in-tests.md) | Treat "already exists" tagging error as success when parallel tests build same image |
1112
| ✅ Accepted | 2026-02-06 | [Agent Skills Content Strategy](./skill-content-strategy-duplication-vs-linking.md) | Three-tier content strategy: self-contained workflows, progressive disclosure, linked docs |

‎docs/decisions/docker-compose-local-validation-placement.md‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
# Decision: Docker Compose Local Validation Placement in the Infrastructure Layer
2+
3+
## Status
4+
5+
Accepted
6+
7+
## Date
8+
9+
2026-02-24
10+
11+
## Context
12+
13+
After rendering the `docker-compose.yml.tera` Tera template, the resulting
14+
`docker-compose.yml` file may contain structural errors that Docker Compose will
15+
reject at `run` time with a non-descriptive failure deep in the deployment
16+
workflow. Issue [#382](../issues/382-docker-compose-template-empty-networks-key.md)
17+
was a concrete example: an empty `networks:` key rendered when no optional
18+
services were configured, which caused `docker compose up` to fail with
19+
`services.tracker.networks must be a list` on the remote VM — far from the
20+
point of origin.
21+
22+
To catch such errors early and provide actionable feedback, a validation step was
23+
added that runs `docker compose config --quiet` against the rendered output
24+
directory immediately after template rendering, before any files are uploaded to
25+
the VM.
26+
27+
### The Question
28+
29+
The rendering pipeline has multiple candidate locations for the validation call:
30+
31+
```text
32+
RenderDockerComposeTemplatesStep::execute() ← application/steps layer
33+
└─ DockerComposeTemplateRenderingService::render() ← application service layer
34+
└─ DockerComposeProjectGenerator::render() ← infrastructure layer ← chosen
35+
├─ EnvRenderer::render() ← writes .env
36+
└─ DockerComposeRenderer::render() ← writes docker-compose.yml
37+
```
38+
39+
Alternatives explicitly considered:
40+
41+
1. **`DockerComposeRenderer::render()`** — the lowest-level renderer, inside
42+
infrastructure, writes only `docker-compose.yml`
43+
2. **`DockerComposeProjectGenerator::render()`** — the infrastructure-layer
44+
generator, writes both `docker-compose.yml` and `.env` into the build
45+
directory
46+
3. **`DockerComposeTemplateRenderingService::render()`** — the application-layer
47+
service that orchestrates the generator
48+
4. **`RenderDockerComposeTemplatesStep::execute()`** — the application-layer step
49+
invoked by the command handler
50+
51+
### Constraints
52+
53+
1. **DDD layering**: Infrastructure operations (spawning a child process, invoking a
54+
CLI tool) must not leak upward into the application layer. The application layer
55+
should remain free of concrete system-call dependencies.
56+
2. **Complete artifact**: `docker compose config` reads variable substitutions from
57+
the `.env` file in the same directory. Validating before `.env` exists produces
58+
false failures or incomplete results.
59+
3. **Single responsibility alignment**: `DockerComposeRenderer::render()` is
60+
responsible for template rendering only; embedding a shell-command invocation
61+
there mixes two distinct concerns inside an already-focused collaborator.
62+
63+
## Decision
64+
65+
Call `validate_docker_compose_file()` inside
66+
**`DockerComposeProjectGenerator::render()`**, immediately after both
67+
`EnvRenderer` and `DockerComposeRenderer` have written their output files.
68+
69+
```rust
70+
// project_generator.rs — after both render() calls succeed:
71+
validate_docker_compose_file(&build_compose_dir)
72+
.map_err(|source| DockerComposeProjectGeneratorError::DockerComposeValidationFailed { source })?;
73+
```
74+
75+
This is the earliest point in the pipeline where:
76+
77+
- the **complete artifact** (both `docker-compose.yml` and `.env`) is available,
78+
making the validation meaningful and accurate, and
79+
- the code is **still inside the infrastructure layer**, respecting the DDD
80+
boundary that keeps process-spawning concerns out of the application layer.
81+
82+
### Why not `DockerComposeRenderer::render()`?
83+
84+
`DockerComposeRenderer` writes only `docker-compose.yml`. At that point, `.env`
85+
does not exist yet. `docker compose config` depends on `.env` for variable
86+
substitution, so validation would be incomplete or outright wrong for
87+
configurations that use `.env` values.
88+
89+
Validating inside a renderer whose responsibility is purely template-to-file
90+
conversion also violates single responsibility.
91+
92+
### Why not the application service or step?
93+
94+
`DockerComposeTemplateRenderingService` and
95+
`RenderDockerComposeTemplatesStep` live in the application layer. Embedding a
96+
call to `validate_docker_compose_file()` — which spawns a `docker` child process
97+
— there would:
98+
99+
- introduce an OS-level, infrastructure dependency into the application layer,
100+
violating DDD layering rules,
101+
- make the application layer harder to test in isolation without a real `docker`
102+
binary present.
103+
104+
The application layer should orchestrate business flows; it should not know how
105+
to invoke `docker compose`.
106+
107+
## Consequences
108+
109+
### Positive
110+
111+
- Validation runs at the earliest correct moment: once and only once, as soon as
112+
the full artifact exists.
113+
- The infrastructure layer encapsulates all `docker` CLI concerns; the application
114+
layer remains free of process-spawning logic.
115+
- `DockerComposeProjectGeneratorError` gains a typed `DockerComposeValidationFailed`
116+
variant, surfacing the exact `docker compose config` output as an actionable
117+
error message to the user at `configure` time.
118+
- Future template changes that introduce structural errors are caught immediately
119+
during development without needing a full deployment cycle.
120+
121+
### Negative / Trade-offs
122+
123+
- `DockerComposeProjectGenerator::render()` is now responsible for more than
124+
pure rendering — it also validates. This is a deliberate widening of scope:
125+
the generator is the "assembly" step that produces the complete artifact, and
126+
post-assembly validation is a natural responsibility for the assembler.
127+
- Validation requires `docker` to be installed on the machine running the
128+
deployer. This is already a project dependency (Docker is listed as a required
129+
tool), so it adds no new requirement but does make the dependency explicit in
130+
the infrastructure code.
131+
132+
## Alternatives Considered
133+
134+
| Location | Rejected Reason |
135+
| ------------------------------------------------- | ------------------------------------------------------------------------ |
136+
| `DockerComposeRenderer::render()` | Artifact incomplete (no `.env`); single-responsibility violation |
137+
| `DockerComposeTemplateRenderingService::render()` | Application layer calling infrastructure (process-spawning) violates DDD |
138+
| `RenderDockerComposeTemplatesStep::execute()` | Same DDD violation; too high up the call stack for an infra concern |
139+
140+
## Related Decisions
141+
142+
- [Tera Minimal Templating Strategy](./tera-minimal-templating-strategy.md) —
143+
philosophy behind how templates are structured and what they are responsible for
144+
- [Actionable Error Messages](./actionable-error-messages.md) —
145+
pattern used by `DockerComposeLocalValidationError::help()` to surface
146+
clear, solution-oriented errors
147+
148+
## References
149+
150+
- Issue [#382](../issues/382-docker-compose-template-empty-networks-key.md) —
151+
the concrete bug that motivated this validation step
152+
- `src/infrastructure/templating/docker_compose/local_validator.rs` —
153+
implementation of `validate_docker_compose_file()`
154+
- `src/infrastructure/templating/docker_compose/template/renderer/project_generator.rs` —
155+
call site within `DockerComposeProjectGenerator::render()`

‎docs/issues/382-docker-compose-template-empty-networks-key.md‎

Lines changed: 50 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ that catches this invalid template output.**
2121

2222
## Goals
2323

24-
- [ ] Fix the template to conditionally render the `networks:` key only when networks exist
25-
- [ ] Ensure consistency across all service blocks in the template
26-
- [ ] Add a unit test that renders the template with a minimal config (no optional services) and validates the output
27-
- [ ] Validate rendered docker-compose.yml with `docker compose config --quiet` after template rendering to fail early
24+
- [x] Fix the template to conditionally render the `networks:` key only when networks exist
25+
- [x] Ensure consistency across all service blocks in the template
26+
- [x] Add a unit test that renders the template with a minimal config (no optional services) and validates the output
27+
- [x] Validate rendered docker-compose.yml with `docker compose config --quiet` after template rendering to fail early
2828

2929
## Root Cause Analysis
3030

@@ -94,14 +94,14 @@ Result: `tracker.networks` is an empty `Vec`, and the template renders an invali
9494
The tracker is the **only service that is always present AND can have zero
9595
networks**:
9696

97-
| Service | Always Present? | Can Have Zero Networks? | Affected? |
98-
| ---------- | ----------------- | ------------------------ | ------------ |
99-
| Tracker | Yes | Yes | **Yes** |
100-
| Caddy | No (needs domain) | No (always has proxy) | No |
101-
| Prometheus | No (needs domain) | No (always has metrics) | No |
102-
| Grafana | No (needs domain) | No (always has metrics) | No |
103-
| MySQL | No (needs mysql) | No (always has database) | No |
104-
| Backup | Yes | Yes | No (guarded) |
97+
| Service | Always Present? | Can Have Zero Networks? | Affected? |
98+
| ---------- | ----------------- | ------------------------ | --------------- |
99+
| Tracker | Yes | Yes | Fixed (guarded) |
100+
| Caddy | No (needs domain) | No (always has proxy) | Fixed (guarded) |
101+
| Prometheus | No (needs domain) | No (always has metrics) | Fixed (guarded) |
102+
| Grafana | No (needs domain) | No (always has metrics) | Fixed (guarded) |
103+
| MySQL | No (needs mysql) | No (always has database) | Fixed (guarded) |
104+
| Backup | Yes | Yes | No (guarded) |
105105

106106
The **backup** service correctly handles this case with a guard (line 238):
107107

@@ -200,9 +200,9 @@ validated until Docker Compose processes it.
200200

201201
### Phase 1: Fix the Template
202202

203-
- [ ] Add a conditional guard around the tracker's `networks:` block in
204-
`templates/docker-compose/docker-compose.yml.tera` (lines 103-106), matching
205-
the pattern already used by the backup service:
203+
- [x] Add a conditional guard around the tracker's `networks:` block in
204+
`templates/docker-compose/docker-compose.yml.tera`, matching the pattern
205+
already used by the backup service:
206206

207207
```tera
208208
{%- if tracker.networks | length > 0 %}
@@ -213,17 +213,19 @@ validated until Docker Compose processes it.
213213
{%- endif %}
214214
```
215215

216-
- [ ] Audit all other service blocks in the template for the same pattern.
217-
Currently, `caddy` (line 80) also renders `networks:` unconditionally, but is
218-
safe because caddy always has at least the proxy network when enabled. Add a
219-
guard anyway for defensive coding.
216+
- [x] Audit all other service blocks in the template for the same pattern.
217+
Guards added for all five service blocks: `tracker`, `caddy`, `prometheus`,
218+
`grafana`, and `mysql`.
220219

221220
### Phase 2: Add Test Coverage
222221

223-
- [ ] Add a unit test that renders the docker-compose template with a minimal
224-
config (SQLite, no domains, no Prometheus) and verifies the output is valid YAML
225-
- [ ] Add a unit test that verifies the rendered output does not contain empty
226-
`networks:` keys for any service
222+
- [x] Add a unit test that renders the docker-compose template with a minimal
223+
config (SQLite, no domains, no Prometheus) and verifies the output does not
224+
contain an empty `networks:` key
225+
(`it_should_not_render_empty_networks_key_for_tracker_when_no_optional_services_are_configured`)
226+
- [x] Add a unit test that verifies the rendered output contains the correct
227+
`networks:` key when Prometheus is enabled
228+
(`it_should_render_networks_key_for_tracker_when_prometheus_is_enabled`)
227229

228230
### Phase 3: Validate Rendered docker-compose.yml After Rendering
229231

@@ -248,14 +250,19 @@ descriptive error on failure.
248250

249251
**Implementation approach:**
250252

251-
- [ ] After template rendering in the `configure` step (where docker-compose.yml
253+
- [x] After template rendering in the `configure` step (where docker-compose.yml
252254
is generated), run `docker compose config --quiet` on the output file
253-
- [ ] If validation fails, report a clear error to the user indicating the
255+
(`validate_docker_compose_file()` in `local_validator.rs`, called from
256+
`DockerComposeProjectGenerator::render()`)
257+
- [x] If validation fails, report a clear error to the user indicating the
254258
rendered template is invalid, including the docker-compose error output
255-
- [ ] This validation should run locally against the `build/<env>/docker-compose/`
259+
(`DockerComposeValidationFailed` error variant with `help()` message)
260+
- [x] This validation should run locally against the `build/<env>/docker-compose/`
256261
directory before files are uploaded to the VM
257-
- [ ] Requires `docker` to be available on the machine running the deployer (it
262+
- [x] Requires `docker` to be available on the machine running the deployer (it
258263
already is, since Docker is a project dependency)
264+
- [x] Add a unit/integration test covering the validation step
265+
(4 tests in `src/infrastructure/templating/docker_compose/local_validator.rs`)
259266

260267
**Benefits:**
261268

@@ -266,36 +273,40 @@ descriptive error on failure.
266273

267274
### Phase 4: Documentation
268275

269-
- [ ] Update the template documentation comments if needed
276+
- [x] Update the template documentation comments if needed (no changes required;
277+
guards are self-documenting)
270278

271279
## Acceptance Criteria
272280

273281
**Quality Checks**:
274282

275-
- [ ] Pre-commit checks pass: `./scripts/pre-commit.sh`
283+
- [x] Pre-commit checks pass: `./scripts/pre-commit.sh`
276284

277285
**Task-Specific Criteria**:
278286

279-
- [ ] Minimal config (SQLite, no domains, no Prometheus) produces valid docker-compose.yml
280-
- [ ] The `run` command succeeds with the configuration that previously failed
281-
- [ ] Template guards are consistent across all service blocks
282-
- [ ] Unit test covers the minimal-config rendering scenario
283-
- [ ] Rendered docker-compose.yml is validated with `docker compose config --quiet` after template rendering
284-
- [ ] Invalid templates produce a clear, actionable error at `configure` time (not `run` time)
287+
- [x] Minimal config (SQLite, no domains, no Prometheus) produces valid docker-compose.yml
288+
- [x] Template guards are consistent across all service blocks
289+
- [x] Unit test covers the minimal-config rendering scenario
290+
- [x] Rendered docker-compose.yml is validated with `docker compose config --quiet` after template rendering
291+
- [x] Invalid templates produce a clear, actionable error at `configure` time (not `run` time)
292+
- [x] The `run` command succeeds with the configuration that previously failed
293+
- Verified with `envs/minimal-fix-test.json` (SQLite, no domains, no Prometheus):
294+
`create → provision → configure → release → run → test` all passed ✅
285295

286296
## Related Documentation
287297

288298
- Template source: `templates/docker-compose/docker-compose.yml.tera`
289299
- Network derivation: `src/domain/tracker/config/mod.rs` (lines 560-585)
290300
- Network topology: `src/domain/topology/network.rs`
291301
- Template context builder: `src/infrastructure/templating/docker_compose/template/wrappers/docker_compose/context/builder.rs`
302+
- Local validator: `src/infrastructure/templating/docker_compose/local_validator.rs`
303+
- ADR: [Docker Compose Local Validation Placement](../decisions/docker-compose-local-validation-placement.md)
292304

293305
## Notes
294306

295-
- The `caddy` service block (line 80) also renders `networks:` unconditionally but
296-
is currently safe because caddy is only included when domains are configured, and
297-
caddy always has the proxy network. Adding a guard there too would be defensive
298-
but recommended for consistency.
307+
- All service `networks:` blocks (`tracker`, `caddy`, `prometheus`, `grafana`,
308+
`mysql`) are now guarded with `{%- if <service>.networks | length > 0 %}`,
309+
matching the pre-existing `backup` guard. The template is now consistent.
299310
- This bug affects any "minimal" deployment configuration without optional services.
300311
As the project adds more minimal deployment examples, this will become a more
301312
common failure mode.

0 commit comments

Comments
 (0)