feat(mariadb): add TLS support via cert-manager - #2680

Uninstall removes the cert-manager Secrets now, but turning managed TLS
off while the instance keeps running does not: the post-delete hook does
not run on an upgrade, and cert-manager does not own what it issued. The
note said the Secrets stay behind without saying what to do about it.

Not automating this on upgrade is deliberate. A hook that deletes Secrets
on every reconcile, including for instances that never had TLS, is a
worse thing to get wrong than an inert Secret that no tenant grant
exposes and nothing reads.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

The guard tested only readability, so a present-but-empty ca.crt passed
it and the run continued into exactly the unnamed client error the guard
was added to replace. Empty is reachable rather than theoretical: the
operator omits expired certificates when it assembles the bundle, so a CA
set that has fully expired leaves the file in place with nothing in it.

Check size as well, and say "missing or empty" so the message matches
both cases. Verified by running the rendered script against an absent and
an empty file.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

The comments described the CA-extraction controller in the present tense,
as something that copies the certificate today. It is not in this repo:
the controller, the spec.caCert field it reads, and the tenant-ca label
all arrive with a separate change, and against the current API the block
is pruned on apply. A reader would have gone looking for a controller
that is not there.

State that it is inert until that lands, why it is declared now anyway,
and that the tenant reaches the anchor meanwhile through the plain name
grant beside it, which needs nothing new.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

…rsion

The text said --ssl-ca alone leaves the hostname unchecked, without
saying for which client. Measured both: the 10.11 client the backup image
ships defaults --ssl-verify-server-cert to off, the 11.x client defaults
it on. The sentence was therefore right for one and wrong for the other,
in the paragraph a reader uses to decide whether their connection is
verified.

Also record why the tenant reaches the anchor through a Role here rather
than through the shared trust-anchor helper: that helper renders the
Secret from a CA supplied as a template value, and this CA only exists at
runtime, so the chart has no PEM to give it.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

The kindIs "invalid" guard on required had no coverage, and writing one
showed why: the schema types the field as a boolean and rejects null
before the template runs, so the guard is defensive rather than a live
path. Assert the rejection instead of inventing a rendering that cannot
happen, which also catches the schema being relaxed later.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

… operator

The comment said the operator offers no way to extend its SAN list, as a
standing fact. It is only true of the version pinned here: upstream has
since added serverCertAdditionalNames, which its SAN builder appends. Left
as an absolute, the next operator bump would inherit the workaround
without anyone reconsidering it.

Say which version the limitation belongs to and what to check at the next
bump, since that is the point where handing the leaf back to the operator
becomes possible and most of this template can go.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

The note said the hostname check is off on 10.11 and on across 11.x. The
default actually changed in 11.4. Measured all four versions this chart
offers: 10.6 and 10.11 leave it off, 11.4 and 11.8 enable it. Name the
versions rather than the series, since the reader is choosing between
exactly those.

Scope the key-bearing-Secret guard to the secrets block as well. It
scanned the whole file, so a Service whose name ends in -tls would have
tripped it while a Secret entry with a trailing comment slipped past —
the wrong answer in both directions for the check that decides whether a
private key can reach a tenant.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

…teardown

Two claims of mine were wrong about mechanism.

The empty-selector warning said such a selector would project every Secret
in the namespace. The lineage webhook computes those labels only for
objects whose ownership resolves to the application, so the reach is the
instance's own Secrets. Still the thing to prevent — that set contains
-ca-tls and -tls — but the stated reason should match the code, since it
is what a reader checks the guard against.

The teardown step told the user to delete both Secrets with no ordering.
The server keypair stays mounted in the running pods until the operator
reissues from its own CA and rolls the StatefulSet, so following that
literally leaves the next pod unable to start. Wait for the rollout
first.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

…ender

These strings flow into the ApplicationDefinition schema and become the
help text beside the fields in the console. They had grown into
paragraphs of rationale — 704 and 613 characters against 48 for the
neighbouring external — which is the wrong length for a form label and
duplicated what the README already explains at the right length.

Say what the field does and that it is opt-in; the reasoning stays in the
README, which is where a reader goes for it.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

Four claims of mine were broader than the thing they described.

The bats header said it pins the invariants deciding whether a private
key can reach a tenant. Three of its tests touch the declaration that the
API does not accept yet, so they pin an intended spelling and nothing
more; if the field lands under another name they stay green while the
feature does nothing. Say which tests are live and which are placeholders.

The certificate comment said the operator skips reconciliation entirely.
It skips issuance, which is why SANs go unvalidated, but it still reads
the Secret for the pod annotation hash and status.

The README mapped the client's verification default onto the version
enum, which selects the server image. The default belongs to the client
library, so an 11.4 client verifies against a 10.6 server.

The backup guard is described as catching a missing or unusable bundle.
On a fresh managed-TLS install the bundle can be present and non-empty
while holding only the operator's client CA, and the failure lands at
verification instead. Note the window, and note in the README that a new
instance stays down until cert-manager issues.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

tls.enabled read as a switch and was not one — TLS is served under every
value, and what the field actually chose was which authority issues the
server certificate. The same key means on/off in other charts here, so a
tenant carrying that habit across would have read this exactly backwards.

Replace it with tls.issuer: operator | cert-manager, defaulting to
operator. The value now states the behaviour instead of needing prose to
undo the name: operator is what runs today, so the fact that nothing
changes on upgrade is visible in the default rather than argued in a
comment. Being an enum, a typo is rejected by the schema, which a bool
could not do.

The explanation the old name required is gone with it — the helper
docblock and the CR comment lose the paragraphs that existed only to say
this was not a switch, and the README states the split in a table.

Not marked breaking: the field is introduced by this series and does not
exist on the target branch, so there is no released spelling to migrate
from. Breaking is measured against what is merged, not against the
intermediate history of the branch.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

…n exclude

The include selector matches by label and carries no name constraint, so
any Secret of this instance that acquires the tenant-ca label is promoted
to the tenant with its data copied verbatim. The label on the source is
policy-guarded; the one on the destination is not. Exclude is evaluated
first and short-circuits, so naming the dangerous Secrets closes the part
of that gap we can enumerate.

Listed by group with the reason each is dangerous: the private keys from
both issuers, the internal database credentials, the backup S3 keys and
restic password, and the registry pull secret.

The guarantee is only as wide as the list. Matching is by exact name with
no globs, so a Secret added by a future operator version, or one created
transiently during a rotation, is not covered — the protection that holds
in general is still that only the extraction controller writes the label.
mariadb-operator has no rotation-snapshot Secret of the kind that makes
this worse elsewhere; its Secret names are fixed.

The existing guard against a key-bearing name is rescoped to
secrets.include while here, since such a name under exclude is the
backstop working rather than a leak.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

The note explaining why this chart spells the field `tls.issuer` called
the other charts' switch `tls.issuer` as well, which inverted it: it read
as though those charts use this same key to mean on/off. They spell it
`tls.enabled`.

Restore that name and hedge the claim to "some other charts". The key is
not uniformly a switch elsewhere either — one chart uses it to select
whether a hostname is added to a certificate its engine serves regardless
— so an unqualified "other charts" overstates how settled the meaning is.

Link the epic tracking the convergence of the TLS surface, so the
divergence the note describes points somewhere instead of only being
asserted.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

… claim

Renaming the field left the suite describing the design it replaced:
titles about keeping TLS off, comments about a tri-state falling through
to external inheritance, one case named for a boolean value the enum no
longer has. The suite is where the contract is read, so it was still
teaching the old one.

The migration note also claimed the backup client's new verification is a
live change to running jobs. It is not: that CronJob reads the root
password from a Secret name the chart does not render, so it cannot start
in the first place. Say that, and point at the supported backup flow
instead of implying the deprecated one works.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

The awk that was supposed to read secrets.include reset on any two-space
key and then re-entered on the next four-space include:, so it swept
services.include as well. It passes today only because no Service happens
to end in -tls — the exact case the comment promised it handled would
have misfired on a Service name. The exclude reader had the same shape.

Gate both on the secrets: section header instead of on indentation alone.
Verified in both directions: a Service named -tls no longer trips the
guard, and a Secret named -tls under secrets.include still does.

Also correct which CA the backup client verifies against: the bundle
carries the chart's CA too under the cert-manager issuer, which is what
makes the unconditional --ssl-ca right in both configurations.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

The eight cases covering the backup client's TLS flags read as coverage
of a live path. They are not: that CronJob cannot start, because it reads
the root password from a Secret name and key the chart does not render,
and the mismatch predates this branch.

Keep the cases — the flag defect was real, and they are what will stop
--ssl-mode returning whenever the job is repaired or dropped — but say in
the header what they actually pin, so the suite does not quietly claim a
path it never exercises.

Signed-off-by: Aleksei Sviridkin <f@lex.la>

…ines

The CA source was spelled `{{ .release }}-ca-bundle`. The webhook builds
the only context these templates are rendered against, and it carries
kind, name and namespace — no release. The template resolves to nothing,
so the name would come out as `-ca-bundle` and match no Secret.

The exclude entry a few lines down already names the same Secret the way
that works, `mariadb-{{ .name }}-ca-bundle`: the release prefix is a
literal and the instance name is the templated part. Spell the source the
same. One Secret described two ways in one file meant one of them was
wrong, and it was this one.

The declaration is inert until the extraction controller lands, so this
changes nothing that runs today. That is also why it could sit here
unnoticed: the test pinned the broken spelling and passed on it, which
would have made the name wrong exactly when it first started being read.
Pin the working spelling instead, and add a guard that no undefined key
is templated into a Secret name.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

…tion

The exclude backstop enumerated the internal credential Secrets but
omitted `-metrics-config`. The operator writes it for every instance —
`MetricsConfigSecretKeyRef` at the pinned 25.10.2 fixes the name as
`<name>-metrics-config` — and its `exporter.cnf` carries

    [client]
    user=...
    password=...

in plaintext. The exporter user is granted SELECT, PROCESS, REPLICATION
CLIENT, REPLICA MONITOR and SLAVE MONITOR on `*.*`, so that password
reads every table in every database.

It is not a hypothetical Secret: the chart sets `metrics.enabled: true`
unconditionally, so it exists on every ready instance. The include side
selects by label with no name constraint, and a nil `resourceNames`
matches everything, so the exclude list is what keeps it away from the
tenant. Exclude beats include, so naming it there is sufficient.

This is inside the enumerable part of the gap, not the part the comment
already disclaims: that carve-out covers Secrets a future operator adds
or creates mid-rotation, and this one exists today under a stable name.

Also complete a sentence in the suite that lost the words naming which
issuer it describes.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

Both comments justify a security decision, and both overstated it.

The exclude list described `-ca`, `-server-cert` and `-client-cert` as
what the operator creates "when it issues its own" certificates. Only
`-server-cert` is conditional. The chart never sets the client refs, so
at the pinned 25.10.2 `WithCA`/`WithCert` gate on those refs being nil
and the operator issues the client leg — and `-ca`, which carries the CA
private key — under both issuers. The list was already right; the
comment invited a future reader to prune two live private-key names as
inapplicable under cert-manager.

The watch label was described as the difference between picking up a
cert-manager renewal and waiting for the next full resync. The operator
requeues every five minutes whenever TLS is on, so the label buys
promptness, not correctness. Say that, and note the operator sets the
same label on Certificates it builds itself, which is why the chart
mirrors it here.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

The CA source name was rewritten to a literal release prefix on the
premise that no release variable exists in the template context. That
premise came from the lineage webhook, which renders a different field:
resourceNames in the secrets selectors, where the context really is only
kind, name and namespace.

caCert.sourceSecretName is rendered by the CA-extraction controller
instead, and that one supplies a release variable explicitly. It also
parses with missingkey=error, so an undefined key would fail loudly
rather than resolve to an empty string — the silent collapse the rewrite
was meant to prevent could not have happened.

Restore `{{ .release }}-ca-bundle`. It is also the better spelling on its
own: the literal hardcodes an assumption about the release prefix and
would point at a Secret that does not exist if that prefix ever changed,
while the variable is the release name by construction. The reference
implementation for this field spells it the same way.

Drop the guard that forbade the variable. The controller's own tests pin
that spelling as correct, so the guard would have started failing the
canonical form the moment the field became live.

Keep the test pinning the source name, and say in it which renderer this
field belongs to, so the two contexts are not conflated again.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

The guard asserting that the trust anchor is surfaced to the tenant
grepped the whole file. The bundle is also named in a comment and in the
caCert declaration, so either of those could satisfy it while the
secrets.include entry it exists to protect was gone. Scope it to the
include block, the way the sibling key-bearing-Secret guard already does.
Verified by deleting the include entry with a matching string left
elsewhere in the file: the guard now fails, where before it passed.

The suite header described a comment as stating that spec.tls.issuer is
true. There is no such field on the MariaDB CR — issuer is a chart value,
and what the chart renders is spec.tls.enabled, which is what the rest of
that sentence already describes. Name the field the assertions actually
check.

Also drop the claim that an 11.4 client verifies by default. The default
that changed in 11.4 is the --ssl-verify-server-cert flag, not whether
verification happens: without --ssl-ca that client has no trust anchor,
and falls back to an anti-tampering check derived from the password hash
which establishes neither the chain nor the hostname, and which cannot be
derived at all for a passwordless user. Passing both flags is what gives
chain and hostname verification, on either client version. The documented
command was already correct; the sentence around it invited dropping the
flag that carries it.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

The backstop guards grepped the whole file for each name. Both lists sit
at the same indentation, so an entry in `include` satisfied a guard that
exists to prove the name is in `exclude` — and moving a name between them
is the single most dangerous edit this file allows, since exclude is what
keeps a Secret away from the tenant.

Moving `-ca`, the operator's CA private key, from exclude to include left
all ten guards green. So did moving `-root`. Extract the exclude block and
match inside it; both mutations now fail on the name that moved, and the
suite is unchanged otherwise.

Also correct two descriptions the TLS work outdated. The cleanup hook's
Role comment still said the rule covered two operator-generated Secrets;
it now covers four, and two of them are issued by cert-manager, which the
block above already explains. The runtime message called them all
operator-generated for the same reason.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

Three assertions matched text that survived the mutation they exist to
catch.

The backup CA guard was pinned by three separate substrings: the two test
expressions and the message. Flipping its exit status to 0, or short
circuiting the condition with `false &&`, leaves all three intact and the
suite green — while the backup runs on with --ssl-ca pointing at a file
that is missing or empty, and in the exit-0 case reports success having
dumped nothing. Match the whole construct through its exit instead.

The exclude entries were checked by name without checking they still sit
under resourceNames. A nil resourceNames matches every name, so dropping
that key turns the backstop into a match-everything selector while each
name is still present in the block. Assert the key.

The leaf certificate pinned renewBefore but not duration. cert-manager
rejects a duration that does not exceed renewBefore, so pinning one of
the pair lets the other drift into a certificate that renders but cannot
be applied.

Each is verified by the mutation it was blind to: exit 1 to exit 0, a
short-circuited condition, a renamed resourceNames key, and an 8760h
duration shrunk to 1h all now fail the suite.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

…ledger

The backup CA case is called "checks the CA file exists and names it" but
only guarded the first half. Deleting the Secret name from the error left
the suite green, so the reason MYSQL_CA_SECRET is passed to the CronJob at
all — telling the operator which Secret to go look at, since the client
reports a missing CA without naming a file or its source — was unpinned.

The cozyrds suite header also counted its own coverage wrong. It claimed
two live tests and three placeholders; there are seven and four. The four
exclude guards are live and went unmentioned, and the case reading the
source key is inert like its neighbours but carried no placeholder marker.
That header is the file's honest-coverage statement, so an understated
ledger is the one error it cannot afford.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>

…pply

The rejection happens at the API server's field manager under Flux v2.8's
Server-Side Apply, not client-side in helm-controller before the request is
sent — the run's own error carries an ErrStatus with code 500, so the request
did arrive. Under v2.7's client-side patch the undeclared field was dropped
silently and the block was genuinely inert, which is where the earlier wording
came from; v2.8 turned the same shape into a failed apply.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>