Skip to content

nixos/postgresql: enrich ensure users and databases support#255961

Closed
Scrumplex wants to merge 13 commits intoNixOS:masterfrom
Scrumplex:nixos/postgresql/better-ensure
Closed

nixos/postgresql: enrich ensure users and databases support#255961
Scrumplex wants to merge 13 commits intoNixOS:masterfrom
Scrumplex:nixos/postgresql/better-ensure

Conversation

@Scrumplex
Copy link
Copy Markdown
Member

@Scrumplex Scrumplex commented Sep 18, 2023 •
edited
Loading

Description of changes

Based on #203474

TODOs

  • Update release notes
  • Add tests for ensureClauses

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Sep 18, 2023
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 18, 2023
@Scrumplex Scrumplex force-pushed the nixos/postgresql/better-ensure branch 7 times, most recently from 018e44e to fb401ad Compare September 18, 2023 21:04
@nixos-discourse
Copy link
Copy Markdown

nixos-discourse commented Sep 18, 2023

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/anybody-wanna-adopt-203474/25524/2

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Sep 18, 2023
@Scrumplex Scrumplex changed the title postgresql: enrich ensure users and databases support nixos/postgresql: enrich ensure users and databases support Sep 20, 2023
@Scrumplex Scrumplex force-pushed the nixos/postgresql/better-ensure branch from fb401ad to 9af7db2 Compare September 20, 2023 14:23
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog This PR adds or changes release notes labels Sep 23, 2023
SuperSandro2000
SuperSandro2000 reviewed Sep 26, 2023
View reviewed changes
nixos/tests/hedgedoc.nix Outdated
Comment on lines 36 to 50
Copy link
Copy Markdown
Member

@SuperSandro2000 SuperSandro2000 Sep 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why the dbname and owner don't match here and why not socket auth is used but I also don't have enough time to also maintain the tests of my packages. sigh. :(

@Scrumplex Scrumplex force-pushed the nixos/postgresql/better-ensure branch from e098c0a to 22e31c0 Compare September 30, 2023 18:07
@Scrumplex Scrumplex marked this pull request as ready for review September 30, 2023 18:08
@SuperSandro2000 SuperSandro2000 mentioned this pull request Oct 1, 2023
Merged
12 tasks
@emilylange emilylange mentioned this pull request Oct 22, 2023
Merged
13 tasks
marsam
marsam approved these changes Oct 25, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@marsam marsam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Thanks a lot!

nixos/modules/services/databases/postgresql.nix Outdated
Copy link
Copy Markdown
Contributor

@marsam marsam Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

args doesn't seem to be used. What do you think of removing it?

Copy link
Copy Markdown
Member Author

@Scrumplex Scrumplex Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

args is actually used a few lines below in ensureUsers

@marsam
Copy link
Copy Markdown
Contributor

marsam commented Oct 25, 2023 •
edited
Loading

@GrahamcOfBorg test postgresql postgresql-ensure

rycee and others added 2 commits October 25, 2023 19:12
@rycee @Scrumplex
nixos/postgresql: make ensureDatabases more complete
d2d0fe9 
Specifically, this commit makes it possible to set various database
parameters beside just the name.
@rycee @Scrumplex
nixos/postgresql: clean up ensure database and user code
54da349 
This reorganizes the ensure code generation to make the code a bit
less dense and also do general cleanups.

Most importantly, the order is changed to first create users, then
databases, and finally clauses and grants. This allows the database
creation to refer to new users as owners.

Co-authored-by: Sefa Eyeoglu <contact@scrumplex.net>
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
rycee and others added 11 commits October 25, 2023 19:12
@rycee @Scrumplex
nixos/postgresql: support setting password for ensured user
eaf9880
@rycee @Scrumplex
nixos/matrix-synapse: use ensure user and database options
4e10fda 
This replaces the explicit database setup script to instead use the
PostgreSQL `ensureUsers` and `ensureDatabases` options.
@Scrumplex
nixos/postgresql: add tests for ensure database and user
7e1e751 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/vaultwarden: use postgresql ensure options in test
72ac20f 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/freshrss: use postgresql ensure options in test
f79aae3 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/pgmanage: use postgresql ensure options in test
9d00a41 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/mastodon: use postgresql ensure options in test
f45cec1 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/documize: use postgresql ensure options in test
9749a94 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/invidious: use postgresql ensure options in test
5639173 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/hedgedoc: use postgresql ensure options in test
fe75ebb 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex
nixos/postgresql: add 23.11 release notes
86f9827 
Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex Scrumplex force-pushed the nixos/postgresql/better-ensure branch from 22e31c0 to 86f9827 Compare October 25, 2023 17:13
@happysalada happysalada mentioned this pull request Nov 6, 2023
Closed
@bendlas bendlas mentioned this pull request Nov 14, 2023
Merged
13 tasks
@bendlas
Copy link
Copy Markdown
Contributor

bendlas commented Nov 14, 2023 •
edited
Loading

Unfortunately, It looks like this will a bit late for helping us adapt to postgres 15 for 23.11. For now, we'll deprecate ensurePermissions and instead advise for ensureDBOwnership + the occasional raw postStart script in the manual.

I would very much like to give a proper look at this approach for 24.05, especially also look at ways this could tie into system - wide approach towards state management and migrations. Also I'd like to look for a way to get the benefits of ensure-style options, while avoiding the pitfalls of convergent configuration management

I wrote out my initial thoughts here: #267365

@Scrumplex
Copy link
Copy Markdown
Member Author

Scrumplex commented Dec 19, 2023

I'll close this as I don't really intend on working on this much further. Though feel free to continue this, as we don't have an alternative to convergent configuration management for databases in Nixpkgs atm.

@Scrumplex Scrumplex closed this Dec 19, 2023
@nixos-discourse
Copy link
Copy Markdown

nixos-discourse commented Dec 20, 2023

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/what-about-state-management/37082/1

@Scrumplex Scrumplex deleted the nixos/postgresql/better-ensure branch September 29, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

+1 more reviewer

@marsam marsam marsam approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Scrumplex @nixos-discourse @marsam @bendlas @SuperSandro2000 @rycee