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.

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

Use log_error file and datadir from mysql_info settings instead of variables mysql_datadir and mysql_hardening_log_file #478

Merged
merged 1 commit into from Aug 25, 2021

Conversation

123quhiwiwk
Copy link
Contributor

As discussed in PR #477, this PR changes the handling with the log_error file.
The log_error file from mysql_info mysql_settings.settings.log_error is used instead of default variable mysql_hardening_log_file.

As drawback, when changing log_error option using mysql_hardening_options, this new file will not be checked, as the configuration in mysql_hardening_mysql_hardening_conf_file happens after checking the permissions of the log_error file (but this also was the case when using the default variable).
So when changing the setting log_error this must be done before running mysql-hardening or run mysql-hardening twice.

@rndmh3ro
Copy link
Member

Funny, I just started working on this, too. You were faster!

Two things though:

  1. Removing the variable mysql_hardening_log_file would be a breaking change. So I'd like to deprecate its usage (by writing it into the changelog and removing the var from the vars-files like you already did), but still allow using it for now.

I think it can be done like this:

- name: Ensure permissions on mysql-logfile are correct
  file:
    path: '{{ item }}'
    state: file
    owner: '{{ mysql_hardening_user }}'
    group: '{{ mysql_hardening_group }}'
    mode: '0640'
  when: item is defined
  loop:
    - "{{ mysql_settings.settings.log_error }}"
    - "{{ mysql_hardening_log_file }}"

If mysql_hardening_log_file is defined, the permissions will be changed, same behaviour as before. Additionally if it's undefined, no error will be thrown.

  1. We should do the same with the variable mysql_datadir. It can be replaced by mysql_settings.settings.datadir.

@123quhiwiwk
Copy link
Contributor Author

I don't know, if I did something wrong, but when using loop as you suggested, the whole task is skipped (if mysql_hardening_log_file or mysql_datadir is not defined).
Sadly, the docu did not help me. Maybe I will notice my mistake tomorrow.
So, my fix for now is using an empty default for variables 🙄

I also change the README, and included my hint, that you should not use mysql_hardening_options to set datadir or log_error, I hope that's okay?

Copy link
Member

@rndmh3ro rndmh3ro left a comment

Choose a reason for hiding this comment

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

Thanks, looks good! Just two minor changes, then this is ready to be merged.

roles/mysql_hardening/CHANGELOG.md Outdated Show resolved Hide resolved
roles/mysql_hardening/README.md Outdated Show resolved Hide resolved
Signed-off-by: 123quhiwiwk <70281681+123quhiwiwk@users.noreply.github.com>
@123quhiwiwk 123quhiwiwk changed the title Mysql: Use log_error from mysql_info instead of default variable Use log_error file and datadir from mysql_info settings instead if variables mysql_datadir and mysql_hardening_log_file Aug 25, 2021
@123quhiwiwk
Copy link
Contributor Author

Oh, that is correct, I thought I did something wrong :)
I see, so I changed the title of PR to make the text in the changelog understandable...

@123quhiwiwk 123quhiwiwk changed the title Use log_error file and datadir from mysql_info settings instead if variables mysql_datadir and mysql_hardening_log_file Use log_error file and datadir from mysql_info settings instead of variables mysql_datadir and mysql_hardening_log_file Aug 25, 2021
@rndmh3ro rndmh3ro merged commit 062dd3f into dev-sec:master Aug 25, 2021
@rndmh3ro
Copy link
Member

Thank you for this nice addition!

divialth pushed a commit to divialth/ansible-collection-hardening that referenced this pull request Aug 3, 2022
…ble (dev-sec#478)

Signed-off-by: 123quhiwiwk <70281681+123quhiwiwk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants