mxgross / EasyBackupBundle

Kimai 2 Plugin for easier backups
MIT License
24 stars 6 forks source link

Path to mysqldump #17

Closed aptevo closed 4 years ago

aptevo commented 4 years ago

I had to change the path to mysqldump to include --force and -h 127.0.0.1 to make it run fine.

mxgross commented 4 years ago

Have you tried setting it in the Kimai Settings page? Does it work? I appended the documentation with a screenshot now:

image

For me it worked without the -n and --force parameters. If this is a issue on more systems I will add some information to the documentation or change the default value for the mysqldump command.

kevinpapst commented 4 years ago

Depending on the permissions the flag --single-transaction might be required as well. Can the params be included in the config (the label "path to mysqldump sounds different)?

mxgross commented 4 years ago

I got another issue were the problem was a database URL like this ATABASE_URL="mysql://kimai2:password@localhost:3307/kimai2?unix_socket=/run/mysqld/mysqld10.sock"

It is no good way how I implemented getting the dbName in Line https://github.com/mxgross/EasyBackupBundle/blob/89570098af66a6f73099f36f54b7ca6218554fb2/Controller/EasyBackupController.php#L252

It must be hardened.

kevinpapst commented 4 years ago

What about you provide a default configuration, which is only full string to be executed with some replacers, eg: /usr/bin/mysqldump -h %host% -u %user% %database% Your plugin will replace these strings with what you detected.

But if the user has different needs, the command can be fully customized and even the replacer could be deleted and replaced with own settings, eg I could use /usr/bin/mysqldump -sock /run/mysqld/mysqld10.sock --single-transaction %database%

That would be much more flexible.

mxgross commented 4 years ago

That's a good idea, thanks, I will do it that way. To prevent users to use this configurable command for other stuff, do you think this is a good thing to add too: https://www.php.net/manual/en/function.escapeshellarg.php Or is there a better way? Currently there is no check for the user input: https://github.com/mxgross/EasyBackupBundle/blob/89570098af66a6f73099f36f54b7ca6218554fb2/Controller/EasyBackupController.php#L255

mxgross commented 4 years ago

I tried to do this

        $rootNode
            ->addDefaultsIfNotSet()
            ->children()
                ->scalarNode('setting_mysqldump_command')
                    ->defaultValue('/usr/bin/mysqldump -u %user% -p %password% -h %host% -port %port% --single-transaction --force %database%')

But kimai interprets this already as parameter. How can I avoid this?

The service "App\Configuration\SystemConfiguration" has a dependency on a non-existent parameter "user". Did you mean one of these: "doctrine.orm.security.user.provider.class", "security.authentication.hide_user_not_found", "fos_user.backend_type_orm", "fos_user.security.interactive_login_listener.class", "fos_user.security.login_manager.class", "fos_user.resetting.email.template", "fos_user.registration.confirmation.template", "fos_user.registration.confirmation.from_email", "fos_user.resetting.email.from_email", "fos_user.storage", "fos_user.firewall_name", "fos_user.model_manager_name", "fos_user.model.user.class", "fos_user.profile.form.type", "fos_user.profile.form.name", "fos_user.profile.form.validation_groups", "fos_user.registration.confirmation.enabled", "fos_user.registration.form.type", "fos_user.registration.form.name", "fos_user.registration.form.validation_groups", "fos_user.change_password.form.type", "fos_user.change_password.form.name", "fos_user.change_password.form.validation_groups", "fos_user.resetting.retry_ttl", "fos_user.resetting.token_ttl", "fos_user.resetting.form.type", "fos_user.resetting.form.name", "fos_user.resetting.form.validation_groups", "kimai.fosuser"?

Thanks

kevinpapst commented 4 years ago

Use {database} instead?

mxgross commented 4 years ago

Of course, some other character, sorry, it is friday ... :laughing: