Add support for client certificates in amtool#2391
Add support for client certificates in amtool#2391Hsn723 wants to merge 3 commits intoprometheus:mainfrom
Conversation
Signed-off-by: Hsn723 <me@atelierhsn.com>
cli/root.go
Outdated
| if caFile != "" { | ||
| tc.CAFile = caFile | ||
| } | ||
| tlsConfig, err := commoncfg.NewTLSConfig(&tc) |
There was a problem hiding this comment.
I would call commoncfg.NewRoundTripperFromConfig() instead and pass the returned value to cr.Transport.
cli/root.go
Outdated
| cr.DefaultAuthentication = clientruntime.BasicAuth(amURL.User.Username(), password) | ||
| } | ||
|
|
||
| if certFile != "" && keyFile != "" { |
There was a problem hiding this comment.
It should be ok to always create and pass a custom Transport, even if TLS flags are empty.
| app.Flag("tls_config.ca_file", "Path to CA file").ExistingFileVar(&caFile) | ||
| app.Flag("tls_config.cert_file", "Path to client certificate").ExistingFileVar(&certFile) | ||
| app.Flag("tls_config.key_file", "Path to client certificate key").ExistingFileVar(&keyFile) | ||
| app.Flag("tls_config.server_name", "Server name for TLS config").StringVar(&serverName) |
There was a problem hiding this comment.
I would add tls_config.insecure_skip_verify too.
Signed-off-by: Hsn723 <me@atelierhsn.com>
Signed-off-by: Hsn723 <me@atelierhsn.com>
|
@simonpasquier It also seems I'm failing some CI tests unrelated to |
|
I suggested that we base the effort on Prometheus' protool and maybe provide a config file so we can support basic auth passwords too. |
|
In both ways, file or not, we should add the necessary helpers in common to support this use case |
|
Sounds like it could be useful. Maybe it is time to revive that PR |
|
Hm although there is which allows to configure TLS, so is that actually needed? |
|
I guess this is obsoleted by #2764 |
|
Hi @Hsn723 , it looks like this PR is out of date and has some overlap with other changes, that have been merged in the mean time. So, I'll close this. We appreciate the time you put into this. If feel like opening an up-to-date PR, that I'll make sure to review it in a timely manor. Kind regards, Solomon Jacobs |
I'd like to propose the ability to provide a client certificate in
amtoolto support use cases where an instance ofalertmanageris behind and Ingress that expects a client certificate.