Skip to content

fix(config): deep-merge extraConfig and reject reserved paths - #6

Merged
tdakkota merged 2 commits into
mainfrom
fix/extraconfig-deep-merge
Jul 28, 2026
Merged

fix(config): deep-merge extraConfig and reject reserved paths#6
tdakkota merged 2 commits into
mainfrom
fix/extraconfig-deep-merge

Conversation

@tdakkota

Copy link
Copy Markdown
Contributor

The bug

renderConfig merged spec.extraConfig over the generated config with maps.Copy(cfg, extra) — a
shallow, top-level copy. Because the operator renders everything that matters under a single
storage key, any extraConfig that mentions storage discarded the whole generated block:
backend, dir, cluster.etcd/rf/shards_per_tenant, s3 and the engine tuning.

Impact

Silent, and in the worst direction: an absent backend defaults to the ephemeral memory
backend (the PVC is mounted and unused, data is lost on restart) and an absent cluster block means
the node leaves the cluster (no etcd, no ring, no replication). Nothing flagged it — both
configs are legal on their own.

The fix

  1. Deep merge. extraConfig is merged recursively: nested objects merge key by key, everything
    else (scalars, lists, explicit nulls) replaces. extraConfig: {storage: {policy: ...}} now adds
    storage.policy and keeps backend, dir and cluster.
  2. Reserved paths are rejected, not merged. A deep merge alone still lets extraConfig
    overwrite storage.cluster.etcd or storage.backend, so the paths the operator renders from the
    spec are reserved: the signal backends, storage.backend/dir/wal_dir/s3, the whole
    storage.cluster subtree, and the storage.* engine tuning keys. Setting one fails the reconcile
    with an error naming the offending path and the spec field to use instead.
  3. Reported via the existing status machinery. Spec-level problems (reserved paths, unparsable
    extraConfig, missing etcd.endpoints, missing S3 bucket) are wrapped in a validationError and
    surface as Degraded with reason InvalidSpec, and are not requeued — retrying cannot fix them,
    a spec change re-triggers reconcile.

Docs: the extraConfig field godoc, the regenerated CRD and a README table of reserved paths.

Tests

Table tests for deepMerge and validateExtraConfig, plus renderConfig regressions asserting
that extraConfig: {storage: {policy: ...}} preserves backend/dir/cluster and that a reserved
path fails validation. make test and make lint pass.

Closes #1

🤖 Generated with Claude Code

@tdakkota
tdakkota force-pushed the fix/extraconfig-deep-merge branch from 13e90a0 to 894f9fa Compare July 28, 2026 11:39
tdakkota and others added 2 commits July 28, 2026 14:50
extraConfig was merged with maps.Copy, so any extraConfig mentioning
storage replaced the whole generated storage block: backend fell back to
memory, the cluster block vanished, and the node silently lost durability
and ring membership.

Merge recursively instead, and reject extraConfig that targets paths the
operator renders itself (signal backends, storage.backend/dir/wal_dir/s3,
storage.cluster and the engine tuning keys). Invalid specs now surface as
Degraded/InvalidSpec on the CR without being requeued.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@tdakkota
tdakkota force-pushed the fix/extraconfig-deep-merge branch from 894f9fa to 7e6b5ae Compare July 28, 2026 11:51
@tdakkota
tdakkota merged commit b8d7199 into main Jul 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(config): extraConfig shallow merge silently wipes the generated storage block

1 participant