The case titled "renders regardless of tls.issuer" set issuer: operator,
which is the chart default, so both sides of its claim were the same
side. Hiding the sentinel behind the cert-manager gate — the one edit
the title rules out — left it green.
Being ungated is the projection's entire contract, so each state is now
its own case: the default, operator, cert-manager, and an explicit null,
which the issuer helper treats as unset rather than as a spelling of the
default.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The exclude checks read .spec.secrets.exclude[].resourceNames[]?, which
sees only name-shaped entries. An exclude entry selecting by label was
therefore invisible to every one of them, and adding one revokes the
tenant's connection string and its trust anchor at once: matchName
returns true for every name when resourceNames is nil, and exclude is
evaluated before include and wins. The test that existed to stop exactly
that edit let it through.
The empty-selector scan keyed on the literal token matchLabels, so an
empty matchExpressions and a bare {} passed it anywhere in the file,
including under services, which the comment claimed the same pass
covered. Both are legal: the selector type inlines metav1.LabelSelector.
One more comment stated the mechanism backwards — an exclude entry that
lost resourceNames makes the backstop total rather than voiding it — and
the check under it required only that some entry carry the key, so an
added entry without one went unnoticed.
Comparing spec.secrets and spec.services whole closes all of it. The
comparison is against the rendered chart rather than the source file,
because templates/cozyrd.yaml globs cozyrds/* and ships every match:
reading one file leaves a second one, dotfiles included, granting
whatever it likes. A document count fails closed on that.
The two per-name tests are kept ahead of the comparison, subsumed but
more legible, because the runner stops at the first failure. The
block-extraction helper is gone: the cozytest runner injects return 0
before a column-zero brace, so its status was discarded there.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The notContains element omitted optional: true, and contains compares
each list element whole rather than as a subset, so the element it
described equalled nothing the template can render. Mounting the server
key Secret as the backup CA volume — the exact substitution the
assertion names — left it green, and only the neighbouring positive
caught the mutation.
The comment two cases below already described this trap as handled,
which is how it survived being reintroduced.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Both suites matched only the Secret name list in the hook's script, so
replacing kubectl delete with kubectl get left all 106 cases green while
the CA private key and the server private key survived the release. The
verb is the claim these tests exist to make; the names alone are just
text that happens to appear in a shell script.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The migration procedure was headed "the order that avoids downtime" and
closed by claiming step 3 is the only step that can refuse a connection.
Step 1 restarts the instance: under cert-manager the CR gains
serverCertSecretRef and serverCASecretRef, so the name of the Secret
projected into the pods changes, and a pod-template change rolls the
StatefulSet. The chart supports single-replica instances, and for those
a rolling restart is plain downtime.
The document already knew this in the other direction — the going-back
section warns that deleting the Secret before the operator has reissued
and rolled the StatefulSet leaves the next pod unable to start, and
prints the rollout-status command — so the two halves contradicted each
other.
Both claims are now scoped to what they are true of: the order still
matters, because it keeps enforcement behind the client migration, and
step 3 remains the only step that refuses a client by policy rather than
by restarting the server under it.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The unit suite pins what the chart renders — the sentinel, its source
name, its key — from the same assumptions the chart itself encodes, so
the two agree by construction and neither can catch the other being
wrong. Only a live mariadb-operator says whether <release>-ca-bundle is
the right name, and only a live apiserver says whether the lineage
webhook promotes the projection to something a tenant can read. Both
converged engines already carry this step; mariadb had none.
The fixture sets no tls.issuer, which is the case worth covering rather
than a gap: TLS is always served and the projection template is ungated,
so the operator reconciles the source here too.
A negative satisfied by an absent label is also satisfied by a webhook
that stamped nothing all run, so the credentials Secret leads as a
positive control. For the same reason the key-bearing names are checked
for private key material before their absence from the registry is
required, the count of names that reached that check is asserted
non-zero, and the read must answer NotFound specifically: an RBAC denial
or an unavailable aggregated apiserver also makes it fail, and reading
that as "safely absent" is how the check would stop discriminating
without ever going red.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The rule sold itself as least privilege while granting -ca-tls and -tls
under every tls.issuer, including operator, where neither Secret is ever
rendered. The comment gave no reason for that, which reads as an
oversight in a rule whose whole point is scope.
Unconditional is what the template's own logic supports, so the reason
is now written rather than the grant narrowed. The delete above is
unconditional too and leans on --ignore-not-found: a release issued by
cert-manager and later switched to tls.issuer: operator still has both
Secrets on the cluster, and uninstall renders the Role from the values
current at that moment. Gating the names on the issuer would drop the
grant for exactly that release, failing the delete on RBAC and leaving
the CA and server private keys in a namespace the tenant shares with its
other applications.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The comment sent the reader to certmanager_test.yaml, which this chart
does not have. Its cert-manager coverage — the rendering of the CA and
leaf Certificates under each tls.issuer value — lives in tls_test.yaml.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The publication guards asserted notExists on the labels
internal.cozystack.io/publish-ca-cert and its -release variant. Neither
name exists anywhere in the tree. The chart renders only
k8s.mariadb.com/watch into secretTemplate.labels, so a notExists on a
path no template can produce holds under every value, and the two cases
titled for a CA publication source could not fail for what they named.
The labels that decide tenant visibility are tenant-ca, which the
ApplicationDefinition's secrets.include selects on, and tenantresource,
which tenantsecrets serves on. Both are now asserted absent on each
key-bearing Certificate. Exclude-by-name in the ApplicationDefinition
stays the backstop behind them, not a substitute.
The documents are addressed by the Secret each Certificate writes plus
an explicit kind rather than by documentIndex, because an index follows
the slot: reordering the template would slide a labelled key-bearing
Certificate out from under the assertion and pass green. metadata.name
cannot serve, as the CA Certificate and the CA Issuer share the name
<release>-ca.
The operator-issuer case promised a label check its body never made; it
asserts only that the template renders nothing. Under that issuer no
Certificate exists to label, so zero documents is the whole statement
available, and the title now says that instead.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The file asserted that the ApplicationDefinition declares the CA source
itself -- sourceSecretName, sourceKey -- and that the operator's ca-bundle
is granted by name in secrets.include. The chart renders none of that any
more. The trust anchor now reaches the tenant as the key-free
"<release>.tenant-ca" projection: a TenantProjection sentinel in the chart
names the extraction source, and the ApplicationDefinition selects the
result on internal.cozystack.io/tenant-ca. Three assertions therefore
pinned fields the file no longer contains, and a fourth pinned a by-name
grant this design deliberately withholds.
Pin the grant surface instead, the way nats-rd and mongodb-rd do: compare
the whole value of secrets.include against the credentials Secret plus the
tenant-ca selector, so neither an added name nor an added label slips past
a per-dimension check, and assert that neither the credentials Secret nor
the projection is named in exclude, which is evaluated first and wins. The
source declaration stays pinned where it is rendered, by
packages/apps/mariadb/tests/tenant_projection_test.yaml, which compares
sourceSecretName against an exact key-free name -- a stronger guard than a
grep here could make, since this file never sees the chart.
Every assertion guarding a key-bearing Secret is kept.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The dashboard Role and the ApplicationDefinition granted the operator's
key-free mariadb-<name>-ca-bundle Secret directly by name. Every other
converged engine (mongodb, postgres, nats, qdrant) instead renders a
TenantProjection that lifts ca.crt into the canonical <release>.tenant-ca
object, reached through core.cozystack.io/tenantsecrets rather than a
per-Secret grant.
Add templates/tenant-projection.yaml sourcing from the operator's
ca-bundle, drop the name grant from the dashboard Role and from the
ApplicationDefinition's secrets.include (the existing tenant-ca
matchLabels selector now has something to match), and remove the
ApplicationDefinition's caCert field, which named a field the
ApplicationDefinition CRD does not define and would have rejected the
whole object under server-side apply.
Update the README and the pinned RBAC tests to the new tenant-ca name.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The comment claimed the block is pruned on apply. It is not: helm-controller
builds its server-side-apply patch against the schema the API server
publishes, so an undeclared field fails that patch with "field not declared
in schema" before any request reaches the server, and the HelmRelease fails
as a whole.
The distinction is load-bearing. It is no install rather than a degraded
one, which makes landing the CRD field a mechanically forced prerequisite
rather than a sequencing preference.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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: LLM
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: LLM
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: LLM
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: LLM
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: LLM
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: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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: LLM
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>
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>
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 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: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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>
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>
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>
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>
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>
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>
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 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>
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 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>
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>
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 dashboard Role already let the tenant read <release>-ca-bundle, but
the ApplicationDefinition did not list it, so it was never marked as a
tenant resource and never appeared through the API. The documentation
tells clients to fetch exactly that Secret, which worked with kubectl and
not in the interface the platform actually gives tenants.
Name it in secrets.include, the same way kafka names its CA certificates.
This works against the current API, unlike the label selector beside it,
which stays for the projection an unmerged controller will publish — so
the trust anchor is reachable now rather than only after that lands.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
cert-manager is deployed with enableCertificateOwnerRef disabled, so
removing a Certificate leaves the Secret it issued in place. Uninstalling
an instance that used managed TLS therefore left <release>-ca-tls, which
holds the CA signing key, and <release>-tls, which holds the server key,
sitting in a namespace the tenant shares with its other applications, for
as long as the namespace exists.
The chart already had the mechanism: the post-delete hook deletes the
operator-generated password Secrets by name under an RBAC rule scoped to
exactly those names. Add these two to both lists. -ca-bundle stays out —
the operator gives it an ownerRef, so Kubernetes collects it, and naming
it here would widen the grant for nothing.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The client reports an unreadable CA only as "TLS/SSL error: No such file
or directory", with no path and no filename — verified against the
MariaDB client the backup image ships. Since the bundle is an optional
mount, that message is exactly what an operator sees during the window
before the trust bundle is reconciled, and it says nothing about what to
look at. Check the file first and fail with its path and the Secret it
comes from.
Correct the migration note as well. It said nothing changes on upgrade
until TLS is opted into, which is not true for instances still using the
deprecated backup CronJob: their backup client now verifies the server
where before it did not. It verifies against the operator's own bundle
and the operator's certificate already covers the host the job dials, so
no action is needed — but a behaviour change to a running job should be
stated rather than contradicted.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The guard against an empty selector matched only "matchLabels: {}". A
bare "matchLabels:" with nothing nested under it is the same empty
selector — it compiles to match-everything and would project every
Secret in the namespace to the tenant — and it slipped through. Check
that a bare selector is followed by an indented label as well.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The section described one certificate covering the in-cluster names plus
the external hostname, which was true while managed TLS followed
external. It no longer does, so the default for an externally exposed
instance is the operator's certificate — and that one cannot carry the
external hostname, because the operator has no way to learn it.
A reader following the verification instructions on such an instance
would have failed hostname verification with no explanation, having done
everything the page asked. Describe the two issuers separately and say
plainly that verifying an external connection is what tls.enabled: true
is for.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The migration section only walked the enable direction. Reverting hands
issuance back to the operator, which means clients have to pick up its CA
again, and cert-manager leaves the Secrets it issued behind when the
Certificates go away. Neither is dangerous, but both are surprising to
find out afterwards.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The example passed the password on the command line, where it lands in
shell history and in process arguments any other user on the host can
read. Prompting costs nothing in an example whose subject is protecting
the connection.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The bundle volume was a required mount. The operator reconciles it only
after certificates settle, so on first install with managed TLS there is
a window where it does not exist, and a required mount leaves the pod in
ContainerCreating rather than failing. With concurrencyPolicy: Forbid
that one stuck pod blocks every subsequent backup for as long as it
hangs. Marking it optional turns that into a run that fails with a
message naming the missing file, retries, and lets the schedule proceed.
Correct the reason given for enforcement defaulting to false while here.
It said the tenant had nothing to verify the server against, but this
same change grants the tenant read access to the bundle, so the anchor
is reachable. The default is still right — enforcement refuses clients
that have not moved yet, which is a decision to make rather than
something an upgrade does — but that is the reason, and it was stated in
five places, all fed from the values annotations.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The check accepted the label anywhere in the ApplicationDefinition, so a
stray occurrence would satisfy it while the selector that actually
exposes the projection stayed empty or wrong. Scope it to the
matchLabels block it has to appear in.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Making managed TLS opt-in left the suite describing the behaviour it
replaced: the header still mapped external=true with tls.enabled unset
to managed, the section comment and several test titles still said TLS
was enabled via external, and every one of those cases only passed
because it now sets tls.enabled explicitly. The suite is where someone
reads the tri-state contract, so it was the one place still teaching the
old one.
The SAN comment also promised more than the test delivers. Pinning the
list catches an accidental edit to the template, but the expected values
are a copy of what the template renders, so an operator-side rename
would leave it green — and the operator skips certificate reconciliation
entirely when both secret refs are supplied, so it does not check
either. Say that the gap is real and that closing it needs a handshake,
rather than implying it is covered.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
tls.enabled defaulted to the value of external, so every existing
external instance would have moved from the operator's CA to a
chart-issued one on the next platform upgrade, without its owner doing
anything. Clients pinning the operator's ca.crt fail chain validation
the moment the new certificate is served, and external instances are
exactly the population where a copied ca.crt is most likely.
The usual argument for applying something on upgrade does not hold
here: taking over the CA does not raise the security floor. These
instances already serve TLS under the operator's CA, and what changes is
which authority issues it — an architectural move toward the platform
trust contract, not protection against a threat. There is no security
gain to weigh the breakage against, so it becomes a choice.
Rendered for an existing external instance, the result is now identical
to what the operator already applies on its own: TLS on, enforcement
off, no chart-issued certificates. Nothing happens on upgrade.
The default flips as an announced change together with the published
trust anchor, the same way tls.required does.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The check rejecting key-bearing Secrets from the tenant include list
matched only unquoted list entries, while its sibling check was already
quote-agnostic. A quoted entry would have passed both the guard and the
comment claiming it could not.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The TLS section reused the <instance> placeholder that the rest of the
document uses for the MariaDB resource name, but the Secret and the
Service are named after the Helm release, which prefixes it. A reader
following the instructions literally asked for a Secret that does not
exist and connected to a hostname that does not resolve.
Use the prefixed form in that section and say once, at the top, which
name is which.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Aleksei Sviridkin