Moves health and metrics endpoints to dedicated listener on separate port#910
Moves health and metrics endpoints to dedicated listener on separate port#910mcblake27 wants to merge 1 commit into
Conversation
…port Health and metrics endpoints were previously attached to the public API router, allowing end users to access /health, /ready, and /metrics directly, which is a potential DDoS vector. This change moves them to a dedicated listener on port 8082, separate from the public API on port 8080. Also updates relevant tests to work with new structure/naming schema. Closes openstack-experimental#719. This contribution was developed with the assistance of AI tools.
gtema
left a comment
gtema
left a comment
There was a problem hiding this comment.
I have updated the original issue with bit more details of the target setup how the new endpoint should work and combine healthz with otel metrics. Part of this change you do not need to implement OTEL service, but build an endpoint that will be further extended with it. Otherwise the change goes in the correct direction.
You need to fix commit message, formatting, and once you fix the deployment manifest the k3s test should also pass
| pub interface_internal: Option<InternalInterface>, | ||
|
|
||
| /// Server listener configuration for the internal interface. | ||
| /// Server listener configuration for the public interface. |
| } | ||
|
|
||
| /// Prometheus scrape endpoint — returns the three audit counters in text | ||
| /// exposition format (v0.0.4). No authentication required; operators are |
There was a problem hiding this comment.
this is the metrics handler that you need to merge with the health endpoint so that at the new listener we have
- /health - the health endpoint
- /metrics - prometheus metrics
In one of the next steps we would evaluate how to populate metrics endpoint with the real data.
There was a problem hiding this comment.
The way I am currently handling this is that when I define the router metrics_app in spawn_metrics_listener, I call .route("/metrics", axum::routing::get(metrics_handler)) to add this metrics handler as a route on the router. Is that satisfactory for what you're describing here, or is there a different way I am supposed to be doing this? My understanding is that that is what you are asking, but I may be misunderstanding.
| } | ||
|
|
||
| fn default_metrics_tcp_address() -> SocketAddr { | ||
| SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 8082) |
There was a problem hiding this comment.
now that you moved the health endpoint to the different address the k3s test fails since the keystone statefulset in kubernetes cannot reach it: https://github.com/openstack-experimental/keystone/blob/main/tools/k8s/keystone/base/statefulset-rs.yaml#L47C1-L72C1 - that place need to be adapted to point at the new place. Keep in mind the potential port conflict since keystone operates with many other components and systems (it does not conflict now, but any decision on changing ports is tricky - keycloak is also running on 8082, just in the different container). Let's maybe use 8099 as something less standard
2 participants
Health and metrics endpoints were previously attached to the public API router, allowing end users to access /health, /ready, and /metrics directly, which is a potential DDoS vector. This change moves them to a dedicated listener on port 8082, separate from the public API on port 8080. Also updates relevant tests to work with new structure/naming schema. Adds modular spawn_metrics_listener function to crates/keystone/src/bin/keystone.rs and API interface structure MetricsInterface in crates/config/src/interface.rs, as well as updating pathways in crates/config/src/lib.rs, crates/keystone/src/api/mod.rs, and crates/keystone/src/api/health.rs.
Closes #719.
This contribution was developed with the assistance of AI tools.