feat: add TLS endpoint to kepler exporter #1899

aharivel · 2024-12-20T16:21:50Z

Add TLS support via the new web configuration file, following the Prometheus Exporter Toolkit style for consistency across exporters.

Usage:

kepler --web.config.file=web-config.yml

Content of web-config.yml:

tls_server_config:
cert_file: /path/to/server.crt
key_file: /path/to/server.key

Add TLS support via the new web configuration file, following the Prometheus Exporter Toolkit style for consistency across exporters. * Usage: kepler --web.config.file=web-config.yml * Content of web-config.yml: tls_server_config: cert_file: /path/to/server.crt key_file: /path/to/server.key Signed-off-by: Anthony Harivel <[email protected]>

github-actions · 2024-12-20T16:22:53Z

🤖 SeineSailor

Here is a concise summary of the pull request changes:

Summary: This pull request adds TLS support to the Kepler exporter by introducing a new web-config.yml file for specifying TLS server certificates and keys. The main function now reads TLS configuration from the file, enabling the server to listen on an HTTPS endpoint when configured.

Key Modifications:

Added web-config.yml file for TLS configuration
Updated main function to read TLS configuration from the file
Server now supports TLS and listens on HTTPS endpoint when configured

Impact: The external interface remains unchanged, but the code behavior changes when TLS is configured, switching from HTTP to HTTPS.

Observations/Suggestions:

It would be beneficial to include tests to verify the TLS configuration and HTTPS endpoint functionality.
Consider adding documentation to explain the new TLS configuration options and their usage.
Review the error handling and logging mechanisms to ensure they are adequate for TLS-related issues.

SamYuan1990

https://prometheus.io/docs/introduction/roadmap/#tls-and-authentication-in-http-serving-endpoints I don't mind for TLS endpoint, but considering with prometheus TLS note for official exporters...

TLS and authentication in HTTP serving endpoints

TLS and authentication are currently being rolled out to the Prometheus, Alertmanager, and the official exporters. Adding this support will make it easier for people to deploy Prometheus components securely without requiring a reverse proxy to add those features externally.

at meanwhile, if we are going to enable TLS for kepler, considering in a k8s cluster with node A,B,C
what will be the hostname in cert for the TLS cert?what will be the changes for kepler pod?
as for example if we are going to use kepler.svc.nodeX.com as host name in k8s cluster, and the key for each node are different.
so, somehow will it impact/it will change our deployment yaml?
as we have to inject/put a specific node cert-key pair as configuration?

mark with change request for more information and discussion.

aharivel · 2024-12-21T10:18:47Z

Hi @SamYuan1990 ,
I wanted to clarify something regarding your comments about Prometheus official support. Could it be that there was a mix-up between the terms "rolled out" and "rules out"?

While I don't claim expertise in Kubernetes security or managing certificates in multi-node deployments, I can point out that some IaaS platforms, like Red Hat OpenStack (RHOSO), are already using node_exporter with TLS. Here are a couple of links to highlight this:

Node Exporter Configuration
Node Exporter Setup Tasks

Furthermore, they are also using Kepler, as shown here:
Kepler Integration Tasks

This leads me to my point: if someone knows how to manage their certificates effectively, Kepler should be capable of supporting TLS endpoints. Why should Kepler be the only exporter in an IaaS setup like OpenStack that does not support TLS, especially when node_exporter, ipmi_exporter, and others do?

Additionally, Kepler is handling sensitive data. For instance, consider the CVEs related to RAPL and side-channel attacks, which I’m sure you’re familiar with. Would organizations that prioritize security really allow this data to be transmitted in plaintext over the network?

Finally, I want to stress that this is an optional feature. For users content with unencrypted data, nothing changes—they can continue as before. But adding this option would present Kepler as a more robust and serious tool for those who care about security.

SamYuan1990 · 2024-12-22T12:25:03Z

Hi @SamYuan1990 , I wanted to clarify something regarding your comments about Prometheus official support. Could it be that there was a mix-up between the terms "rolled out" and "rules out"?

While I don't claim expertise in Kubernetes security or managing certificates in multi-node deployments, I can point out that some IaaS platforms, like Red Hat OpenStack (RHOSO), are already using node_exporter with TLS. Here are a couple of links to highlight this:

Node Exporter Configuration Node Exporter Setup Tasks Furthermore, they are also using Kepler, as shown here: Kepler Integration Tasks

This leads me to my point: if someone knows how to manage their certificates effectively, Kepler should be capable of supporting TLS endpoints. Why should Kepler be the only exporter in an IaaS setup like OpenStack that does not support TLS, especially when node_exporter, ipmi_exporter, and others do?

Additionally, Kepler is handling sensitive data. For instance, consider the CVEs related to RAPL and side-channel attacks, which I’m sure you’re familiar with. Would organizations that prioritize security really allow this data to be transmitted in plaintext over the network?

Finally, I want to stress that this is an optional feature. For users content with unencrypted data, nothing changes—they can continue as before. But adding this option would present Kepler as a more robust and serious tool for those who care about security.

From security point of view, yes. We can make kepler protected by TLS to improve our baseline for sensitive data protection.
Once more, this PR is a great job and I just "mark with change request for more information and discussion."

So from deployment point of view, considering with kepler community operator and kepler community helm chat as deployment approaches.

do we have issue open on kepler operator or kepler helm chat with TLS support?
are we going to integrate with cert mgr or sidecar as istio? by document supporting or we just leave it to user?
what if kepler running as a static pod?

and btw, any impacts to kepler RPM service?

from security point:

what's specific level for kepler's sensitive data? what are they?

Additionally, Kepler is handling sensitive data.

how to protect those data according to it's level? to clear/clarify our security boundary.

do we need considering a key/a cert with password?
do we recommended user to enable tls verify when enable tls?
do we need considering mtls?

for instance:
if we considering pod list is a sensitive data, and which should not listed by plaintext through network, as kubelet having TLS to protect those info. we should add TLS to protect those info as well. I suppose this is from node mgr's points of view.
if we don't want user A to see user B's pod on the kepler metric... should we considering username and password? I suppose this is from user's point of view.

from impl points of view:

is there any chance for us to reuse https://github.com/prometheus/exporter-toolkit
ref
https://github.com/prometheus/node_exporter/blob/d0c1d00d18867297326bc7b1c21de36ae50e6a92/README.mdplain=1#L383
reuse https://github.com/prometheus/exporter-toolkit/blob/master/docs/web-configuration.md
https://github.com/prometheus/exporter-toolkit/tree/master/web
https://github.com/prometheus/node_exporter/blob/d0c1d00d18867297326bc7b1c21de36ae50e6a92/go.mod#L28

aharivel · 2025-01-07T10:10:30Z

Hi @SamYuan1990,
I'm going to let @rootfs , @vimalk78 or @vprashar2929 answer / decide on all the questions regarding K8s related deployment and usage. There are definitely answer on how to manage certificates. Again Kepler is not the only container applications that need security.

However I can answer some of the following question:

Kepler is using RAPL as a source for energy metrics in order to calculate the ratio. The access of this value is restricted to root, and Kepler is not only running as root only for eBPF but also for accessing those values. There are a couple of CVE regarding the usage of this RAPL value. We can easily imagine that when Kepler is reporting pakage energy and other consumptions in clear text that side channel attack is even easier. So yes, if there are sensitive data involved in this, TLS is mandatory.
Regarding the implementation point of view: indeed we can totally reuse the Prometheus toolkit. However there are a couple of concerned about this:

"This repository is currently WIP and experimental." is literally written down the README.
The modification is not out of consequence. Instead of using http server (to run srv.ListenAndServe() ) from the standard import of "net/http", you must change to http server that goes through "github.com/prometheus/exporter-toolkit/web" (to run web.ListenAndServe() ). Same for the flag, you must change it from standard import "flag" to "github.com/prometheus/exporter-toolkit/web/kingpinflag".

With that said, I want to conclude that I don't want to take the responsibility to change the behavior of the http server (handlers, flags, etc.) unless directly specified by @rootfs and I also think that refactoring the http server is out of the context of the commit.

NB: I have use the same semantic of Prometheus exporter-toolkit for the command line usage. That means if anyone later want to move to the toolkit then the usage will be exactly the same and so backward compatible.

SamYuan1990

In this case, I will change my review to LGTM, we can have a TLS impl in short term and for long term plan, move to prometheus community solution. I will leave it to @rootfs , @vprashar2929 , @sthaha or @vimalk78 to merge this PR.
if we are going to merge current, do we need a new issue for prometheus tookit?

rootfs · 2025-01-07T14:43:14Z

this looks good to me, can @sthaha @vimalk78 take a final look? thanks

vimalk78 · 2025-01-07T15:28:11Z

cmd/exporter/exporter.go

+		var tlsServerConfig TLSServerConfig
+		decoder := yaml.NewDecoder(configFile)
+		if err := decoder.Decode(&tlsServerConfig); err != nil {
+			klog.Errorf("Error parsing config file: %v\n", err)


nit: for better debugging, add configPath in error logs

vimalk78 · 2025-01-07T15:32:22Z

Added issue for kepler-exporter for tracking
sustainable-computing-io/kepler-operator#443

SamYuan1990 requested changes Dec 21, 2024

View reviewed changes

SamYuan1990 requested review from sthaha, rootfs and SamYuan1990 December 21, 2024 01:15

SamYuan1990 approved these changes Jan 7, 2025

View reviewed changes

vimalk78 approved these changes Jan 7, 2025

View reviewed changes

vimalk78 mentioned this pull request Jan 7, 2025

Support TLS endpoint to kepler sustainable-computing-io/kepler-operator#443

Open

vimalk78 merged commit 1afff7f into sustainable-computing-io:main Jan 7, 2025
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add TLS endpoint to kepler exporter #1899

feat: add TLS endpoint to kepler exporter #1899

aharivel commented Dec 20, 2024

github-actions bot commented Dec 20, 2024 •

edited

Loading

SamYuan1990 left a comment •

edited

Loading

aharivel commented Dec 21, 2024

SamYuan1990 commented Dec 22, 2024

aharivel commented Jan 7, 2025

SamYuan1990 left a comment •

edited

Loading

rootfs commented Jan 7, 2025

vimalk78 Jan 7, 2025

vimalk78 commented Jan 7, 2025

feat: add TLS endpoint to kepler exporter #1899

feat: add TLS endpoint to kepler exporter #1899

Conversation

aharivel commented Dec 20, 2024

github-actions bot commented Dec 20, 2024 • edited Loading

SamYuan1990 left a comment • edited Loading

Choose a reason for hiding this comment

aharivel commented Dec 21, 2024

SamYuan1990 commented Dec 22, 2024

So from deployment point of view, considering with kepler community operator and kepler community helm chat as deployment approaches.

from security point:

from impl points of view:

aharivel commented Jan 7, 2025

SamYuan1990 left a comment • edited Loading

Choose a reason for hiding this comment

rootfs commented Jan 7, 2025

vimalk78 Jan 7, 2025

Choose a reason for hiding this comment

vimalk78 commented Jan 7, 2025

github-actions bot commented Dec 20, 2024 •

edited

Loading

SamYuan1990 left a comment •

edited

Loading

SamYuan1990 left a comment •

edited

Loading