Skip to content

Commit d4ba7bb

Browse files
scooby87scooby87
committed
fix(postgres): fail closed when autoscaling is enabled without KEDA, guard target
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 <[email protected]>
1 parent 8b19d24 commit d4ba7bb

8 files changed

Lines changed: 85 additions & 5 deletions

File tree

‎api/apps/v1alpha1/postgresql/types.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/apps/postgres/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ For `sslmode=verify-full` to work, the CA bundle retrieved above must be saved t
245245
| Name | Description | Type | Value |
246246
| -------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------- | ----------------- |
247247
| `autoscaling` | Read-replica autoscaling configuration. | `object` | `{}` |
248-
| `autoscaling.enabled` | Enable horizontal autoscaling of read replicas. | `bool` | `false` |
248+
| `autoscaling.enabled` | Enable horizontal autoscaling of read replicas. Requires the platform `keda` package to be enabled by an administrator first; the chart fails to render (rather than break the release) if KEDA is not installed. | `bool` | `false` |
249249
| `autoscaling.minReplicas` | Minimum total instances; raised to the synchronous-quorum floor (`maxSyncReplicas + 1`) and to 2 when either is higher. | `int` | `2` |
250250
| `autoscaling.maxReplicas` | Maximum total instances; raised to the quorum floor if the floor exceeds it. | `int` | `6` |
251251
| `autoscaling.metric` | Read-load signal to scale on. | `string` | `ReadConnections` |

‎packages/apps/postgres/templates/scaledobject.yaml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,21 @@
11
{{- if .Values.autoscaling.enabled }}
2+
{{- /*
3+
Fail closed when KEDA is not installed. KEDA is an optional platform package and
4+
is NOT enabled by default, but autoscaling.enabled is a tenant-facing knob, so a
5+
tenant can turn it on against a cluster where the admin never enabled `keda`.
6+
Without this guard the ScaledObject renders against an unknown kind and the whole
7+
database HelmRelease fails with an opaque `no matches for kind "ScaledObject"`.
8+
We must NOT instead skip rendering: db.yaml already drops the static
9+
`.spec.instances` under active autoscaling, so a missing ScaledObject would let
10+
CNPG collapse the cluster to 1 instance. Stop with an actionable message instead.
11+
*/ -}}
12+
{{- if not (.Capabilities.APIVersions.Has "keda.sh/v1alpha1") }}
13+
{{- fail "postgres: autoscaling.enabled is set, but the KEDA platform package is not installed (keda.sh/v1alpha1 is unknown to the cluster). Ask your platform admin to enable the 'keda' package before enabling database autoscaling." }}
14+
{{- end }}
15+
{{- /* The HPA divides read load by target; 0 is undefined and negatives invert scaling. */ -}}
16+
{{- if lt (int .Values.autoscaling.target) 1 }}
17+
{{- fail "postgres: autoscaling.target must be a positive integer (the desired-replica formula divides the read load by it)." }}
18+
{{- end }}
219
{{- include "cozy-lib.keda.scaledObject" (dict
320
"name" .Release.Name
421
"namespace" .Release.Namespace

‎packages/apps/postgres/tests/autoscaling_keda_guard_test.yaml‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
suite: postgres autoscaling fails closed without KEDA
2+
3+
# Isolated from autoscaling_test.yaml because that suite makes the KEDA CRD
4+
# available cluster-wide via suite-level capabilities. Here we assert the
5+
# opposite: with KEDA absent (default capabilities), enabling autoscaling must
6+
# stop the render with an actionable message rather than emit a ScaledObject
7+
# against an unknown kind - which would fail the tenant's whole HelmRelease, and
8+
# since db.yaml already drops spec.instances under autoscaling, must never
9+
# silently skip the ScaledObject either.
10+
11+
release:
12+
name: postgres-db
13+
namespace: tenant-acme
14+
15+
set:
16+
_cluster:
17+
scheduling: {}
18+
cluster-domain: cozy.local
19+
_namespace:
20+
monitoring: tenant-root
21+
22+
templates:
23+
- templates/scaledobject.yaml
24+
25+
tests:
26+
- it: fails closed when autoscaling is enabled but the KEDA package is not installed
27+
set:
28+
autoscaling:
29+
enabled: true
30+
minReplicas: 2
31+
maxReplicas: 6
32+
target: 150
33+
quorum:
34+
maxSyncReplicas: 2
35+
asserts:
36+
- failedTemplate:
37+
errorMessage: "postgres: autoscaling.enabled is set, but the KEDA platform package is not installed (keda.sh/v1alpha1 is unknown to the cluster). Ask your platform admin to enable the 'keda' package before enabling database autoscaling."
38+
39+
- it: renders nothing when autoscaling is off even without KEDA
40+
asserts:
41+
- hasDocuments:
42+
count: 0

‎packages/apps/postgres/tests/autoscaling_test.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ set:
1616
_namespace:
1717
monitoring: tenant-root
1818

19+
# The ScaledObject template guards on the KEDA CRD being present; make it available
20+
# to every rendering test (individual guard tests override this).
21+
capabilities:
22+
apiVersions:
23+
- keda.sh/v1alpha1
24+
1925
templates:
2026
- templates/db.yaml
2127
- templates/scaledobject.yaml
@@ -245,3 +251,18 @@ tests:
245251
path: metadata.annotations["autoscaling.keda.sh/paused"]
246252
value: "true"
247253
template: templates/scaledobject.yaml
254+
255+
- it: rejects a non-positive autoscaling target
256+
templates:
257+
- templates/scaledobject.yaml
258+
set:
259+
autoscaling:
260+
enabled: true
261+
minReplicas: 2
262+
maxReplicas: 6
263+
target: 0
264+
quorum:
265+
maxSyncReplicas: 2
266+
asserts:
267+
- failedTemplate:
268+
errorMessage: "postgres: autoscaling.target must be a positive integer (the desired-replica formula divides the read load by it)."

‎packages/apps/postgres/values.schema.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@
218218
"default": false
219219
},
220220
"enabled": {
221-
"description": "Enable horizontal autoscaling of read replicas.",
221+
"description": "Enable horizontal autoscaling of read replicas. Requires the platform `keda` package to be enabled by an administrator first; the chart fails to render (rather than break the release) if KEDA is not installed.",
222222
"type": "boolean",
223223
"default": false
224224
},

‎packages/apps/postgres/values.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ quorum:
145145
##
146146

147147
## @typedef {struct} Autoscaling - Horizontal autoscaling of read replicas via KEDA. When active, `replicas` is no longer declared statically — a KEDA ScaledObject drives the CNPG instance count from read load.
148-
## @field {bool} enabled - Enable horizontal autoscaling of read replicas.
148+
## @field {bool} enabled - Enable horizontal autoscaling of read replicas. Requires the platform `keda` package to be enabled by an administrator first; the chart fails to render (rather than break the release) if KEDA is not installed.
149149
## @field {int} minReplicas - Minimum total instances; raised to the synchronous-quorum floor (`maxSyncReplicas + 1`) and to 2 when either is higher.
150150
## @field {int} maxReplicas - Maximum total instances; raised to the quorum floor if the floor exceeds it.
151151
## @field {ReadMetric} metric - Read-load signal to scale on.

0 commit comments

Comments
 (0)