function61 / promswarmconnect

Bridges Docker Swarm services to Prometheus without any changes to Prometheus
https://function61.com/
Apache License 2.0
24 stars 6 forks source link

HTTPS metrics_endpoints #12

Closed treksler closed 4 years ago

treksler commented 5 years ago

Hi,

Is there any way to handle https metrics endpoints? I can specify the port and the path easily enough, but there seems to be no way to specify the protocol.

joonas-fi commented 5 years ago

There currently isn't. I run my own cluster with encrypted overlay networking, so intra-cluster traffic is encrypted anyway.

I could add support to HTTPS, but what is your scope?

Since promswarmconnect deals with IP addresses, the endpoint URLs as relabeled in Prometheus effectively end up as something like this: http://192.168.4.108:9001/metrics.

If we were to add support for https, the URL would look in Prometheus like this: https://192.168.4.108:9001/metrics

That means that when Prometheus opens TLS connection to the endpoint, it should have a valid certificate with the cert's SAN field including 192.168.4.108. Do you generate certs dynamically for your containers when they are started by Docker?

Or are you using insecure_skip_verify? That lessens the usefulness of TLS, since now an attacker can MITM.

I'm unfamiliar with Prometheus's TLS config, does the server_name option mean that the Prometheus <-> endpoint connection is considered valid if server_name is in endpoint cert's SAN field? If so and that's what you are using, then I think it could be a justified idea to add this support.

joonas-fi commented 4 years ago

I am open to implementing https, but I need better understanding on how https endpoints are implemented in practice.

If I don't get an answer I'll close due to inactivity. Happy to reopen though if someone can help me understand the requirements.

ainsey11 commented 4 years ago

I can see how this works, but in practice will be quite hard to do in promswarmconnect, If you're using custom CA's then they will need to be mounted into the container, and then each service will probably need an extra param to identify them as https endpoints

so your container config would look like this:


    image: fn61/promswarmconnect:20190309_1131_dda6bb23
    volumes:
      - /var/run/docker.sock:/var/run/docker.sock
      - /mnt/Data/CA/CA.crt:/etc/CA.crt
    environment:
      - DOCKER_URL=unix:///var/run/docker.sock
      - NETWORK_NAME=stackname_default
    deploy:
      placement:
        constraints: [node.hostname == catsareawesome4] ```

and then your service 

```  nats_monitoring:
    image: ainsey11/nats_prometheus_exporter
    environment:
      - METRICS_ENDPOINT=:7777/metrics
      - METRICS_HTTPS=true
    ports:
      - 7777:7777
    command: ["-varz", "-connz", "-routez", "-subz", "http://nats:8222"] 
    deploy:
      placement:
        constraints: [node.hostname == potatoesarethebest]```
joonas-fi commented 4 years ago

Thanks @ainsey11 for your input. :)

There is also "v2" syntax available, which we maybe could extend to look like METRICS_ENDPOINT:7777/metrics,https=true.

However, the syntax is not the problem. The problem is that before understanding the use cases for https I don't see myself investing the time if nobody will benefit from this. I'm not saying there aren't use cases, but I'd need to understand more first.

Like @ainsey11 said, you'd need the CA in promswarmconnect. You'd also need server certs in each container-to-monitor, which might need to be dynamically generated if you'd want the server certs to actually be valid for the IP we're connecting to.

I don't think adding insecureSkipValidate TLS config would be wise since that kinda defeats the purpose of having encryption.

I changed my mind about closing this issue. We can keep it open. If anybody has any input, go ahead :)

joonas-fi commented 4 years ago

I found myself needing this, because I have an https-only endpoint (even though it's behind a loadbalancer)

I implemented the support. Currently it only works if your port is 443, e.g. you have METRICS_ENDPOINT=:443/metrics.

These changes were made to Prometheus configuration:

Example is visible here https://github.com/function61/prometheus-conf/blob/e5c70f03674a87f1b1ffed9555f8386a7a84b758/prometheus.yml#L14 (same link as from README)