argoproj / argo-rollouts

Progressive Delivery for Kubernetes
https://argo-rollouts.readthedocs.io/
Apache License 2.0
2.57k stars 812 forks source link

Prometheus metrics should support self-signed certs. #2298

Open todaywasawesome opened 1 year ago

todaywasawesome commented 1 year ago

Many metrics providers that are locally hosted on the same Kubernetes cluster use internal, self-signed certs. This can throw errors when trying to use the available https endpoints. We should add support for self-signed certs.

https://github.com/argoproj/argo-rollouts/blob/4d8fbd151e10bb7c08fe6a573f65857cb91ec330/metricproviders/prometheus/prometheus.go#L146-L173

dratler commented 1 year ago

Hi @todaywasawesome , Is this still open for the grab ?

dratler commented 1 year ago

Also I think using self sign certificate will be good for local only unless some 3rd party can use it for wrong

christianh814 commented 1 year ago

I imagine the spec could look like this (Similar to the Web provider)

apiVersion: argoproj.io/v1alpha1
kind: AnalysisTemplate
metadata:
  name: testing
spec:
  args:
  - name: service-name
  - api-token
  - ca-cert
  metrics:
  - name: success-rate
    interval: 5m
    successCondition: len(result) == 0 || result[0] >= 0.95
    failureLimit: 3
    provider:
      prometheus:
        address: https://prometheus.example.com:9090
        caCert: "{{ args.ca-cert }}"
        headers: 
          - key: Authorization
            value: "Bearer {{ args.api-token }}"
        query: sum(irate(istio_requests_total{reporter="source",destination_service=~"{{args.service-name}}",response_code!~"5.*"}[5m]))

Could also work with basic auth too

apiVersion: argoproj.io/v1alpha1
kind: AnalysisTemplate
metadata:
  name: testing
spec:
  args:
  - name: service-name
  - api-token
  - ca-cert
  metrics:
  - name: success-rate
    interval: 5m
    successCondition: len(result) == 0 || result[0] >= 0.95
    failureLimit: 3
    provider:
      prometheus:
        address: https://prometheus.example.com:9090
        caCert: "{{ args.ca-cert }}"
        headers: 
          - key: Authorization
            value: "Basic {{ args.username-and-password }}"
        query: sum(irate(istio_requests_total{reporter="source",destination_service=~"{{args.service-name}}",response_code!~"5.*"}[5m]))
github-actions[bot] commented 1 year ago

This issue is stale because it has been open 60 days with no activity.

gnunn1 commented 7 months ago

With the prometheus provider now supporting insecure:true is this issue still valid? Testing v1.6.0 with OpenShift's monitoring stack it looks like everything is working fine with insecure:true and setting the Authorization header to a bearer token.