Seven guides in docs/ were written next to the code and left in the
source tree, where no operator looks. The website carries them now, so
a second copy here only lets the two drift again: the Backup Classes
page had already fallen behind its original.
docs/release.md, docs/deprecating-applications.md and the two SeaweedFS
runbooks in docs/operations stay. A migration, a bats suite, a workflow
or AGENTS.md reads each of them by path, which is the test for keeping
a document in this repository.
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
With bootstrap.enabled the chart rendered a PerconaServerMongoDBRestore
without backupSource.s3.bucket. The operator CRD requires that field, so
the API server rejected the restore and the HelmRelease failed. The prefix
was missing too, and it matters just as much. For a backupSource restore
the operator builds the PBM storage from backupSource.s3 alone and takes
only the backup name from backupSource.destination, so a restore without
the prefix would look for the backup at the bucket root.
The restore now takes the bucket and prefix from the same helpers that
fill the backup storage, so it reads from where the backups are written.
Both places quote the two values, so a numeric bucket or prefix stays a
string instead of making the resource invalid. The backup storage still
leaves out an empty prefix, so running releases keep the same object. A
destinationPath that names no bucket fails at render time instead of
reaching the operator with an empty bucket.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Each of these templates is wrapped in a top-level condition, and the
case asserts only that a field is absent. If a values change turns the
condition off, the render is empty and the absence holds trivially, so
the case passes without looking at anything. A document selector makes
an empty render fail with "document not found".
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
These cases picked a document by position and then asserted only that
something is absent from it. When a template gains or reorders a
document, the index lands on a different object that lacks the field
just as well, and the case stays green while the property it names is
no longer checked. Selecting by kind or metadata.name keeps each case on
the object it names, and fails with "document not found" when that
object stops rendering.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
The system-bucket storage was applied on every reconcile of a job, not
once per job: with the operator CR already minted nothing returned before
the apply, so a streaming dump saw an SSA patch every poll for up to a
day, and the only thing keeping that patch off a cluster that had
re-acquired the chart's tasks and pitr since the mint was a predicate
term no test held. The storage a CR names is fixed for its dump, so the
apply now runs only before the CR exists; the legacy-render hold covers
that window, and a test pins that an existing CR is never injected over.
The other predicate that decides whether a Backup owns an archive
compared the live entry's bucket and credentialsSecret against the
strategy, and only the bucket half had a fixture. The credential is what
tells a tenant's entry from the platform's when a bucket name collides,
so a case varying only that term joins the foreign-bucket one. The
cleanup shortcut's snapshot-kind term and the 24h running ceiling are
pinned the same way, the latter as an absolute boundary pair.
The generated field description said the schema rejects useSystemBucket
without backup.enabled. The aggregated apiserver does not evaluate CEL on
application schemas yet, so the render-time refusal is the one that
holds; the description and the runbook now say so. The runbook's restore
paragraph still described the unconditional credential swap this branch
narrowed, exempted MariaDB from a retention warning its shipped strategy
does not earn, and pointed at a Plan retention field that does not
exist; the example README and a dispatcher test's comment still carried
the pre-branch claim that the driver never injects and never deletes.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
The prune finalizer on a driver-minted PerconaServerMongoDBBackup means
the platform owns the archive: cleanup deletes the operator CR on that
marker and the operator removes the object from storage. It was stamped
from the app's useSystemBucket flag, which says nothing about where the
storage the CR names actually points. With a strategy carrying no s3
block nothing was injected, the presence-only precondition passed on a
legacy tenant-bucket entry, and the CR went out finalized against the
tenant's own bucket; deleting the Cozystack Backup then deleted the
tenant's archive. The finalizer now follows the storage's coordinates as
read back from the live cluster after the injection, and a strategy
without s3 coordinates, or a storage pointing anywhere else, holds the
job instead of minting.
The injection keyed on the same desired value. The app CR is projected
from the HelmRelease's spec.values, so the flag reads true before
helm-controller renders the revision, while the tenant's own storage,
scheduled tasks and PITR stream stay on the cluster. Injecting over that
entry redirected the tenant's nightly dump and oplog into the shared
bucket under platform credentials. A cluster still carrying tasks or
pitr is now left alone and the job held until the release renders the
flag, and the schema carries a rule marking useSystemBucket without
backup.enabled invalid, the pair the chart already refuses at render.
The storage-race retry counted surviving operator CRs against its
budget, but an errored CR is reaped the moment it is deleted (the
operator drops delete-backup without pruning on a non-ready CR), so the
count never left one and the job minted a CR per poll for the whole
deadline. The count now lives on the BackupJob and survives the reap.
A system-bucket dump in running had no bound at all: a wedged pbm agent
left the job Running forever and a nightly Plan stacked another beside
it. It now fails after 24 hours with the operator CR left in place as
the handle to whatever pbm still writes. The deadline cancel pins its
delete to the resourceVersion the live read answered for, and the
artifact-retry failure names the window that was actually in force.
Tests pin the guards that were exercised only in the harmless direction:
a system-bucket snapshot reaching the prune, the resolved prefix reaching
the cluster, a missing live client deferring the cancel, and the
finalizer refused against foreign coordinates.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
The pbm agents run only when backup.enabled is true, so useSystemBucket
with backups disabled renders a cluster that can never service a
BackupJob, and the tenant learned that thirty minutes into the first job
when the driver's precondition gave up. The chart already fails closed on
the bootstrap clash; do the same here so the combination is a render-time
error with the remedy in it.
The storage-race retry keyed on the app flag while everything else in the
state switch had moved to the flow recorded on the CR's own finalizer, and
that flag term was the only thing keeping a legacy tenant's operator CR
from being deleted and retried on the same operator error — with no test
holding it. Key it on the CR's flow like its neighbours and pin the legacy
side: the error is terminal and the CR survives.
The retry matches the operator's own error wording, which is correct at
the pinned crVersion and nothing else ties together: a bump that rewords
it turns the retry off with the suite still green. Name the coupling at
the pin and at the matcher so a bump re-checks it.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
The opt-out refusal ran before the lookup of the job's existing operator
CR, so it was a per-poll gate rather than a mint-time one: flipping
backup.useSystemBucket to false while a dump streamed routed every later
reconcile into the refusal, failed the job at the deadline and left the
CR minted under the old flag streaming into the platform bucket with a
finalizer no Backup would ever resolve — the stranded archive the running
exemption exists to prevent. The refusal now applies only while the job
has no CR, and everything after the mint reads the flow off the CR's
delete-backup finalizer, which is fixed at mint time, instead of the
mutable app flag. A late dump on the legacy flow is likewise handled by
its CR's flow: the deadline fails the job and leaves the operator CR, as
before this driver existed.
The refusal also identified the injected entry by bucket name and told
the operator to wait for the chart to re-render it, which the docs say
never happens. The entry is now recognised by its credentialsSecret, the
one field on it the driver chose (bucket names collide with an admin's
external bucket and drift when the platform bucket is re-provisioned),
and the message names the manual removal the docs prescribe.
At the pinned operator `requested` is set right after the command is
sent to pbm, so deleting a CR in that state cannot recall it and a late
dump would land with nothing to own it. Only an unobserved or queued CR
is cancelled; a requested one is left as the handle to whatever pbm
writes, and the job fails.
The credential pre-check failed at once where every sibling precondition
waits for the restore deadline, and it re-ran against a restore already
replaying, so a Secret pruned mid-replay marked an in-progress restore
Failed. It now waits like its neighbours and is skipped once the operator
restore exists. The status-write suppression on the waiting helper is
pinned by counting status updates across repeated reconciles.
The legacy deadline's justification claimed psmdb task retention covers
the archive; the operator prunes only its own scheduled backups, so the
object is the tenant's to reclaim, and the comments say that instead. The
bootstrap refusal names the value that actually clears it
(bootstrap.enabled is not reset after a bootstrap restore).
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
The delete-backup finalizer prunes storage only for a `ready` backup at
v1.22.0; a timed-out backup never reaches `ready`, so the deadline-cancel
comment no longer claims the finalizer "takes anything partially written" —
nothing is written until the backup runs, which the cancel prevents. The
psmdbBackupTimedOut godoc no longer says the operator "has not started" the
backup, which is wrong for `requested` (the operator sets it as it dispatches);
it now describes the not-yet-running states it actually gates.
The pre-read CR handed to releaseMongoDBCleanup on the terminating path had no
test that failed when the argument was dropped, because the existing cases
either call the function directly or let the second Get succeed. Added a
cleanupMongoDBBackup case whose operator-CR Get fails after the first: the
release must strip from the passed object, not re-read, or the teardown wedges.
The useSystemBucket field said "retention move[s] to the Plan", but a Plan has
no retention field; corrected to "scheduling moves to a Plan; this flow has no
retention yet" and regenerated the schema, README and mongodb-rd. Softened the
"never pins a namespace in Terminating" wording in the cleanup godoc and RBAC
comment: the driver only bounds its own finalizer, the operator's release-lock
finalizer can still hold the CR while the operator is down.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Every prose surface (the useSystemBucket field, the retentionPolicy field, and
backup-classes.md) tells a tenant that schedule and retentionPolicy are ignored
on this flow, and cozyvalues-gen copies that into the schema, README and the
dashboard. A render-time reject of a non-default retentionPolicy contradicted
all of them: a tenant who followed the documented opt-in with a retentionPolicy
still set got a failed render and a HelmRelease that stopped converging, pointed
at the very doc that says the field does nothing. Drop the reject so
retentionPolicy is inert exactly as documented, handled the same way as schedule
(gated out, no error). The absence of retention on this flow is real but belongs
to a follow-up, so the docs now point at the BackupRetentionPolicy design (#4192)
and the teardown-GC issue (#3966), and recommend an S3 lifecycle policy until
they land.
Cancel a timed-out operator backup instead of only failing the BackupJob. The
deadline fires for a backup the operator has not begun streaming; but `waiting`
is the state pbm reports for a backup queued behind another, and the operator
starts it once the slot frees — after the BackupJob is Failed and never
reconciled again, writing an archive into the shared bucket that no Backup
object represents. The deadline branch now deletes the operator CR before
failing, which prevents that late run entirely: at v1.22.0 the operator returns
for a CR with a deletion timestamp before it dispatches to pbm, so nothing is
written. The delete is best-effort so it never blocks the failure.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Two ways the shared cozy-backups bucket could accumulate objects nothing prunes.
A dump that outran the 30-minute deadline was declared failed while pbm kept
writing. The BackupJob's terminal-phase guard then never runs the branch that
mints the Cozystack Backup, so the archive lands with no Backup object to
represent it and no retention can reach it (the operator CR carries no ownerRef
either). Leave a running dump to finish — a real dataset routinely outruns 30m,
and a broken one still terminates through the operator's error/rejected state.
The deadline keeps bounding the case it was written for: the operator never
starts the backup (state stays "", requested or waiting), where nothing has
been written and failing strands nothing.
retentionPolicy is inert on this flow (the chart's tasks[].keep is gated out and
no Plan-level retention exists yet), so a value a tenant set expecting it to work
would grow the bucket silently. Reject a retentionPolicy changed from the chart
default when useSystemBucket is on, rather than ignore it; the default still
renders so an untouched install is unaffected.
Also on the cleanup path: hand the terminating-namespace release the operator CR
the caller already read and confirmed owned, so the finalizer strip is not gated
on a second Get whose transient failure would silently drop it and leave the CR
finalized in a namespace being torn down. Correct the ownership comments — the
delete-backup finalizer is not one the operator never sets (it finalizes
task-minted backups on the legacy flow); what keeps it unambiguous is that
cleanup only ever resolves the one CR named in driverMetadata. Add the missing
isolation test that a legacy app's own s3-storage is never overwritten by the
injection call site, and move the strategy's storage comment into a Go-template
comment so it no longer ships into every rendered Strategy CR.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
The skip-artifact-cleanup annotation reaches releaseMongoDBCleanup before the
ownership check, and the operator-CR delete sat outside the finalizer gate, so
a legacy backup (no delete-backup finalizer) lost its PerconaServerMongoDBBackup
record when a tenant set the annotation to release a stuck Backup or applied it
across a namespace during teardown. That record is what a psmdb restore-by-name
resolves against, and both the RBAC comment and the docs promise it is never
touched. Gate every write in the release on the finalizer being present: an
unowned (legacy) CR or one that cannot be read at all is left intact, and the
Cozystack Backup still releases.
Surface a failed finalizer strip. The release must proceed even against an
unreachable CR, but the swallowed patch error left the operator CR holding
percona.com/delete-backup — pinning its namespace in Terminating, the outcome
the branch exists to prevent — while the Event reported the success case. Emit
a FinalizerNotStripped Warning naming the object so an operator can find it.
Mark the sibling backup fields (schedule, retentionPolicy, destinationPath,
endpointURL, insecureSkipTLSVerify, s3AccessKey, s3SecretKey) as ignored under
useSystemBucket, so a tenant editing one instead of the flag itself is not
misled by a description that describes only the legacy flow; regenerate the
schema, README and mongodb-rd. Pair the insecureSkipTLSVerify rendering
assertion with a case that flips the platform knob on, so the two together
discriminate the rendered value from a hardcoded default.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
The strategy godoc still said the storage config is NOT inlined and the
strategy only names the storage — which controller-gen copies verbatim into
the CRD, so kubectl explain served the new s3 field and its own denial from
one object. Rewrite it (and the StorageName field doc) to describe both
flows, and regenerate the CRD.
Render insecureSkipTLSVerify on the default MongoDB strategy: the field is
plumbed end to end into the driver-injected storage entry and backed by a
platform knob, but the template set only forcePathStyle, pinning it to
false so a self-signed external S3 that Redis backups honour would fail
MongoDB x509 verification. Pin it in the rendering test alongside the other
s3 coordinates.
Fix the useSystemBucket field description: angle brackets rendered as empty
HTML tags in the README table and the dashboard schema, so wrap the path in
backticks (as the postgres field already does) and note that schedule,
retentionPolicy and PITR are ignored on this flow. Regenerate the chart
schema, README and mongodb-rd.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Several comments and the useSystemBucket field description had drifted from
what the code does.
The strategy header still said the $bucketName gate was "not because it
needs the value" and that the sibling strategies needed the bucket name
while this one only asked for ordering, but the strategy now renders
bucket: {{ $bucketName }} for the injected coordinates, so it needs the
value; the precondition comment claimed the s3-storage entry exists on
backup.enabled=true, which is only the legacy flow. The rendering-test
comment carried the same "needs no bucket coordinates" claim. The
useSystemBucket field description said injection happens "at first BackupJob
time" (it now runs on every BackupJob) and that backups scope to
<namespace>/<release> (the prefix templates the application name, not the
release), and it ran ~700 chars of implementation detail into the CRD that
backup-classes.md already carries; trimmed and corrected, then regenerated
schema/README/types/cozyrds. Same <release> slip fixed in the driver
comment and the docs.
Nothing asserted the new strategy rendering. Added the MongoDB row to the
endpoint-form suite (s3.endpointUrl, scheme preserved both ways) and field
assertions to the rendering suite — bucket, endpointUrl, region,
credentialsSecret, forcePathStyle, and prefix, which must reach the CR as a
literal go-template string so the driver can template it per-application;
a stray brace would empty it with every doc-count test still green.
The restore credential swap broke across flows. It replaced the source
credentialsSecret with whatever the target declared, comparing no buckets,
so a system-bucket backup restored into a legacy target kept the platform
bucket against the tenant's own -s3-creds, and a legacy backup restored
into a system-bucket target got cozy-backups-creds against the tenant
bucket - both AccessDenied. Skip the swap when the source already names the
platform-projected cozy-backups-creds (it outlives the source app, nothing
to repair), and only adopt a target credential whose storage is on the
same bucket as the archive. psmdbStorageS3 now reads .s3.bucket alongside
.s3.credentialsSecret; TestPsmdbTargetCredentialsSecret covers the match,
the cross-bucket refusal, and the empty-bucket no-op.
Also drop the post-apply re-Get: the typed SSA Patch decodes the server's
merged object back into the passed struct, so the precondition reads the
injected storage from it instead of a follow-up Get that hit the informer
cache and briefly reported Ready=False on the happy path.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
The MongoDB backup driver decided what to inject and what to record from
the live cluster's storage set rather than from the app's opt-in flag. That
had three consequences.
The snapshot fallback fired for legacy apps too. Its gate was
`snap.S3 == nil && rendered.S3 != nil`, but the coordinates moved onto the
shared cozy-default strategy, so rendered.S3 is now non-nil for every
MongoDB app. A legacy backup written to the tenant's own bucket got its
snapshot rewritten to the platform bucket, sending restore to the wrong
place. Gate the fallback on the app's backup.useSystemBucket flag instead,
threaded through createMongoDBBackupArtifact, and pin the legacy case.
The injected storage never caught up with the strategy. Injection was
gated on the named storage being absent, so it ran once per cluster and
froze the coordinates at the first backup; a later endpoint/region change
or bucket re-provision was a silent no-op, and Helm could not repair it
because the useSystemBucket chart renders no storages. Key injection off
the app flag and re-apply on every BackupJob (the prefix is deterministic,
so this never splits the archive the way CNPG's serverName would). Add
UseSystemBucket to mongodbapp.MongoDBSpec so a legacy app is protected by
the flag, not by whatever happens to be on the cluster.
The docs said the opposite of what the driver does. backup-classes.md
claimed "the MongoDB driver cannot inject its S3 target per-backup";
correct it and add an opt-in-to-the-system-bucket section that names which
values stop being read and that scheduled backups + PITR must move to a
Plan. Fix the endpoint table row, the tuning reference, the RBAC comment
that said the driver never patches, the strategy header/inline comments,
and the example comment. Also assert notExists on spec.backup.pitr in the
helm test, and lift the duplicated cozy-backups-creds default into a const.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
Extract the two untested contracts from applyMongoDBSystemStorage into
pure functions and unit-test both, mirroring the CNPG driver's
buildBarmanObjectStore split:
- shouldInjectMongoDBSystemStorage decides whether the driver injects the
system-bucket storage; the tests pin both branches, including the one
that leaves a legacy app's manual bucket untouched.
- buildMongoDBSystemStorageEntry builds the {type:s3,s3:{...}} entry,
defaulting credentialsSecret and omitting forcePathStyle when nil.
Also add a helm-unittest for the restore.yaml guard rejecting
bootstrap.enabled together with backup.useSystemBucket.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
MongoDB was the only cozy-default app with no useSystemBucket flow: its
backup storage was always static on the app CR (bucket/endpoint/creds
from backup.* values + a chart-emitted <release>-s3-creds Secret), so a
tenant could not back up to the platform system bucket out of the box the
way postgres/clickhouse/etc. do.
PBM takes bucket/endpointUrl/region/prefix as static CR fields (not
secret refs) and the platform bucket name/endpoint are only known at
BackupJob time, so - unlike ClickHouse's runtime env secretKeyRef - the
chart cannot render them. The fix mirrors the CNPG driver: the chart adds
a useSystemBucket flag that keeps spec.backup.enabled (so the pbm agents
run) but omits the static storages/tasks/pitr and the creds Secret; the
cozy-default MongoDB strategy carries the coordinates (endpointUrl already
scheme'd by the endpoint helper, credentialsSecret cozy-backups-creds);
and the psmdb driver SSA-injects spec.backup.storages[s3-storage] onto the
live PerconaServerMongoDB at BackupJob time under its own field manager
with ForceOwnership, so a Flux re-render never reverts it. Injection is
gated on the storage being absent, so a legacy app that ships its own
static storage is left untouched. The backup snapshot falls back to the
strategy coordinates when the operator status echo is empty, so the
restore path rebuilds backupSource (credentialsSecret cozy-backups-creds
is projected into the restore namespace before dispatch).
RBAC grows patch/update on perconaservermongodbs for the injection. A
bootstrap.enabled + useSystemBucket render guard mirrors postgres.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
The upgrade section still promised a password rotation and a regenerated
credentials Secret. With the operator-copy fallback the chart reads the
current passwords from internal-<release>-users and writes the same values,
so nothing rotates and no session breaks.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Every test rendered with an empty lookup, so the branch that keeps a live
password could be deleted with the suite green. Three fixtures through
kubernetesProvider cover what a fresh render never reaches: a helm-owned
users Secret is reused, a Secret the release does not own stays out of the
manifest while its password still reaches the credentials, and the operator
copy rebuilds a missing users Secret. Values with URI-reserved characters
cover the userinfo encoding, and both Secrets are asserted Opaque again.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
On the not-owned path a missing password key produced a generated password
that reached <release>-credentials but never the users Secret the operator
reads, so the dashboard advertised a password MongoDB never had. The render
now fails and names the Secret and the key. The operator fills every key
itself, so only a hand-made partial Secret reaches this branch.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Two paragraphs under Credentials were hard-wrapped at 80 columns, the one
place in this README that was; the rest follows the one-line-per-paragraph
rule the repository asks for in markdown.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
Assisted-by: LLM
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 <f@lex.la>
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 <f@lex.la>
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 <f@lex.la>
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 <f@lex.la>
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 <f@lex.la>
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 <f@lex.la>
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 <f@lex.la>
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 <f@lex.la>
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 <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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 <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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 <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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 <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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 <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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 <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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 <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
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 <noreply@anthropic.com>
Signed-off-by: Arsolitt <arsolitt@gmail.com>
Adds a MongoDB (Percona Server for MongoDB / psmdb) backup-strategy driver so
`apps.cozystack.io/MongoDB` reaches feature parity with the CNPG, MariaDB and
ClickHouse native drivers — logical backups, restore-to-differently-named
instances, and point-in-time recovery through the unified
BackupJob/RestoreJob/BackupClass/Plan interface (closes#3545).
The driver orchestrates the operator's PerconaServerMongoDBBackup /
PerconaServerMongoDBRestore CRDs:
- BackupJob: creates a one-shot PerconaServerMongoDBBackup, polls
status.state, and persists the S3 destination + storage descriptor
(by-reference, no credentials) onto the Cozystack Backup artifact.
- RestoreJob: drives PerconaServerMongoDBRestore via backupSource (read back
from the source backup status, snapshot fallback) so the same artifact
restores in-place or into a differently-named instance; PITR via
spec.options.pitr (date/latest).
Because psmdb resolves the backup storage by name against the cluster's
spec.backup.storages, the driver names the chart's storage (s3-storage) via the
cozy-default-mongodb Strategy rather than injecting it, and surfaces a clear
Ready=False precondition when a target has backups disabled.
Wiring:
- api/backups/strategy/v1alpha1: MongoDB strategy type + generated CRD/deepcopy.
- internal/backupcontroller: psmdbstrategy_controller + psmdbtypes/mongodbapp
partial-schema mirrors; dispatch, supported-kinds and scheme registration.
- backupstrategy-controller: cozy-default MongoDB route, cozy-default-mongodb
Strategy CR, and psmdb.percona.com RBAC.
- apps/mongodb: backup.insecureSkipTLSVerify knob (psmdb s3 storage has no
CA-bundle field) so backups can target Cozystack's self-signed seaweedfs.
Tests + e2e:
- unit tests for the driver (validation, ensure-idempotency, precondition,
backupSource resolution, snapshot round-trip, PITR option parsing).
- examples/backups/mongodb + a mongodb-2-backup-roundtrip Chainsaw suite that
drives run-all.sh end-to-end (sentinel document round-trips through S3 into a
restored copy while the source stays untouched), per docs/agents/e2e-testing.md.
- docs/operations/backup-classes.md updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Matches the managed MongoDB CR (`spec.crVersion`) to the psmdb-operator 1.22.0
bump so the platform tracks the deployed operator instead of running at the
N-1 skew. The 1.22.0 CR schema is additive (no new required fields), so
existing instances reconcile cleanly under SmartUpdate. PMM (disabled by
default) and the PBM 2.11.0 sidecar stay compatible with operator 1.22.0;
refreshing their image matrix is a separate apps/mongodb concern.
Adaptations:
- templates/mongodb.yaml: spec.crVersion 1.21.1 -> 1.22.0
- tests/mongodb_test.yaml: update crVersion assertion to 1.22.0
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Add the storageclass dynamic-dropdown x-cozystack-options annotation to the
storageClass field of the data apps (openbao, clickhouse, mariadb, redis,
harbor, opensearch, rabbitmq, mongodb, postgres, nats, kafka, qdrant,
foundationdb, http-cache) at the values.yaml source, so the extension is
emitted by cozyvalues-gen instead of hand-edited into values.schema.json.
kafka carries two (Kafka and ZooKeeper structs); foundationdb is nested
under the Storage struct.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
Mark fields that reference cluster resources with the x-cozystack-options
vendor keyword so the dashboard renders them as dynamic dropdowns:
GPU, instanceType, instanceProfile, network (vm-instance), GPU/
instanceType (kubernetes nodeGroups), image (vm-disk), storagePool
(bucket), storageClass (all apps) and disks[].name (vm-instance). Drop
the stale static instanceProfile enum.
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io>
Annotate storageClass as immutable across every stateful app whose
chart exposes one — clickhouse, foundationdb, harbor, http-cache,
kafka (Kafka + ZooKeeper), kubernetes (top-level only), mariadb,
mongodb, nats, openbao, opensearch, postgres, qdrant, rabbitmq,
redis, vm-disk. Without coverage across the catalogue the UI would
render storageClass read-only on some apps and editable on others —
a worse mental model than uniform behaviour.
The annotation lands at the schema layer (CEL XValidation rule
'self == oldSelf' on values.schema.json / types.go / embedded
openAPISchema in cozyrds/<app>.yaml). The cozystack aggregated
apiserver does not yet evaluate CEL on application-backed resources;
that is tracked at cozystack/cozystack#2657. The UI
(cozystack/cozystack-ui#6) consumes the marker today.
The PVC-pinning argument is identical for every chart: Kubernetes
fixes a PVC's storageClassName at creation time, so editing this
field on an existing resource never migrated data. The schema
annotation makes that contract explicit.
kubernetes.nodeGroups[name].storageClass is intentionally NOT
annotated: the field is optional and undefaulted, so a strict
'self == oldSelf' rule would block any future attempt to set it on
an existing node group. A richer ratcheting form is needed there —
see docs/storage-immutability.md.
A refactor in pkg/registry/apps/application/rest.go extracts the
inline schema-build path into buildSpecSchema so it can be
unit-tested. Three regression tests in rest_spec_schema_test.go pin
the contract that x-kubernetes-validations survives the
v1->internal->structural conversion (loaded from the actual MariaDB
ApplicationDefinition fixture on disk, not a hand-edited mock).
Each per-app README gains a one-line blockquote pointing to a new
docs/storage-immutability.md that explains the contract and which
consumers enforce it today.
Requires cozystack/cozyvalues-gen with @immutable support — bump the
pre-commit pin in .github/workflows/pre-commit.yml once the release
exists.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Rewrite resourcesPreset values in every apps/* and extras/* chart from
the legacy flat names to the new instance-type form, using the 1:1
mapping:
nano -> t1.nano micro -> t1.micro
small -> t1.small medium -> c1.small
large -> c1.medium xlarge -> c1.large
2xlarge -> c1.xlarge
Each affected values.yaml carries the canonical 47-value enum (40
instance-type names plus 7 legacy aliases) so the schema accepts both
forms during the deprecation window. cozyvalues-gen regenerates the
matching api/apps/v1alpha1/*/types.go kubebuilder enum and defaults,
the chart values.schema.json, the README parameter table, and the
ApplicationDefinition openAPISchema in packages/system/*-rd/cozyrds.
Three hardcoded legacy literals in chart templates migrated:
- packages/apps/harbor/templates/harbor.yaml:121 (portal)
- packages/apps/opensearch/templates/opensearch.yaml:43 (dashboards)
- packages/system/keycloak/templates/sts.yaml:115 (heap fallback)
packages/apps/foundationdb/Makefile gains the missing hack/update-crd.sh
call. Without it, every regen left foundationdb-rd cozyrds stale and
new FoundationDB app CRs through cozystack-api were rejected at
validation for any non-legacy resourcesPreset value.
Every default-rendered chart produces identical CPU and memory to the
value applied before the rename, courtesy of the legacy alias table in
cozy-lib and the 1:1 default mapping.
Co-authored-by: Matthieu ROBIN <matthieu@hidora.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
## What this PR does
Fixes#2514.
When `external: true` is set on a non-sharded `MongoDB`, Cozystack
rendered a single
`LoadBalancer` Service whose selector matched **all** replica-set
members
(`app.kubernetes.io/replset: rs0`). Round-robin balancing sent ~⅔ of
writes to
secondaries and they were rejected with `MongoServerError: not primary`.
Drivers couldn't recover on their own either: connecting through the LB
returned
the in-cluster DNS names of all members from `isMaster`, none of which
resolve
from outside the cluster, so automatic primary discovery failed.
This PR replaces the manual `external-svc.yaml` (for replica-set mode)
with the
Percona MongoDB operator's native `replsets[].expose` feature:
```yaml
replsets:
- name: rs0
expose:
enabled: true
type: LoadBalancer
externalTrafficPolicy: Local
```
The operator then:
- creates one `LoadBalancer` Service **per pod** (`<release>-rs0-0`,
`-1`, `-2`)
with `statefulset.kubernetes.io/pod-name` selectors,
- watches `Service.status.loadBalancer.ingress` for each pod and
rewrites the
member `host` in `rs.conf` to the external IPs.
External clients use a standard replica-set URI
(`mongodb://...:27017,...:27017,...:27017/?replicaSet=rs0`) and the
driver
discovers the primary correctly from outside the cluster.
For **sharded** mode the existing single `LoadBalancer` is preserved —
it
front-ends stateless `mongos`, which can route writes to the right shard
primary itself. `external-svc.yaml` is now scoped to that case only.
### Tested
- `helm unittest .` in `packages/apps/mongodb/`: 91/91 pass.
- Applied on a dev stand:
- `replsets[0].expose` is set as expected in the rendered PSMDB CR.
- The Percona operator (`v1.21.1`) accepts the spec and creates 3
per-pod
`LoadBalancer` Services with `statefulset.kubernetes.io/pod-name`
selectors. The previous single round-robin Service is no longer
rendered.
- With an LB IP pool present, the operator promotes the allocated LB IPs
into
the PSMDB `status.host` field and into `rs.conf`.
End-to-end mongosh write loop wasn't run on the dev stand because the
underlying network does not advertise/route allocated LoadBalancer IPs
(no
working L2/BGP path), so the rewritten `rs.conf` members are unreachable
in
that environment. The structural change is otherwise fully verified and
the
issue's reproduction scenario expects a real LB-capable network.
### Release note
```release-note
fix(mongodb): expose each replica-set member through its own LoadBalancer when `external: true`, so external clients can discover the primary via the standard replica-set connection string and writes no longer fail with "not primary" errors.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* External LoadBalancer services now only render when both external and
sharding are enabled.
* Replica set external exposure is now conditional based on the external
setting.
* Service configuration now uses LoadBalancer type with Local traffic
policy for external access.
* Service selectors refined for proper mongos targeting in sharded
deployments.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Describe the in-place password rotation performed by the Percona operator
when upgrading to releases that scope the system users secret per CR.
Clarify that consumers of the credentials secret need a pod restart to
pick up the new password, custom user secrets are unaffected, and the
legacy namespace-shared secret is left orphaned and must be cleaned up
manually.
Signed-off-by: Arsolitt <arsolitt@gmail.com>
Update Data lifecycle section to address review feedback:
- Reflect the release-scoped users secret naming.
- Distinguish operator-driven PVC release from actual PV/data
deletion (depends on StorageClass reclaimPolicy).
- Document that TLS secrets (<release>-ssl, <release>-ssl-internal)
are not reclaimed automatically.
- Add a recovery note for the case where the operator is removed
before the CR, leaving finalizers unable to run.
Signed-off-by: Arsolitt <arsolitt@gmail.com>
Myasnikov Daniil