Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metrics endpoint to support TLS #442

Closed
senthilkumarkj opened this issue Oct 18, 2019 · 20 comments
Closed

Metrics endpoint to support TLS #442

senthilkumarkj opened this issue Oct 18, 2019 · 20 comments

Comments

@senthilkumarkj
Copy link

Hello! We've some requirements to expose metrics in an TLS endpoint. Simple HttpServer added this constructor in 0.7.0 to allow https server to be passed.

If we add conf to specify keystore and other related configs, we can create an HttpsServer in JavaAgent.

I've made it work here - senthilkumarkj#1

However, I'm not sure what's the best way to add the new configs for the server. Currently server related conf (host and port) are part of options itself.

I've a couple of proposals.

  1. Add the new configs (such as TLSEnabled, Keystore path, password etc) in the same config file separated by yaml directive --- and config names prefixed with "server" like
---
serverTLSEnabled: true
serverKeyStorePath: <path>
serverKeyStorePassword: secret

But the problem is config file is parsed in collector only. We may need to parse the same file twice one in agent and one in collector.

  1. Add a new server config file. This is what I've done in my CL. But I need to make sure if the server config isn't given, agent should still work and shouldn't complain to be backward compatible.

Please let me know your thoughts on this. Thanks!

@brian-brazil
Copy link
Contributor

For now I'd suggest keeping it working in a local version, and then follow how we do it in Go once that's settled down.

@rolanddb
Copy link

rolanddb commented Dec 7, 2020

Hi, was hoping to expose an HTTPS endpoint. Is there any chance of getting @senthilkumarkj 's changes merged?

@brian-brazil
Copy link
Contributor

This is not something to be figured out in this repo, it's for client_java which this would then pull in. The version of Java we support combined with the features such a thing should offer to provide similar functionality to the Go version makes this challenging, but I'm open to ideas as to how to go about it.

@rolanddb
Copy link

rolanddb commented Dec 7, 2020

@brian-brazil Clear, thanks for the quick response. I didn't find a related issue in the client_java repo. Shall I open an issue there?

I'm not sure what the challenges are regarding Java versions. My understanding was that senthilkumarkj already has it working, and that it was a matter of deciding how the configuration should be laid out.

@brian-brazil
Copy link
Contributor

Different versions of Java support different TLS protocol versions, and later ones also add new methods which we'd want to be using. Basically all the TLS bits of https://github.com/prometheus/exporter-toolkit/tree/master/https#sample-config need to be supported as far as is possible using the same config file, including the automatic cert reloading.

@al-bar
Copy link

al-bar commented Dec 10, 2020

Hi all, I'd like to add to this discussion the request to consider adding also authentication to TLS support.
The only solution I've found at the moment to fulfill the requirement to enrcrypt and authenticate the exposed metrics is to add a proxy in front of the jmx_exporter http interface to authenticate and add TLS, but I definitely need a lighter solution.

@brian-brazil
Copy link
Contributor

That would be part of the above.

@nagukothapalli
Copy link

nagukothapalli commented Jun 30, 2021

any further updates on this, please. we do have the same requirement to enable the MSSL on JMX metric port

@giondo
Copy link

giondo commented Aug 5, 2021

+1 waiting for ssl implementation

@dhoard
Copy link
Collaborator

dhoard commented Aug 13, 2021

I just created a next-release branch PR ( prometheus/client_java#683 ) to add HTTP authentication into HTTPServer. If it's accepted/merged it would easily allow basic HTTP authentication into jmx-exporter.

@fstab
Copy link
Member

fstab commented Aug 13, 2021

Just to be clear: The PR mentioned above adds basic HTTP authentication. It does not add SSL support.

@dhoard
Copy link
Collaborator

dhoard commented Aug 13, 2021

Just to be clear: The PR mentioned above adds basic HTTP authentication. It does not add SSL support.

Correct

@dhoard
Copy link
Collaborator

dhoard commented Dec 15, 2021

Given the upcoming merge of prometheus/client_java#695, I wanted to start the conversation around the configuration parameters for TLS/SSL support.

jmx_exporter currently requires client TLS/SSL for JMX to be configured.

-Djavax.net.ssl.keyStore=/home/user/.keystore
-Djavax.net.ssl.keyStorePassword=changeit
-Djavax.net.ssl.trustStore=/home/user/.truststore
-Djavax.net.ssl.trustStorePassword=changeit

We could reuse the approach and add a new yaml configuration value sslAlias to be used as a configuration flag as well as to define the certificate name to be used for the HTTPServer.

This would allow for a single keystore/truststore to manage.


An alternate approach would be to use specifically defined yaml configuration values for TLS/SSL support. (i.e. not use the Java system properties.)

This would allow independent management of the keystore/truststore used by the HTTPServer.


Thoughts on the configuration approach/parameters?

@fstab
Copy link
Member

fstab commented Dec 15, 2021

See also #664.

@suyuyi
Copy link

suyuyi commented Jul 18, 2022

@dhoard Hi, is there any progress in supporting TLS for JMX exporter? I want to use keyStore/trustStore to implement exposing metrics to Prometheus in TLS.
And I am a little confused about discussion after reading #630 , #664 .
As I know, now I can config SSL/TLS in jmx_exporter, but it's not used to expose metric to Prometheus, it's used to connect to other remote Server which use it.
The only way I can expose metrcs in TLS/SSL now, is to use Nginx as a proxy, am I right?

@dhoard
Copy link
Collaborator

dhoard commented Jul 20, 2022

@suyuyi Currently, using a reverse proxy with TLS/SSL is required. I have worked on some of the initial work for the feature, but I’m currently focused on other things, so haven’t revisited it.

@YitongSun
Copy link

We are currently use Prometheus to expose metrics, is there any plan to support TLS for JMX exporter? thanks

@dhoard
Copy link
Collaborator

dhoard commented Jan 8, 2023

@senthilkumarkj @suyuyi @rolanddb @giondo @nagukothapalli I have been working on the initial code to support HTTPS and HTTP Basic authentication.

A potential pre-release version of the code can be found at...

https://github.com/dhoard/jmx_exporter/releases/tag/jmx_exporter_enhanced-snapshot.

Due to the vast number of changes, it will take time to be merged and released.

@dhoard
Copy link
Collaborator

dhoard commented Jan 26, 2023

See #688

@dhoard
Copy link
Collaborator

dhoard commented Jun 30, 2023

Resolved in release 0.19.0.

@dhoard dhoard closed this as completed Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants