User Info Fetcher: Enable TLS with WebPKI trust by default

[NickLarsenNZ]

Currently, the following configuration leads to the HTTP client connecting without TLS:

apiVersion: opa.stackable.tech/v1alpha1
kind: OpaCluster
metadata:
  name: test-opa
spec:
  clusterConfig:
    userInfo:
      backend:
        keycloak:
          hostname: keycloak.example.com
          clientCredentialsSecret: opa-infofetcher-keycloak-secret
          adminRealm: master
          userRealm: master

And to enable TLS, you have to jump through a few hoops by adding:

          tls:
            verification:
              server:
                caCert:
                  webPki: {}

In this day-in-age, I think it is expected to default to TLS (and the CRA requires secure-by-default).

So I propose that we impl Default for tls:

impl Default for TlsVerification {
    fn default() -> Self {
        Self::Server(TlsServerVerification {
            ca_cert: CaCert::WebPki {},
        })
    }
}

... and explicit steps are to be taken to disable TLS or to ignore verification (or set internal PKI), eg:

          tls: null 

or

          tls:
            verification:
              none: {}

[NickLarsenNZ]

This actually pertains to operator-rs (Tls) and has wider reaching consequences. This issue should probably be renamed, or a new one created in operator-rs and linked to this one?

[NickLarsenNZ]

Some considerations that seem relevant (about the whole ambiguity of Option::None meaning not configured or disabled):

https://github.com/cloudflare/foundations/blob/25534b33d39163b0667db10f112c7ea2f690149b/foundations/src/settings/mod.rs#L68-L90

//! There's a problem here when it comes to settings. If you want TLS to be disabled by default then
//! default value for the tls field in your config will be None, which means that possible knobs
//! for TLS would not be rendered in the default YAML config, hiding the documentation for this part
//! of functionality as well.
//!
//! Instead, the recommended approach is to avoid using Option in such situations and provide
//! an explicit enabled knob to the TlsSettings:

[NickLarsenNZ]

Refinement question:

• should this be an operator-rs change (bigger consequences)
• or drop the shared TLS struct for the user-info-fetcher so that we can quickly resolve this issue?

[fhennig]

Good question.

I think I'd prefer to have this in operator-rs, and not introduce an inconsistency. Also because I do not think that this is important or urgent enough to introduce an inconsistency.

It makes sense to me to actually do this for all TLS, so I would take this out of the UIF first release epic and instead widen the scope to "better default TLS" (in general, not just UIF) and handle it like that.

my 2cts