Skip to content

Commit cdbbda6

Browse files
committed
refactor: [#301] implement PortDerivation for GrafanaConfig
1 parent b8db37f commit cdbbda6

4 files changed

Lines changed: 104 additions & 40 deletions

File tree

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -314,10 +314,10 @@ impl TrackerServiceContext {
314314

315315
### Step 3: Implement PortDerivation for Grafana
316316

317-
- [ ] 3.1 Implement `PortDerivation` for `GrafanaConfig` in domain
318-
- [ ] 3.2 Add unit tests for Grafana port derivation
319-
- [ ] 3.3 Update infrastructure `GrafanaServiceConfig` to use domain trait
320-
- [ ] 3.4 Remove `derive_grafana_ports()` calls from infrastructure
317+
- [x] 3.1 Implement `PortDerivation` for `GrafanaConfig` in domain
318+
- [x] 3.2 Add unit tests for Grafana port derivation
319+
- [x] 3.3 Update infrastructure `GrafanaServiceConfig` to use domain trait
320+
- [x] 3.4 Remove `derive_grafana_ports()` calls from infrastructure
321321

322322
### Step 4: Implement PortDerivation for Tracker (Most Complex)
323323

src/domain/grafana/config.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
use serde::{Deserialize, Serialize};
44

5+
use crate::domain::topology::{PortBinding, PortDerivation};
56
use crate::shared::domain_name::DomainName;
67
use crate::shared::secrets::Password;
78

@@ -116,6 +117,23 @@ impl Default for GrafanaConfig {
116117
}
117118
}
118119

120+
impl PortDerivation for GrafanaConfig {
121+
/// Derives port bindings for Grafana
122+
///
123+
/// Implements PORT-07 and PORT-08:
124+
/// - Without TLS: expose port 3000 directly
125+
/// - With TLS: don't expose (Caddy handles it)
126+
fn derive_ports(&self) -> Vec<PortBinding> {
127+
// PORT-07: Grafana 3000 exposed only without TLS
128+
// PORT-08: Grafana 3000 NOT exposed with TLS
129+
if self.use_tls_proxy {
130+
vec![]
131+
} else {
132+
vec![PortBinding::tcp(3000, "Grafana dashboard")]
133+
}
134+
}
135+
}
136+
119137
#[cfg(test)]
120138
mod tests {
121139
use super::*;
@@ -233,4 +251,53 @@ mod tests {
233251
config.admin_password.expose_secret()
234252
);
235253
}
254+
255+
// =========================================================================
256+
// Port derivation tests (PORT-07, PORT-08)
257+
// =========================================================================
258+
259+
mod port_derivation {
260+
use super::*;
261+
use crate::domain::tracker::Protocol;
262+
263+
#[test]
264+
fn it_should_expose_port_3000_when_tls_disabled() {
265+
// PORT-07: Grafana 3000 exposed only without TLS
266+
let config =
267+
GrafanaConfig::new("admin".to_string(), "password".to_string(), None, false);
268+
269+
let ports = config.derive_ports();
270+
271+
assert_eq!(ports.len(), 1);
272+
let port = &ports[0];
273+
assert_eq!(port.host_port(), 3000);
274+
assert_eq!(port.container_port(), 3000);
275+
assert_eq!(port.protocol(), Protocol::Tcp);
276+
}
277+
278+
#[test]
279+
fn it_should_not_expose_port_when_tls_enabled() {
280+
// PORT-08: Grafana 3000 NOT exposed with TLS
281+
let domain = DomainName::new("grafana.example.com").unwrap();
282+
let config = GrafanaConfig::new(
283+
"admin".to_string(),
284+
"password".to_string(),
285+
Some(domain),
286+
true,
287+
);
288+
289+
let ports = config.derive_ports();
290+
291+
assert!(ports.is_empty());
292+
}
293+
294+
#[test]
295+
fn it_should_include_description_for_grafana_port() {
296+
let config = GrafanaConfig::default();
297+
298+
let ports = config.derive_ports();
299+
300+
assert_eq!(ports[0].description(), "Grafana dashboard");
301+
}
302+
}
236303
}

src/infrastructure/templating/docker_compose/template/wrappers/docker_compose/context/builder.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -146,15 +146,10 @@ impl DockerComposeContextBuilder {
146146
.map(|config| PrometheusServiceConfig::new(config, has_grafana));
147147

148148
// Build Grafana service config if enabled
149-
let grafana = self.grafana_config.map(|config| {
150-
let has_tls = config.use_tls_proxy();
151-
GrafanaServiceConfig::new(
152-
config.admin_user().to_string(),
153-
config.admin_password().clone(),
154-
has_tls,
155-
has_caddy,
156-
)
157-
});
149+
let grafana = self
150+
.grafana_config
151+
.as_ref()
152+
.map(|config| GrafanaServiceConfig::new(config, has_caddy));
158153

159154
// Build Caddy service config if enabled
160155
let caddy = if has_caddy {

src/infrastructure/templating/docker_compose/template/wrappers/docker_compose/context/grafana.rs

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
// External crates
44
use serde::Serialize;
55

6-
use crate::domain::topology::Network;
6+
use crate::domain::grafana::GrafanaConfig;
7+
use crate::domain::topology::{Network, PortDerivation};
78
use crate::shared::secrets::Password;
89

910
use super::port_definition::PortDefinition;
10-
use super::port_derivation::derive_grafana_ports;
1111

1212
/// Grafana service configuration for Docker Compose
1313
///
@@ -41,25 +41,19 @@ impl GrafanaServiceConfig {
4141
///
4242
/// # Arguments
4343
///
44-
/// * `admin_user` - Grafana admin username
45-
/// * `admin_password` - Grafana admin password
46-
/// * `has_tls` - Whether Grafana has TLS enabled (via Caddy)
44+
/// * `config` - The domain Grafana configuration
4745
/// * `has_caddy` - Whether Caddy TLS proxy is enabled (adds `proxy_network`)
4846
#[must_use]
49-
pub fn new(
50-
admin_user: String,
51-
admin_password: Password,
52-
has_tls: bool,
53-
has_caddy: bool,
54-
) -> Self {
47+
pub fn new(config: &GrafanaConfig, has_caddy: bool) -> Self {
5548
let networks = Self::compute_networks(has_caddy);
56-
let port_bindings = derive_grafana_ports(has_tls);
49+
// Use domain PortDerivation trait for port logic
50+
let port_bindings = config.derive_ports();
5751
let ports = port_bindings.iter().map(PortDefinition::from).collect();
5852

5953
Self {
60-
admin_user,
61-
admin_password,
62-
has_tls,
54+
admin_user: config.admin_user().to_string(),
55+
admin_password: config.admin_password().clone(),
56+
has_tls: config.use_tls_proxy(),
6357
ports,
6458
networks,
6559
}
@@ -80,28 +74,39 @@ impl GrafanaServiceConfig {
8074
#[cfg(test)]
8175
mod tests {
8276
use super::*;
77+
use crate::shared::DomainName;
78+
79+
fn make_config(use_tls_proxy: bool) -> GrafanaConfig {
80+
if use_tls_proxy {
81+
GrafanaConfig::new(
82+
"admin".to_string(),
83+
"password".to_string(),
84+
Some(DomainName::new("grafana.example.com").unwrap()),
85+
true,
86+
)
87+
} else {
88+
GrafanaConfig::new("admin".to_string(), "password".to_string(), None, false)
89+
}
90+
}
8391

8492
#[test]
8593
fn it_should_connect_grafana_to_visualization_network() {
86-
let config =
87-
GrafanaServiceConfig::new("admin".to_string(), Password::new("password"), false, false);
94+
let config = GrafanaServiceConfig::new(&make_config(false), false);
8895

8996
assert!(config.networks.contains(&Network::Visualization));
9097
}
9198

9299
#[test]
93100
fn it_should_not_connect_grafana_to_proxy_network_when_caddy_disabled() {
94-
let config =
95-
GrafanaServiceConfig::new("admin".to_string(), Password::new("password"), false, false);
101+
let config = GrafanaServiceConfig::new(&make_config(false), false);
96102

97103
assert_eq!(config.networks, vec![Network::Visualization]);
98104
assert!(!config.networks.contains(&Network::Proxy));
99105
}
100106

101107
#[test]
102108
fn it_should_connect_grafana_to_proxy_network_when_caddy_enabled() {
103-
let config =
104-
GrafanaServiceConfig::new("admin".to_string(), Password::new("password"), true, true);
109+
let config = GrafanaServiceConfig::new(&make_config(true), true);
105110

106111
assert_eq!(
107112
config.networks,
@@ -111,8 +116,7 @@ mod tests {
111116

112117
#[test]
113118
fn it_should_serialize_networks_to_name_strings() {
114-
let config =
115-
GrafanaServiceConfig::new("admin".to_string(), Password::new("password"), true, true);
119+
let config = GrafanaServiceConfig::new(&make_config(true), true);
116120

117121
let json = serde_json::to_value(&config).expect("serialization should succeed");
118122

@@ -123,17 +127,15 @@ mod tests {
123127

124128
#[test]
125129
fn it_should_expose_port_3000_when_tls_disabled() {
126-
let config =
127-
GrafanaServiceConfig::new("admin".to_string(), Password::new("password"), false, false);
130+
let config = GrafanaServiceConfig::new(&make_config(false), false);
128131

129132
assert_eq!(config.ports.len(), 1);
130133
assert_eq!(config.ports[0].binding(), "3000:3000");
131134
}
132135

133136
#[test]
134137
fn it_should_not_expose_ports_when_tls_enabled() {
135-
let config =
136-
GrafanaServiceConfig::new("admin".to_string(), Password::new("password"), true, true);
138+
let config = GrafanaServiceConfig::new(&make_config(true), true);
137139

138140
assert!(config.ports.is_empty());
139141
}

0 commit comments

Comments
 (0)