fix(nats): merge config.merge.accounts with the generated accounts map by yankawai · Pull Request #4134 · cozystack/cozystack

europrinter (yankawai) · 2026-09-07T09:02:25Z

What this PR does

With users set, templates/nats.yaml rendered config.merge.accounts.A.users from the generated passwords and then appended .Values.config.merge verbatim. A tenant config.merge that also carried accounts, which is the only place to set accounts.A.jetstream for the generated account, produced the key twice in the HelmRelease values, and the install failed at post-render with mapping key "accounts" already defined.

The template now builds a single merge map: a deep copy of .Values.config.merge with the generated users merged in under accounts.A.users via mergeOverwrite. The generated user list wins for account A; other keys under accounts.A, other accounts and top-level keys such as max_payload are kept; a config.merge without users passes through unchanged; and no merge block is rendered when neither is set. The values contract does not change.

helm unittest: 67/67 in the chart. The new tests/config_merge_test.yaml covers the six shapes above; against the previous template the same suite fails on the three cases that combine users with config.merge.accounts.

Downstream repositories

Template and tests only: no values, schema, README or defaults change.

Release note

fix(nats): merge config.merge.accounts with the generated accounts instead of rendering the key twice

Summary by CodeRabbit

Bug Fixes
- Improved NATS configuration merging so generated user credentials and custom account settings are combined consistently.
- Preserved tenant-listed users, additional accounts, and top-level configuration during merges.
- Prevented empty merge sections from being rendered when no users or custom merge settings are configured.
Tests
- Added coverage for user generation, account merging, configuration preservation, and empty-configuration scenarios.

coderabbitai · 2026-09-07T09:02:44Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 79c448d5-ecc0-4c76-8393-aa5e5383e47e

📥 Commits

Reviewing files that changed from the base of the PR and between cd675e1 and e1800d4.

📒 Files selected for processing (2)

packages/apps/nats/templates/nats.yaml
packages/apps/nats/tests/config_merge_test.yaml

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Walkthrough

The NATS Helm template now deep-copies and combines tenant config.merge values with generated account users before rendering one merge block. New Helm chart tests cover account merging, key preservation, passthrough behavior, and empty configuration.

Changes

NATS config.merge rendering

Layer / File(s)	Summary
Merge configuration and validation `packages/apps/nats/templates/nats.yaml`, `packages/apps/nats/tests/config_merge_test.yaml`	The template combines generated users with `.Values.config.merge` and renders the result as one YAML block. Tests cover merged accounts, preserved users and keys, passthrough configuration, and omission when no merge values exist.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to e1800

This change prevents duplicate NATS account mappings when generated users and tenant merge configuration are combined, preserving tenant account settings in the rendered HelmRelease. The covered rendering cases indicate no remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary change: merging config.merge.accounts with the generated accounts map in the NATS chart.
Linked Issues check	✅ Passed	The template now merges generated users with config.merge, preserves tenant account settings such as accounts.A.jetstream, avoids duplicate accounts keys, and adds tests for the required configuration…
Out of Scope Changes check	✅ Passed	The changes are limited to the NATS template and focused Helm chart tests. They directly support the linked issue and do not introduce unrelated code, configuration, documentation, or downstream chang…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

scooby87

LGTM (independent cozy-review pass).

The deep-merge is correct: mergeOverwrite $merge lets the generated users list win for the managed account (so a tenant cannot inject a stray user or override generated passwords) while preserving the tenant other keys (jetstream, other accounts, top-level). deepCopy .Values.config.merge before mutating protects .Values. Range order over the passwords map is sorted, so no reconcile churn.

The new test suite is mutation-verified: reverting the template to HEAD~1 turns it RED with the exact bug symptom (mapping key accounts already defined), green after. helm unittest 67/67.

Non-blocking: tenant accounts.<a>.users are silently dropped in favour of the generated ones (correct security posture) — a doc note would help; and the upgrade path re-serialises spec.values once (cosmetic, structurally identical, no pod restart).

With users set the chart emitted accounts.A.users from the generated passwords and then appended .Values.config.merge verbatim. A tenant that also put an accounts block into config.merge, which is the only place to enable JetStream for the generated account, got the key twice in the rendered values and the release failed at post-render. The generated users are now merged into config.merge before it is rendered: the generated list wins for account A, everything else the tenant put under config.merge is kept, and a config.merge without users passes through unchanged. Signed-off-by: Yan Bondarenko <[email protected]>

github-actions · 2026-09-18T09:44:43Z

Successfully created backport PR for release-1.6:

[Backport release-1.6] fix(nats): merge config.merge.accounts with the generated accounts map #4321

…e generated accounts map (#4321) # Description Backport of #4134 to `release-1.6`.

europrinter (yankawai) requested review from IvanHunters, Andrei Kvapil (kvaps), Aleksei Sviridkin (lexfrei), Timofei Larkin (lllamnyp), myasnikovdaniil and scooby87 as code owners September 7, 2026 09:02

github-actions Bot added area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review kind/bug Categorizes issue or PR as related to a bug size/L This PR changes 100-499 lines, ignoring generated files labels Sep 7, 2026

europrinter (yankawai) force-pushed the fix/nats-config-merge-accounts branch from e1800d4 to 2e07995 Compare September 9, 2026 14:54

europrinter (yankawai) requested a review from Matthieu ROBIN (matthieu-robin) as a code owner September 9, 2026 14:54

scooby87 approved these changes Sep 17, 2026

View reviewed changes

Aleksei Sviridkin (lexfrei) mentioned this pull request Sep 18, 2026

ci: a re-run or late approval of a fork PR run tests a tree that exists in no branch #4312

Open

Aleksei Sviridkin (lexfrei) added the kind/backport Categorizes issue or PR as requiring a backport to the current release line label Sep 18, 2026

Aleksei Sviridkin (lexfrei) force-pushed the fix/nats-config-merge-accounts branch from 2e07995 to 334f6ad Compare September 18, 2026 09:24

github-actions Bot added the area/database Issues or PRs related to managed databases (postgres, mariadb, redis, etcd, kafka, clickhouse) label Sep 18, 2026

Aleksei Sviridkin (lexfrei) merged commit 301a7fe into cozystack:main Sep 18, 2026
15 of 16 checks passed

github-actions Bot mentioned this pull request Sep 18, 2026

[Backport release-1.6] fix(nats): merge config.merge.accounts with the generated accounts map #4321

Merged

myasnikovdaniil added a commit that referenced this pull request Sep 23, 2026

[Backport release-1.6] fix(nats): merge config.merge.accounts with th…

f6c8f90

…e generated accounts map (#4321) # Description Backport of #4134 to `release-1.6`.

Navigation Menu

Repository navigation

fix(nats): merge config.merge.accounts with the generated accounts map - #4134

fix(nats): merge config.merge.accounts with the generated accounts map#4134
Aleksei Sviridkin (lexfrei) merged 1 commit into
cozystack:mainfrom
yankawai:fix/nats-config-merge-accounts

europrinter (yankawai) commented Sep 7, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 7, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

scooby87 left a comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

europrinter (yankawai) commented Sep 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Downstream repositories

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

scooby87 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

europrinter (yankawai) commented Sep 7, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 7, 2026 •

edited

Loading