Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

nixos/mysql: declarative users & databases #29387

Merged
merged 1 commit into from
Sep 18, 2017
Merged

nixos/mysql: declarative users & databases #29387

merged 1 commit into from
Sep 18, 2017
+62 −0

Conversation

florianjacob
Copy link
Contributor

@florianjacob florianjacob commented Sep 14, 2017 •
edited
Loading

using Unix socket authentication, ensured on every rebuild.

Motivation for this change

I want to be able to declaratively manage the MySQL / MariaDB databases and users I need for the standard web apps like wordpress, nextcloud, piwik et cetera. This can be used by end users as well as package maintainer.

This helps #29031 by allowing to easily create a MySQL user for backing up, with reduced permissions.

This goes in the direction of and could supersede #6963 , but with scope reduced to local, Unix socket authenticated users.

I tried to circumvent the issue of un-synchronized state between nix configuration and actual databases by calling this ensureDatabases / ensureUsers and making it clear that this only ensures the databases and users are there, but does nothing to delete or rename databases and users or to reduce permissions that were once given. This will be up to the user, to prevent e.g. accidental deletion of databases and generally breaking things.

(Also, I don't even know how the state of which databases existed before and after rebuilds could be diffed and updated accordingly.)

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@florianjacob
nixos/mysql: declarative users & databases
3636cbe 
using Unix socket authentication, ensured on every rebuild.
This was referenced Sep 14, 2017
Closed
Closed
@davidak
Copy link
Member

davidak commented Sep 14, 2017

👍 Looks good, thanks!

fadenb
fadenb approved these changes Sep 15, 2017
View reviewed changes
Copy link
Contributor

@fadenb fadenb left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM
I did not yet test it on a system with existing DBs and users but will try to to that later today.
Works exactly as expected 👍

@joachifm joachifm added 0.kind: enhancement Add something new 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 17, 2017
@fpletz fpletz added this to the 17.09 milestone Sep 18, 2017
@fpletz fpletz merged commit 839e3c7 into NixOS:master Sep 18, 2017
@fpletz
Copy link
Member

fpletz commented Sep 18, 2017

Thanks!

@florianjacob florianjacob deleted the mysql-declarative-setup branch September 18, 2017 11:14
@rvl rvl mentioned this pull request Sep 24, 2017
Merged
6 tasks
@danbst
Copy link
Contributor

danbst commented Nov 6, 2017

I don't use MySQL, but I've stumbled on this:

... IDENTIFIED WITH ${if mysql == pkgs.mariadb then "unix_socket" else "auth_socket"};"

I think it's a bit flaky check. What if I use mariadb from nixpkgs-unstable, but rest of the system is nixos-17.09? It will be detected as mysql. Perhaps better to check (builtins.parseDrvName (mysql.name)).name == "mariadb"

@florianjacob
Copy link
Contributor Author

@danbst you're totally right, did not think about that case. Your proposal with parseDrvName sounds much better, and should be a drop-in replacement for the current condition. Do you want to pose the PR, or should I do it?

@danbst danbst mentioned this pull request Nov 14, 2017
Merged
8 tasks
@ravloony
Copy link

This is cool. I'm going to use this in #31475 instead of the stateful workaround I have now.

@florianjacob
Copy link
Contributor Author

@ravloony thank you! Yes, looks like a good fit. And more usage examples in NixOS services apart from mysql-backup will probably help more people discovering and adopting this. 😄

@ravloony
Copy link

@florianjacob Done. Feel free to have a look and see if there is anything I could do better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fpletz fpletz

@fadenb fadenb

Assignees
No one assigned
Labels
0.kind: enhancement Add something new 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/`
Projects
None yet
Milestone
17.09
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@florianjacob @davidak @fpletz @danbst @ravloony @fadenb @joachifm