Skip to content

Commit 18e9aee

Browse files
committed
Merge #303: refactor(infrastructure): [#301] Phase 4 - Extract ServiceTopology shared type
14a27e4 fix: [#304] add crate-level allow for clippy::large_stack_arrays false positive (Jose Celano) 2f3314a fix(domain): [#301] re-enable aggregate tests with clippy allow attribute (Jose Celano) 1962386 refactor(infrastructure): [#301] extract ServiceTopology shared type (Jose Celano) 195c80e refactor(infrastructure): [#301] remove unused fields from Grafana/Prometheus ServiceContext (Jose Celano) e87b7ae refactor(infrastructure): [#301] remove legacy fields from TrackerServiceContext (Jose Celano) f47326f docs: [#301] update Phase 4 progress - Step 7.5 complete (Jose Celano) 8452fb5 refactor(infrastructure): [#301] remove TrackerPorts type alias (Jose Celano) 7a9d035 refactor(infrastructure): [#301] rename service configs to *Context (Jose Celano) 1e28b43 refactor(domain): [#301] replace fixed_ports.rs with domain types for Caddy/MySQL (Jose Celano) d98df1e docs: [#301] mark Phase 4 acceptance criteria complete (Jose Celano) 49a9eef docs: [#301] update Phase 4 progress - all main goals complete (Jose Celano) 30eb4a1 refactor(domain): [#301] add NetworkDerivation trait and EnabledServices abstraction (Jose Celano) 17a526e refactor: [#301] rename PrometheusServiceConfig and GrafanaServiceConfig constructors (Jose Celano) 9da19b6 refactor: [#301] complete DDD migration - remove TrackerServiceConfig::new() (Jose Celano) 15ae4c9 refactor: [#301] add fixed port functions for Caddy and MySQL in domain (Jose Celano) 62e02ff refactor: [#301] implement PortDerivation for TrackerConfig (Jose Celano) cdbbda6 refactor: [#301] implement PortDerivation for GrafanaConfig (Jose Celano) b8db37f refactor: [#301] implement PortDerivation for PrometheusConfig (Jose Celano) 894da3c refactor: [#301] add PortDerivation trait in domain topology (Jose Celano) Pull request description: ## Summary Closes #301 This PR completes Phase 4 of the Docker Compose Topology Domain Model Refactoring by extracting a shared `ServiceTopology` type to formalize the architectural pattern discovered during cleanup. ## Problem After Phase 3 cleanup that removed unused fields from all ServiceContext types, we observed that all 5 types (`TrackerServiceContext`, `GrafanaServiceContext`, `PrometheusServiceContext`, `CaddyServiceContext`, `MysqlServiceContext`) now share identical fields: ```rust pub ports: Vec<PortDefinition> pub networks: Vec<Network> ``` This wasn't just coincidence - it represents a clear architectural pattern: - **Topology data** (ports/networks) → needed for `docker-compose.yml` template (inter-service relationships) - **Configuration data** (credentials, intervals) → injected via `.env` files (internal service config) ## Solution Extract a shared `ServiceTopology` type to make this pattern explicit: ```rust #[derive(Serialize, Debug, Clone, PartialEq, Default)] pub struct ServiceTopology { pub ports: Vec<PortDefinition>, pub networks: Vec<Network>, } ``` All ServiceContext types now embed this with `#[serde(flatten)]` to maintain template compatibility (flat JSON structure). ## Changes ### New File - `service_topology.rs` - Shared topology structure with documentation explaining the design rationale ### Updated ServiceContext Types All now use the embedded `ServiceTopology` pattern: - `TrackerServiceContext` - `GrafanaServiceContext` - `PrometheusServiceContext` - `CaddyServiceContext` - `MysqlServiceContext` ### API Changes - Added `ports()` and `networks()` accessor methods to each ServiceContext - Updated `builder.rs` to use accessor methods - Updated all tests to use accessor methods ## Testing - All 2075 library tests pass - All E2E tests pass (infrastructure lifecycle + deployment workflow) - Pre-commit checks pass (linting, formatting, spell check, clippy) ## Architecture Diagram ``` ┌─────────────────────────────────────────────────────┐ │ ServiceContext (e.g., Tracker) │ ├─────────────────────────────────────────────────────┤ │ #[serde(flatten)] │ │ pub topology: ServiceTopology │ │ ├── ports: Vec<PortDefinition> ─────┐ │ │ └── networks: Vec<Network> ────┼── JSON │ │ │ flat │ └───────────────────────────────────────────┴─────────┘ ↓ docker-compose.yml.tera template ``` ## Related - Epic: #287 - Docker Compose Topology Domain Model Refactoring - Predecessor: #300 - Phase 3 Port Topology Template Integration - Plan: `docs/refactors/plans/docker-compose-topology-domain-model.md` ACKs for top commit: josecelano: ACK 14a27e4 Tree-SHA512: 3712632ca6c1cad234d31c99e424d7031ea9924d8033b1d5ca2e3743d83f2cd36e6d7861a928b9f21ccfb5a8023d3bdc251f26c4ba152c99c31b5a6d61075bd4
2 parents c157a2a + 14a27e4 commit 18e9aee

29 files changed

Lines changed: 2620 additions & 1333 deletions

File tree

docs/issues/301-phase-4-service-topology-ddd-alignment.md

Lines changed: 124 additions & 69 deletions
Large diffs are not rendered by default.
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
# Issue #304: Clippy `large_stack_arrays` False Positive with `vec![]` Macro
2+
3+
**GitHub Issue**: <https://github.com/torrust/torrust-tracker-deployer/issues/304>
4+
5+
## Problem
6+
7+
Clippy reports a false positive `large_stack_arrays` lint when using the `vec![]` macro
8+
to create vectors of `ServiceTopology` items in tests.
9+
10+
### Error Message
11+
12+
```text
13+
error: allocating a local array larger than 16384 bytes
14+
|
15+
= help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.93.0/index.html#large_stack_arrays
16+
= note: `-D clippy::large-stack-arrays` implied by `-D clippy::pedantic`
17+
= help: to override `-D clippy::pedantic` add `#[allow(clippy::large_stack_arrays)]`
18+
```
19+
20+
### Root Cause
21+
22+
This is a known clippy bug where the `vec![]` macro is incorrectly flagged for creating
23+
a large stack array. The `vec![]` macro creates a `Vec<T>` which allocates on the heap,
24+
not the stack. The lint is a false positive.
25+
26+
**Upstream Issue**: <https://github.com/rust-lang/rust-clippy/issues/12586>
27+
28+
### Affected Code
29+
30+
Tests in `src/domain/topology/aggregate.rs` that create vectors of `ServiceTopology`:
31+
32+
```rust
33+
let topology = DockerComposeTopology::new(vec![
34+
ServiceTopology::with_networks(Service::Tracker, vec![Network::Database]),
35+
ServiceTopology::with_networks(Service::MySQL, vec![Network::Database]),
36+
])
37+
.unwrap();
38+
```
39+
40+
### Workarounds Attempted (Did Not Work)
41+
42+
1. **Outer attribute on module**: `#[allow(clippy::large_stack_arrays)]` on `mod tests`
43+
2. **Inner attribute on submodule**: `#![allow(clippy::large_stack_arrays)]` inside submodules
44+
3. **Outer attribute on test function**: `#[allow(clippy::large_stack_arrays)]` on `#[test]` functions
45+
4. **Inner attribute in function body**: `#![allow(clippy::large_stack_arrays)]` inside function
46+
47+
None of these local suppression methods worked because the lint fires during macro
48+
expansion before the allow attributes are processed.
49+
50+
## Solution
51+
52+
Add a **crate-level allow attribute** in `src/lib.rs`:
53+
54+
```rust
55+
// False positive: clippy reports large_stack_arrays for vec![] macro with ServiceTopology
56+
// This is a known issue: https://github.com/rust-lang/rust-clippy/issues/12586
57+
#![allow(clippy::large_stack_arrays)]
58+
```
59+
60+
This is the only approach that successfully suppresses the false positive.
61+
62+
### Trade-offs
63+
64+
- **Downside**: Suppresses the lint crate-wide, potentially hiding legitimate issues
65+
- **Mitigation**: This lint is specifically for stack allocations. Since we use `Vec<T>`
66+
(heap-allocated) throughout the codebase, legitimate triggers are unlikely
67+
- **Future**: Remove this allow once the upstream clippy issue is fixed
68+
69+
## Affected Rust Versions
70+
71+
- Rust 1.93.0 stable (confirmed on GitHub Actions CI)
72+
- Local nightly 1.95.0 does **not** reproduce the issue
73+
74+
### Fix Timeline
75+
76+
| Event | Date | Version |
77+
| ---------------------------------------------------------------------------------------------- | -------------- | ---------------------------------- |
78+
| Fix merged to clippy master ([PR #12624](https://github.com/rust-lang/rust-clippy/pull/12624)) | April 27, 2024 | - |
79+
| Rust 1.79.0 released | June 13, 2024 | Nightly at merge time |
80+
| Rust 1.80.0 released | July 25, 2024 | **Expected first stable with fix** |
81+
82+
### Potential Regression
83+
84+
The fix was merged in April 2024 and should be in Rust 1.80.0+. However, we're still
85+
seeing this error on Rust 1.93.0 (January 2026). This suggests either:
86+
87+
1. **Regression**: The bug may have regressed in a later clippy version
88+
2. **Different code pattern**: Our `vec![]` with `ServiceTopology` (large struct with
89+
`EnumSet` fields) might trigger a variant not covered by the original fix
90+
3. **CI environment**: Some discrepancy in the clippy version used in CI
91+
92+
Consider reporting this as a potential regression if the issue persists after verifying
93+
the clippy version matches the expected behavior.
94+
95+
## Related Clippy Issues
96+
97+
Other `large_stack_arrays` issues (not directly related to our problem):
98+
99+
- [#13774](https://github.com/rust-lang/rust-clippy/issues/13774) - No span for error (closed)
100+
- [#13529](https://github.com/rust-lang/rust-clippy/issues/13529) - Nested const items (closed)
101+
- [#9460](https://github.com/rust-lang/rust-clippy/issues/9460) - Static struct (closed)
102+
- [#4520](https://github.com/rust-lang/rust-clippy/issues/4520) - Original lint creation (open)
103+
104+
## References
105+
106+
- Upstream clippy issue: <https://github.com/rust-lang/rust-clippy/issues/12586>
107+
- Related PR: #303 (Phase 4 Service Topology DDD Alignment)
108+
109+
## Labels
110+
111+
- `bug`
112+
- `workaround`
113+
- `clippy`
114+
- `technical-debt`

src/application/steps/rendering/docker_compose_templates.rs

Lines changed: 25 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,12 @@ use std::sync::Arc;
2929

3030
use tracing::{info, instrument};
3131

32-
use crate::domain::environment::user_inputs::UserInputs;
3332
use crate::domain::environment::Environment;
3433
use crate::domain::template::TemplateManager;
35-
use crate::domain::tracker::{DatabaseConfig, TrackerConfig};
34+
use crate::domain::topology::EnabledServices;
35+
use crate::domain::tracker::DatabaseConfig;
3636
use crate::infrastructure::templating::docker_compose::template::wrappers::docker_compose::{
37-
DockerComposeContext, DockerComposeContextBuilder, MysqlSetupConfig, TrackerServiceConfig,
37+
DockerComposeContext, DockerComposeContextBuilder, MysqlSetupConfig, TrackerServiceContext,
3838
};
3939
use crate::infrastructure::templating::docker_compose::template::wrappers::env::EnvContext;
4040
use crate::infrastructure::templating::docker_compose::{
@@ -155,12 +155,8 @@ impl<S> RenderDockerComposeTemplatesStep<S> {
155155
self.environment.admin_token().to_string()
156156
}
157157

158-
fn build_tracker_config(&self) -> TrackerServiceConfig {
158+
fn build_tracker_config(&self) -> TrackerServiceContext {
159159
let tracker_config = self.environment.tracker_config();
160-
let user_inputs = &self.environment.context().user_inputs;
161-
162-
let (udp_tracker_ports, http_tracker_ports_without_tls, http_api_port, http_api_has_tls) =
163-
Self::extract_tracker_ports(tracker_config, user_inputs);
164160

165161
// Determine which features are enabled (affects tracker networks)
166162
let has_prometheus = self.environment.prometheus_config().is_some();
@@ -169,16 +165,26 @@ impl<S> RenderDockerComposeTemplatesStep<S> {
169165
DatabaseConfig::Mysql(..)
170166
);
171167
let has_caddy = self.has_caddy_enabled();
168+
let has_grafana = self.environment.grafana_config().is_some();
172169

173-
TrackerServiceConfig::new(
174-
udp_tracker_ports,
175-
http_tracker_ports_without_tls,
176-
http_api_port,
177-
http_api_has_tls,
178-
has_prometheus,
179-
has_mysql,
180-
has_caddy,
181-
)
170+
// Build list of enabled services for topology context
171+
let mut enabled_services = Vec::new();
172+
if has_prometheus {
173+
enabled_services.push(crate::domain::topology::Service::Prometheus);
174+
}
175+
if has_grafana {
176+
enabled_services.push(crate::domain::topology::Service::Grafana);
177+
}
178+
if has_mysql {
179+
enabled_services.push(crate::domain::topology::Service::MySQL);
180+
}
181+
if has_caddy {
182+
enabled_services.push(crate::domain::topology::Service::Caddy);
183+
}
184+
185+
let topology_context = EnabledServices::from(&enabled_services);
186+
187+
TrackerServiceContext::from_domain_config(tracker_config, &topology_context)
182188
}
183189

184190
/// Check if Caddy is enabled (HTTPS with at least one TLS-configured service)
@@ -205,7 +211,7 @@ impl<S> RenderDockerComposeTemplatesStep<S> {
205211

206212
fn create_sqlite_contexts(
207213
admin_token: String,
208-
tracker: TrackerServiceConfig,
214+
tracker: TrackerServiceContext,
209215
) -> (EnvContext, DockerComposeContextBuilder) {
210216
let env_context = EnvContext::new(admin_token);
211217
let builder = DockerComposeContext::builder(tracker);
@@ -215,7 +221,7 @@ impl<S> RenderDockerComposeTemplatesStep<S> {
215221

216222
fn create_mysql_contexts(
217223
admin_token: String,
218-
tracker: TrackerServiceConfig,
224+
tracker: TrackerServiceContext,
219225
port: u16,
220226
database_name: String,
221227
username: String,
@@ -312,47 +318,6 @@ impl<S> RenderDockerComposeTemplatesStep<S> {
312318
env_context
313319
}
314320
}
315-
316-
fn extract_tracker_ports(
317-
tracker_config: &TrackerConfig,
318-
user_inputs: &UserInputs,
319-
) -> (Vec<u16>, Vec<u16>, u16, bool) {
320-
// Extract UDP tracker ports (always exposed - no TLS termination via Caddy)
321-
let udp_ports: Vec<u16> = tracker_config
322-
.udp_trackers()
323-
.iter()
324-
.map(|tracker| tracker.bind_address().port())
325-
.collect();
326-
327-
// Get the set of HTTP tracker ports that have TLS enabled
328-
let tls_enabled_ports: std::collections::HashSet<u16> = user_inputs
329-
.tracker()
330-
.http_trackers_with_tls()
331-
.iter()
332-
.map(|(_, port)| *port)
333-
.collect();
334-
335-
// Extract HTTP tracker ports WITHOUT TLS (these need to be exposed)
336-
let http_ports_without_tls: Vec<u16> = tracker_config
337-
.http_trackers()
338-
.iter()
339-
.map(|tracker| tracker.bind_address().port())
340-
.filter(|port| !tls_enabled_ports.contains(port))
341-
.collect();
342-
343-
// Extract HTTP API port
344-
let api_port = tracker_config.http_api().bind_address().port();
345-
346-
// Check if HTTP API has TLS enabled
347-
let http_api_has_tls = user_inputs.tracker().http_api_tls_domain().is_some();
348-
349-
(
350-
udp_ports,
351-
http_ports_without_tls,
352-
api_port,
353-
http_api_has_tls,
354-
)
355-
}
356321
}
357322

358323
#[cfg(test)]

0 commit comments

Comments
 (0)