From 44679e71a23c37bfe9a047bd4243666d4b12c459 Mon Sep 17 00:00:00 2001 From: Florian Apolloner Date: Sun, 21 Jan 2024 18:29:29 +0100 Subject: [PATCH] Refactor of consul modules (#7826) * Extract common functionality. * Refactor duplicated code into module_utils. * Fixed ansible-test issues. * Address review comments. * Revert changes to consul_acl. It uses deprecated APIs disabled since Consul 1.11 (which is EOL), don't bother updating the module anymore. * Remove unused code. * Merge token into default doc fragment. * JSON all the way down. * extract validation tests into custom file and prep for requests removal. * Removed dependency on requests. * Initial test for consul_kv. * fixup license headers. * Revert changes to consul.py since it utilizes python-consul. * Disable the lookup test for now. * Fix python 2.7 support. * Address review comments. * Address review comments. * Addec changelog fragment. * Mark ConsulModule as private. --- .../7826-consul-modules-refactoring.yaml | 2 + plugins/doc_fragments/consul.py | 44 +++++ plugins/module_utils/consul.py | 88 +++++++++ plugins/modules/consul_policy.py | 183 +++--------------- plugins/modules/consul_role.py | 158 +++------------ plugins/modules/consul_session.py | 172 ++++------------ .../targets/consul/tasks/consul_general.yml | 76 ++++++++ .../targets/consul/tasks/consul_kv.yml | 57 ++++++ .../targets/consul/tasks/consul_session.yml | 59 ------ .../integration/targets/consul/tasks/main.yml | 2 + 10 files changed, 365 insertions(+), 476 deletions(-) create mode 100644 changelogs/fragments/7826-consul-modules-refactoring.yaml create mode 100644 plugins/doc_fragments/consul.py create mode 100644 tests/integration/targets/consul/tasks/consul_general.yml create mode 100644 tests/integration/targets/consul/tasks/consul_kv.yml diff --git a/changelogs/fragments/7826-consul-modules-refactoring.yaml b/changelogs/fragments/7826-consul-modules-refactoring.yaml new file mode 100644 index 0000000000..b9e5a92849 --- /dev/null +++ b/changelogs/fragments/7826-consul-modules-refactoring.yaml @@ -0,0 +1,2 @@ +minor_changes: + - 'consul_policy, consul_role, consul_session - removed dependency on ``requests`` and factored out common parts (https://github.com/ansible-collections/community.general/pull/7826).' \ No newline at end of file diff --git a/plugins/doc_fragments/consul.py b/plugins/doc_fragments/consul.py new file mode 100644 index 0000000000..c2407439c8 --- /dev/null +++ b/plugins/doc_fragments/consul.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- +# Copyright (c) Ansible project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +class ModuleDocFragment: + # Common parameters for Consul modules + DOCUMENTATION = r""" +options: + host: + description: + - Host of the consul agent, defaults to V(localhost). + default: localhost + type: str + port: + type: int + description: + - The port on which the consul agent is running. + default: 8500 + scheme: + description: + - The protocol scheme on which the consul agent is running. + Defaults to V(http) and can be set to V(https) for secure connections. + default: http + type: str + validate_certs: + type: bool + description: + - Whether to verify the TLS certificate of the consul agent. + default: true + token: + description: + - The token to use for authorization. + type: str + ca_path: + description: + - The CA bundle to use for https connections + type: str +""" diff --git a/plugins/module_utils/consul.py b/plugins/module_utils/consul.py index 1e820b7fe0..cb89179855 100644 --- a/plugins/module_utils/consul.py +++ b/plugins/module_utils/consul.py @@ -7,6 +7,12 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type +import json + +from ansible.module_utils.six.moves.urllib import error as urllib_error +from ansible.module_utils.six.moves.urllib.parse import urlencode +from ansible.module_utils.urls import open_url + def get_consul_url(configuration): return '%s://%s:%s/v1' % (configuration.scheme, @@ -27,3 +33,85 @@ class RequestError(Exception): def handle_consul_response_error(response): if 400 <= response.status_code < 600: raise RequestError('%d %s' % (response.status_code, response.content)) + + +def auth_argument_spec(): + return dict( + host=dict(default="localhost"), + port=dict(type="int", default=8500), + scheme=dict(default="http"), + validate_certs=dict(type="bool", default=True), + token=dict(no_log=True), + ca_path=dict(), + ) + + +class _ConsulModule: + """Base class for Consul modules. + + This class is considered private, till the API is fully fleshed out. + As such backwards incompatible changes can occur even in bugfix releases. + """ + + def __init__(self, module): + self.module = module + + def _request(self, method, url_parts, data=None, params=None): + module_params = self.module.params + + if isinstance(url_parts, str): + url_parts = [url_parts] + if params: + # Remove values that are None + params = {k: v for k, v in params.items() if v is not None} + + ca_path = module_params.get("ca_path") + base_url = "%s://%s:%s/v1" % ( + module_params["scheme"], + module_params["host"], + module_params["port"], + ) + url = "/".join([base_url] + list(url_parts)) + + headers = {} + token = self.module.params.get("token") + if token: + headers["X-Consul-Token"] = token + + try: + if data: + data = json.dumps(data) + headers["Content-Type"] = "application/json" + if params: + url = "%s?%s" % (url, urlencode(params)) + response = open_url( + url, + method=method, + data=data, + headers=headers, + validate_certs=module_params["validate_certs"], + ca_path=ca_path, + ) + response_data = response.read() + except urllib_error.URLError as e: + self.module.fail_json( + msg="Could not connect to consul agent at %s:%s, error was %s" + % (module_params["host"], module_params["port"], str(e)) + ) + else: + status = ( + response.status if hasattr(response, "status") else response.getcode() + ) + if 400 <= status < 600: + raise RequestError("%d %s" % (status, response_data)) + + return json.loads(response_data) + + def get(self, url_parts, **kwargs): + return self._request("GET", url_parts, **kwargs) + + def put(self, url_parts, **kwargs): + return self._request("PUT", url_parts, **kwargs) + + def delete(self, url_parts, **kwargs): + return self._request("DELETE", url_parts, **kwargs) diff --git a/plugins/modules/consul_policy.py b/plugins/modules/consul_policy.py index 091716a67c..6b62009f8d 100644 --- a/plugins/modules/consul_policy.py +++ b/plugins/modules/consul_policy.py @@ -19,6 +19,7 @@ description: author: - Håkon Lerring (@Hakon) extends_documentation_fragment: + - community.general.consul - community.general.attributes attributes: check_mode: @@ -29,7 +30,6 @@ options: state: description: - Whether the policy should be present or absent. - required: false choices: ['present', 'absent'] default: present type: str @@ -48,44 +48,12 @@ options: description: description: - Description of the policy. - required: false type: str default: '' rules: type: str description: - Rule document that should be associated with the current policy. - required: false - host: - description: - - Host of the consul agent, defaults to localhost. - required: false - default: localhost - type: str - port: - type: int - description: - - The port on which the consul agent is running. - required: false - default: 8500 - scheme: - description: - - The protocol scheme on which the consul agent is running. - required: false - default: http - type: str - token: - description: - - A management token is required to manipulate the policies. - type: str - validate_certs: - type: bool - description: - - Whether to verify the TLS certificate of the consul agent or not. - required: false - default: true -requirements: - - requests ''' EXAMPLES = """ @@ -135,22 +103,11 @@ operation: """ from ansible.module_utils.basic import AnsibleModule +from ansible_collections.community.general.plugins.module_utils.consul import ( + _ConsulModule, auth_argument_spec) -try: - from requests.exceptions import ConnectionError - import requests - has_requests = True -except ImportError: - has_requests = False - - -TOKEN_PARAMETER_NAME = "token" -HOST_PARAMETER_NAME = "host" -SCHEME_PARAMETER_NAME = "scheme" -VALIDATE_CERTS_PARAMETER_NAME = "validate_certs" NAME_PARAMETER_NAME = "name" DESCRIPTION_PARAMETER_NAME = "description" -PORT_PARAMETER_NAME = "port" RULES_PARAMETER_NAME = "rules" VALID_DATACENTERS_PARAMETER_NAME = "valid_datacenters" STATE_PARAMETER_NAME = "state" @@ -166,50 +123,20 @@ CREATE_OPERATION = "create" _ARGUMENT_SPEC = { NAME_PARAMETER_NAME: dict(required=True), DESCRIPTION_PARAMETER_NAME: dict(required=False, type='str', default=''), - PORT_PARAMETER_NAME: dict(default=8500, type='int'), RULES_PARAMETER_NAME: dict(type='str'), VALID_DATACENTERS_PARAMETER_NAME: dict(type='list', elements='str', default=[]), - HOST_PARAMETER_NAME: dict(default='localhost'), - SCHEME_PARAMETER_NAME: dict(default='http'), - TOKEN_PARAMETER_NAME: dict(no_log=True), - VALIDATE_CERTS_PARAMETER_NAME: dict(type='bool', default=True), - STATE_PARAMETER_NAME: dict(default=PRESENT_STATE_VALUE, choices=[PRESENT_STATE_VALUE, ABSENT_STATE_VALUE]), + STATE_PARAMETER_NAME: dict(default=PRESENT_STATE_VALUE, choices=[PRESENT_STATE_VALUE, ABSENT_STATE_VALUE]) } +_ARGUMENT_SPEC.update(auth_argument_spec()) -def get_consul_url(configuration): - return '%s://%s:%s/v1' % (configuration.scheme, - configuration.host, configuration.port) - - -def get_auth_headers(configuration): - if configuration.token is None: - return {} - else: - return {'X-Consul-Token': configuration.token} - - -class RequestError(Exception): - pass - - -def handle_consul_response_error(response): - if 400 <= response.status_code < 600: - raise RequestError('%d %s' % (response.status_code, response.content)) - - -def update_policy(policy, configuration): - url = '%s/acl/policy/%s' % (get_consul_url(configuration), policy['ID']) - headers = get_auth_headers(configuration) - response = requests.put(url, headers=headers, json={ +def update_policy(policy, configuration, consul_module): + updated_policy = consul_module.put(('acl', 'policy', policy['ID']), data={ 'Name': configuration.name, # should be equal at this point. 'Description': configuration.description, 'Rules': configuration.rules, 'Datacenters': configuration.valid_datacenters - }, verify=configuration.validate_certs) - handle_consul_response_error(response) - - updated_policy = response.json() + }) changed = ( policy.get('Rules', "") != updated_policy.get('Rules', "") or @@ -220,35 +147,24 @@ def update_policy(policy, configuration): return Output(changed=changed, operation=UPDATE_OPERATION, policy=updated_policy) -def create_policy(configuration): - url = '%s/acl/policy' % get_consul_url(configuration) - headers = get_auth_headers(configuration) - response = requests.put(url, headers=headers, json={ +def create_policy(configuration, consul_module): + created_policy = consul_module.put('acl/policy', data={ 'Name': configuration.name, 'Description': configuration.description, 'Rules': configuration.rules, 'Datacenters': configuration.valid_datacenters - }, verify=configuration.validate_certs) - handle_consul_response_error(response) - - created_policy = response.json() - + }) return Output(changed=True, operation=CREATE_OPERATION, policy=created_policy) -def remove_policy(configuration): - policies = get_policies(configuration) +def remove_policy(configuration, consul_module): + policies = get_policies(consul_module) if configuration.name in policies: policy_id = policies[configuration.name]['ID'] - policy = get_policy(policy_id, configuration) - - url = '%s/acl/policy/%s' % (get_consul_url(configuration), - policy['ID']) - headers = get_auth_headers(configuration) - response = requests.delete(url, headers=headers, verify=configuration.validate_certs) - handle_consul_response_error(response) + policy = get_policy(policy_id, consul_module) + consul_module.delete(('acl', 'policy', policy['ID'])) changed = True else: @@ -256,38 +172,30 @@ def remove_policy(configuration): return Output(changed=changed, operation=REMOVE_OPERATION) -def get_policies(configuration): - url = '%s/acl/policies' % get_consul_url(configuration) - headers = get_auth_headers(configuration) - response = requests.get(url, headers=headers, verify=configuration.validate_certs) - handle_consul_response_error(response) - policies = response.json() +def get_policies(consul_module): + policies = consul_module.get('acl/policies') existing_policies_mapped_by_name = dict( (policy['Name'], policy) for policy in policies if policy['Name'] is not None) return existing_policies_mapped_by_name -def get_policy(id, configuration): - url = '%s/acl/policy/%s' % (get_consul_url(configuration), id) - headers = get_auth_headers(configuration) - response = requests.get(url, headers=headers, verify=configuration.validate_certs) - handle_consul_response_error(response) - return response.json() +def get_policy(id, consul_module): + return consul_module.get(('acl', 'policy', id)) -def set_policy(configuration): - policies = get_policies(configuration) +def set_policy(configuration, consul_module): + policies = get_policies(consul_module) if configuration.name in policies: index_policy_object = policies[configuration.name] policy_id = policies[configuration.name]['ID'] - rest_policy_object = get_policy(policy_id, configuration) + rest_policy_object = get_policy(policy_id, consul_module) # merge dicts as some keys are only available in the partial policy policy = index_policy_object.copy() policy.update(rest_policy_object) - return update_policy(policy, configuration) + return update_policy(policy, configuration, consul_module) else: - return create_policy(configuration) + return create_policy(configuration, consul_module) class Configuration: @@ -295,15 +203,9 @@ class Configuration: Configuration for this module. """ - def __init__(self, token=None, host=None, scheme=None, validate_certs=None, name=None, description=None, port=None, - rules=None, valid_datacenters=None, state=None): - self.token = token # type: str - self.host = host # type: str - self.scheme = scheme # type: str - self.validate_certs = validate_certs # type: bool + def __init__(self, name=None, description=None, rules=None, valid_datacenters=None, state=None): self.name = name # type: str self.description = description # type: str - self.port = port # type: int self.rules = rules # type: str self.valid_datacenters = valid_datacenters # type: str self.state = state # type: str @@ -320,50 +222,25 @@ class Output: self.policy = policy # type: dict -def check_dependencies(): - """ - Checks that the required dependencies have been imported. - :exception ImportError: if it is detected that any of the required dependencies have not been imported - """ - - if not has_requests: - raise ImportError( - "requests required for this module. See https://pypi.org/project/requests/") - - def main(): """ Main method. """ module = AnsibleModule(_ARGUMENT_SPEC, supports_check_mode=False) - - try: - check_dependencies() - except ImportError as e: - module.fail_json(msg=str(e)) + consul_module = _ConsulModule(module) configuration = Configuration( - token=module.params.get(TOKEN_PARAMETER_NAME), - host=module.params.get(HOST_PARAMETER_NAME), - scheme=module.params.get(SCHEME_PARAMETER_NAME), - validate_certs=module.params.get(VALIDATE_CERTS_PARAMETER_NAME), name=module.params.get(NAME_PARAMETER_NAME), description=module.params.get(DESCRIPTION_PARAMETER_NAME), - port=module.params.get(PORT_PARAMETER_NAME), rules=module.params.get(RULES_PARAMETER_NAME), valid_datacenters=module.params.get(VALID_DATACENTERS_PARAMETER_NAME), state=module.params.get(STATE_PARAMETER_NAME), ) - try: - if configuration.state == PRESENT_STATE_VALUE: - output = set_policy(configuration) - else: - output = remove_policy(configuration) - except ConnectionError as e: - module.fail_json(msg='Could not connect to consul agent at %s:%s, error was %s' % ( - configuration.host, configuration.port, str(e))) - raise + if configuration.state == PRESENT_STATE_VALUE: + output = set_policy(configuration, consul_module) + else: + output = remove_policy(configuration, consul_module) return_values = dict(changed=output.changed, operation=output.operation, policy=output.policy) module.exit_json(**return_values) diff --git a/plugins/modules/consul_role.py b/plugins/modules/consul_role.py index 872949a296..e38afba562 100644 --- a/plugins/modules/consul_role.py +++ b/plugins/modules/consul_role.py @@ -19,6 +19,7 @@ description: author: - Håkon Lerring (@Hakon) extends_documentation_fragment: + - community.general.consul - community.general.attributes attributes: check_mode: @@ -34,7 +35,6 @@ options: state: description: - whether the role should be present or absent. - required: false choices: ['present', 'absent'] default: present type: str @@ -42,7 +42,6 @@ options: description: - Description of the role. - If not specified, the assigned description will not be changed. - required: false type: str policies: type: list @@ -51,7 +50,6 @@ options: - List of policies to attach to the role. Each policy is a dict. - If the parameter is left blank, any policies currently assigned will not be changed. - Any empty array (V([])) will clear any policies previously set. - required: false suboptions: name: description: @@ -70,7 +68,6 @@ options: - List of service identities to attach to the role. - If not specified, any service identities currently assigned will not be changed. - If the parameter is an empty array (V([])), any node identities assigned will be unassigned. - required: false suboptions: name: description: @@ -95,7 +92,6 @@ options: - List of node identities to attach to the role. - If not specified, any node identities currently assigned will not be changed. - If the parameter is an empty array (V([])), any node identities assigned will be unassigned. - required: false suboptions: name: description: @@ -110,36 +106,6 @@ options: - This will result in effective policy only being valid in this datacenter. type: str required: true - host: - description: - - Host of the consul agent, defaults to V(localhost). - required: false - default: localhost - type: str - port: - type: int - description: - - The port on which the consul agent is running. - required: false - default: 8500 - scheme: - description: - - The protocol scheme on which the consul agent is running. - required: false - default: http - type: str - token: - description: - - A management token is required to manipulate the roles. - type: str - validate_certs: - type: bool - description: - - Whether to verify the TLS certificate of the consul agent. - required: false - default: true -requirements: - - requests ''' EXAMPLES = """ @@ -204,28 +170,11 @@ operation: """ from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.basic import missing_required_lib from ansible_collections.community.general.plugins.module_utils.consul import ( - get_consul_url, get_auth_headers, handle_consul_response_error) -import traceback + _ConsulModule, auth_argument_spec) -REQUESTS_IMP_ERR = None - -try: - from requests.exceptions import ConnectionError - import requests - HAS_REQUESTS = True -except ImportError: - HAS_REQUESTS = False - REQUESTS_IMP_ERR = traceback.format_exc() - -TOKEN_PARAMETER_NAME = "token" -HOST_PARAMETER_NAME = "host" -SCHEME_PARAMETER_NAME = "scheme" -VALIDATE_CERTS_PARAMETER_NAME = "validate_certs" NAME_PARAMETER_NAME = "name" DESCRIPTION_PARAMETER_NAME = "description" -PORT_PARAMETER_NAME = "port" POLICIES_PARAMETER_NAME = "policies" SERVICE_IDENTITIES_PARAMETER_NAME = "service_identities" NODE_IDENTITIES_PARAMETER_NAME = "node_identities" @@ -254,19 +203,15 @@ SERVICE_ID_RULE_SPEC = dict( ) _ARGUMENT_SPEC = { - TOKEN_PARAMETER_NAME: dict(no_log=True), - PORT_PARAMETER_NAME: dict(default=8500, type='int'), - HOST_PARAMETER_NAME: dict(default='localhost'), - SCHEME_PARAMETER_NAME: dict(default='http'), - VALIDATE_CERTS_PARAMETER_NAME: dict(type='bool', default=True), NAME_PARAMETER_NAME: dict(required=True), DESCRIPTION_PARAMETER_NAME: dict(required=False, type='str', default=None), POLICIES_PARAMETER_NAME: dict(type='list', elements='dict', options=POLICY_RULE_SPEC, mutually_exclusive=[('name', 'id')], required_one_of=[('name', 'id')], default=None), SERVICE_IDENTITIES_PARAMETER_NAME: dict(type='list', elements='dict', options=SERVICE_ID_RULE_SPEC, default=None), NODE_IDENTITIES_PARAMETER_NAME: dict(type='list', elements='dict', options=NODE_ID_RULE_SPEC, default=None), - STATE_PARAMETER_NAME: dict(default=PRESENT_STATE_VALUE, choices=[PRESENT_STATE_VALUE, ABSENT_STATE_VALUE]), + STATE_PARAMETER_NAME: dict(default=PRESENT_STATE_VALUE, choices=[PRESENT_STATE_VALUE, ABSENT_STATE_VALUE]) } +_ARGUMENT_SPEC.update(auth_argument_spec()) def compare_consul_api_role_policy_objects(first, second): @@ -280,11 +225,7 @@ def compare_consul_api_role_policy_objects(first, second): return first == second -def update_role(role, configuration): - url = '%s/acl/role/%s' % (get_consul_url(configuration), - role['ID']) - headers = get_auth_headers(configuration) - +def update_role(role, configuration, consul_module): update_role_data = { 'Name': configuration.name, 'Description': configuration.description, @@ -370,10 +311,7 @@ def update_role(role, configuration): if not node_id_specified and role.get('NodeIdentities') is not None: update_role_data["NodeIdentities"] = role.get('NodeIdentities') - response = requests.put(url, headers=headers, json=update_role_data, verify=configuration.validate_certs) - handle_consul_response_error(response) - - resulting_role = response.json() + resulting_role = consul_module.put(('acl', 'role', role['ID']), data=update_role_data) changed = ( role['Description'] != resulting_role['Description'] or role.get('Policies', None) != resulting_role.get('Policies', None) or @@ -384,10 +322,7 @@ def update_role(role, configuration): return Output(changed=changed, operation=UPDATE_OPERATION, role=resulting_role) -def create_role(configuration): - url = '%s/acl/role' % get_consul_url(configuration) - headers = get_auth_headers(configuration) - +def create_role(configuration, consul_module): # check if the user omitted policies, service identities, or node identities policy_specified = True if len(configuration.policies) == 1 and configuration.policies[0] is None: @@ -423,28 +358,21 @@ def create_role(configuration): create_role_data["NodeIdentities"] = [x.to_dict() for x in configuration.node_identities] if not configuration.check_mode: - response = requests.put(url, headers=headers, json=create_role_data, verify=configuration.validate_certs) - handle_consul_response_error(response) - - resulting_role = response.json() - + resulting_role = consul_module.put('acl/role', data=create_role_data) return Output(changed=True, operation=CREATE_OPERATION, role=resulting_role) else: return Output(changed=True, operation=CREATE_OPERATION) -def remove_role(configuration): - roles = get_roles(configuration) +def remove_role(configuration, consul_module): + roles = get_roles(consul_module) if configuration.name in roles: role_id = roles[configuration.name]['ID'] if not configuration.check_mode: - url = '%s/acl/role/%s' % (get_consul_url(configuration), role_id) - headers = get_auth_headers(configuration) - response = requests.delete(url, headers=headers, verify=configuration.validate_certs) - handle_consul_response_error(response) + consul_module.delete(('acl', 'role', role_id)) changed = True else: @@ -452,33 +380,25 @@ def remove_role(configuration): return Output(changed=changed, operation=REMOVE_OPERATION) -def get_roles(configuration): - url = '%s/acl/roles' % get_consul_url(configuration) - headers = get_auth_headers(configuration) - response = requests.get(url, headers=headers, verify=configuration.validate_certs) - handle_consul_response_error(response) - roles = response.json() +def get_roles(consul_module): + roles = consul_module.get('acl/roles') existing_roles_mapped_by_id = dict((role['Name'], role) for role in roles if role['Name'] is not None) return existing_roles_mapped_by_id -def get_consul_version(configuration): - url = '%s/agent/self' % get_consul_url(configuration) - headers = get_auth_headers(configuration) - response = requests.get(url, headers=headers, verify=configuration.validate_certs) - handle_consul_response_error(response) - config = response.json()["Config"] +def get_consul_version(consul_module): + config = consul_module.get('agent/self')["Config"] return ConsulVersion(config["Version"]) -def set_role(configuration): - roles = get_roles(configuration) +def set_role(configuration, consul_module): + roles = get_roles(consul_module) if configuration.name in roles: role = roles[configuration.name] - return update_role(role, configuration) + return update_role(role, configuration, consul_module) else: - return create_role(configuration) + return create_role(configuration, consul_module) class ConsulVersion: @@ -556,13 +476,8 @@ class Configuration: Configuration for this module. """ - def __init__(self, token=None, host=None, scheme=None, validate_certs=None, name=None, description=None, port=None, - policies=None, service_identities=None, node_identities=None, state=None, check_mode=None): - self.token = token # type: str - self.host = host # type: str - self.port = port # type: int - self.scheme = scheme # type: str - self.validate_certs = validate_certs # type: bool + def __init__(self, name=None, description=None, policies=None, service_identities=None, + node_identities=None, state=None, check_mode=None): self.name = name # type: str self.description = description # type: str if policies is not None: @@ -597,44 +512,29 @@ def main(): Main method. """ module = AnsibleModule(_ARGUMENT_SPEC, supports_check_mode=True) - - if not HAS_REQUESTS: - module.fail_json(msg=missing_required_lib("requests"), - exception=REQUESTS_IMP_ERR) + consul_module = _ConsulModule(module) try: configuration = Configuration( - token=module.params.get(TOKEN_PARAMETER_NAME), - host=module.params.get(HOST_PARAMETER_NAME), - port=module.params.get(PORT_PARAMETER_NAME), - scheme=module.params.get(SCHEME_PARAMETER_NAME), - validate_certs=module.params.get(VALIDATE_CERTS_PARAMETER_NAME), name=module.params.get(NAME_PARAMETER_NAME), description=module.params.get(DESCRIPTION_PARAMETER_NAME), policies=module.params.get(POLICIES_PARAMETER_NAME), service_identities=module.params.get(SERVICE_IDENTITIES_PARAMETER_NAME), node_identities=module.params.get(NODE_IDENTITIES_PARAMETER_NAME), state=module.params.get(STATE_PARAMETER_NAME), - check_mode=module.check_mode - + check_mode=module.check_mode, ) except ValueError as err: module.fail_json(msg='Configuration error: %s' % str(err)) return - try: + version = get_consul_version(consul_module) + configuration.version = version - version = get_consul_version(configuration) - configuration.version = version - - if configuration.state == PRESENT_STATE_VALUE: - output = set_role(configuration) - else: - output = remove_role(configuration) - except ConnectionError as e: - module.fail_json(msg='Could not connect to consul agent at %s:%s, error was %s' % ( - configuration.host, configuration.port, str(e))) - raise + if configuration.state == PRESENT_STATE_VALUE: + output = set_role(configuration, consul_module) + else: + output = remove_role(configuration, consul_module) return_values = dict(changed=output.changed, operation=output.operation, role=output.role) module.exit_json(**return_values) diff --git a/plugins/modules/consul_session.py b/plugins/modules/consul_session.py index a69f8c918f..6ab071e143 100644 --- a/plugins/modules/consul_session.py +++ b/plugins/modules/consul_session.py @@ -16,12 +16,11 @@ description: cluster. These sessions can then be used in conjunction with key value pairs to implement distributed locks. In depth documentation for working with sessions can be found at http://www.consul.io/docs/internals/sessions.html -requirements: - - requests author: - Steve Gargan (@sgargan) - Håkon Lerring (@Hakon) extends_documentation_fragment: + - community.general.consul - community.general.attributes attributes: check_mode: @@ -76,26 +75,6 @@ options: the associated lock delay has expired. type: list elements: str - host: - description: - - The host of the consul agent defaults to localhost. - type: str - default: localhost - port: - description: - - The port on which the consul agent is running. - type: int - default: 8500 - scheme: - description: - - The protocol scheme on which the consul agent is running. - type: str - default: http - validate_certs: - description: - - Whether to verify the TLS certificate of the consul agent. - type: bool - default: true behavior: description: - The optional behavior that can be attached to the session when it @@ -109,10 +88,6 @@ options: type: int version_added: 5.4.0 token: - description: - - The token key identifying an ACL rule set that controls access to - the key value pair. - type: str version_added: 5.6.0 ''' @@ -148,95 +123,49 @@ EXAMPLES = ''' ''' from ansible.module_utils.basic import AnsibleModule - -try: - import requests - from requests.exceptions import ConnectionError - has_requests = True -except ImportError: - has_requests = False +from ansible_collections.community.general.plugins.module_utils.consul import ( + auth_argument_spec, _ConsulModule +) -def execute(module): +def execute(module, consul_module): state = module.params.get('state') if state in ['info', 'list', 'node']: - lookup_sessions(module) + lookup_sessions(module, consul_module) elif state == 'present': - update_session(module) + update_session(module, consul_module) else: - remove_session(module) + remove_session(module, consul_module) -class RequestError(Exception): - pass +def list_sessions(consul_module, datacenter): + return consul_module.get( + 'session/list', + params={'dc': datacenter}) -def handle_consul_response_error(response): - if 400 <= response.status_code < 600: - raise RequestError('%d %s' % (response.status_code, response.content)) +def list_sessions_for_node(consul_module, node, datacenter): + return consul_module.get( + ('session', 'node', node), + params={'dc': datacenter}) -def get_consul_url(module): - return '%s://%s:%s/v1' % (module.params.get('scheme'), - module.params.get('host'), module.params.get('port')) +def get_session_info(consul_module, session_id, datacenter): + return consul_module.get( + ('session', 'info', session_id), + params={'dc': datacenter}) -def get_auth_headers(module): - if 'token' in module.params and module.params.get('token') is not None: - return {'X-Consul-Token': module.params.get('token')} - else: - return {} - - -def list_sessions(module, datacenter): - url = '%s/session/list' % get_consul_url(module) - headers = get_auth_headers(module) - response = requests.get( - url, - headers=headers, - params={ - 'dc': datacenter}, - verify=module.params.get('validate_certs')) - handle_consul_response_error(response) - return response.json() - - -def list_sessions_for_node(module, node, datacenter): - url = '%s/session/node/%s' % (get_consul_url(module), node) - headers = get_auth_headers(module) - response = requests.get( - url, - headers=headers, - params={ - 'dc': datacenter}, - verify=module.params.get('validate_certs')) - handle_consul_response_error(response) - return response.json() - - -def get_session_info(module, session_id, datacenter): - url = '%s/session/info/%s' % (get_consul_url(module), session_id) - headers = get_auth_headers(module) - response = requests.get( - url, - headers=headers, - params={ - 'dc': datacenter}, - verify=module.params.get('validate_certs')) - handle_consul_response_error(response) - return response.json() - - -def lookup_sessions(module): +def lookup_sessions(module, consul_module): datacenter = module.params.get('datacenter') state = module.params.get('state') try: if state == 'list': - sessions_list = list_sessions(module, datacenter) + sessions_list = list_sessions(consul_module, datacenter) # Ditch the index, this can be grabbed from the results if sessions_list and len(sessions_list) >= 2: sessions_list = sessions_list[1] @@ -244,14 +173,14 @@ def lookup_sessions(module): sessions=sessions_list) elif state == 'node': node = module.params.get('node') - sessions = list_sessions_for_node(module, node, datacenter) + sessions = list_sessions_for_node(consul_module, node, datacenter) module.exit_json(changed=True, node=node, sessions=sessions) elif state == 'info': session_id = module.params.get('id') - session_by_id = get_session_info(module, session_id, datacenter) + session_by_id = get_session_info(consul_module, session_id, datacenter) module.exit_json(changed=True, session_id=session_id, sessions=session_by_id) @@ -260,10 +189,8 @@ def lookup_sessions(module): module.fail_json(msg="Could not retrieve session info %s" % e) -def create_session(module, name, behavior, ttl, node, +def create_session(consul_module, name, behavior, ttl, node, lock_delay, datacenter, checks): - url = '%s/session/create' % get_consul_url(module) - headers = get_auth_headers(module) create_data = { "LockDelay": lock_delay, "Node": node, @@ -273,19 +200,15 @@ def create_session(module, name, behavior, ttl, node, } if ttl is not None: create_data["TTL"] = "%ss" % str(ttl) # TTL is in seconds - response = requests.put( - url, - headers=headers, + create_session_response_dict = consul_module.put( + 'session/create', params={ 'dc': datacenter}, - json=create_data, - verify=module.params.get('validate_certs')) - handle_consul_response_error(response) - create_session_response_dict = response.json() + data=create_data) return create_session_response_dict["ID"] -def update_session(module): +def update_session(module, consul_module): name = module.params.get('name') delay = module.params.get('delay') @@ -296,7 +219,7 @@ def update_session(module): ttl = module.params.get('ttl') try: - session = create_session(module, + session = create_session(consul_module, name=name, behavior=behavior, ttl=ttl, @@ -317,22 +240,15 @@ def update_session(module): module.fail_json(msg="Could not create/update session %s" % e) -def destroy_session(module, session_id): - url = '%s/session/destroy/%s' % (get_consul_url(module), session_id) - headers = get_auth_headers(module) - response = requests.put( - url, - headers=headers, - verify=module.params.get('validate_certs')) - handle_consul_response_error(response) - return response.content == "true" +def destroy_session(consul_module, session_id): + return consul_module.put(('session', 'destroy', session_id)) -def remove_session(module): +def remove_session(module, consul_module): session_id = module.params.get('id') try: - destroy_session(module, session_id) + destroy_session(consul_module, session_id) module.exit_json(changed=True, session_id=session_id) @@ -341,12 +257,6 @@ def remove_session(module): session_id, e)) -def test_dependencies(module): - if not has_requests: - raise ImportError( - "requests required for this module. See https://pypi.org/project/requests/") - - def main(): argument_spec = dict( checks=dict(type='list', elements='str'), @@ -358,10 +268,6 @@ def main(): 'release', 'delete']), ttl=dict(type='int'), - host=dict(type='str', default='localhost'), - port=dict(type='int', default=8500), - scheme=dict(type='str', default='http'), - validate_certs=dict(type='bool', default=True), id=dict(type='str'), name=dict(type='str'), node=dict(type='str'), @@ -375,7 +281,7 @@ def main(): 'node', 'present']), datacenter=dict(type='str'), - token=dict(type='str', no_log=True), + **auth_argument_spec() ) module = AnsibleModule( @@ -387,14 +293,10 @@ def main(): ], supports_check_mode=False ) - - test_dependencies(module) + consul_module = _ConsulModule(module) try: - execute(module) - except ConnectionError as e: - module.fail_json(msg='Could not connect to consul agent at %s:%s, error was %s' % ( - module.params.get('host'), module.params.get('port'), e)) + execute(module, consul_module) except Exception as e: module.fail_json(msg=str(e)) diff --git a/tests/integration/targets/consul/tasks/consul_general.yml b/tests/integration/targets/consul/tasks/consul_general.yml new file mode 100644 index 0000000000..2fc28efc25 --- /dev/null +++ b/tests/integration/targets/consul/tasks/consul_general.yml @@ -0,0 +1,76 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: ensure unknown scheme fails + consul_session: + state: info + id: dummy + scheme: non_existent + token: "{{ consul_management_token }}" + register: result + ignore_errors: true + +- assert: + that: + - result is failed + +- name: ensure SSL certificate is checked + consul_session: + state: info + id: dummy + port: 8501 + scheme: https + token: "{{ consul_management_token }}" + register: result + ignore_errors: true + +- name: previous task should fail since certificate is not known + assert: + that: + - result is failed + - "'certificate verify failed' in result.msg" + +- name: ensure SSL certificate isn't checked when validate_certs is disabled + consul_session: + state: info + id: dummy + port: 8501 + scheme: https + token: "{{ consul_management_token }}" + validate_certs: false + register: result + +- name: previous task should succeed since certificate isn't checked + assert: + that: + - result is changed + +- name: ensure a secure connection is possible + consul_session: + state: info + id: dummy + port: 8501 + scheme: https + token: "{{ consul_management_token }}" + ca_path: '{{ remote_dir }}/cert.pem' + register: result + +- assert: + that: + - result is changed + +- name: ensure connection errors are handled properly + consul_session: + state: info + id: dummy + token: "{{ consul_management_token }}" + port: 1234 + register: result + ignore_errors: true + +- assert: + that: + - result is failed + - result.msg.startswith('Could not connect to consul agent at localhost:1234, error was') diff --git a/tests/integration/targets/consul/tasks/consul_kv.yml b/tests/integration/targets/consul/tasks/consul_kv.yml new file mode 100644 index 0000000000..6cca73137a --- /dev/null +++ b/tests/integration/targets/consul/tasks/consul_kv.yml @@ -0,0 +1,57 @@ +--- +# Copyright (c) 2024, Florian Apolloner (@apollo13) +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Create a key + consul_kv: + key: somekey + value: somevalue + token: "{{ consul_management_token }}" + register: result + +- assert: + that: + - result is changed + - result.data.Value == 'somevalue' + +#- name: Test the lookup +# assert: +# that: +# - lookup('community.general.consul_kv', 'somekey', token=consul_management_token) == 'somevalue' + +- name: Update a key with the same data + consul_kv: + key: somekey + value: somevalue + token: "{{ consul_management_token }}" + register: result + +- assert: + that: + - result is not changed + - result.data.Value == 'somevalue' + +- name: Remove a key from the store + consul_kv: + key: somekey + state: absent + token: "{{ consul_management_token }}" + register: result + +- assert: + that: + - result is changed + - result.data.Value == 'somevalue' + +- name: Remove a non-existant key from the store + consul_kv: + key: somekey + state: absent + token: "{{ consul_management_token }}" + register: result + +- assert: + that: + - result is not changed + - not result.data \ No newline at end of file diff --git a/tests/integration/targets/consul/tasks/consul_session.yml b/tests/integration/targets/consul/tasks/consul_session.yml index 96a2ae6c96..7d6d084021 100644 --- a/tests/integration/targets/consul/tasks/consul_session.yml +++ b/tests/integration/targets/consul/tasks/consul_session.yml @@ -80,65 +80,6 @@ that: - result is failed -- name: ensure unknown scheme fails - consul_session: - state: info - id: '{{ session_id }}' - scheme: non_existent - token: "{{ consul_management_token }}" - register: result - ignore_errors: true - -- assert: - that: - - result is failed - -- name: ensure SSL certificate is checked - consul_session: - state: info - id: '{{ session_id }}' - port: 8501 - scheme: https - token: "{{ consul_management_token }}" - register: result - ignore_errors: true - -- name: previous task should fail since certificate is not known - assert: - that: - - result is failed - - "'certificate verify failed' in result.msg" - -- name: ensure SSL certificate isn't checked when validate_certs is disabled - consul_session: - state: info - id: '{{ session_id }}' - port: 8501 - scheme: https - token: "{{ consul_management_token }}" - validate_certs: false - register: result - -- name: previous task should succeed since certificate isn't checked - assert: - that: - - result is changed - -- name: ensure a secure connection is possible - consul_session: - state: info - id: '{{ session_id }}' - port: 8501 - scheme: https - token: "{{ consul_management_token }}" - environment: - REQUESTS_CA_BUNDLE: '{{ remote_dir }}/cert.pem' - register: result - -- assert: - that: - - result is changed - - name: delete a session consul_session: state: absent diff --git a/tests/integration/targets/consul/tasks/main.yml b/tests/integration/targets/consul/tasks/main.yml index 3cd0fef5b8..9f5677cf11 100644 --- a/tests/integration/targets/consul/tasks/main.yml +++ b/tests/integration/targets/consul/tasks/main.yml @@ -89,6 +89,8 @@ - 1 - 2 - 3 + - import_tasks: consul_general.yml + - import_tasks: consul_kv.yml - import_tasks: consul_session.yml - import_tasks: consul_policy.yml - import_tasks: consul_role.yml