fix(backups): keep the backupstrategy-controller Helm release under the Secret size limit (#4941)

## What this PR does

Fixes a size regression from #4929. Helm stores each release revision in
one Secret and the apiserver refuses a Secret over 1 MiB. With all
default Strategy CRs rendered the `backupstrategy-controller` release
crosses that limit, so the forced upgrade that creates them fails with
`Secret "sh.helm.release.v1.backupstrategy-controller.vN" is invalid:
data: Too long: may not be more than 1048576 bytes`, the CRDs stay and
the default strategies never appear. The release was already within
about 1 KB of the limit before #4929, which added tests to the chart.

The strategy CRDs are 2.2 MB of YAML and Helm stores them twice per
revision, as chart files and in the rendered manifest. Almost all of it
is the upstream `PodTemplateSpec` schema inlined by the Altinity, Job
and FoundationDB strategies. Kafka, RabbitMQ and Redis already mark
their `Template corev1.PodTemplateSpec` as `Schemaless` and
`PreserveUnknownFields`, this PR does the same for
`AltinitySpec.Template`, `JobSpec.Template` and
`FoundationDBTemplate.BackupDeploymentPodTemplateSpec`. Go types are
unchanged and every description is kept. The CRDs go from 2.2 MB to 317
KB and the largest release revision from over the limit to about 250 KB,
24% of it. The size rationale now lives once in the package doc, the
field docs of the six schemaless templates share one short line.

What changes for users: the apiserver no longer validates these three
fields and `kubectl explain` no longer lists children under them
(`kubectl explain pod.spec` still documents them), same as for Kafka,
RabbitMQ and Redis today. Unknown keys are stored instead of pruned, the
typed decode in the drivers drops them either way. Labels and
annotations under `template.metadata` were rejected by the strict v1.6
schema and are now accepted and kept. A Go template in a non-string slot
never worked, it was rejected on apply, now the apiserver accepts it and
the typed decode in the controller is what rejects it, as for the other
three kinds (I did not run that case live).

The drivers read these fields through the non-strict client and
`internal/template` rewrites only string leaves, nothing under
`internal/backupcontroller` relies on pruning. Go tests for
`internal/backupcontroller`, `internal/template` and `api` pass.

`hack/strategy-crd-schemaless.bats` fails on any strategy CRD that
inlines a pod schema and prints the property path and the two markers to
add. `hack/helm-release-size.bats` estimates the stored release of every
chart under `packages/` the way Helm encodes it (release JSON, gzip,
base64) and fails above 90% of the limit, plus the worst case of this
chart. The estimate runs 1 to 2 percent above real Secrets. A chart has
to render offline to be measured with its manifest: the 36 charts that
do not are listed in `NO_OFFLINE_RENDER` and measured on files and
templates only, any other render failure fails the test, and so does a
listed chart that starts rendering.

Checked on a throwaway kind cluster: v1.6.4 upgrades to this branch and
the forced follow-up creates all default strategies, a fresh install
works, strategy objects created under the old schema stay valid, a Job
strategy with templated args and pod template labels is accepted.

### Downstream repositories

- [x] No downstream repository is affected by this change

### Release note

```release-note
fix(backups): the backupstrategy-controller release no longer fails with "Secret ... Too long" on the upgrade that creates the default backup strategies, the Altinity, Job and FoundationDB strategies store their pod templates schemaless like Kafka, RabbitMQ and Redis
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Backup strategy templates for Altinity, FoundationDB, Jobs, Kafka,
RabbitMQ, and Redis now preserve fields not defined in the API schema.
The API server no longer validates these templates against the pod
schema; validation occurs when the driver creates the workload. This
lets strategies include pod-template fields without requiring those
fields to be listed in the strategy schema.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
myasnikovdaniil authored and GitHub committed 2026-10-21 14:27:15 +05:00
commit 3cd261277e
16 files changed
+367 -26507

No files matched your search

@@ -66,6 +66,13 @@ type AltinitySpec struct {
// exposes .Name, .Namespace, and
// .ApplicationRef.{APIGroup,Kind,Name} so to-copy
// restores can address the source release.
//
// Stored schemaless: the apiserver keeps it as written rather than
// checking it against the pod schema, and it is validated when the
// driver creates the workload from it.
//
// +kubebuilder:validation:Schemaless
// +kubebuilder:pruning:PreserveUnknownFields
Template corev1.PodTemplateSpec `json:"template"`
}
@@ -108,7 +108,14 @@ type FoundationDBTemplate struct {
// Admins use it to mount credentials, CA bundles, or any other
// per-tenant secrets the backup_agent needs. Templating is supported
// on every string field.
//
// Stored schemaless: the apiserver keeps it as written rather than
// checking it against the pod schema, and it is validated when the
// driver creates the workload from it.
//
// +optional
// +kubebuilder:validation:Schemaless
// +kubebuilder:pruning:PreserveUnknownFields
BackupDeploymentPodTemplateSpec *corev1.PodTemplateSpec `json:"backupDeploymentPodTemplateSpec,omitempty"`
}
@@ -15,6 +15,17 @@ limitations under the License.
*/
// Package v1alpha1 contains API Schema definitions for the v1alpha1 API group.
//
// Fields that embed a large upstream struct, a PodTemplateSpec above all, are
// marked Schemaless with PreserveUnknownFields. backupstrategy-controller
// renders these CRDs from templates, so Helm stores each one twice in every
// release revision, and a revision is one Secret capped at 1 MiB; with the
// upstream schemas inlined the revision that renders the default strategies
// no longer fits and that upgrade fails. The Go types stay typed, and the
// apiserver validates the content when a driver creates the workload from it.
// hack/strategy-crd-schemaless.bats fails on a strategy CRD that inlines a
// pod template schema.
//
// +kubebuilder:object:generate=true
// +groupName=strategy.backups.cozystack.io
package v1alpha1
@@ -61,6 +61,13 @@ type JobSpec struct {
// - `.Parameters` — the resolved BackupClass strategy parameters.
// - `.Backup` — only on restore runs: `.Backup.Name`, `.Backup.Namespace`
// and `.Backup.ApplicationRef` describing the source backup.
//
// Stored schemaless: the apiserver keeps it as written rather than
// checking it against the pod schema, and it is validated when the
// driver creates the workload from it.
//
// +kubebuilder:validation:Schemaless
// +kubebuilder:pruning:PreserveUnknownFields
Template corev1.PodTemplateSpec `json:"template"`
}
+4 -5
View File
@@ -71,11 +71,10 @@ type KafkaSpec struct {
// - `.Backup` — only on restore: `.Backup.Name`, `.Backup.Namespace` and
// `.Backup.ApplicationRef` describing the source backup.
//
// Stored schemaless deliberately: the full PodTemplateSpec OpenAPI schema is
// ~590 KB, and inlining it here (after Altinity/Job/FoundationDB) would push
// the chart's Helm state Secret over the 1 MiB cap. See job_types.go for the
// same field carried with its schema, and strategy-kafka-default.yaml for
// the rendered pod.
// Stored schemaless: the apiserver keeps it as written rather than
// checking it against the pod schema, and it is validated when the
// driver creates the workload from it. See
// strategy-kafka-default.yaml for the rendered pod.
//
// +kubebuilder:validation:Schemaless
// +kubebuilder:pruning:PreserveUnknownFields
@@ -83,13 +83,9 @@ type RabbitmqSpec struct {
// .ApplicationRef.{APIGroup,Kind,Name} so a to-copy
// restore can address the source release's object key.
//
// The template is stored schemaless (its content is preserved but not
// validated by the apiserver): the driver renders and validates the pod
// spec, and generating the full PodTemplateSpec OpenAPI schema here would
// add ~590 KB to this CRD. The backupstrategy-controller chart inlines
// every definitions/*.yaml into one Helm release, whose state Secret is
// capped at 1 MiB - a fourth full PodTemplateSpec schema (after Altinity,
// Job and FoundationDB) pushes that Secret over the cap and fails install.
// Stored schemaless: the apiserver keeps it as written rather than
// checking it against the pod schema, and it is validated when the
// driver creates the workload from it.
//
// +kubebuilder:validation:Schemaless
// +kubebuilder:pruning:PreserveUnknownFields
+3 -8
View File
@@ -76,14 +76,9 @@ type RedisSpec struct {
// and `.Backup.ApplicationRef` describing the source backup, so a
// to-copy restore reads the SOURCE release's object-storage prefix.
//
// Stored schemaless (content preserved, not validated by the apiserver): the
// driver renders and validates the pod spec, and generating the full
// PodTemplateSpec OpenAPI schema here would add ~590 KB to this CRD. The
// backupstrategy-controller chart inlines every definitions/*.yaml into one
// Helm release whose state Secret is capped at 1 MiB, so a full
// PodTemplateSpec schema here (a fifth after Altinity, Job, FoundationDB and
// Redis's own siblings) pushes that Secret over the cap and fails install.
// Same reason as rabbitmq_types.go.
// Stored schemaless: the apiserver keeps it as written rather than
// checking it against the pod schema, and it is validated when the
// driver creates the workload from it.
//
// +kubebuilder:validation:Schemaless
// +kubebuilder:pruning:PreserveUnknownFields
+155
View File
@@ -0,0 +1,155 @@
#!/usr/bin/env bats
# Helm keeps every release revision in one Secret, and the apiserver refuses a
# Secret over 1 MiB. A chart past that budget installs and then fails the one
# upgrade whose manifest is large enough ("Secret sh.helm.release.v1.<name>.vN
# is invalid: data: Too long"), and that release stays failed. The stored
# release carries the whole chart directory as it reaches helm-controller:
# the packages artifact applies no .helmignore, so tests/, images/ and the
# Makefile count too. hack/helm-release-size.py reproduces the encoding.
#
# The budget is 90 percent of the limit: the estimate runs about 1 percent
# above real Secrets, and the rest is room for a chart to grow by more than
# one change before anyone has to act.
#
# Harness compatibility note: CI runs this file under Bats through
# `make bats-unit-tests`. It also remains compatible with the narrower legacy
# `hack/cozytest.sh` translator, so it avoids `run`, `$status`, `$output`,
# `skip`, and setup()/teardown(). Paths stay repo-root-relative for both
# runners.
#
# Run with: bats hack/helm-release-size.bats
load test_helper
LIMIT=1048576
BUDGET=943718
# Charts that do not render offline with default values (required values,
# cluster lookups). They are measured on their stored files and templates
# alone, a lower bound. Any other chart must render: a render failure there
# would quietly shrink the measurement to the same lower bound. A chart that
# stops rendering offline on purpose goes here; one that starts rendering
# must come off, so the list never hides a renderable chart.
NO_OFFLINE_RENDER="
packages/apps/bucket
packages/apps/clickhouse
packages/apps/foundationdb
packages/apps/harbor
packages/apps/kafka
packages/apps/kubernetes
packages/apps/kubernetes-nodes
packages/apps/mariadb
packages/apps/mongodb
packages/apps/nats
packages/apps/openbao
packages/apps/opensearch
packages/apps/postgres
packages/apps/qdrant
packages/apps/rabbitmq
packages/apps/redis
packages/apps/tenant
packages/apps/valkey
packages/apps/vm-disk
packages/apps/vm-instance
packages/apps/vpc
packages/apps/vpn
packages/core/platform
packages/extra/bootbox
packages/extra/computeplane
packages/extra/etcd
packages/extra/external-dns
packages/extra/gateway
packages/extra/info
packages/extra/ingress
packages/extra/monitoring
packages/extra/seaweedfs
packages/library/cozy-lib
packages/system/kuberture
packages/system/telepresence
packages/tests/cozy-lib-tests
"
@test "the estimator counts chart files the way Helm stores them" {
tmp=$(mktemp -d)
mkdir -p "$tmp/chart/templates" "$tmp/chart/tests"
printf 'apiVersion: v2\nname: probe\nversion: 0.0.0\n' > "$tmp/chart/Chart.yaml"
# Incompressible, so the stored size cannot be smaller than its base64.
head -c 300000 /dev/urandom > "$tmp/chart/tests/blob"
size=$(printf '' | python3 -I hack/helm-release-size.py "$tmp/chart")
if [ "$size" -lt 400000 ]; then
echo "estimate $size for a chart carrying 300000 random bytes; files are not counted" >&2
false
fi
rm -rf "$tmp"
}
@test "every chart's stored release fits the budget with its default render" {
tmp=$(mktemp -d)
: > "$tmp/over"
: > "$tmp/render"
n=0
for c in packages/*/*/; do
c=${c%/}
[ -f "$c/Chart.yaml" ] || continue
n=$((n + 1))
listed=no
case "$NO_OFFLINE_RENDER" in *"
$c
"*) listed=yes ;; esac
if helm template release "$c" -n default > "$tmp/manifest" 2> "$tmp/err"; then
if [ "$listed" = yes ]; then
echo "$c renders offline now; remove it from NO_OFFLINE_RENDER in hack/helm-release-size.bats" >> "$tmp/render"
fi
elif [ "$listed" = yes ]; then
: > "$tmp/manifest"
else
echo "$c failed to render offline; fix the render, or add it to NO_OFFLINE_RENDER in hack/helm-release-size.bats if it needs cluster state: $(grep -m 1 '^Error' "$tmp/err" | cut -c1-300)" >> "$tmp/render"
continue
fi
size=$(python3 -I hack/helm-release-size.py "$c" < "$tmp/manifest")
if [ "$size" -gt "$BUDGET" ]; then
echo "$c: $size bytes (budget $BUDGET, limit $LIMIT)" >> "$tmp/over"
fi
done
if [ "$n" -lt 50 ]; then
echo "only $n charts found under packages/; the sweep checked almost nothing" >&2
false
fi
if [ -s "$tmp/render" ]; then
cat "$tmp/render" >&2
false
fi
if [ -s "$tmp/over" ]; then
echo "charts whose stored Helm release is over budget:" >&2
cat "$tmp/over" >&2
false
fi
rm -rf "$tmp"
}
@test "backupstrategy-controller fits the budget on its largest revision" {
# The default render gates every Strategy CR off. The revision that renders
# all of them, with the Velero location on as in production, is the one
# that grew past the limit once.
c=packages/system/backupstrategy-controller
tmp=$(mktemp -d)
# Rendered to a file rather than piped: in a pipe the estimator's status
# would stand for the render's, and a failed render would be measured as an
# empty manifest and pass.
if ! helm template backupstrategy-controller "$c" -n cozy-backup-controller \
--set backupStorage.bucketNameOverride=cozy-backups-0123 \
--set velero.bslEnabled=true > "$tmp/manifest"; then
echo "$c failed to render its largest revision; nothing was measured" >&2
false
fi
if ! grep -q '^kind: Kafka$' "$tmp/manifest"; then
echo "$c rendered without the default Strategy CRs; this is not its largest revision" >&2
false
fi
size=$(python3 -I hack/helm-release-size.py "$c" < "$tmp/manifest")
rm -rf "$tmp"
if [ "$size" -gt "$BUDGET" ]; then
echo "$c: $size bytes with every default Strategy rendered (budget $BUDGET, limit $LIMIT)" >&2
false
fi
}
+106
View File
@@ -0,0 +1,106 @@
#!/usr/bin/env python3
"""Estimate the size of the Secret in which Helm stores a release of a chart.
Helm's Secret storage driver keeps each revision as one Secret whose `release`
value is base64(gzip(json(release))), and the apiserver refuses a Secret whose
data exceeds 1 MiB. A chart that crosses it installs fine at first and then
fails the one upgrade whose rendered manifest is large enough, with "Secret
sh.helm.release.v1.<name>.vN is invalid: data: Too long". The stored release
carries the whole chart, not just what renders: every file under the chart dir
that is not Chart.yaml, values.yaml, values.schema.json, templates/ or charts/
ends up in chart.files, and the production path (the packages OCI artifact
loaded by helm-controller) applies no .helmignore, so tests/, images/ and the
Makefile are stored too.
This reproduces the encoding: the release JSON with the chart's templates and
files as base64 []byte, the rendered manifest read from stdin (`helm template`
output), gzip at Helm's BestCompression, then base64. It does not reproduce
Helm's gzip byte for byte or the runtime fields (info, config); measured
against real release Secrets it lands within about 0.5 percent, which is why
callers compare it to a threshold with headroom rather than to the limit.
Usage: helm template ... | helm-release-size.py <chart-dir> [--exclude GLOB ...]
Prints the estimated stored size in bytes.
"""
import base64
import fnmatch
import gzip
import json
import os
import sys
# Paths Helm keeps out of chart.files: they are stored elsewhere in the chart
# object (metadata, values, schema, templates) or are subcharts.
NOT_FILES = {"Chart.yaml", "Chart.lock", "values.yaml", "values.schema.json"}
NOT_FILE_DIRS = ("templates/", "charts/")
def go_json(obj):
# encoding/json escapes <, > and & inside strings; match it so the size
# does not drift with how much HTML-ish text a chart carries.
s = json.dumps(obj, separators=(",", ":"), ensure_ascii=False)
return s.replace("<", "\\u003c").replace(">", "\\u003e").replace("&", "\\u0026")
def main(argv):
if len(argv) < 2:
sys.stderr.write(__doc__)
return 2
chart = argv[1].rstrip("/")
excludes = []
rest = argv[2:]
while rest:
if rest[0] != "--exclude" or len(rest) < 2:
sys.stderr.write("unexpected argument: %s\n" % rest[0])
return 2
excludes.append(rest[1])
rest = rest[2:]
templates, files = [], []
for root, dirs, names in os.walk(chart):
dirs.sort()
for n in sorted(names):
path = os.path.join(root, n)
rel = os.path.relpath(path, chart)
if any(fnmatch.fnmatch(rel, g) for g in excludes):
continue
with open(path, "rb") as fh:
data = base64.b64encode(fh.read()).decode()
entry = {"name": rel, "data": data}
if rel.startswith("templates/"):
templates.append(entry)
elif rel in NOT_FILES or rel.startswith(NOT_FILE_DIRS):
continue
else:
files.append(entry)
def read(rel):
p = os.path.join(chart, rel)
return open(p).read() if os.path.exists(p) else ""
release = {
"name": "release",
"info": {"status": "deployed", "description": "Upgrade complete"},
"chart": {
"metadata": {"name": os.path.basename(chart), "version": "0.0.0", "apiVersion": "v2"},
"lock": None,
"templates": templates,
# Helm stores the parsed values and the user config; raw text with
# comments and no config errs high (about 6 KB on
# backupstrategy-controller), the safe side for a budget.
"values": read("values.yaml"),
"schema": base64.b64encode(read("values.schema.json").encode()).decode() or None,
"files": files,
},
"manifest": sys.stdin.read(),
"version": 1,
"namespace": "default",
}
raw = go_json(release).encode()
stored = base64.b64encode(gzip.compress(raw, compresslevel=9))
print(len(stored))
return 0
if __name__ == "__main__":
sys.exit(main(sys.argv))
+42
View File
@@ -0,0 +1,42 @@
#!/usr/bin/env bats
# A strategy CRD that inlines the upstream pod schema adds about 600 KB, and
# backupstrategy-controller stores each CRD twice in every Helm release
# revision, inside one Secret capped at 1 MiB. Three of them did once, and the
# upgrade that renders the default strategies failed with "Too long". Pod
# templates in api/backups/strategy are therefore Schemaless with
# PreserveUnknownFields (see the package doc there). hack/helm-release-size.bats
# catches the total; this names the field that caused it.
#
# Harness compatibility note: CI runs this file under Bats through
# `make bats-unit-tests`. It also remains compatible with the narrower legacy
# `hack/cozytest.sh` translator, so it avoids `run`, `$status`, `$output`,
# `skip`, and setup()/teardown(). Paths stay repo-root-relative for both
# runners.
#
# Run with: bats hack/strategy-crd-schemaless.bats
load test_helper
DEFS=packages/system/backupstrategy-controller/definitions
@test "no strategy CRD inlines a pod schema" {
tmp=$(mktemp -d)
n=0
for f in "$DEFS"/strategy.backups.cozystack.io_*.yaml; do
n=$((n + 1))
# A PodSpec is the object whose containers[] items carry an image field;
# report the property path that holds it, so the message names the field.
yq -N '[.. | select(tag == "!!map" and has("properties") and (.properties.containers.items.properties.image != null)) | path | join(".")] | .[]' "$f" \
| sed "s#^#$(basename "$f"): #" >> "$tmp/inlined"
done
if [ "$n" -lt 10 ]; then
echo "only $n strategy CRDs under $DEFS; nothing meaningful was checked" >&2
false
fi
if [ -s "$tmp/inlined" ]; then
echo "strategy CRDs inlining a pod schema; mark the Go field +kubebuilder:validation:Schemaless and +kubebuilder:pruning:PreserveUnknownFields:" >&2
cat "$tmp/inlined" >&2
false
fi
rm -rf "$tmp"
}
File diff suppressed because it is too large. Load diff
File diff suppressed because it is too large. Load diff
@@ -75,11 +75,10 @@ spec:
- `.Backup` — only on restore: `.Backup.Name`, `.Backup.Namespace` and
`.Backup.ApplicationRef` describing the source backup.
Stored schemaless deliberately: the full PodTemplateSpec OpenAPI schema is
~590 KB, and inlining it here (after Altinity/Job/FoundationDB) would push
the chart's Helm state Secret over the 1 MiB cap. See job_types.go for the
same field carried with its schema, and strategy-kafka-default.yaml for
the rendered pod.
Stored schemaless: the apiserver keeps it as written rather than
checking it against the pod schema, and it is validated when the
driver creates the workload from it. See
strategy-kafka-default.yaml for the rendered pod.
x-kubernetes-preserve-unknown-fields: true
required:
- template
@@ -88,13 +88,9 @@ spec:
.ApplicationRef.{APIGroup,Kind,Name} so a to-copy
restore can address the source release's object key.
The template is stored schemaless (its content is preserved but not
validated by the apiserver): the driver renders and validates the pod
spec, and generating the full PodTemplateSpec OpenAPI schema here would
add ~590 KB to this CRD. The backupstrategy-controller chart inlines
every definitions/*.yaml into one Helm release, whose state Secret is
capped at 1 MiB - a fourth full PodTemplateSpec schema (after Altinity,
Job and FoundationDB) pushes that Secret over the cap and fails install.
Stored schemaless: the apiserver keeps it as written rather than
checking it against the pod schema, and it is validated when the
driver creates the workload from it.
x-kubernetes-preserve-unknown-fields: true
required:
- template
@@ -70,14 +70,9 @@ spec:
and `.Backup.ApplicationRef` describing the source backup, so a
to-copy restore reads the SOURCE release's object-storage prefix.
Stored schemaless (content preserved, not validated by the apiserver): the
driver renders and validates the pod spec, and generating the full
PodTemplateSpec OpenAPI schema here would add ~590 KB to this CRD. The
backupstrategy-controller chart inlines every definitions/*.yaml into one
Helm release whose state Secret is capped at 1 MiB, so a full
PodTemplateSpec schema here (a fifth after Altinity, Job, FoundationDB and
Redis's own siblings) pushes that Secret over the cap and fails install.
Same reason as rabbitmq_types.go.
Stored schemaless: the apiserver keeps it as written rather than
checking it against the pod schema, and it is validated when the
driver creates the workload from it.
x-kubernetes-preserve-unknown-fields: true
required:
- template