Skip to content

Commit 01da06e

Browse files
committed
refactor: [#272] Replace tls with domain+use_tls_proxy for Grafana
Step 7.5.4 of configuration restructuring: - Replace tls: Option<TlsSection> with domain: Option<String> + use_tls_proxy - Update GrafanaSection DTO with new fields and validation - Update GrafanaConfig domain type with new constructor signature - Update docker compose context builder to use use_tls_proxy() - Update show command tests for Grafana - Update envs/manual-https-test.json to use new configuration format - Update spec document marking Step 7.5.4 complete This change simplifies the configuration model for Grafana, making TLS proxy enablement explicit rather than inferred from the presence of a tls section. Note: Grafana has no configurable bind address, so no localhost+TLS validation is needed.
1 parent 55dfa94 commit 01da06e

File tree

7 files changed

+232
-80
lines changed

7 files changed

+232
-80
lines changed

docs/issues/272-add-https-support-with-caddy.md

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1467,15 +1467,15 @@ The implementation is split into incremental steps, one service type at a time,
14671467

14681468
##### Step 7.5.4: Grafana
14691469

1470-
- [ ] Add `domain: Option<String>` and `use_tls_proxy: Option<bool>` to `GrafanaSection` DTO
1471-
- [ ] Update `GrafanaConfig` domain type
1472-
- [ ] Add validation rules (note: Grafana has no configurable bind address, so localhost validation not needed)
1473-
- [ ] Update Caddy template for Grafana
1474-
- [ ] Update show command `ServiceInfo` for Grafana
1475-
- [ ] Update `envs/manual-https-test.json` for Grafana
1476-
- [ ] Remove `TlsSection` from Grafana
1477-
- [ ] Add unit tests
1478-
- [ ] Run E2E tests
1470+
- [x] Add `domain: Option<String>` and `use_tls_proxy: Option<bool>` to `GrafanaSection` DTO
1471+
- [x] Update `GrafanaConfig` domain type
1472+
- [x] Add validation rules (note: Grafana has no configurable bind address, so localhost validation not needed)
1473+
- [x] Update Caddy template for Grafana
1474+
- [x] Update show command `ServiceInfo` for Grafana
1475+
- [x] Update `envs/manual-https-test.json` for Grafana
1476+
- [x] Remove `TlsSection` from Grafana
1477+
- [x] Add unit tests
1478+
- [x] Run E2E tests
14791479

14801480
##### Step 7.5.5: Cleanup and Final Verification
14811481

src/application/command_handlers/create/config/environment_config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ impl EnvironmentCreationConfig {
410410

411411
// Check Grafana
412412
if let Some(ref grafana) = self.grafana {
413-
if grafana.tls.is_some() {
413+
if grafana.use_tls_proxy == Some(true) {
414414
return true;
415415
}
416416
}

src/application/command_handlers/create/config/grafana.rs

Lines changed: 124 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@ use schemars::JsonSchema;
88
use serde::{Deserialize, Serialize};
99

1010
use crate::application::command_handlers::create::config::errors::CreateConfigError;
11-
use crate::application::command_handlers::create::config::https::TlsSection;
1211
use crate::domain::grafana::GrafanaConfig;
13-
use crate::domain::tls::TlsConfig;
1412
use crate::shared::secrets::PlainPassword;
1513

1614
use crate::shared::DomainName;
@@ -35,14 +33,13 @@ use crate::shared::DomainName;
3533
/// }
3634
/// ```
3735
///
38-
/// With TLS configuration:
36+
/// With TLS proxy configuration:
3937
/// ```json
4038
/// {
4139
/// "admin_user": "admin",
4240
/// "admin_password": "admin",
43-
/// "tls": {
44-
/// "domain": "grafana.example.com"
45-
/// }
41+
/// "domain": "grafana.example.com",
42+
/// "use_tls_proxy": true
4643
/// }
4744
/// ```
4845
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
@@ -56,12 +53,21 @@ pub struct GrafanaSection {
5653
/// to prevent accidental exposure in logs or debug output.
5754
pub admin_password: PlainPassword,
5855

59-
/// Optional TLS configuration for HTTPS
56+
/// Domain name for external HTTPS access (optional)
6057
///
61-
/// When present, Grafana will be proxied through Caddy with HTTPS enabled.
62-
/// The domain specified will be used for Let's Encrypt certificate acquisition.
58+
/// When present, defines the domain at which Grafana will be accessible.
59+
/// Caddy uses this for automatic certificate management.
6360
#[serde(default, skip_serializing_if = "Option::is_none")]
64-
pub tls: Option<TlsSection>,
61+
pub domain: Option<String>,
62+
63+
/// Whether to use TLS proxy via Caddy (default: false)
64+
///
65+
/// When true:
66+
/// - Caddy handles HTTPS termination with automatic certificates
67+
/// - Requires a domain to be configured
68+
/// - Grafana is accessed via HTTPS through Caddy
69+
#[serde(default, skip_serializing_if = "Option::is_none")]
70+
pub use_tls_proxy: Option<bool>,
6571
}
6672

6773
impl Default for GrafanaSection {
@@ -70,7 +76,8 @@ impl Default for GrafanaSection {
7076
Self {
7177
admin_user: default_config.admin_user().to_string(),
7278
admin_password: default_config.admin_password().expose_secret().to_string(),
73-
tls: None,
79+
domain: None,
80+
use_tls_proxy: None,
7481
}
7582
}
7683
}
@@ -84,26 +91,38 @@ impl GrafanaSection {
8491
///
8592
/// # Errors
8693
///
87-
/// Returns `CreateConfigError::InvalidDomain` if the TLS domain is invalid.
94+
/// Returns `CreateConfigError::InvalidDomain` if the domain is invalid.
95+
/// Returns `CreateConfigError::TlsProxyWithoutDomain` if `use_tls_proxy`
96+
/// is true but no domain is provided.
8897
pub fn to_grafana_config(&self) -> Result<GrafanaConfig, CreateConfigError> {
89-
let config = match &self.tls {
90-
Some(tls_section) => {
91-
tls_section.validate()?;
92-
let domain = DomainName::new(&tls_section.domain).map_err(|e| {
98+
let use_tls_proxy = self.use_tls_proxy.unwrap_or(false);
99+
100+
// Validate: use_tls_proxy requires domain
101+
if use_tls_proxy && self.domain.is_none() {
102+
return Err(CreateConfigError::TlsProxyWithoutDomain {
103+
service_type: "Grafana".to_string(),
104+
bind_address: "N/A (hardcoded port 3000)".to_string(),
105+
});
106+
}
107+
108+
// Parse domain if present
109+
let domain =
110+
match &self.domain {
111+
Some(domain_str) => Some(DomainName::new(domain_str).map_err(|e| {
93112
CreateConfigError::InvalidDomain {
94-
domain: tls_section.domain.clone(),
113+
domain: domain_str.clone(),
95114
reason: e.to_string(),
96115
}
97-
})?;
98-
GrafanaConfig::with_tls(
99-
self.admin_user.clone(),
100-
self.admin_password.clone(),
101-
TlsConfig::new(domain),
102-
)
103-
}
104-
None => GrafanaConfig::new(self.admin_user.clone(), self.admin_password.clone()),
105-
};
106-
Ok(config)
116+
})?),
117+
None => None,
118+
};
119+
120+
Ok(GrafanaConfig::new(
121+
self.admin_user.clone(),
122+
self.admin_password.clone(),
123+
domain,
124+
use_tls_proxy,
125+
))
107126
}
108127
}
109128

@@ -116,15 +135,17 @@ mod tests {
116135
let section = GrafanaSection::default();
117136
assert_eq!(section.admin_user, "admin");
118137
assert_eq!(section.admin_password, "admin");
119-
assert!(section.tls.is_none());
138+
assert!(section.domain.is_none());
139+
assert!(section.use_tls_proxy.is_none());
120140
}
121141

122142
#[test]
123143
fn it_should_convert_to_grafana_config() {
124144
let section = GrafanaSection {
125145
admin_user: "custom_admin".to_string(),
126146
admin_password: "secure_password".to_string(),
127-
tls: None,
147+
domain: None,
148+
use_tls_proxy: None,
128149
};
129150

130151
let result = section.to_grafana_config();
@@ -150,7 +171,8 @@ mod tests {
150171
let section = GrafanaSection {
151172
admin_user: "admin".to_string(),
152173
admin_password: "secret_password".to_string(),
153-
tls: None,
174+
domain: None,
175+
use_tls_proxy: None,
154176
};
155177

156178
let config = section.to_grafana_config().unwrap();
@@ -160,4 +182,76 @@ mod tests {
160182
assert!(debug_output.contains("[REDACTED]"));
161183
assert!(!debug_output.contains("secret_password"));
162184
}
185+
186+
#[test]
187+
fn it_should_convert_with_domain_and_tls_proxy() {
188+
let section = GrafanaSection {
189+
admin_user: "admin".to_string(),
190+
admin_password: "password".to_string(),
191+
domain: Some("grafana.example.com".to_string()),
192+
use_tls_proxy: Some(true),
193+
};
194+
195+
let result = section.to_grafana_config();
196+
assert!(result.is_ok());
197+
198+
let config = result.unwrap();
199+
assert_eq!(config.tls_domain(), Some("grafana.example.com"));
200+
assert!(config.use_tls_proxy());
201+
}
202+
203+
#[test]
204+
fn it_should_convert_with_domain_without_tls_proxy() {
205+
let section = GrafanaSection {
206+
admin_user: "admin".to_string(),
207+
admin_password: "password".to_string(),
208+
domain: Some("grafana.example.com".to_string()),
209+
use_tls_proxy: Some(false),
210+
};
211+
212+
let result = section.to_grafana_config();
213+
assert!(result.is_ok());
214+
215+
let config = result.unwrap();
216+
assert_eq!(
217+
config.domain(),
218+
Some(&DomainName::new("grafana.example.com").unwrap())
219+
);
220+
assert!(!config.use_tls_proxy());
221+
}
222+
223+
#[test]
224+
fn it_should_return_error_when_tls_proxy_enabled_without_domain() {
225+
let section = GrafanaSection {
226+
admin_user: "admin".to_string(),
227+
admin_password: "password".to_string(),
228+
domain: None,
229+
use_tls_proxy: Some(true),
230+
};
231+
232+
let result = section.to_grafana_config();
233+
assert!(result.is_err());
234+
235+
let err = result.unwrap_err();
236+
assert!(matches!(
237+
err,
238+
CreateConfigError::TlsProxyWithoutDomain { .. }
239+
));
240+
}
241+
242+
#[test]
243+
fn it_should_return_error_for_invalid_domain() {
244+
let section = GrafanaSection {
245+
admin_user: "admin".to_string(),
246+
admin_password: "password".to_string(),
247+
domain: Some(String::new()),
248+
use_tls_proxy: Some(true),
249+
};
250+
251+
let result = section.to_grafana_config();
252+
assert!(result.is_err());
253+
254+
let err = result.unwrap_err();
255+
assert!(matches!(err, CreateConfigError::InvalidDomain { .. }));
256+
}
163257
}

src/application/command_handlers/show/info/grafana.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,11 @@ mod tests {
9191
#[test]
9292
fn it_should_create_grafana_info_with_https_from_config() {
9393
use crate::domain::grafana::GrafanaConfig;
94-
use crate::domain::tls::TlsConfig;
9594
use crate::shared::domain_name::DomainName;
9695

9796
let domain = DomainName::new("grafana.tracker.local").unwrap();
98-
let config = GrafanaConfig::with_tls(
99-
"admin".to_string(),
100-
"pass".to_string(),
101-
TlsConfig::new(domain),
102-
);
97+
let config =
98+
GrafanaConfig::new("admin".to_string(), "pass".to_string(), Some(domain), true);
10399
let ip = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1));
104100

105101
let info = GrafanaInfo::from_config(&config, ip);
@@ -111,7 +107,7 @@ mod tests {
111107

112108
#[test]
113109
fn it_should_create_grafana_info_with_http_from_config_without_tls() {
114-
let config = GrafanaConfig::new("admin".to_string(), "pass".to_string());
110+
let config = GrafanaConfig::new("admin".to_string(), "pass".to_string(), None, false);
115111
let ip = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1));
116112

117113
let info = GrafanaInfo::from_config(&config, ip);

0 commit comments

Comments
 (0)