nixos/mysql: fix typing-induced bugs #58702

florianjacob · 2019-04-01T19:12:35Z

Motivation for this change

Fixes issues brought up by @schneefux in #58582.

This also tries to prevent mysql.initialScript from ending up in /nix/store when given as nix path type, i.e. initialScript = /etc/nixos/setup-credentials.sql in contrast to initialScript = "/etc/nixos/setup-credentials.sql";, as it might be used for sensitive data like credentials. Uses toString for this, as explained here: https://stackoverflow.com/questions/43850371/when-does-a-nix-path-type-make-it-into-the-nix-store-and-when-not

Things done

and increase test coverage to catch this

which was wrongly specified as types.lines Prevent it from getting copied to nix store as people might use it for credentials, and make the tests cover it.

florianjacob · 2019-04-01T19:28:39Z

@schneefux can you give this a test, please? I don't personally use those two features, which is why those errors slipped through. Therefore tried to improve test coverage for those.

This should also fix the issue you would have come up with next, as initialScript was wrongly typed by me as multiline as I thought it's something like extraConfig, while it is actually a file. Should also stop initialScript from turning up in /nix/store, as you brought to my mind that people will likely store credentials in it as there is no other way to create users with passwords declaratively.

florianjacob · 2019-04-01T19:28:56Z

@GrahamcOfBorg test mysql

schneefux · 2019-04-05T16:50:45Z

Yeah, that works. Good job, thank you!

aanderse

nixpkgs/nixos/modules/services/databases/mysql.nix

Lines 366 to 375 in 14571f5

    
                                     # TODO: this silently falls through if database.schema does not exist, 
        
                                     # we should catch this somehow and exit, but can't do it here because we're in a subshell. 
        
                                     if [ -f "${database.schema}" ] 
        
                                     then 
        
                                         cat ${database.schema} 
        
                                     elif [ -d "${database.schema}" ] 
        
                                     then 
        
                                         cat ${database.schema}/mysql-databases/*.sql 
        
                                     fi 
        
                                     ''}

Can you please provide some documentation explaining how the user can make use of line 373? Seems like a useful feature that most people wouldn't even know exists.

flokli · 2019-05-21T09:34:18Z

Ping :-)

Ran into the current breakage at #57550 (comment) too.

infinisil · 2019-05-24T17:28:48Z

I'll merge this for now

@aanderse This can be done in a follow-up PR. For now fixing this regression is more important.

aanderse · 2019-05-24T17:30:54Z

@infinisil @flokli Yes, sorry. This sorta fell off my radar and I should have circled back. Thanks.

florianjacob · 2019-06-13T20:19:25Z

Can you please provide some documentation explaining how the user can make use of line 373? Seems like a useful feature that most people wouldn't even know exists.

@aanderse sorry for my absence, and thanks for your review. While you're right and I think know how that option is intended, I never used it and will therefore refrain from trying to improve it, as this was how I introduced this regression in the first place. :/

aanderse · 2019-06-13T20:57:02Z

@florianjacob no problem, fair enough.

Florian Jacob added 2 commits April 1, 2019 20:01

nixos/mysql: fix support for non-specified database schema

77978c1

and increase test coverage to catch this

nixos/mysql: fix initialScript option

14571f5

which was wrongly specified as types.lines Prevent it from getting copied to nix store as people might use it for credentials, and make the tests cover it.

florianjacob requested a review from infinisil as a code owner April 1, 2019 19:12

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Apr 1, 2019

aanderse requested changes Apr 14, 2019

View reviewed changes

infinisil merged commit f631167 into NixOS:master May 24, 2019

schneefux mentioned this pull request May 27, 2019

mysql.initialDatabases.….schema is not of type null or path #58582

Closed

florianjacob deleted the fix-mysql branch June 13, 2019 20:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/mysql: fix typing-induced bugs #58702

nixos/mysql: fix typing-induced bugs #58702

florianjacob commented Apr 1, 2019 •

edited

florianjacob commented Apr 1, 2019

florianjacob commented Apr 1, 2019

schneefux commented Apr 5, 2019

aanderse left a comment

flokli commented May 21, 2019

infinisil commented May 24, 2019

aanderse commented May 24, 2019

florianjacob commented Jun 13, 2019

aanderse commented Jun 13, 2019

	# TODO: this silently falls through if database.schema does not exist,
	# we should catch this somehow and exit, but can't do it here because we're in a subshell.
	if [ -f "${database.schema}" ]
	then
	cat ${database.schema}
	elif [ -d "${database.schema}" ]
	then
	cat ${database.schema}/mysql-databases/*.sql
	fi
	''}

nixos/mysql: fix typing-induced bugs #58702

nixos/mysql: fix typing-induced bugs #58702

Conversation

florianjacob commented Apr 1, 2019 • edited

Motivation for this change

Things done

florianjacob commented Apr 1, 2019

florianjacob commented Apr 1, 2019

schneefux commented Apr 5, 2019

aanderse left a comment

Choose a reason for hiding this comment

flokli commented May 21, 2019

infinisil commented May 24, 2019

aanderse commented May 24, 2019

florianjacob commented Jun 13, 2019

aanderse commented Jun 13, 2019

florianjacob commented Apr 1, 2019 •

edited