Moves health and metrics endpoints to dedicated listener on separate port#910
Conversation
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
a0d979d to
e56b4f2
Compare
| /// Keystone health/metrics API interface. | ||
| #[derive(Debug, Deserialize, Clone)] | ||
| pub struct MetricsInterface { | ||
| /// Default address for the health/metrics endpoint. Defaults to `0.0.0.0:8082`. |
There was a problem hiding this comment.
the docstring still says 8082 while the code does 8099
| name: keystone-raft | ||
| protocol: TCP | ||
| - containerPort: 8099 | ||
| name: keystone-metrics |
There was a problem hiding this comment.
as the k3s job log says kubernetes has length limitations (15 chars). Let's just use "metrics" here
87fb297 to
7d7d932
Compare
|
@mcblake27 please sync your fork repository so that it can be rebased solving the merge conflict |
e7c325b to
6b011fd
Compare
|
@gtema Apologies, was having rebasing issues on my end but I believe it is okay now, let me know if there are still issues |
|
@mcblake27 format failure is genuine |
c3efc75 to
ae539ee
Compare
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 moves them to a dedicated listener on port 8099, separate from the public API on port 8080. Adds a new config section to configure the address. Updates the k8s StatefulSet manifest to point liveness, readiness, and startup probes at the new port. Co-developed-by: Claude (Anthropic) Closes openstack-experimental#719 Signed-off-by: Milo Blake <mcblake@andrew.cmu.edu>
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 moves them to a dedicated listener on port 8099, separate from the public API on port 8080. Adds a new `[interface_metrics]` config section to configure the address. Updates the k8s StatefulSet manifest to point liveness, readiness, and startup probes at the new port. Co-developed-by: Claude (Anthropic) Closes openstack-experimental#719 Signed-off-by: Milo Blake <mcblake@andrew.cmu.edu>
Signed-off-by: Milo Blake <mcblake@andrew.cmu.edu>
Signed-off-by: Milo Blake <mcblake@andrew.cmu.edu>
…port (openstack-experimental#910) * feat: Move health and metrics to dedicated 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 moves them to a dedicated listener on port 8099, separate from the public API on port 8080. Adds a new config section to configure the address. Updates the k8s StatefulSet manifest to point liveness, readiness, and startup probes at the new port. Co-developed-by: Claude (Anthropic) Closes openstack-experimental#719 Signed-off-by: Milo Blake <mcblake@andrew.cmu.edu> * feat: Move health and metrics to dedicated 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 moves them to a dedicated listener on port 8099, separate from the public API on port 8080. Adds a new `[interface_metrics]` config section to configure the address. Updates the k8s StatefulSet manifest to point liveness, readiness, and startup probes at the new port. Co-developed-by: Claude (Anthropic) Closes openstack-experimental#719 Signed-off-by: Milo Blake <mcblake@andrew.cmu.edu> * Fix: Apply rustfmt formatting Signed-off-by: Milo Blake <mcblake@andrew.cmu.edu> * fix: Updated URL port in start-api.sh to 8099 Signed-off-by: Milo Blake <mcblake@andrew.cmu.edu> --------- Signed-off-by: Milo Blake <mcblake@andrew.cmu.edu>
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.