The barman-cloud plugin injects its sidecar without resource requests or
limits. Tenant namespaces ship a LimitRange that defaults containers to
128Mi, so the sidecar inherits that default and is OOMKilled mid-backup:
the ObjectStore stays healthy, the Cluster stays Ready, and only the
backup fails. A cgroup high-water mark of 254 MiB on a 38 MB database
shows the default cannot hold it.
The shared cozy-lib helper and the Go path that builds the platform's own
ObjectStore now request 100m/256Mi and cap memory at 1Gi. No CPU limit is
set, matching the entityOperator precedent in the kafka chart: a
throttled sidecar stalls WAL archiving instead of failing it.
The helper is renamed from checksumSidecarConfiguration to
sidecarConfiguration, since it no longer carries only the checksum pin.
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
The pin check matched job="kube-state-metrics" anywhere in the trigger query, so
one pinned selector satisfied it no matter what the other three did: unpinning
$joinReplica, $idleFloor or $joinCluster individually left the suite green while
the join went back to matching any exporter's series. Anchor one assertion per
selector on the label that identifies its site so a single dropped pin fails.
Assisted-by: LLM
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The read-load trigger query joined on kube_pod_labels without pinning
job="kube-state-metrics", while the sibling DatabaseAutoscaler* alert rules
already pin it. A second exporter emitting a kube_pod_labels series (or a
duplicate scrape) would multiply the join and distort the desired-count
arithmetic. Pin the job on every kube_pod_labels selector in the query for
parity with the alerts. Follow-up from #3954 (#4187).
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The serverAddress host regex accepted any dotted tail ending in .svc, so a
single-label host like foo.svc passed the in-cluster-DNS check. It then reached
the namespace-pin, whose extraction regex left the unresolvable host unchanged
and rejected it as a namespace outside the tenant subtree - the right outcome by
the wrong path, with a misleading message. Anchor the host to two DNS labels
before .svc so it is refused at the host check with the actionable message, and
drop the stray dot the old label class allowed.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Closes IvanHunters' MAJOR on scaledobject.yaml:39. The render guard rejected
replicas > maxReplicas but not the symmetric low side. Under dry-run/transition the
chart renders the static instances=replicas while the ScaledObject carries
minReplicaCount=effectiveMin with both scale directions paused. The pause disables
the HPA's scaling policies, not its min-replica clamp, so when replicas < effectiveMin
the HPA raises the live count to the floor on its first reconcile — a scale-up in a
mode documented as "actuates neither way" — and because the rendered instances value
never changes, the client-side two-way patch never pulls it back, leaving permanent
extra standbys and PVCs. Fail closed and tell the operator to stage replicas to the
floor first. Under dry-run/transition this floor guard is strictly broader than the
db.yaml quorum wedge (any replicas <= maxSyncReplicas is also < effectiveMin), so it
supersedes it there; the wedge stays exercised on the disabled path.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The serverAddress SSRF guard had two holes beyond the external-domain suffix
already closed. It extracted the host by truncating at the first `:` or `/`,
which a `http://<svc>.<ns>.svc:port@evil.example.com/` userinfo payload defeats:
the truncated text passes the `.svc` check while net/url dials the host after the
`@`. And it validated only the host shape, never the namespace, so a tenant could
point the cluster-privileged KEDA operator at a sibling tenant's or a system
namespace's Service (the egress NetworkPolicy's `cluster` entity does not stop
in-cluster egress). Parse the URL authority properly (scheme, path/query/fragment,
userinfo to the last `@`, then port) and pin the host's namespace to the tenant's
own subtree via cozy-lib.rbac.allParentTenantsAndThis. The namespace pin narrows
what a tenant may set and needs API-owner sign-off (noted at the guard).
Separately, nothing bounded maxReplicas: the schema carries no maximum and the
aggregated apiserver does not range-validate the CR, so a tenant could set
maxReplicas: 100000 and drive KEDA to scale CNPG toward that count. Add a render
guard rejecting a maxReplicas above a sane read-replica ceiling.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The serverAddress in-cluster check accepted an unanchored trailing domain
(`\.svc(\.[a-zA-Z0-9.-]+)?$`), so a crafted host like x.svc.attacker.com — a
`.svc` label followed by any external domain — passed the guard. A tenant could
then point the cluster-privileged KEDA operator off-cluster (the egress
NetworkPolicy is the only remaining barrier, and it is inert where the CNI does
not enforce CiliumNetworkPolicy).
Anchor the optional suffix to this cluster's domain (from _cluster.cluster-domain,
injected by the platform from networking.clusterDomain): a legitimate vmselect is
<service>.<namespace>.svc or ...svc.<clusterDomain>, nothing else. The prior tests
only exercised non-.svc hosts and gave false confidence; add a case for the
.svc.<external> bypass.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Close two render-guard follow-ups the design proposal names for this PR:
- Fail closed when replicas exceeds autoscaling.maxReplicas. The stock HPA's
currentReplicas > maxReplicaCount branch writes /scale = maxReplicaCount
unconditionally on the first reconcile — even under the both-direction pause of
dryRun/transition — so a resting/seed count above the ceiling would shed the
surplus standbys and their PVCs in one step, past the scale-down pacing. Mirrors
the existing effectiveMin > maxReplicas guard.
- Refuse an autoscaling.serverAddress whose host is not an in-cluster Service DNS
name (…svc[.<domain>]). The cluster-privileged KEDA operator fetches it every
poll, so an external/IP address is an SSRF surface (cloud metadata endpoint,
another tenant); the query body is already namespace-scoped, this restricts the
destination. Defense-in-depth pairs with the operator egress policy (keda package).
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Under active read-replica autoscaling the chart seeds spec.instances at a
constant max(replicas, effectiveMin) rather than omitting it (an omitted field
defaults to 1, which the CNPG quorum webhook rejects, and — under server-side
apply — the phase-2 omit prunes the field whenever KEDA has not yet written
/scale, collapsing an idle-at-floor cluster to 1 and shedding its replica PVCs).
Keeping the constant only works if Flux does not force-own and revert it. The
postgres ApplicationDefinition therefore carries
release.cozystack.io/helm-server-side-apply: "false", so the tenant HelmRelease
applies client-side: helm-controller patches the CNPG Cluster from the
previous-vs-new rendered manifest without consulting live state, the unchanged
constant yields no patch, and KEDA's higher live /scale count is preserved. The
constant also equals the disabled/dryRun/transition render (instances: replicas)
whenever replicas sits at/above the floor, so those flips are patch-free too.
Correct the merge-semantics comments across db.yaml, _autoscaling.tpl and
values.yaml to describe the two-way previous-vs-new patch precisely (not a
"three-way no-op"), and document the residual floor-change hazard: editing
replicas / minReplicas / quorum.maxSyncReplicas moves the seed, so that one
reconcile does patch spec.instances to the new seed. Add autoscaling.serverAddress
so a monitoring:true tenant can point the ScaledObject at a vmselect that carries
both the CNPG metrics and kube_pod_labels. Regenerated README/schema/types/cozyrds.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Address review findings on the KEDA read-replica autoscaler where the
implementation diverged from the merged design proposal.
Fail-safe read load. Gate the `or vector(0)` floor on the existence of ANY CNPG
instance pod (`count(kube_pod_labels{...instance_role=~".+"}) > 0`) — primary or
replica — so the query is empty ONLY when no CNPG pod labels exist at all (a
broken kube-state-metrics pipeline), not during initial provisioning when the
sole replica-side pod is CNPG's join Job (jobRole, not instanceRole). Also pin
`ignoreNullValues: "false"` on the trigger: KEDA's prometheus scaler defaults it
true, turning an EMPTY result into metric 0 (not an error) — the HPA would then
desire 0 and clamp to the floor, i.e. silently scale a broken-pipeline cluster
DOWN. With it false an empty result is a scaler error, so the HPA HOLDS and
DatabaseAutoscalerScalerErrors fires; gating on any instance role keeps a healthy
cluster (including a long basebackup) from tripping that alert.
Constant seed = max(replicas, effectiveMin), so enable/dryRun/transition flips
are a merge no-op with no dip and never undercut the quorum floor.
Fail closed when the quorum floor exceeds maxReplicas (was: effectiveMax silently
raised both bounds, unbounded for a large maxSyncReplicas).
Dry-run / transition keep the HPA (unidirectional pause pair, not the HPA-deleting
full pause) but both SelectPolicy=Disabled, so desired normalizes to current — the
recommendation is read from the read-load metric, not a diverging desired count.
Observability. DatabaseAutoscalerAtMax keys on desired >= maxReplicas, NOT the
ScalingLimited condition (which is also true at the floor, so it fired on every
idle cluster with wrong "raise maxReplicas" advice). Dashboard read-load panels
are per-cluster (label_cnpg_io_cluster) with a CPU panel for ReadCPUUtilization
and the exact clamp in descriptions; ScaleStuck's Pending intersection is scoped
to CNPG pods; alerts.yaml narrative comments moved to Go-template comments so they
do not ship into the PrometheusRule.
Guard the seeding invariant: TestBuildHelmReleaseSpecNoForceOrDriftDetection.
Regenerated README/schema/types.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Address IvanHunters' deep review round on #3954:
- [MAJOR] Enabling autoscaling directly on an existing non-quorum database no
longer collapses it to 1 instance and deletes every replica PVC. The chart now
SEEDS spec.instances at the quorum floor (effectiveMin) on BOTH the quorum and
non-quorum active paths instead of omitting the field in the non-quorum case;
the constant seed is a three-way-merge no-op so KEDA still owns the runtime
count via /scale, and the enable dip is capped at the floor (>=2) rather than
collapsing to 1. Documents the client-side-3-way-merge (not SSA) assumption
that the no-op relies on.
- [MINOR] Fail closed on autoscaling.minReplicas > autoscaling.maxReplicas instead
of silently resolving the ceiling upward (which defeated maxReplicas as a cost
knob). The stray maxReplicas: 0 case now rejects rather than clamps.
- [MINOR] The disable-wedge guard, its values note and its test no longer emit the
invalid `kubectl scale --subresource=scale` (scale has no --subresource); use
`kubectl scale cluster.postgresql.cnpg.io/<name> --replicas=N`.
- [MINOR] Correct the enablement note: raising minReplicas/maxSyncReplicas on a
live cluster is NOT harmless when KEDA's live count exceeds the new floor (it
decommissions surplus replicas + PVCs); prescribe pin-first for raises too.
- [MINOR] Add DatabaseAutoscalerKedaOperatorDown (absent(keda_build_info)): when
the operator is unhealthy a ScaledObject never becomes an HPA and no other alert
can fire. Intersect DatabaseAutoscalerScaleStuck with a Pending pod so a healthy
long basebackup (Running pod, desired>current >15m) no longer false-fires.
- [MINOR] Keep the KEDA CRDs on package uninstall (crds.additionalAnnotations
resource-policy: keep via the shim) so removing the keda package doesn't
cascade-delete every tenant ScaledObject.
- [MINOR] cozy-lib helper: reject a non-numeric bound (int "two" -> 0 -> a
scale-to-zero ScaledObject) and render an explicit pollingInterval/cooldownPeriod
of 0 (`if not kindIs invalid`, not `with`).
- [MINOR] Note the read-load fail-open: a broken metrics join reads as idle and
silently pins autoscaling at the floor under real load.
New/updated tests cover the non-quorum seed, the min>max guard, the corrected
kubectl message, the operator-down + Pending-intersected alerts, the CRD keep
policy, and the non-numeric-bound helper guard.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The database-autoscaling dashboard hardcoded "datasource": "VictoriaMetrics" on
its namespace template variable, but cozystack provisions VictoriaMetrics
datasources as vm-shortterm / vm-longterm — there is no datasource named
"VictoriaMetrics", so on import the variable query failed ("datasource not
found"), the namespace variable never populated, and every panel (which filters
on namespace=~"$namespace") showed no data. Switch to the repo-standard pattern
used by db/cloudnativepg and the other dashboards: a `datasource`-typed template
variable (ds_prometheus) referenced as { type: prometheus, uid: ${ds_prometheus} }
from the namespace variable and every panel/target (the panels previously carried
no datasource at all and only worked by falling back to the default).
Also reframe the disable-wedge guard message to lead with the invariant (replicas
must exceed quorum.maxSyncReplicas) so a pure-quorum user who never enabled
autoscaling isn't told to "disable autoscaling"; the disable path is kept as a
parenthetical cause.
Addresses the adversarial branch-review findings on #3954.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Address the remaining IvanHunters review points on #3954:
- [MAJOR] Disabling autoscaling wedged the HelmRelease under synchronous quorum.
An autoscaled + maxSyncReplicas>=1 cluster runs with instances seeded from
effectiveMin while replicas stays at its default, so flipping autoscaling off
rendered instances: <replicas> next to the quorum and CNPG's webhook rejected
it ("maxSyncReplicas must be lower than the number of instances"). Add a
symmetric render-time guard on the static-count path (covers off/dryRun/
transition): fail with an actionable message when replicas <= maxSyncReplicas.
- [MINOR] Role-scope the freeze brake's currentInstances count
(label_cnpg_io_instance_role=~".+") so lingering CNPG join/bootstrap Job pods
(cnpg.io/cluster but jobRole, not instanceRole) don't inflate it and nudge
desired up while the brake is meant to hold it.
- [MINOR] Default maxReplicationLagSeconds to 0 (brake opt-in): only the
non-braking path is validated live, so ship the validated default and let the
freeze be enabled explicitly until its branch is exercised under load.
- [MINOR] Document the floor-change and non-quorum-disable dips (same three-way-
merge hazard) and the pin-replicas-first mitigation.
- [NIT] Coerce the cozy-lib helper's `paused` (eq "true") so a caller passing the
string "false" does not pause the ScaledObject.
- [NIT] Trim the keda.clusterDomain injection comment so it no longer ships ~8
verbatim lines into the cozystack-values Secret on every cluster.
Regenerate types/schema/README/postgres-rd. New tests cover the disable-wedge
guard, the role-scoped freeze count, and the string-"false" paused coercion.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Address the branch-review nits:
- Fail closed when the monitoring namespace is unset. serverAddress builds
vmselect-shortterm.<ns-monitoring>.svc, and an empty ns-monitoring rendered the
malformed vmselect-shortterm..svc:8481, surfacing late as a KEDA scaler error.
The platform always injects it, but a hand-rendered/misconfigured tenant now
gets an actionable render-time failure mirroring the KEDA-CRD and target guards.
Tested.
- Document that the database-autoscaling PrometheusRule matches every KEDA HPA /
ScaledObject error (the underlying series carry no owning-app label), which is
correct only while database autoscaling is KEDA's sole consumer in cozystack;
note to scope the rules if a non-database ScaledObject is ever added.
- Correct the cozy-lib.keda.scaledObject usage example: `paused` should reflect
dry-run OR migration transition, not dry-run alone, so a second engine copying
the example does not drop the transition pause.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Two review notes from IvanHunters:
- effectiveMin/effectiveMax wrapped minReplicas/maxReplicas/maxSyncReplicas/
maxReplicationLagSeconds in Sprig `default`, which treats a numeric 0 as empty
and silently substitutes the fallback (e.g. maxReplicas: 0 became 6, discarding
the tenant's value). values.yaml/schema already default these when omitted, so
read them directly; an explicit out-of-range value now flows into max() and is
clamped to the quorum floor rather than swapped for a different number. Add a
test that maxReplicas: 0 clamps to effectiveMin instead of becoming 6.
- The replication-lag brake only floored the product $braking with `or vector(0)`,
so if a CNPG build stops emitting cnpg_pg_replication_lag or
cnpg_collector_wal_records the individual gate resolved to an empty vector.
Floor each gate ($lagHigh, $writing) individually so the fail-open decision is
explicit and per-gate: the brake engages only on positively-observed lag AND
writing, and a missing signal degrades to "scale normally" (safer than freezing
on absent data) rather than relying on the emptiness of the product. Assert both
gates carry the floor.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
The read-load query's base term is floored with `or vector(0)`, but the
replication-lag brake's frozen additive term (count(kube_pod_labels) * target)
was not. PromQL `+` is a set intersection, so if count(kube_pod_labels) is
momentarily empty — a kube-state-metrics scrape gap or KSM rollout — the frozen
term is empty and the whole `base + frozen*braking` sum collapses to empty,
defeating the base term's own graceful degradation. KEDA's prometheus scaler
(ignoreNullValues: true by default) then reads that as 0 and pins desired to
minReplicaCount, ignoring real read load exactly when metrics are flaky. This
hits the default config (maxReplicationLagSeconds: 30, brake active). Floor the
frozen summand with `or vector(0)` so base + 0 = base, and assert the floor in
the lag-brake unittest.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Cover the load-bearing replica selector that both read-load queries depend on:
the join must restrict to standby pods via label_cnpg_io_instance_role="replica",
the exact key kube-state-metrics exposes for CNPG's cnpg.io/instance_role. A
wrong key (e.g. a camelCase label_cnpg_io_instanceRole) would match no series,
floor the read term to 0, and silently pin desired to the minimum - the suite
previously never asserted the selector at all. Verified live on dev9 (CNPG
1.30.0) that the snake_case key is the one present in kube_pod_labels and the
camelCase form does not exist. Adds the assertion for both the ReadConnections
and ReadCPUUtilization paths.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Narrow the previous unconditional seed to a conditional one, keeping design
proposal §3 (chart omits the field, HPA is the sole writer) intact for the
default path. Without synchronous quorum (maxSyncReplicas=0) CNPG's defaulted
instances=1 is admissible, so the field is omitted exactly as §3 specifies and
KEDA owns it via /scale with nothing contested. Only when maxSyncReplicas>=1 -
where an omitted/defaulted instances=1 fails the admission webhook
("maxSyncReplicas must be lower than the number of instances") - does the chart
seed the quorum floor (effectiveMin) so a fresh Cluster is admitted; the constant
re-render keeps three-way merge from reverting KEDA's higher live value.
This confines the deviation to the case §3 did not consider (sync quorum), so no
design-proposal amendment is needed for the documented path. Verified live on a
dev cluster: the quorum render (instances=2, maxSyncReplicas=1) and the
non-quorum render (instances omitted) are both admitted by the CNPG webhook. Adds
a render test for the non-quorum omit path alongside the quorum seed tests.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Under active autoscaling the chart previously omitted .spec.instances entirely,
letting the KEDA HPA own it via /scale. But CNPG defaults an absent
.spec.instances to 1, and its admission webhook rejects a Cluster where
maxSyncReplicas >= instances ("maxSyncReplicas must be lower than the number of
instances"). So a fresh database created with autoscaling.enabled: true AND
synchronous quorum (maxSyncReplicas >= 1) was rejected at admission before KEDA
could ever scale it to effectiveMin (= maxSyncReplicas+1) — the whole tenant
HelmRelease failed. The default maxSyncReplicas: 0 path was unaffected, which is
why it went unnoticed. Verified live: a rendered Cluster with maxSyncReplicas: 1
and an omitted instances is rejected by the dev-cluster webhook; with the seeded
floor it is admitted.
Render instances: effectiveMin under active autoscaling instead of omitting it,
so a fresh Cluster starts at a valid, quorum-satisfying count. Because the chart
emits the same constant floor on every reconcile, Helm's three-way merge is a
no-op on the field and never reverts KEDA's higher live value, so the HPA still
owns the runtime count. Update the enablement docs (a new DB can enable directly
even with synchronous replication; existing DBs above the floor rebase to it
unless the two-phase transition flow is used) and add render tests for the fresh
create + maxSyncReplicas>=1 case.
Addresses the fresh-create-with-quorum admission gap found in branch review.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
autoscaling.enabled is a tenant-facing knob, but KEDA is an optional platform
package that is not installed by default. A tenant enabling autoscaling on a
cluster without KEDA would render a ScaledObject against an unknown kind and the
whole database HelmRelease would fail with an opaque `no matches for kind
"ScaledObject"`. Guard the ScaledObject template on
`.Capabilities.APIVersions.Has "keda.sh/v1alpha1"` and stop with an actionable
message instead. Skipping the render is deliberately NOT an option: db.yaml
already drops the static .spec.instances under active autoscaling, so a missing
ScaledObject would let CNPG collapse the cluster to 1 instance. Document the
KEDA prerequisite on the `enabled` field (README/schema).
Also reject a non-positive autoscaling.target at render time: it is a plain
integer now, but the desired-replica formula divides the read load by it, so 0
is undefined and negatives invert scaling. cozyvalues-gen has no minimum
directive, so the bound is a template guard.
Add unittests for both guards (a dedicated suite renders without the KEDA CRD so
the fail-closed path is exercised; the main suite now advertises keda.sh/v1alpha1
via capabilities).
Addresses IvanHunters (KEDA-absent) and CodeRabbit (non-positive target) review
feedback on #3954.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Two review findings on the generated API:
- autoscaling.target was a resource.Quantity, which accepts suffixed values
like `150m`. The chart splices target straight into the PromQL query and the
KEDA threshold, where `150m` means 150 minutes to PromQL and is not a usable
KEDA float threshold. Retype it as a dimensionless integer so the schema
rejects suffixed input - helm unittest's schema validation now enforces it,
and the existing autoscaling suite exercises the integer contract.
- ConfigSpec carried a root `metric` field (and spec.metric in the postgres-rd
schema) that nothing reads - the chart consumes only autoscaling.metric - so
setting it silently did nothing. Remove the dead root param, keeping the
ReadMetric enum that autoscaling.metric references.
Regenerate types.go, zz_generated.deepcopy.go, values.schema.json, README.md
and the postgres-rd schema; update the autoscaling tests to the integer target.
Addresses CodeRabbit review feedback on #3954.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Wire autoscaling.maxReplicationLagSeconds into the read-load query: while the
max standby lag has exceeded the threshold within the cooldown window and the
primary is actively writing WAL, the query returns currentInstances*target,
pinning desired to the current count and freezing scaling in both directions
(safer than pinning maxReplicas, which would still allow scale-down under
lag). The lag term uses max_over_time as a hysteresis band; the write gate
(WAL-record rate > 0) keeps an idle primary from tripping it. Metrics
verified live on CNPG 1.30.0: cnpg_pg_replication_lag and
cnpg_collector_wal_records exist and carry namespace/pod (the proposal's
cnpg_pg_stat_replication_sent_diff_bytes does not). Also floor the read load
at zero via `or vector(0)` so an idle cluster returns target rather than an
empty result.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Make enabling autoscaling on an existing database count-preserving, and
add a second read-load signal:
- autoscaling.transition (migration phase 1): keep .spec.instances
rendered as a floor (from replicas, which the operator stages to the
live count) AND pause the ScaledObject, so KEDA stands up without
contending with Flux for the field. Flip transition off (phase 2) to
drop the static field and hand the count to KEDA — suspend the
HelmRelease across that apply, since helm-controller uses Helm's
three-way merge (not SSA) and would otherwise race CNPG's default-to-1.
The procedure is documented on the values field.
- autoscaling.metric (ReadConnections | ReadCPUUtilization): the query
helper emits either the active-connections sum or the postgres-container
CPU sum, both + target.
active = enabled && !dryRun && !transition; the ScaledObject is paused
whenever dryRun or transition is set. Covered by helm unittest (phase-1
holds the staged count + pauses; CPU metric query) and regenerated
schema/README/Go types/postgres-rd.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
Implement the KEDA-based read-replica autoscaler for postgres per the
merged design proposal (cozystack/community#44):
- cozy-lib.keda.scaledObject: engine-agnostic helper rendering a KEDA
ScaledObject (scale target, effective bounds, prometheus trigger,
scale-down behavior, optional pause), fail-closed on bad input.
- postgres: an `autoscaling` values block (enabled / minReplicas /
maxReplicas / target / maxReplicationLagSeconds / dryRun). Under active
autoscaling the chart drops the static .spec.instances so Flux and the
KEDA-managed HPA no longer contend, and renders a ScaledObject whose
minReplicaCount folds in the synchronous-quorum floor
(max(minReplicas, maxSyncReplicas+1, 2)); when the floor exceeds
maxReplicas, quorum wins. dry-run keeps the static count and pauses the
ScaledObject. The WorkloadMonitor tracks the quorum floor instead of a
stale static value.
Covered by helm unittest (cozy-lib helper: valid render, pause, three
fail-closed guards; postgres: instances dropped, quorum-floored bounds,
quorum-over-max, dry-run pause). Regenerated values.schema.json, README,
Go types and postgres-rd.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
An in-place RestoreJob (targetApplicationRef omitted) re-bootstraps the
Postgres app under its own name. On stock v1.6.2 it never comes back up,
for two independent reasons in the same restore patch - both surface only
in place, because a to-copy restore targets a different app.
1. Archive-vs-recovery serverName collision. The driver set the restored
cluster's WAL-archive serverName equal to the recovery source
(bootstrap.serverName = bootstrap.oldName = sourceServerName) while the
chart pinned the archiver serverName to the release name. In place those
coincide, so the cluster archived onto the source's own WAL prefix and
barman-cloud-check-wal-archive refused to start ("Expected empty
archive"). Fixed by giving the restored cluster a fresh
bootstrap.newServerName, keyed by the RestoreJob UID (unique per
restore, stable across reconciles); the chart archives there while
externalClusters still recovers from bootstrap.serverName. This is what
the manual CNPG strategies hit (serverName defaults to the cluster
name); the cozy-default flow dodged it (serverName is <ns>-<app>).
2. useSystemBucket guard. The restore populates explicit S3 coordinates
(destinationPath / s3CredentialsSecret / endpointCA) but left
backup.useSystemBucket=true, so the chart's "bootstrap.enabled is
incompatible with useSystemBucket" guard failed the HelmRelease upgrade
and the cluster never re-rendered. This is what the cozy-default
(system-bucket) flow hits on an in-place restore. Fixed by clearing
useSystemBucket in the restore patch, since the app is now on the
explicit-creds path. The driver's app view (postgresapp.Backup) gains
the field, without omitempty so a false survives the merge patch.
Both are exercised only by an in-place restore; the postgres backup e2e
drove only to-copy, so neither surfaced in CI. A step 50 in-place restore
now guards reason 1 (manual-strategy flow); reason 2 is covered by a Go
unit assertion (a system-bucket in-place e2e needs the platform bucket in
CI and is left as a follow-up).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## What this PR does
Adds an engine-agnostic controller that publishes a key-free
`<release>.tenant-ca` Secret (only `ca.crt`) for a managed application,
so a tenant can verify the application's TLS without ever seeing a
private key. The source is declared, not guessed: a chart renders a
namespaced `TenantProjection` sentinel (group `internal.cozystack.io`)
naming the CA Secret to lift; the controller watches the sentinel,
extracts `ca.crt`, and writes the projection owner-referenced to the
sentinel, so garbage collection is native. The projection carries the
`internal.cozystack.io/tenant-ca` label the lineage webhook turns into
tenant visibility. Postgres is the first consumer.
The trust boundary is RBAC: no tenant role grants any verb on
`internal.cozystack.io`, so a tenant cannot forge a sentinel. A
companion `ValidatingAdmissionPolicy` that pins the writer to
helm-controller ships separately as defence in depth.
This is the controller-and-CRD slice, split out from #3299 for focused
review. It is the only piece required for the feature to work; the
render-time helper, the writer policy, the capability-gate, and the
release-prefix validation land as their own PRs.
### Downstream repositories
Walked the trigger map against the diff. This adds an internal CRD and
controller and a sentinel to the Postgres chart; no app is added or
renamed and no `values.schema.json` changes, so the typed provider and
Ansible are unaffected. The one to confirm is the Postgres README edit —
if the website mirrors it, it needs a refresh.
- [ ] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
### Release note
```release-note
feat(cozystack-controller): publish a key-free per-application CA trust anchor (`<release>.tenant-ca`) that tenants read to verify managed-application TLS, declared through a new `TenantProjection` CRD. Postgres is the first consumer.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added tenant-facing CA certificate projections for PostgreSQL
deployments.
* Tenant secrets now provide only the CA certificate, excluding private
keys.
* Added support for configuring and monitoring tenant CA projections.
* **Documentation**
* Updated PostgreSQL TLS instructions to retrieve certificates through
tenant secrets.
* **Bug Fixes**
* Improved protection against invalid certificates, secret collisions,
and accidental exposure of private key material.
* Added validation to ensure projected certificates remain synchronized
with their sources.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
helm-unittest coverage for the S3 request-checksum fix (was mutation-negative
before): assert spec.instanceSidecarConfiguration.env pins
AWS_REQUEST_CHECKSUM_CALCULATION=when_required on all rendered barman-cloud
ObjectStores — postgres backup + bootstrap-recovery
(packages/apps/postgres/tests/backup_storage_test.yaml) and the Keycloak DB
(new packages/system/keycloak/tests/db_backup_test.yaml). Verified by mutation:
reverting the template change now fails these assertions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io>
CloudNativePG stores its self-signed CA in <release>-ca alongside the
private key and offers no way to label it, so a tenant cannot be handed
the trust anchor without also being handed the key. Render a
TenantProjection that lifts ca.crt alone into the key-free
<release>.tenant-ca Secret, and select that label from the postgres
tenant-resource definition so the projection is served through the
tenantsecrets API. Document the new retrieval path in the chart README.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Two defects of the same class — the WAL-archive S3 prefix (serverName)
changing or defaulting differently between components:
Recovery default regression (review blocker): native CNPG defaulted an
absent externalClusters serverName to the entry name (= bootstrap.oldName),
but the barman-cloud plugin defaults it to the NEW cluster's name, so a
legacy-flow restore with bootstrap.serverName unset would read the wrong
S3 prefix (hard failure, or silently restoring another cluster's data).
The chart now always emits serverName, defaulting to bootstrap.oldName.
Live-cluster serverName flip (found by the gated postgres backup e2e on a
real cluster): the chart renders spec.plugins without serverName
(effective prefix = cluster name) while a strategy template may name a
different one; the driver's ForceOwnership patch then flipped the prefix
mid-stream, splitting the WAL archive across two prefixes — the WAL
holding the backup's start was lost between them and the restore failed
with "WAL not found". The driver now preserves the live Cluster's
effective serverName (explicit parameter, or the cluster name when the
plugin is attached without one) and only applies the strategy's value to
a Cluster with no plugin attached; both charts pin serverName explicitly,
and the example strategy no longer sets one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Review follow-up (cozystack#3300, non-blocking): the barman-cloud ObjectStore CRD
validates spec.retentionPolicy against ^[1-9][0-9]*[dwm]$, so an operator who
explicitly sets backup.retentionPolicy="" would render retentionPolicy: "" and
the API server would reject the ObjectStore, breaking backup provisioning. Guard
the field in the apps/postgres and keycloak chart ObjectStores with
{{- if .Values.backup.retentionPolicy }} (the Go driver path is already safe via
omitempty). Add a helm-unittest asserting an empty retentionPolicy is omitted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Review follow-ups for the CNPG barman-cloud plugin migration (cozystack#3300):
- docs/operations/backup-classes.md: the 'Upgrade notes from chart-managed
backups' section still described the removed native spec.backup.barmanObjectStore
behavior. Rewrite it for the plugin flow (chart renders spec.plugins + a
barmancloud.cnpg.io/ObjectStore; the driver SSA-applies an ObjectStore and
patches spec.plugins), and document the real upgrade hazard: the
plugin-barman-cloud operator + cert-manager must be present in
cozy-postgres-operator or the Cluster references an unregistered plugin and
backups/WAL archiving stall.
- apps/postgres backup_storage_test.yaml: add a positive bootstrap-recovery
rendering test. Asserts externalClusters[].plugin references
barman-cloud.cloudnative-pg.io with barmanObjectName=<release>-recovery and
serverName on the plugin parameter, and that the -recovery ObjectStore carries
spec.configuration but NOT serverName (the plugin forbids it there). Previously
the recovery path had only fail-fast guard tests, no positive coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Address review feedback on #3069:
- ttlSecondsAfterFinished 120 -> 3600 so a failed init Job leaves a 1h
debugging window before the TTL controller collects it (the TTL also
reaps failed Jobs, not just successful ones).
- Load templates/init-script.yaml in the unittest suite: the Job's
checksum/config annotation includes that template, so rendering
init-job.yaml in isolation failed in CI. Select the Job via
documentSelector and assert the 3600 TTL.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
The post-install/post-upgrade init Job is a one-shot reconciler of
users/databases/roles. Its hook-delete-policy was `before-hook-creation`
only, which never fires on uninstall, so the completed Job and its Pod
lingered in the tenant namespace forever after the Postgres CR was
deleted (#3055).
Add `ttlSecondsAfterFinished: 120` so the Kubernetes TTL controller GCs
the Job and its Pod ~2 minutes after completion, regardless of the Helm
hook lifecycle. This deliberately avoids touching the hook-timeout
semantics that the bootstrap.recovery path depends on.
Add a helm-unittest suite locking in the TTL and the bootstrap-skip
behaviour to prevent regression.
Fixes#3055
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
Resolve the conflict in the generated packages/system/postgres-rd/cozyrds/postgres.yaml by regenerating openAPISchema and keysOrder from the merged values. The CRD now carries both the chart's tls.enabled field and main's backup useSystemBucket / BackupClass-driven fields.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Arsolitt <arsolitt@gmail.com>
Remove the jq post-processing patch that widened tls.enabled type to
["boolean","null"] in values.schema.json and the openAPISchema embedded
in the cozyrds CRD. Plain "type":"boolean" is emitted directly by
cozyvalues-gen and is the correct form.
The cozystack-api server unmarshals openAPISchema into
apiextv1.JSONSchemaProps whose Type field is a single string; an array
value silently zeros the field and disables server-side defaulting for
the entire app.
Tri-state semantics are preserved on the Helm template side via
kindIs "invalid": omitting tls.enabled triggers the external-based
fallback; null is no longer a valid input and is rejected by the schema.
Remove two helm-unittest cases that injected tls.enabled: null (now
intentionally rejected) and update the suite comment to match.
Signed-off-by: Arsolitt <arsolitt@gmail.com>
Drop the templates/certmanager.yaml suite entirely (the file no longer
exists) and rewrite the spec.certificates assertions to match the new
operator-managed flow: certificates block emitted only when both TLS is
on AND external is true, with serverAltDNSNames carrying just the
external hostname.
Signed-off-by: Arsolitt <arsolitt@gmail.com>
Pin the 1:1 equivalence between legacy flat preset names and their
instance-type counterparts in helm-unittest for three charts that
exercise different preset shapes:
- postgres — top-level only
- clickhouse — top + nested clickhouseKeeper preset
- opensearch — top + nested dashboards preset
Each case asserts that nano <-> t1.nano, small <-> t1.small,
medium <-> c1.small, large <-> c1.medium, and micro <-> t1.micro
render the same CPU and memory.
packages/apps/{clickhouse,opensearch}/Makefile gain a `test:` target
that calls helm unittest so hack/helm-unit-tests.sh actually runs the
new suites in CI; without it the suites would land as dead weight.
hack/check-rd-presets.sh enforces two invariants in `make unit-tests`:
- every packages/system/*-rd/cozyrds/*.yaml openAPISchema carries
the canonical 47-preset enum (40 instance-type + 7 legacy);
- no chart template carries a hardcoded legacy preset literal in
a defaultingSanitize (list "<legacy>" ...) form.
Both turn an accidental drop of a per-chart Makefile step or an
accidental legacy hardcode into a CI failure rather than silent
production-time validation rejection.
Co-authored-by: Matthieu ROBIN <matthieu@hidora.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
Yan Bondarenko