(feat): Support mtls certificate based authentication

[sandeepkunusoth]

This feature enables secure, passwordless authentication for client applications connecting to Valkey by leveraging mutual TLS (mTLS) and Valkey's native Access Control Lists (ACLs).

#59 (comment)

CRD changes

spec:
  tls:
    ....
    authClients: Required        # Allowed values: Required | Optional | Disabled (Default: Disabled)
    authClientsUser: CN          # Allowed values: CN | URI | Off (Default: Off)

authClients(Required | Optional | Disabled) maps to (valkey.conf): tls-auth-clients yes (or optional / no).
authClientsUser(CN | URI | Off) maps to (valkey.conf): tls-auth-clients-user CN (or URI/Off).

[sandeepkunusoth]

should we change allowed values for authClients to yes|no|optional instead of Required | Optional | Disabled so that we can pass directly this value to tls-auth-clients in valkey.conf?

[sandeepkunusoth]

i wanted to check again on this issue.
does it make sense to introduce new feilds under spec.tls for authClients, authClientUser as in #242

or just do it through spec.config overrides.

adding this in operator spec. enables us to do validation at admission CEL. i think this is only benefit. if later we have some custom defaults set in operator i think it makes sense to expose this via spec.tls?

[melancholictheory]

dedicated fields under spec.tls is the right call, and CEL validation isn't the only reason, it's the one that matters least here.

the bigger one: turning on tls-auth-clients (Required or optional) changes what the operator itself has to do, not just external clients. the operator opens its own connections to the nodes for survey, failover, and the replication wiring, and once the server verifies client certs those connections need to present one too, or the server rejects the operator. if this is set through raw spec.config, the operator has no idea client auth was enabled, so it keeps dialing without a client cert and quietly breaks its own control path. a first-class field is the signal that lets the operator mount and present a client cert for its own connections. that can't come from a config override it doesn't parse.

the other one from running mTLS replication: trust has to be symmetric. with tls-auth-clients optional the source also verifies the replica's client cert, so it isn't enough for the replica to trust the source, the source has to trust the replica's CA too. a one-directional trust config passes the handshake from the replica side and then fails under the reverse check. worth designing the CA/trust side of these fields with that in mind (it also connects to the cross-region case in #323, where the two ends have different CAs).

on the enum: i'd lean toward the operator-facing names (Required/Optional/Disabled) over passing yes/optional/no straight through. the value maps cleanly enough, and keeping the API vocabulary stable means a future valkey rename of the directive doesn't leak into the CRD.

[jdheyburn]

@sandeepkunusoth

should we change allowed values for authClients to yes|no|optional instead of Required | Optional | Disabled so that we can pass directly this value to tls-auth-clients in valkey.conf?

I think we need to think about the abstraction that spec.networking.tls is providing. I would rather be consistent with the language that the current abstraction is providing - which I believe today is operator focused (Required|Optional|Disabled). +1 to @melancholictheory

I am fine with adding fields under spec.networking.tls, but perhaps we need think about the long term design of tls to support all the use cases that Valkey allows for (mentioned in this comment).

[daanvinken]

The TLS tree this should land on is now on main (Phase 1 of #360):

spec:
  networking:
    tls:
      certificates:
        server:
          secretName: valkey-server-tls

tls-auth-clients is still hardcoded optional. There is no client CA slot and no operator client cert.

This issue is Phase 2 of that design, not a new spec.tls object and not spec.config. @jdheyburn shape on #360:

spec:
  networking:
    tls:
      certificates:
        server:
          secretName: valkey-server-tls
        operator:
          secretName: valkey-operator-tls
      clientAuth:
        mode: Required                 # Disabled | Optional | Required
        certificateUser: CN            # Off | CN | URI
        ca:
          - secretName: valkey-client-ca

mode maps to tls-auth-clients (no / optional / yes). certificateUser maps to tls-auth-clients-user. Keep the operator-facing enum names.

A first-class field is required because Required (and Optional, when a cert is presented) makes the operator, exporter, probes, and replica links into TLS clients. A spec.config override does not mount or present a client cert, so the operator can lock itself out.

#242 still uses the old flat authClients fields and reuses the server cert as the operator client cert. It needs a rewrite onto clientAuth + certificates.operator, or a new PR for Phase 2.

Split serverCA / outbound stays Phase 3.

@sandeepkunusoth I have some bandwidth to work on this (and woud like to start using mTLS on live clusters), but don't want to step on your toes. Can I help you in drafting up a first PR or support you in other ways? Thanks in advance!

[sandeepkunusoth]

rebased PR for scope 1 there are still some comments i need to fix. will fix it soon. #242