[Backport release-1.6] fix(postgres): refuse system databases and never drop them by mattia-eleuteri · Pull Request #5157 · cozystack/cozystack

mattia-eleuteri · 2026-10-29T09:46:39Z

What this PR does

Backport of #5155 to release-1.6. The cherry-pick conflicted, because the 1.6 init-script.yaml predates the username and database-name guards and the role passes of main, so the change was applied by hand. The commit keeps the cherry-pick reference.

It does the same three things as on main. The render refuses postgres, template0 and template1 in databases. The delete pass never selects them for DROP DATABASE. A new release system databases pass hands a system database a previous version took over back to the superuser: it drops the event trigger and its function, runs REASSIGN OWNED / DROP OWNED for <db>_admin and <db>_readonly in that database, gives public back to pg_database_owner (PostgreSQL 15+) and clears the comment. The orphaned-roles pass then drops the roles. The SQL of the new pass is identical to main.

Differences from main:

The refusal is a loop of its own, since 1.6 has no database-name guard to sit next to.
The databases description in values.yaml is not changed, which avoids regenerating the README, schema, Go types and cozyrd on this branch.

Testing

tests/system_databases_test.yaml is the suite from main, unchanged. helm unittest packages/apps/postgres passes 56 of 56; with the release-1.6 template, 5 of the 6 new cases fail.
I replayed the scenario with this branch's rendered init.sh against a postgres:17 container:
1. The release-1.6 script with databases: {appdb, template1, postgres} takes over both system databases.
2. Removing them makes the release-1.6 script fail with cannot drop the currently open database.
3. This branch's script then exits 0. It leaves postgres and template1 owned by postgres with no comment, and drops their four roles. In a new database, CREATE SCHEMA succeeds and no event trigger exists.
4. A second run is a no-op.

Upgrade note (as on main): a release that declares one of the three names fails to render until the entry is removed, and removing it is now safe.

Screenshots

Downstream repositories

Release note

fix(postgres): backport to 1.6. databases now refuses the PostgreSQL system databases postgres, template0 and template1, and the init job never runs DROP DATABASE on them. A system database a previous version took over is handed back to the superuser on the next upgrade. A release that declares one of them must remove the entry before upgrading.

postgres, template0 and template1 were accepted in databases and handled like an application database: the init job made <db>_admin their owner, installed the auto-grant event trigger in them and, once the entry left values, ran DROP DATABASE on them. That DROP can only fail (template databases cannot be dropped, and the job is connected to postgres), so every later upgrade failed in the init job and the release stayed in UpgradeFailed even after the entry was put back. template1 also passed the takeover on to every database created afterwards, and the trigger left behind named a dropped role, so CREATE SCHEMA failed in them. The render now refuses the three names. The delete pass never selects them, and a system database a previous version took over is handed back to the superuser instead: the trigger and its function are dropped, what the <db>_admin and <db>_readonly roles own there is reassigned, public returns to pg_database_owner and the comment is cleared, after which the orphaned-roles pass drops the roles. A release that declares one of them has to remove the entry before it upgrades. This branch has no database-name guard yet, so the refusal sits in its own loop, and the values description is left as it is. Assisted-by: LLM (cherry picked from commit 0407214) Signed-off-by: Mattia Eleuteri <[email protected]>

coderabbitai · 2026-10-29T09:46:47Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: cozystack/cozystack
Branch: release-1.6
File: .coderabbit.yaml
Commit: bf4dfdd
Layers: Repository YAML
Review profile: CHILL
Plan: Advanced
Run ID: 633dd490-54c7-46cc-bd86-430f79c5162e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

Trigger review

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

…ases-1.6

mattia-eleuteri · 2026-11-02T10:10:26Z

CI note: the remaining red checks come from this PR being opened from a fork, not from its content. Build Talos (and Build packages/apps/kubernetes on #5154) fail when pushing the PR image to iad.ocir.io/.../cozystack: denied: Anonymous users are only allowed read access, as fork PRs get no registry credentials. E2E Tests depend on those images. Unit & controller tests pass since the branch was updated with release-1.6 (they failed before on an unrelated tenant chart test). A maintainer re-run from a branch of this repository, or pushing the backport branch here, would give the full signal.

…them to the superuser The release pass reassigned what <db>_admin and <db>_readonly owned in a taken-over system database to the superuser. A tenant that held <db>_admin could have left a SECURITY DEFINER function there, which would then run with superuser rights for anyone PUBLIC can execute it. The objects now go to a NOLOGIN cozystack:orphaned role and are made SECURITY INVOKER, and the database and public get their owners back separately. A database created from template1 while it was taken over inherited the auto-grant trigger naming template1_admin, and objects owned by that role. On this branch the orphaned-roles pass only runs in postgres, so DROP ROLE failed on them; releasing template1 now clears both in every other database. Assisted-by: LLM (cherry picked from commit 482b290) Signed-off-by: Mattia Eleuteri <[email protected]>

A database created from template1 while it was taken over inherited a public schema owned by template1_admin. The orphaned-roles pass then handed it to cozystack:orphaned, leaving the database owner without CREATE on it from PostgreSQL 15 on. Releasing template1 now returns that schema to pg_database_owner (the superuser before 15) while template1_admin still owns it. Assisted-by: LLM (cherry picked from commit c0fb063) Signed-off-by: Mattia Eleuteri <[email protected]>

## What this PR does `databases` accepts `postgres`, `template0` and `template1`, and the init job treats them like application databases: it comments them `database managed by helm`, creates `<db>_admin` and `<db>_readonly`, makes `<db>_admin` the owner of the database and its schemas, and installs the `trigger_auto_grant` event trigger. When the entry later leaves values, the delete pass runs `DROP DATABASE` on it, which can only fail: - `template1`, `template0`: `cannot drop a template database`; - `postgres`: `cannot drop the currently open database`, since the job is connected to it. The init job then fails on every upgrade, so the HelmRelease sits in `UpgradeFailed` with `post-upgrade hooks failed`, and putting the entry back does not help. `template1` also hands the takeover (owner of `public`, default privileges, trigger) to every database created afterwards. Once its roles are dropped by hand, the trigger left behind names a role that no longer exists, so `CREATE SCHEMA` fails in every new database, even as superuser. This PR: - **Refuses** the three names at render with a message that says what to do. `cozyvalues-gen` has no way to constrain the keys of a `map[string]Database`, so the refusal sits next to the existing database-name guard, like the `users.postgres` refusal. The field description says it too. - **Never selects them for `DROP DATABASE`.** The delete pass excludes them whatever values say. - **Hands a taken-over system database back.** In a new `release system databases` pass before the delete pass, for each of the three still commented `database managed by helm`: drop the trigger and `auto_grant_schema_privileges()`, `REASSIGN OWNED` / `DROP OWNED` for its two roles in that database, give `public` back to `pg_database_owner` (PostgreSQL 15+), and clear the comment. The existing orphaned-roles pass then drops the roles; it never visits `template1`, hence the per-database step. `template0` accepts no connection and the job that commented it never got further, so only its comment is cleared. **Upgrade note.** A release that declares one of the three names fails to render until the entry is removed. Removing it is now safe: the next run releases the database instead of dropping it. **Testing** - `tests/system_databases_test.yaml`: 5 of its 6 cases fail on `main` without the change. `helm unittest packages/apps/postgres`: 215 passed. - I replayed the scenario against a `postgres:17` container, running the rendered `init.sh` with `PGUSER=postgres`: 1. The `main` script with `databases: {appdb, template1, postgres}` reproduces the takeover. 2. The `main` script with only `appdb` then fails with `cannot drop the currently open database`. 3. This branch's script on that same state exits 0. It releases both databases (owner `postgres`, no comment, no event trigger, `public` owned by `pg_database_owner`) and drops `postgres_admin`, `postgres_readonly`, `template1_admin` and `template1_readonly`. A database created afterwards gets no trigger, and `CREATE SCHEMA` succeeds in it. 4. A second run is a no-op. 5. A `template0` commented and given roles by hand is released the same way. Open PR #5091 touches the same script (stable database order); the hunks do not overlap. Backport: the automatic cherry-pick onto `release-1.6` conflicts in `init-script.yaml` and the generated cozyrd, so the backport to `release-1.6` is done by hand in #5157 (same SQL, replayed against that branch's script). `release-1.7` keeps `kind/backport`. ### Screenshots  ### Downstream repositories  - [x] No downstream repository is affected by this change (walked the map: a template guard, the init script, a test suite and one field description regenerated into README, schema, Go types and cozyrd; no field, default, enum or name change) - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note  ```release-note fix(postgres): databases now refuses the PostgreSQL system databases postgres, template0 and template1, and the init job never runs DROP DATABASE on them. A system database a previous version took over is handed back to the superuser on the next upgrade (event trigger removed, ownership and public schema restored, its <db>_admin and <db>_readonly roles dropped). A release that declares one of them must remove the entry before upgrading. ```   ## Summary by CodeRabbit * **Behavior Changes** * PostgreSQL system databases (`postgres`, `template0`, and `template1`) are refused as managed application databases. * Previously managed system databases are released during cleanup and excluded from database deletion. Cleanup restores eligible ownership and security settings and clears system database comments. * **Documentation** * Updated PostgreSQL configuration guidance to clarify that these system database names are refused. Names that merely contain these strings remain accepted.

## What this PR does `databases` accepts `postgres`, `template0` and `template1`, and the init job treats them like application databases: it comments them `database managed by helm`, creates `<db>_admin` and `<db>_readonly`, makes `<db>_admin` the owner of the database and its schemas, and installs the `trigger_auto_grant` event trigger. When the entry later leaves values, the delete pass runs `DROP DATABASE` on it, which can only fail: - `template1`, `template0`: `cannot drop a template database`; - `postgres`: `cannot drop the currently open database`, since the job is connected to it. The init job then fails on every upgrade, so the HelmRelease sits in `UpgradeFailed` with `post-upgrade hooks failed`, and putting the entry back does not help. `template1` also hands the takeover (owner of `public`, default privileges, trigger) to every database created afterwards. Once its roles are dropped by hand, the trigger left behind names a role that no longer exists, so `CREATE SCHEMA` fails in every new database, even as superuser. This PR: - **Refuses** the three names at render with a message that says what to do. `cozyvalues-gen` has no way to constrain the keys of a `map[string]Database`, so the refusal sits next to the existing database-name guard, like the `users.postgres` refusal. The field description says it too. - **Never selects them for `DROP DATABASE`.** The delete pass excludes them whatever values say. - **Hands a taken-over system database back.** In a new `release system databases` pass before the delete pass, for each of the three still commented `database managed by helm`: drop the trigger and `auto_grant_schema_privileges()`, `REASSIGN OWNED` / `DROP OWNED` for its two roles in that database, give `public` back to `pg_database_owner` (PostgreSQL 15+), and clear the comment. The existing orphaned-roles pass then drops the roles; it never visits `template1`, hence the per-database step. `template0` accepts no connection and the job that commented it never got further, so only its comment is cleared. **Upgrade note.** A release that declares one of the three names fails to render until the entry is removed. Removing it is now safe: the next run releases the database instead of dropping it. **Testing** - `tests/system_databases_test.yaml`: 5 of its 6 cases fail on `main` without the change. `helm unittest packages/apps/postgres`: 215 passed. - I replayed the scenario against a `postgres:17` container, running the rendered `init.sh` with `PGUSER=postgres`: 1. The `main` script with `databases: {appdb, template1, postgres}` reproduces the takeover. 2. The `main` script with only `appdb` then fails with `cannot drop the currently open database`. 3. This branch's script on that same state exits 0. It releases both databases (owner `postgres`, no comment, no event trigger, `public` owned by `pg_database_owner`) and drops `postgres_admin`, `postgres_readonly`, `template1_admin` and `template1_readonly`. A database created afterwards gets no trigger, and `CREATE SCHEMA` succeeds in it. 4. A second run is a no-op. 5. A `template0` commented and given roles by hand is released the same way. Open PR #5091 touches the same script (stable database order); the hunks do not overlap. Backport: the automatic cherry-pick onto `release-1.6` conflicts in `init-script.yaml` and the generated cozyrd, so the backport to `release-1.6` is done by hand in #5157 (same SQL, replayed against that branch's script). `release-1.7` keeps `kind/backport`. ### Screenshots  ### Downstream repositories  - [x] No downstream repository is affected by this change (walked the map: a template guard, the init script, a test suite and one field description regenerated into README, schema, Go types and cozyrd; no field, default, enum or name change) - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note  ```release-note fix(postgres): databases now refuses the PostgreSQL system databases postgres, template0 and template1, and the init job never runs DROP DATABASE on them. A system database a previous version took over is handed back to the superuser on the next upgrade (event trigger removed, ownership and public schema restored, its <db>_admin and <db>_readonly roles dropped). A release that declares one of them must remove the entry before upgrading. ```   ## Summary by CodeRabbit * **Behavior Changes** * PostgreSQL system databases (`postgres`, `template0`, and `template1`) are refused as managed application databases. * Previously managed system databases are released during cleanup and excluded from database deletion. Cleanup restores eligible ownership and security settings and clears system database comments. * **Documentation** * Updated PostgreSQL configuration guidance to clarify that these system database names are refused. Names that merely contain these strings remain accepted.  Backported-from: b53ca1f

mattia-eleuteri mentioned this pull request Oct 29, 2026

fix(postgres): refuse system databases and never drop them #5155

Merged

1 of 12 tasks

Merge branch 'release-1.6' into backport/postgres-refuse-system-datab…

1878376

…ases-1.6

mattia-eleuteri marked this pull request as ready for review November 2, 2026 10:10

mattia-eleuteri requested review from IvanHunters, Andrey Kolkov (androndo), Andrei Kvapil (kvaps), Aleksei Sviridkin (lexfrei), Timofei Larkin (lllamnyp), myasnikovdaniil and Kirill Ilin (sircthulhu) as code owners November 2, 2026 10:10

mattia-eleuteri added 2 commits November 2, 2026 11:28

Navigation Menu

Repository navigation

[Backport release-1.6] fix(postgres): refuse system databases and never drop them - #5157

[Backport release-1.6] fix(postgres): refuse system databases and never drop them#5157
mattia-eleuteri wants to merge 4 commits into
cozystack:release-1.6from
mattia-eleuteri:backport/postgres-refuse-system-databases-1.6

mattia-eleuteri commented Oct 29, 2026

Uh oh!

coderabbitai Bot commented Oct 29, 2026 •

edited

Loading

Review skipped

Uh oh!

mattia-eleuteri commented Nov 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

mattia-eleuteri commented Oct 29, 2026

What this PR does

Screenshots

Downstream repositories

Release note

Uh oh!

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

Uh oh!

Review skipped

Uh oh!

mattia-eleuteri commented Nov 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented Oct 29, 2026 •

edited

Loading