Skip to content

Commit 17903f0

Browse files
committed
refactor: [#281] simplify ServiceInfo by always computing from tracker config
- Remove from_service_endpoints method (was using runtime state lacking TLS info) - Update show handler to always use from_tracker_config with instance IP - Split from_tracker_config into focused helper methods: - build_udp_tracker_urls - build_http_tracker_info - build_api_endpoint_info - build_health_check_info - collect_grafana_tls_domain - Remove #[allow(clippy::too_many_lines)] attribute This simplifies the code by establishing from_tracker_config as the single source of truth for ServiceInfo construction, using the tracker configuration plus instance IP rather than storing incomplete runtime endpoints.
1 parent 891c818 commit 17903f0

2 files changed

Lines changed: 108 additions & 111 deletions

File tree

src/application/command_handlers/show/handler.rs

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -142,22 +142,12 @@ impl ShowCommandHandler {
142142

143143
// Add service info for Released/Running states
144144
if Self::should_show_services(any_env.state_name()) {
145-
// If HTTPS is configured, always compute from tracker config to show TLS domains
146-
// Otherwise, try stored endpoints first for backward compatibility
147-
let services = if any_env.https_config().is_some() {
148-
// HTTPS enabled: compute from config to show proper TLS domains
149-
let tracker_config = any_env.tracker_config();
150-
let grafana_config = any_env.grafana_config();
151-
ServiceInfo::from_tracker_config(tracker_config, instance_ip, grafana_config)
152-
} else if let Some(endpoints) = any_env.service_endpoints() {
153-
// No HTTPS: use stored endpoints (backward compatibility)
154-
ServiceInfo::from_service_endpoints(endpoints)
155-
} else {
156-
// Fallback: compute from tracker config
157-
let tracker_config = any_env.tracker_config();
158-
let grafana_config = any_env.grafana_config();
159-
ServiceInfo::from_tracker_config(tracker_config, instance_ip, grafana_config)
160-
};
145+
// Always compute from tracker config to show proper service information
146+
// including TLS domains, localhost hints, and HTTPS status
147+
let tracker_config = any_env.tracker_config();
148+
let grafana_config = any_env.grafana_config();
149+
let services =
150+
ServiceInfo::from_tracker_config(tracker_config, instance_ip, grafana_config);
161151
info = info.with_services(services);
162152

163153
// Add Prometheus info if configured

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

Lines changed: 102 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
55
use std::net::IpAddr;
66

7-
use crate::domain::environment::runtime_outputs::ServiceEndpoints;
87
use crate::domain::grafana::GrafanaConfig;
98
use crate::domain::tracker::config::is_localhost;
109
use crate::domain::tracker::TrackerConfig;
@@ -124,13 +123,44 @@ impl ServiceInfo {
124123
/// * `instance_ip` - The IP address of the deployed instance
125124
/// * `grafana_config` - Optional Grafana configuration (for TLS domain info)
126125
#[must_use]
127-
#[allow(clippy::too_many_lines)]
128126
pub fn from_tracker_config(
129127
tracker_config: &TrackerConfig,
130128
instance_ip: IpAddr,
131129
grafana_config: Option<&GrafanaConfig>,
132130
) -> Self {
133-
let udp_trackers = tracker_config
131+
let mut tls_domains = Vec::new();
132+
133+
let udp_trackers = Self::build_udp_tracker_urls(tracker_config, instance_ip);
134+
135+
let (https_http_trackers, direct_http_trackers, localhost_http_trackers) =
136+
Self::build_http_tracker_info(tracker_config, instance_ip, &mut tls_domains);
137+
138+
let (api_endpoint, api_uses_https, api_is_localhost_only) =
139+
Self::build_api_endpoint_info(tracker_config, instance_ip, &mut tls_domains);
140+
141+
Self::collect_grafana_tls_domain(grafana_config, &mut tls_domains);
142+
143+
let (health_check_url, health_check_uses_https, health_check_is_localhost_only) =
144+
Self::build_health_check_info(tracker_config, instance_ip, &mut tls_domains);
145+
146+
Self::new(
147+
udp_trackers,
148+
https_http_trackers,
149+
direct_http_trackers,
150+
localhost_http_trackers,
151+
api_endpoint,
152+
api_uses_https,
153+
api_is_localhost_only,
154+
health_check_url,
155+
health_check_uses_https,
156+
health_check_is_localhost_only,
157+
tls_domains,
158+
)
159+
}
160+
161+
/// Build UDP tracker URLs from configuration
162+
fn build_udp_tracker_urls(tracker_config: &TrackerConfig, instance_ip: IpAddr) -> Vec<String> {
163+
tracker_config
134164
.udp_trackers()
135165
.iter()
136166
.map(|udp| {
@@ -139,20 +169,26 @@ impl ServiceInfo {
139169
.map_or_else(|| instance_ip.to_string(), |d| d.as_str().to_string());
140170
format!("udp://{}:{}/announce", host, udp.bind_address().port())
141171
})
142-
.collect();
172+
.collect()
173+
}
143174

144-
// Separate HTTP trackers by TLS configuration and localhost status
175+
/// Build HTTP tracker information, separating by TLS and localhost status
176+
///
177+
/// Returns (`https_trackers`, `direct_trackers`, `localhost_trackers`)
178+
fn build_http_tracker_info(
179+
tracker_config: &TrackerConfig,
180+
instance_ip: IpAddr,
181+
tls_domains: &mut Vec<TlsDomainInfo>,
182+
) -> (Vec<String>, Vec<String>, Vec<LocalhostServiceInfo>) {
145183
let mut https_http_trackers = Vec::new();
146184
let mut direct_http_trackers = Vec::new();
147185
let mut localhost_http_trackers = Vec::new();
148-
let mut tls_domains = Vec::new();
149186

150187
for (index, http) in tracker_config.http_trackers().iter().enumerate() {
151188
if http.use_tls_proxy() {
152189
if let Some(domain) = http.domain() {
153190
// TLS-enabled tracker - use HTTPS domain URL
154-
// Note: localhost + TLS is rejected at config validation time,
155-
// so we don't need to check for it here
191+
// Note: localhost + TLS is rejected at config validation time
156192
https_http_trackers.push(format!("https://{}/announce", domain.as_str()));
157193
tls_domains.push(TlsDomainInfo {
158194
domain: domain.as_str().to_string(),
@@ -175,13 +211,29 @@ impl ServiceInfo {
175211
}
176212
}
177213

178-
// Build API endpoint based on TLS configuration and localhost status
179-
let api_is_localhost_only = is_localhost(&tracker_config.http_api().bind_address());
180-
let (api_endpoint, api_uses_https) = if tracker_config.http_api().use_tls_proxy() {
181-
if let Some(domain) = tracker_config.http_api().domain() {
214+
(
215+
https_http_trackers,
216+
direct_http_trackers,
217+
localhost_http_trackers,
218+
)
219+
}
220+
221+
/// Build API endpoint information
222+
///
223+
/// Returns (`endpoint_url`, `uses_https`, `is_localhost_only`)
224+
fn build_api_endpoint_info(
225+
tracker_config: &TrackerConfig,
226+
instance_ip: IpAddr,
227+
tls_domains: &mut Vec<TlsDomainInfo>,
228+
) -> (String, bool, bool) {
229+
let api = tracker_config.http_api();
230+
let is_localhost_only = is_localhost(&api.bind_address());
231+
232+
let (endpoint, uses_https) = if api.use_tls_proxy() {
233+
if let Some(domain) = api.domain() {
182234
tls_domains.push(TlsDomainInfo {
183235
domain: domain.as_str().to_string(),
184-
internal_port: tracker_config.http_api().bind_address().port(),
236+
internal_port: api.bind_address().port(),
185237
});
186238
(format!("https://{}/api", domain.as_str()), true)
187239
} else {
@@ -190,7 +242,7 @@ impl ServiceInfo {
190242
format!(
191243
"http://{}:{}/api", // DevSkim: ignore DS137138
192244
instance_ip,
193-
tracker_config.http_api().bind_address().port()
245+
api.bind_address().port()
194246
),
195247
false,
196248
)
@@ -200,13 +252,20 @@ impl ServiceInfo {
200252
format!(
201253
"http://{}:{}/api", // DevSkim: ignore DS137138
202254
instance_ip,
203-
tracker_config.http_api().bind_address().port()
255+
api.bind_address().port()
204256
),
205257
false,
206258
)
207259
};
208260

209-
// Add Grafana TLS domain if configured
261+
(endpoint, uses_https, is_localhost_only)
262+
}
263+
264+
/// Collect Grafana TLS domain if configured
265+
fn collect_grafana_tls_domain(
266+
grafana_config: Option<&GrafanaConfig>,
267+
tls_domains: &mut Vec<TlsDomainInfo>,
268+
) {
210269
if let Some(grafana) = grafana_config {
211270
if let Some(domain) = grafana.tls_domain() {
212271
tls_domains.push(TlsDomainInfo {
@@ -215,89 +274,37 @@ impl ServiceInfo {
215274
});
216275
}
217276
}
218-
219-
// Build health check URL based on TLS configuration and localhost status
220-
let health_check_is_localhost_only =
221-
is_localhost(&tracker_config.health_check_api().bind_address());
222-
let (health_check_url, health_check_uses_https) =
223-
if let Some(domain) = tracker_config.health_check_api().tls_domain() {
224-
tls_domains.push(TlsDomainInfo {
225-
domain: domain.to_string(),
226-
internal_port: tracker_config.health_check_api().bind_address().port(),
227-
});
228-
(format!("https://{domain}/health_check"), true)
229-
} else {
230-
(
231-
format!(
232-
"http://{}:{}/health_check", // DevSkim: ignore DS137138
233-
instance_ip,
234-
tracker_config.health_check_api().bind_address().port()
235-
),
236-
false,
237-
)
238-
};
239-
240-
Self::new(
241-
udp_trackers,
242-
https_http_trackers,
243-
direct_http_trackers,
244-
localhost_http_trackers,
245-
api_endpoint,
246-
api_uses_https,
247-
api_is_localhost_only,
248-
health_check_url,
249-
health_check_uses_https,
250-
health_check_is_localhost_only,
251-
tls_domains,
252-
)
253277
}
254278

255-
/// Build `ServiceInfo` from stored `ServiceEndpoints`
256-
///
257-
/// This method extracts service URLs from the runtime outputs
258-
/// that were stored when services were started.
279+
/// Build health check endpoint information
259280
///
260-
/// Note: This method is for backward compatibility with stored endpoints.
261-
/// New deployments should use `from_tracker_config` which has full TLS awareness.
262-
#[must_use]
263-
pub fn from_service_endpoints(endpoints: &ServiceEndpoints) -> Self {
264-
let udp_trackers = endpoints
265-
.udp_trackers
266-
.iter()
267-
.map(ToString::to_string)
268-
.collect();
269-
270-
// For backward compatibility, all HTTP trackers go to direct access
271-
// (stored endpoints don't have TLS or localhost information)
272-
let direct_http_trackers = endpoints
273-
.http_trackers
274-
.iter()
275-
.map(ToString::to_string)
276-
.collect();
277-
278-
let api_endpoint = endpoints
279-
.api_endpoint
280-
.as_ref()
281-
.map_or_else(String::new, ToString::to_string);
282-
283-
let health_check_url = endpoints
284-
.health_check_url
285-
.as_ref()
286-
.map_or_else(String::new, ToString::to_string);
281+
/// Returns (`url`, `uses_https`, `is_localhost_only`)
282+
fn build_health_check_info(
283+
tracker_config: &TrackerConfig,
284+
instance_ip: IpAddr,
285+
tls_domains: &mut Vec<TlsDomainInfo>,
286+
) -> (String, bool, bool) {
287+
let health_check = tracker_config.health_check_api();
288+
let is_localhost_only = is_localhost(&health_check.bind_address());
289+
290+
let (url, uses_https) = if let Some(domain) = health_check.tls_domain() {
291+
tls_domains.push(TlsDomainInfo {
292+
domain: domain.to_string(),
293+
internal_port: health_check.bind_address().port(),
294+
});
295+
(format!("https://{domain}/health_check"), true)
296+
} else {
297+
(
298+
format!(
299+
"http://{}:{}/health_check", // DevSkim: ignore DS137138
300+
instance_ip,
301+
health_check.bind_address().port()
302+
),
303+
false,
304+
)
305+
};
287306

288-
Self::new(
289-
udp_trackers,
290-
Vec::new(), // No HTTPS trackers from legacy endpoints
291-
direct_http_trackers,
292-
Vec::new(), // No localhost tracker info from legacy endpoints
293-
api_endpoint,
294-
false, // Legacy endpoints don't have TLS info
295-
false, // Legacy endpoints don't have localhost info
296-
health_check_url,
297-
false, // Legacy endpoints don't have health check TLS info
298-
false, // Legacy endpoints don't have localhost info
299-
Vec::new(), // No TLS domains from legacy endpoints
300-
)
307+
(url, uses_https, is_localhost_only)
301308
}
302309

303310
/// Returns true if any service has TLS enabled

0 commit comments

Comments
 (0)