mTLS Support for Web Metrics

[gavin-db]

Summary

Proposal: Teach argo-rollouts’ web provider to look for 2 new secrets, to configure TLS for its HTTP client:

• web-provider-client-tls, which specifies the client-side TLS key
• web-provider-truststore, which specifies the trusted CAs, for validating the server.

Details The web-provider-client-tls secret is of type kubernetes.io/tls, i.e., contains keys tls.crt and tls.key with data in DER format. This secret specifies the client’s TLS keys.

The web-provider-truststore contains one or more keys, each with a trusted CA’s certificate chain in PEM format. This secret specifies trusted CAs for validating server’s certificates.

Here’s how the secrets make their way into TLS config for the HTTP client:

clientCert, err = tls.X509KeyPair(tlsCert, tlsKey)

caCertPool := x509.NewCertPool()
caCertPool.AppendCertsFromPEM(caCert)

client := &http.Client{
    Transport: &http.Transport{
        TLSClientConfig: &tls.Config{
            Certificates: []tls.Certificate{clientCert}
            RootCAs:      caCertPool,
        },
    },
}

Questions

1. Are there any other places in the argo-rollouts API where these parameters ought to be exposed? Is there a top-level settings / configuration interface for argo-rollouts?
2. The current web provider admits a setting Insecure: true, that translates to setting InsecureSkipVerify = true in the TLS config. This makes the client skip verification of server certificates. How should web-provider-truststore interact with Insecure: true. Potential Solution: Let Insecure: true override the CA trust store.

Related work

• ArgoRollouts’ Wavefront provider accesses the necessary API tokens using a k8s secret.
• Argo-cd looks for a kubernetes secret named argocd-server-tls , to decide which certificate to use for TLS. (link). Argocd-repo-server uses an identical approach, looking for a secret named argocd-repo-server-tls. (link)

Example

apiVersion: v1
kind: Secret
metadata:
 name: web-provider-client-tls
type: kubernetes.io/tls
data:
 tls.crt: |
   <redacted> ...    
 tls.key: |
   <redacted> ... 

apiVersion: v1
kind: Secret
metadata:
 name: web-provider-truststore
data:
 ca.crt: |
   <redacted> ...

Use Cases

This would enable clients to more securely communicate with API endpoints to conduct analyses.

---

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.

[zachaller]

Are there any other places in the argo-rollouts API where these parameters ought to be exposed? Is there a top-level settings / configuration interface for argo-rollouts?

So I think I would move this into the webhook config see example below

The current web provider admits a setting Insecure: true, that translates to setting InsecureSkipVerify = true in the TLS config. This makes the client skip verification of server certificates. How should web-provider-truststore interact with Insecure: true. Potential Solution: Let Insecure: true override the CA trust store.

I think when InsecureSkipVerify is true the trust store won't be used at all even if defined but I would have to code it up to see how that behaves but agree if Insecure: true is set we should not do any verifications (bad practice but nice for testing self signed certs etc etc)

I think having some spec like this on the web provider makes sense it lets the user define where the secret lives and does not tie it to some magic name. Feel free to adjust or suggest something more appropriate here I also go back on forth on if going to to the key value would make sense or not or just have fixed "magic" key names such as tls.crt, tls.key, ca.crt etc.

metrics:
  - name: webmetric
    successCondition: result == true
    provider:
      web:
        url: "http://my-server.com/api/v1/measurement?service={{ args.service-name }}"
        timeoutSeconds: 20 # defaults to 10 seconds
        certificates:
          truststore:
            secreteRef:
              namespace: <namespace of secrete>
              name: <secrete name>
          client-tls:
            secretRef:
              namespace: <namespace of secrete>
              name: <secrete name>
        headers:
          - key: Authorization
            value: "Bearer {{ args.api-token }}"
        jsonPath: "{$.data.ok}"

another example where we go into the key level, however I lean towards first option above

        certificates:
          truststore:
            secreteKeyRef:
              namespace: <namespace of secrete>
              name: <secrete name>
              tlsCrtKey: tls.crt
              tlsPrivateKeyKey: tls.key

[zachaller]

One behavior to consider would be self serve in the instance where rollouts is running cluster wide and hence why having a namespace field on the secret ref. Example as a cluster operator I want to let my users add their own tls settings they can specify a namespace however if no namespace is specified it uses the namespace that the controller is running in.

[gavin-db]

@zachaller to clarify one point - in your implementation, it looks like you intend for the argo-controller to fetch secrets at runtime using the k8s go client library and a role w/ read permissions on secrets within a namespace.

The alternative would be to require the secrets to be mounted as volumes at controller deploy time. In this paradigm, developers would likely reference secrets by name, and would need to have configured the controller to mount the secrets ahead of time.

Is your preference for #1?

[zachaller]

I think both would be ok, with the secret mounting option I would imagine that file path would be in the config?

[github-actions[bot]]

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

[gavin-db]

We have a working implementation we have been using for some time internally - will try to find time to contribute upstream.