replace password with unix socket authentication for root user #4

DanScharon · 2023-03-16T10:12:30Z

this pull request implements two things:

load differences in installation through distribution dependent variables
replace password authentication for the database root user with authentication via unix socket

authentication via unix socket has been set as the default for the MariaDB user 'root'@'localhost' since MariaDB 10.4.3
Since then there is no need anymore to set a database root user password and it is actually better to not do so because it can have bad side affects, e.g. in Debian/Ubuntu some maintainer scripts rely on this function and also automysqlbackup. For the latter you would have to store again a password in a cleartext file.
For the creation of the opencast user and database it is sufficient to use become for root permissions.

database_root_password is still left in as optional because some users may provision to an already installed MariaDB which has a database root user password already set.

…uth plugin, ensure that the tasks run for both Debian and RedHat based distros

…md accordingly

…sword ;)

lkiesow

Sorry, we seem to have overlooked this pull request.
I've left a review and hope we can get it in soon.

Can you rebase this onto the current main branch, make sure that the tests pass and take a look at the comments?

lkiesow · 2023-05-27T14:21:16Z

defaults/main.yml

+# mariadb root user password
+database_root_password: '4567'


There should be an explanation that this is now only being used for authentication and that this password is not set

I had already adapted the README but I have added additional context in defaults/main.yml file now

lkiesow · 2023-05-27T14:23:35Z

tasks/main.yml

+  include_vars: "{{ item }}"
+  with_first_found:
+    - "{{ ansible_distribution }}-{{ ansible_distribution_release }}.yml"
+    - "{{ ansible_distribution }}.yml"
+    - "{{ ansible_os_family }}.yml"
+  tags: always


Why this complicated? We know that only the last will ever catch, won't it?

Suggested change

include_vars: "{{ item }}"

with_first_found:

- "{{ ansible_distribution }}-{{ ansible_distribution_release }}.yml"

- "{{ ansible_distribution }}.yml"

- "{{ ansible_os_family }}.yml"

tags: always

include_vars: "{{ ansible_os_family }}.yml"

tags: always

agreed. I had put the others there as a preparation for possible further distribution deviations, but there seem to be no more

lkiesow · 2023-05-27T14:27:37Z

tasks/main.yml

+  ansible.builtin.package:
+    name: "{{ item }}"
+    state: present
+  with_items: "{{ mariadb_packages }}"


Shouldn't this also work (and doesn't require a loop):

Suggested change

ansible.builtin.package:

name: "{{ item }}"

state: present

with_items: "{{ mariadb_packages }}"

ansible.builtin.package:

name: "{{ mariadb_packages }}"

state: present

As an alternative, you could even use a simple inline switch here.
That way, you wouldn't have to look up somewhere else what will be installed:

- name: Install MariaDB on RedHat Systems ansible.builtin.package: name: - mariadb-server - mariadb - python3-{{ 'PyMySQL' if ansible_os_family == 'RedHat' else 'pymysql' }}

Yes, this also works. Not every module accepts lists as parameters so I had used a loop here, but the package module apparently does :) for inline switches I would have to use several because not only the python package name differs and then I can just put it into a variable because we need a distribution distinction anyways

lkiesow · 2023-05-27T14:30:39Z

tasks/main.yml

@@ -39,7 +38,7 @@
 - name: Configure mariadb
  notify: Restart MariaDB
  ansible.builtin.ini_file:
-    dest: /etc/my.cnf.d/opencast.cnf
+    dest: "{{ mariadb_configuration_file }}"


Multiple places to comment on this, but let me make the comment here once.

Why set this to different places on Debian and Red Hat? The previous location should be fine. Setting this to something else will just cause confusion for people upgrading from an older version of this role and I don't see any benefit.

Because /etc/my.cnf.d as a config inclusion path does not work on Debian and derivatives. The path does not exist by default and mariadb in Debian does not look for it. So if people upgrade from an older role version it hadn't worked anyways before in Debian and derivatives.

lkiesow · 2023-05-27T14:32:20Z

tasks/main.yml


 - name: Ensure root user can only connect from the local machine
+  community.mysql.mysql_query:


Does this matter at all if the next step is to force the root user to use Unix sockets?

Yes, because even after you have forced the root user to use the unix_socket plugin you'd also want to clean up entries that still refer to remote hosts because after a (accidental) deactivation of the plugin they'd become active/relevant again

This patch fixes the Molecule tests by running the playbooks on LXC containers in GitHub Actions.

This role fails on el9 systems. Setting a socket location fixes the problem.

This patch runs the tests also on el9 and marks the role as being compatible with that version.

This patch fixes the specification for supported `EL` versions which had a typo in the dictionary key.

…uth plugin, ensure that the tasks run for both Debian and RedHat based distros

…r conf file so that it survives package upgrades

DanScharon · 2023-06-02T15:11:22Z

Sorry, we seem to have overlooked this pull request. I've left a review and hope we can get it in soon.

Can you rebase this onto the current main branch, make sure that the tests pass and take a look at the comments?

Thank you for the review! I have incorporated your feedback now

tibroc

I tested this on two different oparating systems (Debian 12 and CentOS Stream 9) and it seems to work fine, if you adjust the socket location accordingly (see comment). I think this is a good improvement of the role and we should merge this (and sorry again for the delay). Thank you @DanScharon !

tibroc · 2023-10-24T11:28:22Z

tasks/main.yml

@@ -90,6 +65,8 @@
    priv: "{{ database_name }}.*:ALL,GRANT"
    login_user: "{{ database_root_user }}"
    login_password: "{{ database_root_password }}"
+    check_implicit_admin: true
+    login_unix_socket: /run/mysqld/mysqld.sock


This needs a os-specific variable as well. E.g. on CentOS Stream 9 the defautl socket location is /var/lib/mysql/mysql.sock.

tibroc · 2023-10-24T11:29:19Z

tasks/main.yml


 - name: Delete anonymous user
  community.mysql.mysql_user:
    user: ""
-    host: "{{ item }}"
+    host_all: yes


Suggested change

host_all: yes

host_all: true

DanScharon added 3 commits March 15, 2023 18:46

make database_root_password optional and use unix_socket as default a…

40734bd

…uth plugin, ensure that the tasks run for both Debian and RedHat based distros

provide a default value for database_root_password and update README.…

7d7b172

…md accordingly

if auth plugin is currently not unix_socket we need database_root_pas…

c207413

…sword ;)

lkiesow requested changes May 27, 2023

View reviewed changes

lkiesow requested a review from tibroc May 27, 2023 14:35

lkiesow and others added 7 commits June 2, 2023 16:37

Run Molecule Tests on LXC

6b806e8

This patch fixes the Molecule tests by running the playbooks on LXC containers in GitHub Actions.

MariaDB Socket Configuration for el9

f13042c

This role fails on el9 systems. Setting a socket location fixes the problem.

Mark as ready for el9

5c6b7ae

This patch runs the tests also on el9 and marks the role as being compatible with that version.

Fix os version specification

cabe111

This patch fixes the specification for supported `EL` versions which had a typo in the dictionary key.

make database_root_password optional and use unix_socket as default a…

50e6526

…uth plugin, ensure that the tasks run for both Debian and RedHat based distros

use a separate file for config adjustments and not the standard serve…

032641f

…r conf file so that it survives package upgrades

simplify tasks and add contextual explanation

bf70326

tibroc requested changes Oct 24, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

replace password with unix socket authentication for root user #4

replace password with unix socket authentication for root user #4

DanScharon commented Mar 16, 2023

lkiesow left a comment

lkiesow May 27, 2023

DanScharon Jun 2, 2023

lkiesow May 27, 2023

DanScharon Jun 2, 2023

lkiesow May 27, 2023

DanScharon Jun 2, 2023

lkiesow May 27, 2023

DanScharon Jun 2, 2023

lkiesow May 27, 2023

DanScharon Jun 2, 2023

DanScharon commented Jun 2, 2023

tibroc left a comment

tibroc Oct 24, 2023

tibroc Oct 24, 2023


		- name: Ensure root user can only connect from the local machine
		community.mysql.mysql_query:

replace password with unix socket authentication for root user #4

Are you sure you want to change the base?

replace password with unix socket authentication for root user #4

Conversation

DanScharon commented Mar 16, 2023

lkiesow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanScharon commented Jun 2, 2023

tibroc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment