feat(mongodb): add TLS support via PSMDB operator-managed certificates - #2692

Signed-off-by: Arsolitt <[email protected]>
Assisted-By: Claude <[email protected]>

Address review feedback from lexfrei (B3): the tri-state tls.enabled field
could not actually disable TLS. The PSMDB operator defaults spec.tls to
preferTLS whenever it is unset, so tls.enabled: false rendered no spec.tls
while the operator still served TLS — a misleading no-op.

Remove the field and unconditionally render spec.secrets.ssl and
spec.tls.mode: preferTLS to match operator behaviour. splitHorizons SAN
injection now keys off external alone. Also drop the Makefile jq patch that
re-injected the tls schema property.

Assisted-By: Claude <[email protected]>
Signed-off-by: Arsolitt <[email protected]>

The tenant-facing paths handed over both Secrets the PSMDB operator creates
for TLS. <release>-ca-cert holds the CA private key, so read access is a full
trust-chain compromise; <release>-ssl holds the server private key, so read
access allows impersonation. A client verifying the endpoint needs ca.crt and
nothing else, so neither object belongs on a tenant-facing path.

Declare the trust anchor by name instead. The operator creates its CA Secret
itself and offers no way to label it, so the ApplicationDefinition names it and
the CA-extraction controller lifts ca.crt alone into the key-free projection.
The include selector matches that projection by label, and the dashboard Role
drops both key-bearing names without gaining the projection by name: a name
grant conveys whatever occupies the name, while the label path conveys only
what the platform vouched for.

Witness both delivery paths against a running cluster, since the two that
matter are separate. The Role is asserted as an exact grant with bounded rule
counts, because a rule with no resourceNames — or one granting every resource
— would otherwise pass an absence check while conveying every Secret in the
namespace. The label path is asserted as a negative on all three key-bearing
Secrets. A bats check pins the declaration's names and keys, which nothing else
covers: the field is pruned at admission until the controller ships, so drift
there would surface as an empty trust anchor with nothing red.

Drop the mongodb-rd unit test. That package has no test target, so the file
never ran, and what it asserted was the leak.

Assisted-By: Claude <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>

With no external hostname to advertise — zero replicas, or a tenant host the
controller has not populated yet — the horizons range produced nothing and the
key was emitted with a null value. The CRD types it as an object without
nullable, so the API server rejected the whole PerconaServerMongoDB and the
HelmRelease with it. The values schema permits zero replicas, so this was
reachable. Guard on both and omit the key entirely.

Pin the surrounding invariants, which were asserted in comments and enforced
nowhere. The chart must render no cert-manager objects of its own, since the
operator owns the chain and anything rendered here would be orphaned while
looking like it participated. It must set neither of the two fields that
switch TLS off at defaulting time while the rendered mode still reads
preferTLS. And the external-access case now supplies a tenant host, so it
isolates the flag it names rather than passing on the host guard.

Correct the header comment, which fused two false claims: that setting
spec.secrets.ssl is what keeps TLS on, and that it is what engages the
cert-manager integration. The operator defaults that name unconditionally and
runs the cert-manager path regardless.

Assisted-By: Claude <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>

The unit-test runner discovers charts by probing for a test target, and this
chart had none — so its suites had never run in CI.

Assisted-By: Claude <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>

…impact

The TLS notes made several claims the operator's own behaviour contradicts.
The CA is issued for a year and renewed about a month early, not for a decade
with rotation disabled — so a bundle copied once stops verifying, and the
retrieval steps now say to re-read it. Sharded mongos SANs are complete,
because the operator emits them for every topology; the real gap is that no SAN
covers the single external LoadBalancer fronting mongos, so external hostname
verification is unsupported there. preferTLS is the operator's own default
rather than a posture adopted to accommodate backups, which run under the same
certificates. External access in replica set mode is one LoadBalancer per
member with driver-side primary discovery, not a single load-balanced
endpoint — the old description contradicted the external SAN note. And the
posture section named only one of its two inherited weaknesses: unverified
clients are accepted as well as plaintext ones.

The lifecycle section was wrong in both directions. The operator owns its TLS
Secrets and its cert-manager objects through the CR, so they are collected on
uninstall rather than left behind. The rotation snapshots are the opposite of
what was written: the operator deletes them itself once the merged CA has
landed, and reads the previous CA back out of them while a rotation is in
flight — so the advice to delete them by hand could destroy the only copy of
the old CA. Only an interrupted rotation strands them.

Document what an upgrade actually does to a release already running with
external access: the new hostnames become certificate SANs, the leaf is
reissued, and the replica set rolls. Gate the retrieval steps on the component
that publishes the trust anchor and on cert-manager, without which the declared
source does not exist and the command returns nothing.

Assisted-By: Claude <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>

The uninstall section promised that no manual cleanup of the TLS Secrets is
needed, reversing the instruction it replaced. That promise rests on an
ownerReference the operator patches on at reconcile time rather than sets at
creation, and two paths skip the patch: a dry-run reconcile returns before it,
and a Secret already present under the same name with a mismatched
cert-manager.io/certificate-name annotation is counted toward success and
returned on unpatched. The deletion finalizer does not cover the gap — it
enumerates only the users, internal-<release>, internal-<release>-users and
encryption-key Secrets. So a Secret predating the release at one of these names
can stay unowned and survive uninstall, and for <release>-ca-cert that orphans
the CA private key.

State the reconcile-time mechanism and tell operators to confirm the CA Secret
is gone. Pin the outcome with a chainsaw step asserting the CR owns all three
TLS Secrets on a normal install, so the claim goes red if the operator stops
patching instead of rotting as prose.

The external-hostname note described a permanently empty _namespace.host as a
bootstrap window that self-heals. A tenant with no host of its own and no
ancestor supplying one resolves to an empty host for good, leaving external
TLS hostname verification silently unavailable. Name that case alongside the
transient one, and say why the render is not failed closed: it would break
existing hostless tenants already running external access over IP.

Assisted-By: Claude <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>

Every assertion in the suite passed while checking nothing. containsDocument
without `any: true` asserts that EVERY rendered document matches, so negating
it asks whether "not every document is a Certificate" — true of any chart
rendering more than one object, and this one renders eight. A cert-manager
Certificate appended to templates/mongodb.yaml, the most plausible place for
one to appear, left the suite fully green.

Add `any: true` to each containsDocument so the negation means "no document
anywhere is of this kind". Unscoped assertions are ANDed across template
files, which is what makes the negatives correct: they go red if any file
renders the forbidden kind.

Add a positive control asserting the chart does render the PerconaServerMongoDB
CR through the same matcher shape, scoped to the one template that emits it.
The four negatives are all satisfied by a matcher that never matches, which is
exactly how this suite failed; the control fails if that returns.

Verified by injecting a Certificate into the chart: the suite now fails and
previously passed.

Assisted-By: Claude <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>

…etime

The external-hostname note claimed the operator and the CR version were "both
shipped by this chart". Only crVersion is: the PSMDB operator is a separate
platform component with its own lifecycle, and the SAN fold is gated on both.
On a platform carrying an older operator nothing reports the unmet gate — the
chart still renders splitHorizons, the CR is still accepted and the cluster
still comes up healthy, while the external hostnames never enter the SAN list
and a verifying client fails the handshake. State which half the chart owns and
point at the operator version as the thing to check.

The leaf certificate lifetime was stated as a flat 90 days. It is an operator
default this chart deliberately does not pin: nothing in the tenant contract
keys off it, the operator reissues without client action, and pinning would
freeze the chart against upstream shortening it. Attribute the number to the
operator version and pin the field's absence, so "inherited rather than chosen"
stays falsifiable — the same treatment allowInvalidCertificates already gets.

Assisted-By: Claude <[email protected]>
Signed-off-by: Aleksei Sviridkin <[email protected]>

The CA retrieval command was documented without the other half: what to do
with the bundle once it is in hand. A tenant reading only the retrieval step
has the certificate and no worked example of a verified connection.

Pass the two TLS inputs as mongosh flags rather than folding tls=true into
the connection string, because the correct separator differs by topology --
the replica-set URI already carries a ?replicaSet=rs0 query and the sharded
one carries none, so a single copyable form would be wrong for one of them.

Call out that omitting the flags is silent. Under preferTLS the server
accepts the plaintext connection and nothing reports the downgrade, so a
working connection is not evidence that TLS was negotiated.

Signed-off-by: Aleksei Sviridkin <[email protected]>

… sizes

The upgrade-restart note said the replica set stays available through the
rollout. That holds at three members and fails below them, and the chart
permits both smaller sizes: replicas 1 and 2 render unsafeFlags.replsetSize
and are not excluded from the SAN change, so they gain horizons and roll on
upgrade exactly as a three-member release does. At one replica the rollout
takes the only mongod down; at two the write majority is still two, so
restarting either member stalls writes until it returns. An operator running
a single-replica external release would have read the old text and declined
to book a window.

Cover the two replicas case, which was untested, and assert horizons and the
sub-quorum marker together at both sizes, so gating horizons to sizes that
can absorb a restart turns the tests red and sends someone back to this
paragraph. The default-size case carries the pair as a positive control: the
marker proves nothing unless it is absent where a restart is survivable.

Signed-off-by: Aleksei Sviridkin <[email protected]>

A tenant verifying a MongoDB endpoint needs the CA certificate, and
every Secret that holds one also holds a private key: the operator's
<release>-ca-cert carries the CA key, the leaf <release>-ssl the server
key. Granting either would hand over the key along with the anchor.

Render a TenantProjection naming the source, so the CA-extraction
controller can publish the key-free <release>.tenant-ca, and select that
projection by label from the managed-service definition. The sentinel
carries no ownerReferences: it is resolved back to its release through
the label Flux stamps, and an owner reference short-circuits that walk
and silently costs the tenant its anchor.

The sentinel is ungated. The operator issues the certificate chain
whatever the chart asks for, so the source exists for every release and
there is no plaintext leg to skip.

The source is <release>-ssl rather than <release>-ca-cert. It is the
name this chart itself sets in spec.secrets.ssl, so the reference cannot
drift from the CR; the operator writes it on both its cert-manager path
and the self-signed fallback it uses when cert-manager is absent, where
no <release>-ca-cert exists at all; and it holds the merged old and new
CA through a rotation, where <release>-ca-cert holds only the new one
and would stop verifying pods still serving the previous leaf.

Withhold the key-bearing Secrets from the tenant explicitly as well. The
label selector matches on no name, so a single mis-stamp would promote a
private key; the exclude list is evaluated first and closes the
enumerable set.

Signed-off-by: Aleksei Sviridkin <[email protected]>

The existing steps only assert that key-bearing Secrets are NOT promoted
to the tenant, and every one of those is satisfied by an absent label —
they hold just as well if nothing was ever published. Add the positive:
the projection exists, carries exactly ca.crt, matches the source
certificate by fingerprint, and is reachable through the tenantsecrets
registry rather than only as a raw Secret.

Signed-off-by: Aleksei Sviridkin <[email protected]>

The cert-manager exclusion suite justified its `any: true` requirement
with a specific number of rendered documents. The argument only needs
"more than one"; the number was already tied to one value set and the
sentinel adds another document.

Signed-off-by: Aleksei Sviridkin <[email protected]>

The README said the sentinel would sit inert with nothing else
affected. The CRD is a template of the controller's own chart, so a
cluster without the controller has no TenantProjection kind and the
release fails to render at all. The caveat also guarded a state that
cannot occur, since the controller is a mandatory platform component —
drop it rather than restate it, matching the postgres chart, and keep
the paragraph explaining the choice of source Secret.

Enumerate all three key-bearing Secrets in the registry negative. Two of
the three were listed, which leaves the third free to acquire the label
and leak a key, the exact reasoning the step above it already spells
out.

Collect TenantProjection status on failure. The controller reports
SourceNotFound and its siblings there, and without it a failed
projection surfaces only as an absent Secret with no reason given.

Rename the source-shape step, which no longer looks at the CA Secret,
and cross-reference the two halves of the leaf Secret's name.

Signed-off-by: Aleksei Sviridkin <[email protected]>

The name-clash note claimed the shared TLS helper documents
<release>-ca-cert as the conventional key-free anchor. It documents the
opposite: the canonical name is <release>.tenant-ca, and it names this
engine as the reason, because MongoDB already claims <release>-ca-cert
with a key-bearing Secret. There is no clash to warn about and the chart
follows the convention rather than deviating from it. The paragraph
above already says <release>-ca-cert holds the CA private key, so drop
the note instead of inverting it.

Guard the ownerReference assertions against an absent list. length() of
null is a type error rather than a failed comparison, and the harness
returns it as an error, which aborts the poll instead of retrying — so
the timeout never applies while the operator is still patching the
reference on.

Restore the two-variable form of the fingerprint comparison. Inlining
both command substitutions into the test swallows a non-zero exit: with
no openssl on PATH both sides are empty, they compare equal, and the
check passes having compared nothing.

Signed-off-by: Aleksei Sviridkin <[email protected]>

The assertions withholding the key-bearing TLS Secrets from the tenant
Role addressed it as document 0 and, in two cases, asserted only with
notContains. Neither survives a rearrangement.

notContains passes when its path does not resolve, so a test built from
it alone goes green against the wrong document. Reversing the order of
the two documents in the template moved the RoleBinding into position 0,
and "does not grant the key-bearing TLS secrets when external is
enabled" and "does not grant the tenant-ca projection by name" both
stayed green while the Role granted <release>-ca-cert by name — the
Secret holding the CA private key.

Select the Role by kind, and pair every notContains with a positive
assertion on the same path so a rule reordering that moves rules[1]
reddens too. Indexing survives neither rearrangement; selecting survives
the first; the positive anchor catches the second.

Signed-off-by: Aleksei Sviridkin <[email protected]>

Every object this chart creates is named after the Helm release, not
after the MongoDB resource, so a MongoDB named catest yields
mongodb-catest.tenant-ca. The retrieval commands used a bare <release>
placeholder, which reads as the resource name and produces a command
that returns nothing. Spell the prefix out in the two commands meant to
be copied, and define the placeholder once for the prose.

Split the sentinel's shape test in two. The absence of ownerReferences
is the invariant — it breaks the walk from the projection back to the
release, and the tenant stops receiving the anchor. An explicit
namespace or a chart-added label breaks nothing by itself; grouping all
three under one name presented them as equally load-bearing.

Signed-off-by: Aleksei Sviridkin <[email protected]>