From bccc55ea7b6e8b3002bafe887d764160e21fefba Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Tue, 16 Jul 2024 10:42:09 +0200 Subject: [PATCH 01/18] clickhouse_user: set default role --- plugins/modules/clickhouse_user.py | 54 +++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index 8c82c11..b1e6d77 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -82,6 +82,11 @@ type: list elements: str version_added: '0.5.0' + default_role: + description: + - Grants and sets the role as default for the user. + type: str + version_added: '0.6.0' ''' EXAMPLES = r''' @@ -94,6 +99,7 @@ name: test_user password: qwerty type_password: sha256_password + default_role: accountant - name: If user exists, update password community.clickhouse.clickhouse_user: @@ -155,6 +161,7 @@ class ClickHouseUser(): def __init__(self, module, client, name, password, type_password, cluster): + self.changed = False self.module = module self.client = client self.name = name @@ -163,6 +170,7 @@ def __init__(self, module, client, name, password, type_password, cluster): self.cluster = cluster # Set default values, then update self.user_exists = False + self.granted_roles = {} self.__populate_info() def __populate_info(self): @@ -181,6 +189,18 @@ def __populate_info(self): if result != []: self.user_exists = True + if self.user_exists: + query = ("SELECT granted_role_name, " + "granted_role_is_default, with_admin_option " + "FROM system.role_grants " + "WHERE granted_role_name='%s'" % self.name) + result = execute_query(self.module, self.client, query) + for row in result: + self.granted_roles[row[0]] = { + "granted_role_is_default": row[1], + "with_admin_option": row[2], + } + def create(self): list_settings = self.module.params['settings'] query = "CREATE USER %s" % self.name @@ -204,11 +224,24 @@ def create(self): if not self.module.check_mode: execute_query(self.module, self.client, query) + if self.module.params['default_role']: + self.__grant_role(self.module.params['default_role']) + self.__set_default_role(self.module.params['default_role']) + return True def update(self, update_password): + if self.module.params['default_role']: + default_role = self.module.params['default_role'] + + if default_role not in self.granted_roles: + self.__grant_role(default_role) + + if not self.granted_roles[default_role]["granted_role_is_default"]: + self.__set_default_role(default_role) + if update_password == 'on_create': - return False + return False or self.changed # If update_password is always # TODO: When ClickHouse will allow to retrieve password hashes, @@ -235,6 +268,24 @@ def drop(self): return True + def __grant_role(self, role): + query = "GRANT %s TO %s" % (role, self.name) + executed_statements.append(query) + + if not self.module.check_mode: + execute_query(self.module, self.client, query) + + self.changed = True + + def __set_default_role(self, role): + query = "SET DEFAULT ROLE %s TO %s" % (role, self.name) + executed_statements.append(query) + + if not self.module.check_mode: + execute_query(self.module, self.client, query) + + self.changed = True + def main(): argument_spec = client_common_argument_spec() @@ -249,6 +300,7 @@ def main(): default='on_create', no_log=False ), settings=dict(type='list', elements='str'), + default_role=dict(type='str', default=None), ) # Instantiate an object of module class From 4ea6ef8d459ae4c309022a1acdde4da1e4c71bef Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Fri, 19 Jul 2024 09:44:52 +0200 Subject: [PATCH 02/18] Add integration tests --- plugins/modules/clickhouse_user.py | 6 +- .../targets/clickhouse_user/tasks/initial.yml | 106 ++++++++++++++++++ 2 files changed, 110 insertions(+), 2 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index b1e6d77..97494c4 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -236,10 +236,12 @@ def update(self, update_password): if default_role not in self.granted_roles: self.__grant_role(default_role) - - if not self.granted_roles[default_role]["granted_role_is_default"]: self.__set_default_role(default_role) + else: + if not self.granted_roles[default_role]["granted_role_is_default"]: + self.__set_default_role(default_role) + if update_password == 'on_create': return False or self.changed diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index c3c73f4..236a121 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -142,3 +142,109 @@ that: - result is changed - result.executed_statements != [] + +# Test default_role argument +- name: Create test roles + loop: + - accountant + - sales + community.clickhouse.clickhouse_role: + name: "{{ item }}" + +- name: Set default role in check mode + register: result + check_mode: true + community.clickhouse.clickhouse_user: + state: present + name: test_user + default_role: accountant + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements == ["GRANT accountant TO test_user", "SET DEFAULT ROLE accountant TO test_user"] + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["default_roles_list"] == [] + +- name: Set default role in real mode + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + default_role: accountant + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements == ["GRANT accountant TO test_user", "SET DEFAULT ROLE accountant TO test_user"] + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["default_roles_list"] == ["accountant"] + +- name: Set another role as default in check mode + register: result + check_mode: true + community.clickhouse.clickhouse_user: + state: present + name: test_user + default_role: sales + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements == ["GRANT sales TO test_user", "SET DEFAULT ROLE sales TO test_user"] + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check the state has not been changed + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["default_roles_list"] == ["accountant"] + +- name: Set another role as default in real mode + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + default_role: sales + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements == ["GRANT sales TO test_user", "SET DEFAULT ROLE sales TO test_user"] + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["default_roles_list"] == ["sales"] From 303d20778d690740fd52058c7082903b5c42e555 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Fri, 19 Jul 2024 09:47:16 +0200 Subject: [PATCH 03/18] Add changelog fragment --- changelogs/fragments/0-clickhouse_user.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/0-clickhouse_user.yml diff --git a/changelogs/fragments/0-clickhouse_user.yml b/changelogs/fragments/0-clickhouse_user.yml new file mode 100644 index 0000000..fe91dba --- /dev/null +++ b/changelogs/fragments/0-clickhouse_user.yml @@ -0,0 +1,2 @@ +minor_changes: +- clickhouse_user - add the ``default_role`` argument to set a default role (https://github.com/ansible-collections/community.clickhouse/pull/70). From 5acd524277663de724906f300794573e7b840f2c Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Fri, 19 Jul 2024 10:20:03 +0200 Subject: [PATCH 04/18] tmp --- plugins/modules/clickhouse_user.py | 6 +++- .../targets/clickhouse_user/tasks/initial.yml | 28 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index 97494c4..4896405 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -157,6 +157,7 @@ PRIV_ERR_CODE = 497 executed_statements = [] +debug = [] class ClickHouseUser(): @@ -195,6 +196,7 @@ def __populate_info(self): "FROM system.role_grants " "WHERE granted_role_name='%s'" % self.name) result = execute_query(self.module, self.client, query) + debug.append("%s" % result) for row in result: self.granted_roles[row[0]] = { "granted_role_is_default": row[1], @@ -235,6 +237,8 @@ def update(self, update_password): default_role = self.module.params['default_role'] if default_role not in self.granted_roles: + # TODO debug remove + debug.append("%s" % self.granted_roles) self.__grant_role(default_role) self.__set_default_role(default_role) @@ -351,7 +355,7 @@ def main(): client.disconnect_connection() # Users will get this in JSON output after execution - module.exit_json(changed=changed, executed_statements=executed_statements) + module.exit_json(changed=changed, executed_statements=executed_statements, debug=debug) if __name__ == '__main__': diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index 236a121..a829529 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -248,3 +248,31 @@ that: - result is not changed - result["users"]["test_user"]["default_roles_list"] == ["sales"] + +- name: Set the role again + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + default_role: sales + +- name: Check ret values + ansible.builtin.assert: + that: + - result is not changed + - result.executed_statements == [] + +- name: Set the role again in check mode + register: result + check_mode: true + community.clickhouse.clickhouse_user: + state: present + name: test_user + default_role: sales + +- name: Check ret values + ansible.builtin.assert: + that: + - result is not changed + - result.executed_statements == [] + From 7ab290255bb134c028bea13d003b7561b0ae09c5 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Fri, 19 Jul 2024 10:36:13 +0200 Subject: [PATCH 05/18] Change implementation --- plugins/modules/clickhouse_user.py | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index 4896405..4fb7fcb 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -157,7 +157,6 @@ PRIV_ERR_CODE = 497 executed_statements = [] -debug = [] class ClickHouseUser(): @@ -171,12 +170,12 @@ def __init__(self, module, client, name, password, type_password, cluster): self.cluster = cluster # Set default values, then update self.user_exists = False - self.granted_roles = {} + self.default_roles_list = [] self.__populate_info() def __populate_info(self): # Collecting user information - query = ("SELECT name, storage, auth_type " + query = ("SELECT name, storage, auth_type, default_roles_list " "FROM system.users " "WHERE name = '%s'" % self.name) @@ -189,19 +188,7 @@ def __populate_info(self): if result != []: self.user_exists = True - - if self.user_exists: - query = ("SELECT granted_role_name, " - "granted_role_is_default, with_admin_option " - "FROM system.role_grants " - "WHERE granted_role_name='%s'" % self.name) - result = execute_query(self.module, self.client, query) - debug.append("%s" % result) - for row in result: - self.granted_roles[row[0]] = { - "granted_role_is_default": row[1], - "with_admin_option": row[2], - } + self.default_roles_list = result[0][3] def create(self): list_settings = self.module.params['settings'] @@ -236,16 +223,10 @@ def update(self, update_password): if self.module.params['default_role']: default_role = self.module.params['default_role'] - if default_role not in self.granted_roles: - # TODO debug remove - debug.append("%s" % self.granted_roles) + if default_role not in self.default_roles_list: self.__grant_role(default_role) self.__set_default_role(default_role) - else: - if not self.granted_roles[default_role]["granted_role_is_default"]: - self.__set_default_role(default_role) - if update_password == 'on_create': return False or self.changed @@ -355,7 +336,7 @@ def main(): client.disconnect_connection() # Users will get this in JSON output after execution - module.exit_json(changed=changed, executed_statements=executed_statements, debug=debug) + module.exit_json(changed=changed, executed_statements=executed_statements) if __name__ == '__main__': From 1a720668ccc5be684ecce550c505f1ef59142091 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Fri, 19 Jul 2024 10:43:36 +0200 Subject: [PATCH 06/18] Change test formatting --- .../targets/clickhouse_user/tasks/initial.yml | 74 +++++++++++-------- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index a829529..a5ff9b7 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -3,8 +3,7 @@ # and should not be used as examples of how to write Ansible roles # #################################################################### -# Test 1 -- name: Test 1 - Create test_user in check mode +- name: Create test_user in check mode register: result check_mode: true community.clickhouse.clickhouse_user: @@ -12,33 +11,31 @@ name: test_user password: querty -- name: Test 1 - Check ret values in +- name: Check ret values in ansible.builtin.assert: that: - result is changed - result.executed_statements == ["CREATE USER test_user IDENTIFIED WITH sha256_password BY '********'"] -- name: Test 1 - Check the actual state +- name: Check the actual state register: result community.clickhouse.clickhouse_info: login_host: localhost client_kwargs: connect_timeout: 20 -- name: Test 1 - Check result +- name: Check result ansible.builtin.assert: that: - result is not changed - result["users"]["test_user"] is not defined - -# Test 2 -- name: Test 2 - Create test_user +- name: Create test_user community.clickhouse.clickhouse_user: state: present name: test_user -- name: Test 2 - Check the actual state +- name: Check the actual state register: result community.clickhouse.clickhouse_info: login_host: localhost @@ -50,63 +47,55 @@ that: - result["users"]["test_user"] != {} - -# Test 3 -- name: Test 3 - Create test_user if it exists +- name: Create test_user if it exists register: result community.clickhouse.clickhouse_user: state: present name: test_user -- name: Test 3 - Check result +- name: Check result ansible.builtin.assert: that: - result is not changed - result.executed_statements == [] - -# Test 4 -- name: Test 4 - Drop test_user +- name: Drop test_user community.clickhouse.clickhouse_user: state: absent name: test_user -- name: Test 4 - Check the actual state +- name: Check the actual state register: result community.clickhouse.clickhouse_info: login_host: localhost client_kwargs: connect_timeout: 20 -- name: Test 4 - Check result +- name: Check result ansible.builtin.assert: that: - result["users"]["test_user"] is not defined - -# Test 5 -- name: Test 5 - Drop test_user if it does not exists +- name: Drop test_user if it does not exists register: result community.clickhouse.clickhouse_user: state: absent name: test_user -- name: Test 5 - Check ret values +- name: Check ret values ansible.builtin.assert: that: - result is not changed - result.executed_statements == [] - -# Test 6 -- name: Test 6 - Create test_user +- name: Create test_user register: result community.clickhouse.clickhouse_user: state: present name: test_user password: querty -- name: Test 6 - Create test_user again with update_password always +- name: Create test_user again with update_password always register: result community.clickhouse.clickhouse_user: state: present @@ -114,20 +103,18 @@ password: querty update_password: always -- name: Test 6 - Check result +- name: Check result ansible.builtin.assert: that: - result is changed - result.executed_statements == ["ALTER USER test_user IDENTIFIED WITH sha256_password BY '********'"] -- name: Test 6 - Drop test_user +- name: Drop test_user community.clickhouse.clickhouse_user: state: absent name: test_user - -# Test 7 -- name: Test 7 - Create test_user with settings +- name: Create test_user with settings register: result community.clickhouse.clickhouse_user: state: present @@ -137,7 +124,7 @@ - max_memory_usage = 15000 READONLY - max_memory_usage_for_all_queries = 15000 MIN 15000 MAX 16000 WRITABLE -- name: Test 7 - Check result +- name: Check result ansible.builtin.assert: that: - result is changed @@ -276,3 +263,26 @@ - result is not changed - result.executed_statements == [] +- name: Set the first role in real mode + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + default_role: accountant + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements == ["GRANT accountant TO test_user", "SET DEFAULT ROLE accountant TO test_user"] + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["default_roles_list"] == ["accountant"] From 86614baaf9008561c3b0cd90c5e0fe273ad90412 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Fri, 9 Aug 2024 09:24:49 +0200 Subject: [PATCH 07/18] Make the argument a list --- plugins/modules/clickhouse_user.py | 32 +++++++++++-------- .../targets/clickhouse_user/tasks/initial.yml | 22 ++++++++----- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index 4fb7fcb..380b443 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -82,10 +82,11 @@ type: list elements: str version_added: '0.5.0' - default_role: + default_roles: description: - - Grants and sets the role as default for the user. - type: str + - Grants and sets the role(s) as default for the user. + type: list + elements: str version_added: '0.6.0' ''' @@ -99,7 +100,9 @@ name: test_user password: qwerty type_password: sha256_password - default_role: accountant + default_roles: + - accountant + - manager - name: If user exists, update password community.clickhouse.clickhouse_user: @@ -213,19 +216,20 @@ def create(self): if not self.module.check_mode: execute_query(self.module, self.client, query) - if self.module.params['default_role']: - self.__grant_role(self.module.params['default_role']) - self.__set_default_role(self.module.params['default_role']) + if self.module.params['default_roles']: + self.__grant_role(self.module.params['default_roles']) + self.__set_default_roles(self.module.params['default_roles']) return True def update(self, update_password): - if self.module.params['default_role']: - default_role = self.module.params['default_role'] + if self.module.params['default_roles']: + default_roles = self.module.params['default_roles'] - if default_role not in self.default_roles_list: - self.__grant_role(default_role) - self.__set_default_role(default_role) + for role in default_roles: + if role not in self.default_roles_list: + self.__grant_role(role) + self.__set_default_roles(role) if update_password == 'on_create': return False or self.changed @@ -264,7 +268,7 @@ def __grant_role(self, role): self.changed = True - def __set_default_role(self, role): + def __set_default_roles(self, role): query = "SET DEFAULT ROLE %s TO %s" % (role, self.name) executed_statements.append(query) @@ -287,7 +291,7 @@ def main(): default='on_create', no_log=False ), settings=dict(type='list', elements='str'), - default_role=dict(type='str', default=None), + default_roles=dict(type='list', elements='str', default=None), ) # Instantiate an object of module class diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index a5ff9b7..b4d1751 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -130,7 +130,7 @@ - result is changed - result.executed_statements != [] -# Test default_role argument +# Test default_roles argument - name: Create test roles loop: - accountant @@ -144,7 +144,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user - default_role: accountant + default_roles: + - accountant - name: Check ret values ansible.builtin.assert: @@ -168,7 +169,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user - default_role: accountant + default_roles: + - accountant - name: Check ret values ansible.builtin.assert: @@ -193,7 +195,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user - default_role: sales + default_roles: + - sales - name: Check ret values ansible.builtin.assert: @@ -217,7 +220,7 @@ community.clickhouse.clickhouse_user: state: present name: test_user - default_role: sales + default_roles: sales - name: Check ret values ansible.builtin.assert: @@ -241,7 +244,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user - default_role: sales + default_roles: + - sales - name: Check ret values ansible.builtin.assert: @@ -255,7 +259,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user - default_role: sales + default_roles: + - sales - name: Check ret values ansible.builtin.assert: @@ -268,7 +273,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user - default_role: accountant + default_roles: + - accountant - name: Check ret values ansible.builtin.assert: From 9449d8553f06dd2b46900a94f6212cc32884f660 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Fri, 9 Aug 2024 09:58:32 +0200 Subject: [PATCH 08/18] Correct the changelog fragment --- changelogs/fragments/0-clickhouse_user.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelogs/fragments/0-clickhouse_user.yml b/changelogs/fragments/0-clickhouse_user.yml index fe91dba..1cd35d2 100644 --- a/changelogs/fragments/0-clickhouse_user.yml +++ b/changelogs/fragments/0-clickhouse_user.yml @@ -1,2 +1,2 @@ minor_changes: -- clickhouse_user - add the ``default_role`` argument to set a default role (https://github.com/ansible-collections/community.clickhouse/pull/70). +- clickhouse_user - add the ``default_roles`` argument to set a default role (https://github.com/ansible-collections/community.clickhouse/pull/70). From 78929f96343290d2cec2c903a587b94301c0df6e Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Fri, 9 Aug 2024 10:06:08 +0200 Subject: [PATCH 09/18] Use alter user to assign multiple roles as default --- plugins/modules/clickhouse_user.py | 23 +++++++++++-------- .../targets/clickhouse_user/tasks/initial.yml | 17 ++++++++------ 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index 380b443..f4183ce 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -226,10 +226,14 @@ def update(self, update_password): if self.module.params['default_roles']: default_roles = self.module.params['default_roles'] + roles_to_set = [] for role in default_roles: if role not in self.default_roles_list: - self.__grant_role(role) - self.__set_default_roles(role) + roles_to_set.append(role) + + if roles_to_set: + self.__grant_roles(roles_to_set) + self.__set_default_roles(roles_to_set) if update_password == 'on_create': return False or self.changed @@ -259,17 +263,18 @@ def drop(self): return True - def __grant_role(self, role): - query = "GRANT %s TO %s" % (role, self.name) - executed_statements.append(query) + def __grant_roles(self, roles_to_set): + for role in roles_to_set: + query = "GRANT %s TO %s" % (role, self.name) + executed_statements.append(query) - if not self.module.check_mode: - execute_query(self.module, self.client, query) + if not self.module.check_mode: + execute_query(self.module, self.client, query) self.changed = True - def __set_default_roles(self, role): - query = "SET DEFAULT ROLE %s TO %s" % (role, self.name) + def __set_default_roles(self, roles_to_set): + query = "ALTER USER %s DEFAULT ROLE %s" % (self.name, ', '.join(roles_to_set)) executed_statements.append(query) if not self.module.check_mode: diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index b4d1751..ba8aa1a 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -134,6 +134,7 @@ - name: Create test roles loop: - accountant + - manager - sales community.clickhouse.clickhouse_role: name: "{{ item }}" @@ -146,12 +147,13 @@ name: test_user default_roles: - accountant + - manager - name: Check ret values ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT accountant TO test_user", "SET DEFAULT ROLE accountant TO test_user"] + - result.executed_statements == ["GRANT accountant TO test_user", "GRANT manager TO test_user", "ALTER USER test_user DEFAULT ROLE accountant, manager"] - name: Check the actual state register: result @@ -171,12 +173,13 @@ name: test_user default_roles: - accountant + - manager - name: Check ret values ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT accountant TO test_user", "SET DEFAULT ROLE accountant TO test_user"] + - result.executed_statements == ["GRANT accountant TO test_user", "GRANT manager TO test_user", "ALTER USER test_user DEFAULT ROLE accountant, manager"] - name: Check the actual state register: result @@ -187,7 +190,7 @@ ansible.builtin.assert: that: - result is not changed - - result["users"]["test_user"]["default_roles_list"] == ["accountant"] + - result["users"]["test_user"]["default_roles_list"] == ["accountant", "manager"] - name: Set another role as default in check mode register: result @@ -202,7 +205,7 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT sales TO test_user", "SET DEFAULT ROLE sales TO test_user"] + - result.executed_statements == ["GRANT sales TO test_user", "ALTER USER test_user DEFAULT ROLE sales"] - name: Check the actual state register: result @@ -213,7 +216,7 @@ ansible.builtin.assert: that: - result is not changed - - result["users"]["test_user"]["default_roles_list"] == ["accountant"] + - result["users"]["test_user"]["default_roles_list"] == ["accountant", "manager"] - name: Set another role as default in real mode register: result @@ -226,7 +229,7 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT sales TO test_user", "SET DEFAULT ROLE sales TO test_user"] + - result.executed_statements == ["GRANT sales TO test_user", "ALTER USER test_user DEFAULT ROLE sales"] - name: Check the actual state register: result @@ -280,7 +283,7 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT accountant TO test_user", "SET DEFAULT ROLE accountant TO test_user"] + - result.executed_statements == ["GRANT accountant TO test_user", "ALTER USER test_user DEFAULT ROLE accountant"] - name: Check the actual state register: result From 87340f0650e411b7e51ffa4087afddef58b9a9d6 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Fri, 16 Aug 2024 13:49:26 +0200 Subject: [PATCH 10/18] add roles argument --- plugins/modules/clickhouse_user.py | 50 ++++++++++++++++--- .../targets/clickhouse_user/tasks/initial.yml | 17 ++++++- 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index f4183ce..02b6f75 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -82,9 +82,18 @@ type: list elements: str version_added: '0.5.0' + roles: + description: + - Grants specified roles for the user. + type: list + elements: str + version_added: '0.6.0' default_roles: description: - - Grants and sets the role(s) as default for the user. + - Sets specified roles as default for the user. + - The roles must be explicitly granted to the user whether manually + before using this argument or by using the I(roles) + argument in the same task. type: list elements: str version_added: '0.6.0' @@ -100,9 +109,11 @@ name: test_user password: qwerty type_password: sha256_password - default_roles: + roles: - accountant - manager + default_roles: + - accountant - name: If user exists, update password community.clickhouse.clickhouse_user: @@ -173,7 +184,10 @@ def __init__(self, module, client, name, password, type_password, cluster): self.cluster = cluster # Set default values, then update self.user_exists = False - self.default_roles_list = [] + self.current_default_roles = [] + self.current_roles = [] + # Fetch actual values from DB and + # update the attributes with them self.__populate_info() def __populate_info(self): @@ -191,7 +205,16 @@ def __populate_info(self): if result != []: self.user_exists = True - self.default_roles_list = result[0][3] + self.current_default_roles = result[0][3] + + if self.user_exists: + self.current_roles = self.__fetch_user_groups() + + def __fetch_user_groups(self): + query = ("SELECT granted_role_name FROM system.role_grants " + "WHERE user_name = '%s'" % self.name) + result = execute_query(self.module, self.client, query) + return [row[0] for row in result] def create(self): list_settings = self.module.params['settings'] @@ -216,23 +239,35 @@ def create(self): if not self.module.check_mode: execute_query(self.module, self.client, query) + if self.module.params['roles']: + self.__grant_role(self.module.params['roles']) + if self.module.params['default_roles']: - self.__grant_role(self.module.params['default_roles']) self.__set_default_roles(self.module.params['default_roles']) return True def update(self, update_password): + if self.module.params['roles']: + desired_roles = self.module.params['roles'] + + roles_to_set = [] + for role in desired_roles: + if role not in self.current_roles: + roles_to_set.append(role) + + if roles_to_set: + self.__grant_roles(roles_to_set) + if self.module.params['default_roles']: default_roles = self.module.params['default_roles'] roles_to_set = [] for role in default_roles: - if role not in self.default_roles_list: + if role not in self.current_default_roles: roles_to_set.append(role) if roles_to_set: - self.__grant_roles(roles_to_set) self.__set_default_roles(roles_to_set) if update_password == 'on_create': @@ -296,6 +331,7 @@ def main(): default='on_create', no_log=False ), settings=dict(type='list', elements='str'), + roles=dict(type='list', elements='str', default=None), default_roles=dict(type='list', elements='str', default=None), ) diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index ba8aa1a..d322e82 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -145,6 +145,9 @@ community.clickhouse.clickhouse_user: state: present name: test_user + roles: + - accountant + - manager default_roles: - accountant - manager @@ -171,6 +174,9 @@ community.clickhouse.clickhouse_user: state: present name: test_user + roles: + - accountant + - manager default_roles: - accountant - manager @@ -198,6 +204,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user + roles: + - sales default_roles: - sales @@ -223,6 +231,7 @@ community.clickhouse.clickhouse_user: state: present name: test_user + roles: sales default_roles: sales - name: Check ret values @@ -247,6 +256,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user + roles: + - sales default_roles: - sales @@ -262,6 +273,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user + roles: + - sales default_roles: - sales @@ -276,6 +289,8 @@ community.clickhouse.clickhouse_user: state: present name: test_user + roles: + - accountant default_roles: - accountant @@ -283,7 +298,7 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT accountant TO test_user", "ALTER USER test_user DEFAULT ROLE accountant"] + - result.executed_statements == ["ALTER USER test_user DEFAULT ROLE accountant"] - name: Check the actual state register: result From a3ef29b7f975c7045d0d6a8821cec7b5302f3ea1 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Mon, 19 Aug 2024 14:16:24 +0200 Subject: [PATCH 11/18] tmp --- plugins/modules/clickhouse_user.py | 89 +++++++++++++++---- .../targets/clickhouse_user/tasks/initial.yml | 26 ++++-- 2 files changed, 93 insertions(+), 22 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index 02b6f75..78d1600 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -85,6 +85,7 @@ roles: description: - Grants specified roles for the user. + - To append specified roles to existing ones, also add I(append=true) to your task. type: list elements: str version_added: '0.6.0' @@ -94,13 +95,32 @@ - The roles must be explicitly granted to the user whether manually before using this argument or by using the I(roles) argument in the same task. + - To append specified roles to existing ones, also add I(append_roles=true) to your task. type: list elements: str version_added: '0.6.0' + append_roles: + description: + - When set to C(true), appends roles specified in I(roles) to existing + user roles instead of removing the user from not specified roles. + - The default is C(false), which will remove the user from all not specified roles. + - Requires I(roles) to be set in the task. + type: bool + default: false + version_added: '0.6.0' + append_default_roles: + description: + - When set to C(true), appends roles specified in I(default_roles) to existing + default roles instead of unsetting not specified ones. + - The default is C(false), which will unset all not specified roles. + - Requires I(default_roles) to be set in the task. + type: bool + default: false + version_added: '0.6.0' ''' EXAMPLES = r''' -- name: Create user +- name: Create user granting roles and setting default role community.clickhouse.clickhouse_user: login_host: localhost login_user: alice @@ -114,6 +134,18 @@ - manager default_roles: - accountant + append_roles: true + +- name: Append the sales role to alice's roles + community.clickhouse.clickhouse_user: + login_host: localhost + login_user: alice + login_db: foo + login_password: my_password + name: test_user + roles: + - sales + append_roles: true - name: If user exists, update password community.clickhouse.clickhouse_user: @@ -251,23 +283,40 @@ def update(self, update_password): if self.module.params['roles']: desired_roles = self.module.params['roles'] - roles_to_set = [] + roles_to_grant = [] for role in desired_roles: if role not in self.current_roles: - roles_to_set.append(role) + roles_to_grant.append(role) + + if roles_to_grant: + self.__grant_roles(roles_to_grant) - if roles_to_set: - self.__grant_roles(roles_to_set) + if not self.module.params['append_roles']: + roles_to_revoke = [] + for role in self.current_roles: + if role not in self.desired_roles: + roles_to_revoke.append(role) + + if roles_to_revoke: + self.__revoke_roles(roles_to_revoke) if self.module.params['default_roles']: default_roles = self.module.params['default_roles'] - roles_to_set = [] - for role in default_roles: - if role not in self.current_default_roles: - roles_to_set.append(role) + if self.module.params['append_roles']: + roles_to_set = [] + for role in default_roles: + if role not in self.current_default_roles: + roles_to_set.append(role) + + if roles_to_set: + self.__set_default_roles(roles_to_set) - if roles_to_set: + elif not self.module.params['append_roles']: + # Use sets to make a list of unique roles + set1 = set(self.current_default_roles) + set2 = set(default_roles) + roles_to_set = list(set1.union(set2)) self.__set_default_roles(roles_to_set) if update_password == 'on_create': @@ -299,12 +348,20 @@ def drop(self): return True def __grant_roles(self, roles_to_set): - for role in roles_to_set: - query = "GRANT %s TO %s" % (role, self.name) - executed_statements.append(query) + query = "GRANT %s TO %s" % (', '.join(roles_to_set), self.name) + executed_statements.append(query) + + if not self.module.check_mode: + execute_query(self.module, self.client, query) - if not self.module.check_mode: - execute_query(self.module, self.client, query) + self.changed = True + + def __revoke_roles(self, roles_to_revoke): + query = "REVOKE %s FROM %s" % (' ,'.join(roles_to_revoke), self.name) + executed_statements.append(query) + + if not self.module.check_mode: + execute_query(self.module, self.client, query) self.changed = True @@ -333,6 +390,8 @@ def main(): settings=dict(type='list', elements='str'), roles=dict(type='list', elements='str', default=None), default_roles=dict(type='list', elements='str', default=None), + append_roles=dict(type='bool', default=False), + append_default_roles=dict(type='bool', default=False), ) # Instantiate an object of module class diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index d322e82..184b83a 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -156,7 +156,7 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT accountant TO test_user", "GRANT manager TO test_user", "ALTER USER test_user DEFAULT ROLE accountant, manager"] + - result.executed_statements == ["GRANT accountant, manager TO test_user", "ALTER USER test_user DEFAULT ROLE accountant, manager"] - name: Check the actual state register: result @@ -167,6 +167,7 @@ ansible.builtin.assert: that: - result is not changed + - result["users"]["test_user"]["roles"] == [] - result["users"]["test_user"]["default_roles_list"] == [] - name: Set default role in real mode @@ -185,7 +186,7 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT accountant TO test_user", "GRANT manager TO test_user", "ALTER USER test_user DEFAULT ROLE accountant, manager"] + - result.executed_statements == ["GRANT accountant, manager TO test_user", "ALTER USER test_user DEFAULT ROLE accountant, manager"] - name: Check the actual state register: result @@ -196,6 +197,7 @@ ansible.builtin.assert: that: - result is not changed + - result["users"]["test_user"]["roles"] == ["accountant", "manager"] - result["users"]["test_user"]["default_roles_list"] == ["accountant", "manager"] - name: Set another role as default in check mode @@ -213,7 +215,9 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT sales TO test_user", "ALTER USER test_user DEFAULT ROLE sales"] + - result.executed_statements[0] == "GRANT sales TO test_user" + - result.executed_statements[1] == "REVOKE account, manager FROM test_user" + - result.executed_statements[2] == "ALTER USER test_user DEFAULT ROLE sales" - name: Check the actual state register: result @@ -224,6 +228,7 @@ ansible.builtin.assert: that: - result is not changed + - result["users"]["test_user"]["roles"] == ["accountant", "manager"] - result["users"]["test_user"]["default_roles_list"] == ["accountant", "manager"] - name: Set another role as default in real mode @@ -238,7 +243,9 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT sales TO test_user", "ALTER USER test_user DEFAULT ROLE sales"] + - result.executed_statements[0] == "GRANT sales TO test_user" + - result.executed_statements[1] == "REVOKE account, manager FROM test_user" + - result.executed_statements[2] == "ALTER USER test_user DEFAULT ROLE sales" - name: Check the actual state register: result @@ -249,6 +256,7 @@ ansible.builtin.assert: that: - result is not changed + - result["users"]["test_user"]["roles"] == ["sales"] - result["users"]["test_user"]["default_roles_list"] == ["sales"] - name: Set the role again @@ -284,7 +292,7 @@ - result is not changed - result.executed_statements == [] -- name: Set the first role in real mode +- name: Append the first role in real mode register: result community.clickhouse.clickhouse_user: state: present @@ -293,12 +301,15 @@ - accountant default_roles: - accountant + append_roles: true + append_default_roles: true - name: Check ret values ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["ALTER USER test_user DEFAULT ROLE accountant"] + - result.executed_statements[0] == ["GRANT accountant TO test_user"] + - result.executed_statements[1] == ["ALTER USER test_user DEFAULT ROLE sales, accountant"] - name: Check the actual state register: result @@ -309,4 +320,5 @@ ansible.builtin.assert: that: - result is not changed - - result["users"]["test_user"]["default_roles_list"] == ["accountant"] + - result["users"]["test_user"]["roles"] == ["sales", "accountant"] + - result["users"]["test_user"]["default_roles_list"] == ["sales", "accountant"] From 119a1eecc38d99d9a331f7b0ff3e4930422d33fb Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Mon, 19 Aug 2024 15:18:50 +0200 Subject: [PATCH 12/18] tmp --- plugins/modules/clickhouse_user.py | 85 +++++--- .../targets/clickhouse_user/tasks/initial.yml | 181 ++++++++++++++++-- 2 files changed, 231 insertions(+), 35 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index 78d1600..1a1a703 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -85,7 +85,8 @@ roles: description: - Grants specified roles for the user. - - To append specified roles to existing ones, also add I(append=true) to your task. + - To append specified roles to existing ones, also add I(append_roles=true) to your task. + - To revoke all roles, pass an empty list and I(append_roles=false). type: list elements: str version_added: '0.6.0' @@ -95,7 +96,8 @@ - The roles must be explicitly granted to the user whether manually before using this argument or by using the I(roles) argument in the same task. - - To append specified roles to existing ones, also add I(append_roles=true) to your task. + - To append specified roles to existing ones, also add I(append_default_roles=true) to your task. + - To unset all roles as default, pass an empty list and I(append_default_roles=false). type: list elements: str version_added: '0.6.0' @@ -104,7 +106,7 @@ - When set to C(true), appends roles specified in I(roles) to existing user roles instead of removing the user from not specified roles. - The default is C(false), which will remove the user from all not specified roles. - - Requires I(roles) to be set in the task. + - Ignored without I(roles) set. type: bool default: false version_added: '0.6.0' @@ -113,7 +115,7 @@ - When set to C(true), appends roles specified in I(default_roles) to existing default roles instead of unsetting not specified ones. - The default is C(false), which will unset all not specified roles. - - Requires I(default_roles) to be set in the task. + - Ignored without I(default_roles) set. type: bool default: false version_added: '0.6.0' @@ -147,6 +149,24 @@ - sales append_roles: true +- name: Unset all alice's default roles + community.clickhouse.clickhouse_user: + login_host: localhost + login_user: alice + login_db: foo + login_password: my_password + name: test_user + default_roles: [] + +- name: Revoke all roles from alice + community.clickhouse.clickhouse_user: + login_host: localhost + login_user: alice + login_db: foo + login_password: my_password + name: test_user + roles: [] + - name: If user exists, update password community.clickhouse.clickhouse_user: login_host: localhost @@ -280,7 +300,7 @@ def create(self): return True def update(self, update_password): - if self.module.params['roles']: + if self.module.params['roles'] is not None: desired_roles = self.module.params['roles'] roles_to_grant = [] @@ -294,31 +314,36 @@ def update(self, update_password): if not self.module.params['append_roles']: roles_to_revoke = [] for role in self.current_roles: - if role not in self.desired_roles: + if role not in desired_roles: roles_to_revoke.append(role) if roles_to_revoke: self.__revoke_roles(roles_to_revoke) - if self.module.params['default_roles']: + if self.module.params['default_roles'] is not None: default_roles = self.module.params['default_roles'] - - if self.module.params['append_roles']: - roles_to_set = [] - for role in default_roles: - if role not in self.current_default_roles: - roles_to_set.append(role) - - if roles_to_set: + cur_def_roles_set = set(self.current_default_roles) + req_def_roles_set = set(default_roles) + + if self.module.params['append_roles'] is False: + if not req_def_roles_set: + # Update roles info in case all roles were revoked + # in the same task and then unset if the roles list + # is not empty + self.current_roles = self.__fetch_user_groups() + if self.current_roles: + self.__unset_default_roles() + + elif cur_def_roles_set != req_def_roles_set: + self.__set_default_roles(default_roles) + + else: + if cur_def_roles_set != req_def_roles_set: + # Append roles to default roles. + # Use set union to make a list of unique roles + roles_to_set = list(cur_def_roles_set.union(req_def_roles_set)) self.__set_default_roles(roles_to_set) - elif not self.module.params['append_roles']: - # Use sets to make a list of unique roles - set1 = set(self.current_default_roles) - set2 = set(default_roles) - roles_to_set = list(set1.union(set2)) - self.__set_default_roles(roles_to_set) - if update_password == 'on_create': return False or self.changed @@ -357,7 +382,7 @@ def __grant_roles(self, roles_to_set): self.changed = True def __revoke_roles(self, roles_to_revoke): - query = "REVOKE %s FROM %s" % (' ,'.join(roles_to_revoke), self.name) + query = "REVOKE %s FROM %s" % (', '.join(roles_to_revoke), self.name) executed_statements.append(query) if not self.module.check_mode: @@ -366,6 +391,11 @@ def __revoke_roles(self, roles_to_revoke): self.changed = True def __set_default_roles(self, roles_to_set): + self.current_roles = self.__fetch_user_groups() + for role in roles_to_set: + if role not in self.current_roles and role not in self.module.params["roles"]: + self.module.fail_json("User %s is not in %s role. Grant it explicitly first." % (self.name, role)) + query = "ALTER USER %s DEFAULT ROLE %s" % (self.name, ', '.join(roles_to_set)) executed_statements.append(query) @@ -374,6 +404,15 @@ def __set_default_roles(self, roles_to_set): self.changed = True + def __unset_default_roles(self): + query = "SET DEFAULT ROLE NONE TO %s" % self.name + executed_statements.append(query) + + if not self.module.check_mode: + execute_query(self.module, self.client, query) + + self.changed = True + def main(): argument_spec = client_common_argument_spec() diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index 184b83a..7d3bd08 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -156,7 +156,8 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT accountant, manager TO test_user", "ALTER USER test_user DEFAULT ROLE accountant, manager"] + - result.executed_statements[0] == "GRANT accountant, manager TO test_user" + - result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE accountant, manager" or result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE manager, accountant" - name: Check the actual state register: result @@ -186,7 +187,8 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements == ["GRANT accountant, manager TO test_user", "ALTER USER test_user DEFAULT ROLE accountant, manager"] + - result.executed_statements[0] == "GRANT accountant, manager TO test_user" + - result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE accountant, manager" or result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE manager, accountant" - name: Check the actual state register: result @@ -197,10 +199,10 @@ ansible.builtin.assert: that: - result is not changed - - result["users"]["test_user"]["roles"] == ["accountant", "manager"] + - result["users"]["test_user"]["roles"] == ["accountant", "manager"] or result["users"]["test_user"]["roles"] == ["manager", "accountant"] - result["users"]["test_user"]["default_roles_list"] == ["accountant", "manager"] -- name: Set another role as default in check mode +- name: Grant and set another role as default in check mode register: result check_mode: true community.clickhouse.clickhouse_user: @@ -216,7 +218,7 @@ that: - result is changed - result.executed_statements[0] == "GRANT sales TO test_user" - - result.executed_statements[1] == "REVOKE account, manager FROM test_user" + - result.executed_statements[1] == "REVOKE accountant, manager FROM test_user" or result.executed_statements[1] == "REVOKE manager, accountant FROM test_user" - result.executed_statements[2] == "ALTER USER test_user DEFAULT ROLE sales" - name: Check the actual state @@ -228,8 +230,8 @@ ansible.builtin.assert: that: - result is not changed - - result["users"]["test_user"]["roles"] == ["accountant", "manager"] - - result["users"]["test_user"]["default_roles_list"] == ["accountant", "manager"] + - result["users"]["test_user"]["roles"] == ["accountant", "manager"] or result["users"]["test_user"]["roles"] == ["manager", "accountant"] + - result["users"]["test_user"]["default_roles_list"] == ["accountant", "manager"] or result["users"]["test_user"]["default_roles_list"] == ["manager", "accountant"] - name: Set another role as default in real mode register: result @@ -244,7 +246,7 @@ that: - result is changed - result.executed_statements[0] == "GRANT sales TO test_user" - - result.executed_statements[1] == "REVOKE account, manager FROM test_user" + - result.executed_statements[1] == "REVOKE accountant, manager FROM test_user" or result.executed_statements[1] == "REVOKE manager, accountant FROM test_user" - result.executed_statements[2] == "ALTER USER test_user DEFAULT ROLE sales" - name: Check the actual state @@ -308,8 +310,110 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements[0] == ["GRANT accountant TO test_user"] - - result.executed_statements[1] == ["ALTER USER test_user DEFAULT ROLE sales, accountant"] + - result.executed_statements[0] == "GRANT accountant TO test_user" + - result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE sales, accountant" or result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE accountant, sales" + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["roles"] == ["sales", "accountant"] or result["users"]["test_user"]["roles"] == ["accountant", "sales"] + - result["users"]["test_user"]["default_roles_list"] == ["sales", "accountant"] or result["users"]["test_user"]["default_roles_list"] == ["accountant", "sales"] + +- name: Unset all default roles in check mode + register: result + check_mode: true + community.clickhouse.clickhouse_user: + state: present + name: test_user + default_roles: [] + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements[0] == "SET DEFAULT ROLE NONE TO test_user" + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["roles"] == ["sales", "accountant"] or result["users"]["test_user"]["roles"] == ["accountant", "sales"] + - result["users"]["test_user"]["default_roles_list"] == ["sales", "accountant"] or result["users"]["test_user"]["default_roles_list"] == ["accountant", "sales"] + +- name: Unset all default roles in real mode + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + default_roles: [] + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements[0] == "SET DEFAULT ROLE NONE TO test_user" + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["roles"] == ["sales", "accountant"] or result["users"]["test_user"]["roles"] == ["accountant", "sales"] + - result["users"]["test_user"]["default_roles_list"] == [] + +- name: Revoke all roles in check mode + register: result + check_mode: true + community.clickhouse.clickhouse_user: + state: present + name: test_user + roles: [] + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements[0] == "REVOKE accountant, sales FROM test_user" or result.executed_statements[0] == "REVOKE sales, accountant FROM test_user" + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["roles"] == ["sales", "accountant"] or result["users"]["test_user"]["roles"] == ["accountant", "sales"] + - result["users"]["test_user"]["default_roles_list"] == [] + +- name: Revoke all roles in real mode + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + roles: [] + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements[0] == "REVOKE accountant, sales FROM test_user" or result.executed_statements[0] == "REVOKE sales, accountant FROM test_user" - name: Check the actual state register: result @@ -320,5 +424,58 @@ ansible.builtin.assert: that: - result is not changed - - result["users"]["test_user"]["roles"] == ["sales", "accountant"] - - result["users"]["test_user"]["default_roles_list"] == ["sales", "accountant"] + - result["users"]["test_user"]["roles"] == [] + - result["users"]["test_user"]["default_roles_list"] == [] + +- name: Revoke all roles and unset all default roles in check mode + register: result + check_mode: true + community.clickhouse.clickhouse_user: + state: present + name: test_user + roles: [] + default_roles: [] + +- name: Check ret values + ansible.builtin.assert: + that: + - result is not changed + - result.executed_statements == [] + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["roles"] == [] + - result["users"]["test_user"]["default_roles_list"] == [] + +- name: Revoke all roles and unset all default roles in real mode + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + roles: [] + default_roles: [] + +- name: Check ret values + ansible.builtin.assert: + that: + - result is not changed + - result.executed_statements == [] + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["roles"] == [] + - result["users"]["test_user"]["default_roles_list"] == [] From 672ec81a0838dfa8c1b71573f78dd96338929dbb Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Mon, 19 Aug 2024 16:37:22 +0200 Subject: [PATCH 13/18] Update changelog fragment --- changelogs/fragments/0-clickhouse_user.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/changelogs/fragments/0-clickhouse_user.yml b/changelogs/fragments/0-clickhouse_user.yml index 1cd35d2..a7252b5 100644 --- a/changelogs/fragments/0-clickhouse_user.yml +++ b/changelogs/fragments/0-clickhouse_user.yml @@ -1,2 +1,5 @@ minor_changes: -- clickhouse_user - add the ``default_roles`` argument to set a default role (https://github.com/ansible-collections/community.clickhouse/pull/70). +- clickhouse_user - add the ``roles`` argument to grant roles (https://github.com/ansible-collections/community.clickhouse/pull/70). +- clickhouse_user - add the ``default_roles`` argument to set default roles (https://github.com/ansible-collections/community.clickhouse/pull/70). +- clickhouse_user - add the ``append_roles`` argument to append roles passed through ``roles`` argument (https://github.com/ansible-collections/community.clickhouse/pull/70). +- clickhouse_user - add the ``append_default_roles`` argument to append roles passed through ``default_roles`` argument (https://github.com/ansible-collections/community.clickhouse/pull/70). From 5887e2f7ac3a631089c7c5d67e4b0af67b61db62 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Wed, 21 Aug 2024 07:09:13 +0200 Subject: [PATCH 14/18] Fix examples --- plugins/modules/clickhouse_user.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index 1a1a703..7730226 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -138,7 +138,7 @@ - accountant append_roles: true -- name: Append the sales role to alice's roles +- name: Append the sales role to test_user's roles community.clickhouse.clickhouse_user: login_host: localhost login_user: alice @@ -149,7 +149,7 @@ - sales append_roles: true -- name: Unset all alice's default roles +- name: Unset all test_user's default roles community.clickhouse.clickhouse_user: login_host: localhost login_user: alice @@ -158,7 +158,7 @@ name: test_user default_roles: [] -- name: Revoke all roles from alice +- name: Revoke all roles from test_user community.clickhouse.clickhouse_user: login_host: localhost login_user: alice From ce2ee34d9f8ec3abb65d1d3f890babe6ac23bdeb Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Thu, 22 Aug 2024 10:14:05 +0200 Subject: [PATCH 15/18] New options --- plugins/modules/clickhouse_user.py | 130 ++++++++++-------- .../targets/clickhouse_user/tasks/initial.yml | 6 +- 2 files changed, 73 insertions(+), 63 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index 7730226..ca54b26 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -84,9 +84,9 @@ version_added: '0.5.0' roles: description: - - Grants specified roles for the user. - - To append specified roles to existing ones, also add I(append_roles=true) to your task. - - To revoke all roles, pass an empty list and I(append_roles=false). + - Grants specified roles to the user. + - To append or remove roles, use the I(roles_mode) argument. + - To revoke all roles, pass an empty list (C([]))and I(default_roles_mode=listed_only). type: list elements: str version_added: '0.6.0' @@ -96,28 +96,33 @@ - The roles must be explicitly granted to the user whether manually before using this argument or by using the I(roles) argument in the same task. - - To append specified roles to existing ones, also add I(append_default_roles=true) to your task. - - To unset all roles as default, pass an empty list and I(append_default_roles=false). + - To append or remove roles, use the I(default_roles_mode) argument. + - To unset all roles as default, pass an empty list (C([])) and I(default_roles_mode=listed_only). type: list elements: str version_added: '0.6.0' - append_roles: + roles_mode: description: - - When set to C(true), appends roles specified in I(roles) to existing - user roles instead of removing the user from not specified roles. - - The default is C(false), which will remove the user from all not specified roles. - - Ignored without I(roles) set. - type: bool - default: false + - When C(listed_only) (default), makes the user a member of only roles specified in I(roles). + It will remove the user from all other roles. + - When C(append), appends roles specified in I(roles) to existing user roles. + - When C(remove), removes roles specified in I(roles) from user roles. + - The argument is ignored without I(roles) set. + type: str + choices: ['append', 'listed_only', 'remove'] + default: 'listed_only' version_added: '0.6.0' - append_default_roles: + default_roles_mode: description: - - When set to C(true), appends roles specified in I(default_roles) to existing + - When C(listed_only) (default), sets only roles specified in I(default_roles) as user default roles. + It will unset all other roles as default roles. + - When C(append), appends roles specified in I(default_roles) to existing user default roles. default roles instead of unsetting not specified ones. - - The default is C(false), which will unset all not specified roles. + - When C(remove), removes roles specified in I(default_roles) from user default roles. - Ignored without I(default_roles) set. - type: bool - default: false + type: str + choices: ['append', 'listed_only', 'remove'] + default: 'listed_only' version_added: '0.6.0' ''' @@ -136,7 +141,6 @@ - manager default_roles: - accountant - append_roles: true - name: Append the sales role to test_user's roles community.clickhouse.clickhouse_user: @@ -147,7 +151,7 @@ name: test_user roles: - sales - append_roles: true + roles_mode: append - name: Unset all test_user's default roles community.clickhouse.clickhouse_user: @@ -291,58 +295,62 @@ def create(self): if not self.module.check_mode: execute_query(self.module, self.client, query) - if self.module.params['roles']: + if self.module.params['roles'] and self.module.params['roles_mode'] != 'remove': self.__grant_role(self.module.params['roles']) - if self.module.params['default_roles']: + if self.module.params['default_roles'] and self.module.params['default_roles_mode'] != 'remove': self.__set_default_roles(self.module.params['default_roles']) return True def update(self, update_password): if self.module.params['roles'] is not None: - desired_roles = self.module.params['roles'] - - roles_to_grant = [] - for role in desired_roles: - if role not in self.current_roles: - roles_to_grant.append(role) - - if roles_to_grant: - self.__grant_roles(roles_to_grant) - - if not self.module.params['append_roles']: - roles_to_revoke = [] - for role in self.current_roles: - if role not in desired_roles: - roles_to_revoke.append(role) + desired = set(self.module.params['roles']) + current = set(self.current_roles) + if self.module.params['roles_mode'] == 'remove': + # Remove only roles already present in current roles + roles_to_revoke = list(desired & current) if roles_to_revoke: self.__revoke_roles(roles_to_revoke) + elif self.module.params['roles_mode'] == 'append': + # Grant only roles from decired that + # are not already present in current roles + roles_to_grant = list(desired - current) + if roles_to_grant: + self.__grant_roles(roles_to_grant) + + elif self.module.params['roles_mode'] == 'listed_only': + if self.module.params['roles'] == [] and self.current_roles: + self.__revoke_roles(self.current_roles) + elif self.module.params['roles'] != []: + roles_to_grant = list(desired - current) + roles_to_revoke = list(current - desired) + if roles_to_grant: + self.__grant_roles(roles_to_grant) + if roles_to_revoke: + self.__revoke_roles(roles_to_revoke) + if self.module.params['default_roles'] is not None: - default_roles = self.module.params['default_roles'] - cur_def_roles_set = set(self.current_default_roles) - req_def_roles_set = set(default_roles) - - if self.module.params['append_roles'] is False: - if not req_def_roles_set: - # Update roles info in case all roles were revoked - # in the same task and then unset if the roles list - # is not empty - self.current_roles = self.__fetch_user_groups() - if self.current_roles: - self.__unset_default_roles() - - elif cur_def_roles_set != req_def_roles_set: - self.__set_default_roles(default_roles) - - else: - if cur_def_roles_set != req_def_roles_set: - # Append roles to default roles. - # Use set union to make a list of unique roles - roles_to_set = list(cur_def_roles_set.union(req_def_roles_set)) - self.__set_default_roles(roles_to_set) + desired = set(self.module.params['default_roles']) + current = set(self.current_default_roles) + + if self.module.params['default_roles_mode'] == 'remove': + if self.module.params['default_roles'] != []: + # In this case, "desired" means "desired to get removed" + self.__set_default_roles(list(current - desired)) + + elif self.module.params['default_roles_mode'] == 'append': + if self.module.params['default_roles'] != [] and desired != current: + self.__set_default_roles(list(current.union(desired))) + + elif self.module.params['default_roles_mode'] == 'listed_only': + if self.module.params['default_roles'] == [] and self.current_roles: + self.__unset_default_roles() + + elif self.module.params['default_roles'] != [] and desired != current: + self.__set_default_roles(self.module.params['default_roles']) if update_password == 'on_create': return False or self.changed @@ -429,8 +437,10 @@ def main(): settings=dict(type='list', elements='str'), roles=dict(type='list', elements='str', default=None), default_roles=dict(type='list', elements='str', default=None), - append_roles=dict(type='bool', default=False), - append_default_roles=dict(type='bool', default=False), + roles_mode=dict(type='str', choices=['listed_only', 'append', 'remove'], + default='listed_only'), + default_roles_mode=dict(type='str', choices=['listed_only', 'append', 'remove'], + default='listed_only'), ) # Instantiate an object of module class diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index 7d3bd08..7d1a893 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -130,7 +130,7 @@ - result is changed - result.executed_statements != [] -# Test default_roles argument +# Test roles and default_roles argument - name: Create test roles loop: - accountant @@ -303,8 +303,8 @@ - accountant default_roles: - accountant - append_roles: true - append_default_roles: true + roles_mode: append + default_roles_mode: append - name: Check ret values ansible.builtin.assert: From 612945c4bb2a5ea53ec82395d06ddc4a7d86f335 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Thu, 22 Aug 2024 10:15:55 +0200 Subject: [PATCH 16/18] Update changelog fragment with new options --- changelogs/fragments/0-clickhouse_user.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelogs/fragments/0-clickhouse_user.yml b/changelogs/fragments/0-clickhouse_user.yml index a7252b5..ee3da27 100644 --- a/changelogs/fragments/0-clickhouse_user.yml +++ b/changelogs/fragments/0-clickhouse_user.yml @@ -1,5 +1,5 @@ minor_changes: - clickhouse_user - add the ``roles`` argument to grant roles (https://github.com/ansible-collections/community.clickhouse/pull/70). - clickhouse_user - add the ``default_roles`` argument to set default roles (https://github.com/ansible-collections/community.clickhouse/pull/70). -- clickhouse_user - add the ``append_roles`` argument to append roles passed through ``roles`` argument (https://github.com/ansible-collections/community.clickhouse/pull/70). -- clickhouse_user - add the ``append_default_roles`` argument to append roles passed through ``default_roles`` argument (https://github.com/ansible-collections/community.clickhouse/pull/70). +- clickhouse_user - add the ``roles_mode`` argument to specify how to handle roles passed through ``roles`` argument (https://github.com/ansible-collections/community.clickhouse/pull/70). +- clickhouse_user - add the ``default_roles_mode`` argument to specify how to handle roles passed through ``default_roles`` argument (https://github.com/ansible-collections/community.clickhouse/pull/70). From d4002adb259d59f1de85077b930f92c3fd639d08 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Thu, 22 Aug 2024 10:34:16 +0200 Subject: [PATCH 17/18] Fixes --- plugins/modules/clickhouse_user.py | 2 +- .../targets/clickhouse_user/tasks/initial.yml | 109 +++++++++++++++++- 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index ca54b26..caead80 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -336,7 +336,7 @@ def update(self, update_password): desired = set(self.module.params['default_roles']) current = set(self.current_default_roles) - if self.module.params['default_roles_mode'] == 'remove': + if self.module.params['default_roles_mode'] == 'remove' and desired & current: if self.module.params['default_roles'] != []: # In this case, "desired" means "desired to get removed" self.__set_default_roles(list(current - desired)) diff --git a/tests/integration/targets/clickhouse_user/tasks/initial.yml b/tests/integration/targets/clickhouse_user/tasks/initial.yml index 7d1a893..1b4c212 100644 --- a/tests/integration/targets/clickhouse_user/tasks/initial.yml +++ b/tests/integration/targets/clickhouse_user/tasks/initial.yml @@ -156,7 +156,7 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements[0] == "GRANT accountant, manager TO test_user" + - result.executed_statements[0] == "GRANT accountant, manager TO test_user" or result.executed_statements[0] == "GRANT manager, accountant TO test_user" - result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE accountant, manager" or result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE manager, accountant" - name: Check the actual state @@ -187,7 +187,7 @@ ansible.builtin.assert: that: - result is changed - - result.executed_statements[0] == "GRANT accountant, manager TO test_user" + - result.executed_statements[0] == "GRANT accountant, manager TO test_user" or result.executed_statements[0] == "GRANT manager, accountant TO test_user" - result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE accountant, manager" or result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE manager, accountant" - name: Check the actual state @@ -479,3 +479,108 @@ - result is not changed - result["users"]["test_user"]["roles"] == [] - result["users"]["test_user"]["default_roles_list"] == [] + +- name: Set default role in real mode + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + roles: + - accountant + - manager + default_roles: + - accountant + - manager + +- name: Remove role and default role in check mode + register: result + check_mode: true + community.clickhouse.clickhouse_user: + state: present + name: test_user + roles: + - manager + default_roles: + - manager + roles_mode: remove + default_roles_mode: remove + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements[0] == "REVOKE manager FROM test_user" + - result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE accountant" + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["roles"] == ["accountant", "manager"] or result["users"]["test_user"]["roles"] == ["manager", "accountant"] + - result["users"]["test_user"]["default_roles_list"] == ["accountant", "manager"] + +- name: Remove role and default role in real mode + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + roles: + - manager + default_roles: + - manager + roles_mode: remove + default_roles_mode: remove + +- name: Check ret values + ansible.builtin.assert: + that: + - result is changed + - result.executed_statements[0] == "REVOKE manager FROM test_user" + - result.executed_statements[1] == "ALTER USER test_user DEFAULT ROLE accountant" + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["roles"] == ["accountant"] + - result["users"]["test_user"]["default_roles_list"] == ["accountant"] + +- name: Remove role and default role in real mode again + register: result + community.clickhouse.clickhouse_user: + state: present + name: test_user + roles: + - manager + default_roles: + - manager + roles_mode: remove + default_roles_mode: remove + +- name: Check ret values + ansible.builtin.assert: + that: + - result is not changed + - result.executed_statements == [] + +- name: Check the actual state + register: result + community.clickhouse.clickhouse_info: + login_host: localhost + +- name: Check result + ansible.builtin.assert: + that: + - result is not changed + - result["users"]["test_user"]["roles"] == ["accountant"] + - result["users"]["test_user"]["default_roles_list"] == ["accountant"] From 0521833901bb1f2cd47e294218f6dceeca22ff60 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Thu, 22 Aug 2024 10:37:42 +0200 Subject: [PATCH 18/18] Fix a typo --- plugins/modules/clickhouse_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/clickhouse_user.py b/plugins/modules/clickhouse_user.py index caead80..888188d 100644 --- a/plugins/modules/clickhouse_user.py +++ b/plugins/modules/clickhouse_user.py @@ -86,7 +86,7 @@ description: - Grants specified roles to the user. - To append or remove roles, use the I(roles_mode) argument. - - To revoke all roles, pass an empty list (C([]))and I(default_roles_mode=listed_only). + - To revoke all roles, pass an empty list (C([])) and I(default_roles_mode=listed_only). type: list elements: str version_added: '0.6.0'