From 27514b954e55d91ec3bbd2ae76a9689c684bfb10 Mon Sep 17 00:00:00 2001 From: Georg Pfuetzenreuter Date: Mon, 21 Oct 2024 22:05:40 +0200 Subject: [PATCH 1/3] Refactor modules The existing modules rely on the "pdnsapi" library which is proprietary and has not received recent updates. Other Python libraries for interfacing with the PowerDNS API suffer from the same issues or are feature incomplete. Solve this and make the modules more easily extensible by directly interfacing with the PowerDNS HTTP REST API, utilizing the requests library which is already installed as a dependency of a common Salt deployment. For more efficient interaction as part of state modules which repeatedly call the execution module, add the requests-toolbelt library as the only additional dependency for easier implementation of a base session. As part of this refactoring the functions are renamed to follow a more streamlined scheme differentiating between read and write operations, and the output is aligned with what is returned directly by PowerDNS. As a result, this is a breaking change. Signed-off-by: Georg Pfuetzenreuter --- _modules/powerdns.py | 378 ++++++++++++++++++++++++------------------- _states/powerdns.py | 314 ++++++++++++++++++++++++++++------- 2 files changed, 466 insertions(+), 226 deletions(-) diff --git a/_modules/powerdns.py b/_modules/powerdns.py index ad6f54f..1478576 100644 --- a/_modules/powerdns.py +++ b/_modules/powerdns.py @@ -1,4 +1,3 @@ -# -*- coding: utf-8 -*- ''' Module to provide access to the power DNS http API @@ -12,220 +11,261 @@ This data can also be passed into pillar. Options passed into opts will overwrite options passed into pillar. ''' -from __future__ import absolute_import -# Import python libs -import logging -from distutils.version import LooseVersion # pylint: disable=import-error,no-name-in-module -import json -import re -from pprint import pformat -from six import string_types - -# Import salt libs -from salt.exceptions import get_error_message as _get_error_message - - -# Import third party libs try: - import pdnsapi as api - from pdnsapi.exceptions import ( - PDNSAccessDeniedException, PDNSNotFoundException, - PDNSProtocolViolationException, PDNSServerErrorException, - PDNSException) - - HAS_PDNSAPI = True + from requests import HTTPError, Request + from requests_toolbelt import sessions + HAS_REQUESTS = True except ImportError: - HAS_PDNSAPI = False + HAS_REQUESTS = False +import logging log = logging.getLogger(__name__) +from simplejson.errors import JSONDecodeError def __virtual__(): - ''' - Only load this module if pdnsapi is installed - ''' - if HAS_PDNSAPI: - return 'powerdns' - else: - return (False, 'The powerdns execution module cannot be loaded: the pdnsapi library is not available.') - -def _canonicalize_name(name): - if not name.endswith('.'): - return name + '.' - else: - return name - - -def _connect(): - url = __salt__['config.option']('pdns.url') - server_id = __salt__['config.option']('pdns.server_id') - api_key = __salt__['config.option']('pdns.api_key') - - log.debug("Attempting to connect: '%s' '%s' '%s'" % (url, server_id, api_key)) - - try: - conn = api.init_api(url, server_id, api_key) - - except PDNSException as e: - log.error("Exception while opening API connection: '%s'" % (e)) - return False - - log.debug("connected: '%s' '%s' '%s'" % (url, server_id, api_key)) - return conn - -def list_zones(): - conn = _connect() - - if not conn: - return "Failed to connect to powerDNS" - - log.debug("Attempting to pull zonelist") - zonelist = conn.zones - log.debug("Zonelist: %s" % (zonelist)) - - return [zone.name for zone in zonelist] - -def zone_exists(name): - conn = _connect() - - if not conn: + ''' + Only load this module if requests is installed + ''' + if HAS_REQUESTS: + return 'powerdns' + else: + return (False, 'The powerdns execution module cannot be loaded: the requests and/or requests_toolbelt libraries are not available.') + +def _init(): + url = __salt__['config.option']('pdns.url') + server_id = __salt__['config.option']('pdns.server_id') + api_key = __salt__['config.option']('pdns.api_key') + + session = sessions.BaseUrlSession(base_url=f'{url}/api/v1/servers/{server_id}/') + session.headers = { + 'Accept': 'application/json', + 'X-API-Key': str(api_key), + } + + log.debug(session.base_url) + log.debug(session.headers) + + get_root(session) + + return session + +def new_session(is_state_module): + if is_state_module: + return _init() + +def get_root(session=None): + log.debug('Requesting root') + if session is None: + session = _init() + log.debug(session.base_url) + test_request = Request(method='GET', url=session.base_url.rstrip('/')) + log.debug(test_request.url) + test_request = session.prepare_request(test_request) + try: + session.send(test_request).raise_for_status() + except HTTPError as e: + log.debug(f'Exception while connecting to the PowerDNS API: {e}') + return False + +def _session_get(session, path, raw=False): + response = session.get(path).json() + + if raw: + return response, None + + if isinstance(response, list) or isinstance(response, dict) and not 'error' in response: + return response, True + + elif isinstance(response, dict) and 'error' in response: + return response['error'], False + + return None, False + +def get_zones(session=None): + log.debug('Requesting zones') + if session is None: + session = _init() + body, result = _session_get(session, 'zones') + if not result: + return f'Failed to query zones: {body}' + log.debug("Zonelist: %s" % (body)) + + return [zone['name'] for zone in body] + +def get_zone(name, session=None): + log.debug(f'Requesting zone "{name}"') + if session is None: + session = _init() + body, result = _session_get(session, f'zones/{name}') + if not result: + return f'Failed to query zone: {body}' + + return body + +def get_zone_rrsets(name, raw=False, session=None): + log.debug(f'Requesting zone "{name}" records') + zone = get_zone(name, session) + + if isinstance(zone, dict): + rrsets = zone.get('rrsets', []) + if raw: + return rrsets + + return [ + { + 'name': rrset.get('name'), + 'records': rrset.get('records', []), + 'ttl': rrset.get('ttl'), + 'type': rrset.get('type'), + } for rrset in rrsets + ] + + return zone + +def get_zone_exists(name, session=None): + if session is None: + session = _init() + response, _ = _session_get(session, f'zones/{name}', raw=True) + if isinstance(response, dict): + if 'error' in response: + error = response['error'] + if error == 'Not Found': return False + else: + return f'Failed to query zone: {error}' - try: - zone = conn.get_zone(name) - except PDNSException as e: - return False + # just checking a couple arbitrary attributes to make sure it's actually a zone object + elif 'id' in response and 'url' in response: + return True + return None - return True +def get_records(zone, recname, rectype=None, session=None): + log.debug(f'Requesting zone "{zone}" record "{recname}"') + if session is None: + session = _init() -def get_zone(name): - conn = _connect() + recname = canonicalize_recname(zone, recname) + rrsets = get_zone_rrsets(zone, raw=False, session=session) - if not conn: - return "Failed to connect to powerDNS" + log.debug(rrsets) - try: - zone = conn.get_zone(name) - except PDNSException as e: - return "Exception while getting zone: '%s'" % (e) + records = [] + if not isinstance(rrsets, list): + return rrsets - return [{'name': record.name, 'type': record.type, 'ttl': record.ttl, 'records': [record2 for record2 in record.records]} for record in zone.records] + for rrset in rrsets: + if rrset['name'] == recname and ( rrset['type'] == rectype and rectype is not None ): + records.append(rrset) -def get_record(zone, name, rtype): - conn = _connect() + return records - if not conn: - return "Failed to connect to powerDNS" +def canonicalize_name(name): + if not name.endswith('.'): + name = f'{name}.' - try: - record, _ = _get_record_zone(conn, zone, name, rtype) - except PDNSException as e: - return "Could not get record '%s'" % (e) + return name - return { 'zone': zone, 'name': record.name, 'type': record.type, 'ttl': record.ttl, 'records': [rec for rec in record.records]} +def canonicalize_recname(zone, recname): + canonzone = canonicalize_name(zone) + if recname == '.': + return canonzone -def _get_record_zone(conn, zone, name, rtype): - canonical_zone = _canonicalize_name(zone) + if recname.endswith('.') or recname.endswith(canonzone): + return recname - zone_rec = conn.get_zone(canonical_zone) + #if zone in recname: + # return canonzone - if not name.endswith(zone): - name = name + '.' + zone - record = zone_rec.get_record(_canonicalize_name(name), rtype) + if recname.endswith(zone): + name = recname - return record, zone_rec + else: + name = f'{recname}.{zone}' -def del_record(zone, name, rtype): - conn = _connect() + return canonicalize_name(name) - if not conn: - return "Failed to connect to powerDNS" +def _handle_result(result, expect): + status = result.status_code + log.debug(f'{status} {result.text}') - try: - record, zone_rec = _get_record_zone(conn, zone, name, rtype) - except PDNSException as e: - return "Could not get record '%s'" % (e) + try: + output = result.json() + except JSONDecodeError: + output = result.text - try: - zone_rec.delete_record(record) - except PDNSException as e: - return "Could not delete record '%s'" % (e) + if isinstance(output, dict) and 'error' in output: + output = output['error'] - return True + if status == expect: + return True, status, output -def add_zone(zone, name_servers=None, records=None): - conn = _connect() + return False, status, output - if not conn: - return "Failed to connect to powerDNS" +def post_zone(zone, payload, session=None): + if session is None: + session = _init() - canonical_zone = _canonicalize_name(zone) + if get_zone_exists(zone): + return False, f'Failed to post: zone "{zone}" already exists', None - try: - zone = conn.create_zone(canonical_zone, name_servers, records) - except PDNSException as e: - return "Failed to create zone: '%s'" % (e) + log.debug(payload) - return [{'name': record.name, 'type': record.type, 'ttl': record.ttl, 'records': [record2 for record2 in record.records]} for record in zone.records] + result = session.post(f'zones', json=payload) -def del_zone(zone): - conn = _connect() + return _handle_result(result, 201) - if not conn: - log.error("Failed to connect to powerDNS") - return False +def patch_zone(zone, payload, changetype, session=None): + if session is None: + session = _init() - canonical_zone = _canonicalize_name(zone) + if not get_zone_exists(zone): + return False, f'Failed to patch: zone "{zone}" does not exist', None - try: - zone = conn.delete_zone(canonical_zone) - except PDNSException as e: - log.error("Failed to delete zone: '%s'" % (e)) - return False + log.debug(payload) - return True + result = session.patch(f'zones/{zone}', json=payload) + + return _handle_result(result, 204) -def add_record(zone, name, rtype, ttl=300, **kwargs): - conn = _connect() +def patch_rrsets(zone, changetype, session, recname=None, rectype=None, record=None, recttl=None, rrsets=None): + payload = {} - if not conn: - log.error("Failed to connect to powerDNS") - return False - - if 'records' not in kwargs: - log.error("Must specify records. Ex: records='[ list, of, records ]'") - return False + if rrsets: + payload['rrsets'] = rrsets + for i, _ in enumerate(payload['rrsets']): + payload['rrsets'][i]['changetype'] = changetype - canonical_zone = _canonicalize_name(zone) + else: + payload['rrsets'] = [ + { + 'name': canonicalize_recname(zone, recname), + 'changetype': changetype, + 'type': rectype, + } + ] - try: - zone_rec = conn.get_zone(canonical_zone) - except PDNSException as e: - log.error("Could not get zone '%s': '%s'" % (canonical_zone, e)) - return False + if changetype == 'REPLACE' and not rrsets: + payload['rrsets'][0].update( + { + 'records': [ + { + 'content': record, + } + ], + 'ttl': recttl, + } + ) - if not name.endswith(zone): - name = name + '.' + zone - - record = api.Record(_canonicalize_name(name), rtype, kwargs['records'], ttl) - - try: - foo = zone_rec.add_record(record) - except PDNSException as e: - log.error("add_record failed: '%s'" % (e)) - return False + return patch_zone(zone, payload, changetype, session) - return True +def create_record(zone, recname, recttl, rectype, record, session=None): + return patch_rrsets(zone, recname=recname, rectype=rectype, changetype='REPLACE', session=session, record=record, recttl=recttl) -# return { 'zone': canonical_zone, 'name': record.name, 'type': record.type, 'ttl': record.ttl, 'records': [rec for rec in record.records]} +def delete_record(zone, recname, rectype, session=None): + return patch_rrsets(zone, recname=recname, rectype=rectype, changetype='DELETE', session=session) -def argtest(*args, **kwargs): - #log.error("'%s'" % (pformat(kwargs))) - if '__id__' in kwargs: - kwargs['YAY'] = 'Called from STATE' - kwargs['args'] = args - return kwargs diff --git a/_states/powerdns.py b/_states/powerdns.py index a66b8bf..1844a38 100644 --- a/_states/powerdns.py +++ b/_states/powerdns.py @@ -1,77 +1,277 @@ -# -*- coding: utf-8 -*- -''' -''' - -# Define the module's virtual name __virtualname__ = 'powerdns' - def __virtual__(): if 'powerdns.get_zone' in __salt__: return __virtualname__ return False -def test(name, *args, **kwargs): - zzz = __salt__['powerdns.argtest'](args, **kwargs) - ret = {'name': name, - 'changes': {}, - 'result': zzz, - 'comment': ''} +import logging +log = logging.getLogger(__name__) - return ret +def zone_present(name, kind=None, rrsets=None, masters=None, dnssec=None, nsec3param=None, nsec3narrow=None, presigned=None, soa_edit=None, soa_edit_api=None, api_rectify=None, catalog=None, nameservers=None, master_tsig_key_ids=None, slave_tsig_key_ids=None): + + want_data = { + key: value + for key, value in locals().items() + if value is not None + } + + zone = __salt__['powerdns.canonicalize_name'](name) + want_data['name'] = zone + + if 'kind' in want_data: + want_data['kind'] = want_data['kind'].capitalize() + + ret = {'name': zone, 'changes': {}, 'result': False, 'comment': ''} + + session = __salt__['powerdns.new_session'](True) + log.debug('powerdns: got session') + + exists = __salt__['powerdns.get_zone_exists'](zone, session) -def zone_present(name, name_servers=None, records=None): - ret = {'name': name, - 'changes': {}, - 'result': False, - 'comment': ''} + log.debug(f'powerdns: zone exists => {exists}') + log.debug(f'powerdns: want data => {want_data}') - if __salt__['powerdns.zone_exists'](name): - ret['result'] = True - ret['comment'] = 'Zone already present.' + if exists: + if 'nameservers' in want_data: + del want_data['nameservers'] + + have_data = { + key: value + for key, value in __salt__['powerdns.get_zone'](zone, session).items() + if key in want_data + } + + log.debug(f'powerdns: have data => {have_data}') + + payload = {} + for key, want_value in want_data.items(): + log.debug(f'powerdns: reading wanted key "{key}"') + have_value = have_data[key] + if want_value != have_value: + log.debug('powerdns: key differs') + ret['changes'][key] = { + 'old': have_value, + 'new': want_value, + } + + payload.update( + { + key: want_value, + } + ) + + if not ret['changes']: + ret['result'] = True + ret['comment'] = 'Zone is already in the correct state.' + return ret + + if ret['changes']: + if __opts__['test']: + ret['result'] = None + ret['comment'] = 'Zone would be modified.' return ret - zone = __salt__['powerdns.add_zone'](name, name_servers, records) - if type(zone) is list: - ret['result'] = True - ret['comment'] = 'Zone present.' - ret['changes'] = { name : { 'old': '', 'new': zone } } - + if not 'rrsets' in payload: + payload['rrsets'] = [] + + ok, status, output = __salt__['powerdns.patch_zone'](zone, payload, 'REPLACE', session) + + if ok: + ret['result'] = True + ret['comment'] = 'Zone modified: {status} - {output}' + + else: + ret['result'] = False + ret['comment'] = f'Zone modification failed: {status} - {output}' + return ret -def zone_absent(name): - ret = {'name': name, - 'changes': {}, - 'result': False, - 'comment': ''} + else: # zone does not exist + ret['changes'] = { + 'new': want_data, + 'old': {}, + } + + if __opts__['test']: + ret['result'] = None + ret['comment'] = 'Zone would be created.' + return ret + + payload = want_data + ok, status, output = __salt__['powerdns.post_zone'](zone, payload, session) + + if ok: + ret['result'] = True + ret['comment'] = f'Zone created: {status} - {output}' - if not __salt__['powerdns.zone_exists'](name): - ret['result'] = True - ret['comment'] = 'Zone already absent.' - return ret else: - zone = __salt__['powerdns.get_zone'](name) + ret['result'] = False + ret['comment'] = f'Zone creation failed: {status} - {output}' - if __salt__['powerdns.del_zone'](name): - ret['result'] = True - ret['comment'] = 'Zone absent.' - ret['changes'] = { name : { 'old': zone, 'new': '' } } - return ret -def record_present(zone, name, record_type, ttl=300, records=[]): - ret = {'name': name, - 'changes': {}, - 'result': False, - 'comment': ''} - old_record = __salt__['powerdns.get_record'](zone, name, record_type) - if __salt__['powerdns.add_record'](zone, name, record_type, ttl, records=records): - ret['result'] = True - ret['comment'] = "Record present" - new_record = __salt__['powerdns.get_record'](zone, name, record_type) - if new_record == old_record: - ret['changes'] = {} - else: - ret['changes'] = { name : { 'new': { 'zone': zone, 'name': name, 'type': record_type, 'ttl': ttl, 'records': records }, 'old': old_record } } - +def rrsets_present(name, rrsets): + zone = __salt__['powerdns.canonicalize_name'](name) + ret = {'name': zone, 'changes': {}, 'result': False, 'comment': ''} + + session = __salt__['powerdns.new_session'](True) + log.debug('powerdns: got session') + + have_rrsets = __salt__['powerdns.get_zone_rrsets'](zone, session) + + want_rrsets = [] + payload = [] + + for rrset in rrsets: + this_rrset = { + 'name': __salt__['powerdns.canonicalize_recname'](zone, rrset['name']), + 'type': rrset['type'], + } + + if 'ttl' in rrset: + this_rrset['ttl'] = rrset['ttl'] + + this_rrset['records'] = [ + { + 'content': record, + 'disabled': False, + } + for record in rrset['records'] + ] + + want_rrsets.append(this_rrset) + + up_to_date_rrsets = [] + only_ttl_rrsets = [] + only_records_rrsets = [] + + log.debug('powerdns: rrset payload BEFORE mangling') + log.debug(want_rrsets) + + # TODO: this needs a second iteration over have_rrsets to remove records which are no longer in Salt + + for i, want_rrset in enumerate(want_rrsets): + log.debug(want_rrset) + for have_rrset in have_rrsets: + log.debug(have_rrset) + if want_rrset['name'] == have_rrset['name'] and have_rrset['type'] == want_rrset['type']: + ttl_ok = False + records_ok = [] + + if 'ttl' in want_rrset and want_rrset['ttl'] == have_rrset['ttl'] or not 'ttl' in want_rrset: + ttl_ok = True + + for ir, want_record in enumerate(want_rrset['records']): + log.debug(f'powerdns: reading wanted record {want_record}') + + for have_record in have_rrset['records']: + log.debug(f'powernds: comparing against {have_record}') + + if have_record == want_record: + log.debug('powerdns: matches') + records_ok.append(ir) + break + + elif have_record['content'] == want_record['content'] and have_record['disabled'] != want_record['disabled']: + log.debug('powerdns: status mismatch') + + if not want_rrset['name'] in ret['changes']: + ret['changes'][want_rrset['name']] = [{}] + + if not 'old' in ret['changes'][want_rrset['name']][i] and not 'new' in ret['changes'][want_rrset['name']][i]: + ret['changes'][want_rrset['name']][i]['old'] = {} + ret['changes'][want_rrset['name']][i]['new'] = {} + + ret['changes'][want_rrset['name']][i]['old'].update({have_record['content']: not have_record['disabled']}) + ret['changes'][want_rrset['name']][i]['new'].update({want_record['content']: not want_record['disabled']}) + + break + + else: + log.debug(f'powerdns: not found') + + if not want_rrset['name'] in ret['changes']: + ret['changes'][want_rrset['name']] = [{}] + + if not 'old' in ret['changes'][want_rrset['name']][i] and not 'new' in ret['changes'][want_rrset['name']][i]: + ret['changes'][want_rrset['name']][i]['old'] = {} + ret['changes'][want_rrset['name']][i]['new'] = {} + + ret['changes'][want_rrset['name']][i]['old'].update({want_record['content']: None}) + ret['changes'][want_rrset['name']][i]['new'].update({want_record['content']: not want_record['disabled']}) + + all_records_ok = len(records_ok) == len(want_rrset['records']) + if ttl_ok and all_records_ok: + up_to_date_rrsets.append(i) + + elif not ttl_ok and all_records_ok: + only_ttl_rrsets.append(i) + if not want_rrset['name'] in ret['changes']: + ret['changes'][want_rrset['name']] = [{}] + ret['changes'][want_rrset['name']][i] = { + 'old': {'ttl': have_rrset['ttl']}, + 'new': {'ttl': want_rrset['ttl']}, + } + + elif ttl_ok and not all_records_ok: + only_records_rrsets.append(i) + + # update of individual records not possible / this might be ok to drop ; just always write all records in the set + #for ir in records_ok: + # want_rrset['records'].pop(ir) + + break + + else: + ret['changes'][want_rrset['name']] = [ + { + 'old': {}, + 'new': { + 'ttl': want_rrset['ttl'], + 'records': { + record['content']: not record['disabled'] + for record in want_rrset.get('records', []) + } + }, + }, + ] + + for i in up_to_date_rrsets: + want_rrsets.pop(i-1) + + # API throws "No change for RRset" if attempting to patch only the TTL without records, bug? + #for i in only_ttl_rrsets: + # del want_rrsets[i]['records'] + + # API requires ttl for record updates, this might be ok to drop + #for i in only_records_rrsets: + # if 'ttl' in want_rrsets[i]: + # del want_rrsets[i]['ttl'] + + log.debug('powerdns: rrset changes') + log.debug(ret['changes']) + log.debug('powerdns: rrset payload AFTER mangling') + log.debug(want_rrsets) + + if want_rrsets: + if __opts__['test']: + ret['result'] = None + ret['comment'] = 'Resource sets would be updated.' + return ret + + ok, status, output = __salt__['powerdns.patch_rrsets'](zone, 'REPLACE', session, rrsets=want_rrsets) + + if ok: + ret['result'] = True + ret['comment'] = f'Resource sets updated: {status} - {output}' + + else: + ret['result'] = False + ret['comment'] = f'Resource set update failed: {status} - {output}' + return ret + + ret['result'] = True + ret['comment'] = 'Resource sets are already up to date.' + return ret From 950407b64af3f7449b7044cf38b4f9889afb543a Mon Sep 17 00:00:00 2001 From: Georg Pfuetzenreuter Date: Tue, 22 Oct 2024 22:06:25 +0200 Subject: [PATCH 2/3] Support rrset updates in zone_present() Avoid splitting rrsets updates to a separate state function when it is handled in the same PowerDNS API payload, rather integrate the logic from rrsets_presents() into zone_present(). The second function can be deleted in a later patch once functionality is deemed equivalent. Signed-off-by: Georg Pfuetzenreuter --- _states/powerdns.py | 159 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 146 insertions(+), 13 deletions(-) diff --git a/_states/powerdns.py b/_states/powerdns.py index 1844a38..1a806a3 100644 --- a/_states/powerdns.py +++ b/_states/powerdns.py @@ -48,18 +48,151 @@ def zone_present(name, kind=None, rrsets=None, masters=None, dnssec=None, nsec3p for key, want_value in want_data.items(): log.debug(f'powerdns: reading wanted key "{key}"') have_value = have_data[key] - if want_value != have_value: - log.debug('powerdns: key differs') - ret['changes'][key] = { - 'old': have_value, - 'new': want_value, - } - - payload.update( - { - key: want_value, + + if type(want_value) != type(have_value): + log.error('powerdns: comparison of values with different instance types is not supported') + return False + + if isinstance(want_value, list): + if key == 'nameservers': + want_value.sort() + have_value.sort() + + if key == 'rrsets': + for i, rrset in enumerate(want_value): + want_value[i-1]['name'] = __salt__['powerdns.canonicalize_recname'](zone, rrset['name']) + + payload.update( + { + key: [ + { + 'changetype': 'REPLACE', + **rrset + } for rrset in want_value + ], + } + ) + + if not 'rrsets' in ret['changes']: + ret['changes']['rrsets'] = {} + + for want_rrset in want_value: # for dict in list of rrsets + log.debug(f'powerdns: want rrset {want_rrset}') + rrset_name = f'{want_rrset["name"]}_{want_rrset["type"]}' + + for have_rrset in have_value: + log.debug(f'powerdns: have rrset {have_rrset}') + + if want_rrset['name'] == have_rrset['name'] and want_rrset['type'] == have_rrset['type']: + log.debug('powerdns: match!') + + for rrset_key in want_rrset.keys(): + log.debug(f'powerdns: parsing key "{rrset_key}"') + + if rrset_key == 'name': + continue + + if rrset_key in have_rrset: + log.debug('powerdns: found wanted key in existing keys') + + if isinstance(want_rrset[rrset_key], str) or isinstance(want_rrset[rrset_key], int): + if want_rrset[rrset_key] != have_rrset[rrset_key]: + if rrset_name not in ret['changes']['rrsets']: + ret['changes']['rrsets'][rrset_name] = { + 'old': {}, + 'new': {}, + } + + ret['changes']['rrsets'][rrset_name]['old'] = have_rrset[rrset_key] + ret['changes']['rrsets'][rrset_name]['new'] = want_rrset[rrset_key] + + elif isinstance(want_rrset[rrset_key], list): + if rrset_key == 'comments': + log.error('powerdns: comments in rrsets are not supported by the formula') + + have_rrset_records = sorted(have_rrset[rrset_key], key=lambda record: record['content']) + want_rrset_records = sorted(want_rrset[rrset_key], key=lambda record: record['content']) + + if have_rrset_records == want_rrset_records: + log.debug('powerdns: records match exactly') + continue + + new_rrset_records = [record for record in want_rrset_records if record['content'] not in [record['content'] for record in have_rrset_records]] + + for i, want_rrset_record in enumerate(want_rrset_records): + i = i-1 + want_rrset_record_disabled = want_rrset_record.get('disabled', False) + if want_rrset_record['content'] == have_rrset_records[i]['content']: + if want_rrset_record_disabled != have_rrset_records[i]['disabled']: + if rrset_name not in ret['changes']['rrsets']: + ret['changes']['rrsets'][rrset_name] = { + 'old': {}, + 'new': {}, + } + + for x in ['old', 'new']: + if 'records' not in ret['changes']['rrsets'][rrset_name][x]: + ret['changes']['rrsets'][rrset_name][x]['records'] = [] + + ret['changes']['rrsets'][rrset_name]['old']['records'].append({have_rrset_records[i]['name']: not have_rrset_records[i]['disabled']}) + ret['changes']['rrsets'][rrset_name]['new']['records'].append({want_rrset_record['name']: want_rrset_record_disabled}) + + if new_rrset_records: + log.debug(f'powerdns: new rrset records: {new_rrset_records}') + + for new_rrset_record in new_rrset_records: + if rrset_name not in ret['changes']['rrsets']: + ret['changes']['rrsets'][rrset_name] = { + 'old': {}, + 'new': {}, + } + + for x in ['old', 'new']: + if 'records' not in ret['changes']['rrsets'][rrset_name][x]: + ret['changes']['rrsets'][rrset_name][x]['records'] = [] + + ret['changes']['rrsets'][rrset_name]['old']['records'].append({new_rrset_record['content']: None}) + ret['changes']['rrsets'][rrset_name]['new']['records'].append({new_rrset_record['content']: not new_rrset_record.get('disabled', False)}) + + else: + if rrset_name not in ret['changes']['rrsets']: + ret['changes']['rrsets'][rrset_name] = { + 'old': {}, + 'new': {}, + } + + ret['changes']['rrsets'][rrset_name]['old'][rrset_key] = None + ret['changes']['rrsets'][rrset_name]['new'][rrset_key] = want_rrset[rrset_key] + + log.debug('powerdns: breaking after analyzing match') + break + + # no existing rrset found with the given name + else: + log.debug(f'powerdns: rrset "{rrset_name}" not found') + ret['changes']['rrsets'][rrset_name] = { + 'old': {}, + 'new': want_rrset, + } + + if not ret['changes']['rrsets']: + del ret['changes']['rrsets'] + + if isinstance(want_value, str) or isinstance(want_value, list) and key != 'rrsets': + if want_value != have_value: + log.debug('powerdns: value differs') + ret['changes'][key] = { + 'old': have_value, + 'new': want_value, } - ) + + payload.update( + { + key: want_value, + } + ) + + log.debug(f'powerdns: payload: {payload}') if not ret['changes']: ret['result'] = True @@ -75,11 +208,11 @@ def zone_present(name, kind=None, rrsets=None, masters=None, dnssec=None, nsec3p if not 'rrsets' in payload: payload['rrsets'] = [] - ok, status, output = __salt__['powerdns.patch_zone'](zone, payload, 'REPLACE', session) + ok, status, output = __salt__['powerdns.patch_zone'](zone, payload, session) if ok: ret['result'] = True - ret['comment'] = 'Zone modified: {status} - {output}' + ret['comment'] = f'Zone modified: {status} - {output}' else: ret['result'] = False From f89cd00cbee3e2112c32eb10006e93a656ac42c0 Mon Sep 17 00:00:00 2001 From: Georg Pfuetzenreuter Date: Mon, 28 Oct 2024 02:34:31 +0100 Subject: [PATCH 3/3] Refactor zone_present() Factor in rrset and record changes and drop the separate rrsets_present() state function. Utilize dictdiffer to avoid lots of the complicated and convoluted comparison code at the cost of a slightly less uniform but still useful diff in the changes output. Signed-off-by: Georg Pfuetzenreuter --- _modules/powerdns.py | 4 +- _states/powerdns.py | 402 +++++++++++-------------------------------- 2 files changed, 104 insertions(+), 302 deletions(-) diff --git a/_modules/powerdns.py b/_modules/powerdns.py index 1478576..7dfe8a7 100644 --- a/_modules/powerdns.py +++ b/_modules/powerdns.py @@ -218,7 +218,7 @@ def post_zone(zone, payload, session=None): return _handle_result(result, 201) -def patch_zone(zone, payload, changetype, session=None): +def patch_zone(zone, payload, session=None): if session is None: session = _init() @@ -260,7 +260,7 @@ def patch_rrsets(zone, changetype, session, recname=None, rectype=None, record=N } ) - return patch_zone(zone, payload, changetype, session) + return patch_zone(zone, payload, session) def create_record(zone, recname, recttl, rectype, record, session=None): return patch_rrsets(zone, recname=recname, rectype=rectype, changetype='REPLACE', session=session, record=record, recttl=recttl) diff --git a/_states/powerdns.py b/_states/powerdns.py index 1a806a3..2608bc4 100644 --- a/_states/powerdns.py +++ b/_states/powerdns.py @@ -5,9 +5,12 @@ def __virtual__(): return __virtualname__ return False +from copy import deepcopy +from dictdiffer import diff as dictdiff import logging log = logging.getLogger(__name__) + def zone_present(name, kind=None, rrsets=None, masters=None, dnssec=None, nsec3param=None, nsec3narrow=None, presigned=None, soa_edit=None, soa_edit_api=None, api_rectify=None, catalog=None, nameservers=None, master_tsig_key_ids=None, slave_tsig_key_ids=None): want_data = { @@ -22,7 +25,7 @@ def zone_present(name, kind=None, rrsets=None, masters=None, dnssec=None, nsec3p if 'kind' in want_data: want_data['kind'] = want_data['kind'].capitalize() - ret = {'name': zone, 'changes': {}, 'result': False, 'comment': ''} + ret = {'name': zone, 'changes': {'old': {}, 'new': {}}, 'result': False, 'comment': ''} session = __salt__['powerdns.new_session'](True) log.debug('powerdns: got session') @@ -33,167 +36,132 @@ def zone_present(name, kind=None, rrsets=None, masters=None, dnssec=None, nsec3p log.debug(f'powerdns: want data => {want_data}') if exists: - if 'nameservers' in want_data: - del want_data['nameservers'] - - have_data = { - key: value - for key, value in __salt__['powerdns.get_zone'](zone, session).items() - if key in want_data - } - + have_data = __salt__['powerdns.get_zone'](zone, session) log.debug(f'powerdns: have data => {have_data}') + payload = have_data.copy() - payload = {} - for key, want_value in want_data.items(): - log.debug(f'powerdns: reading wanted key "{key}"') - have_value = have_data[key] + for have_key, have_value in have_data.items(): + log.debug(f'powerdns: reading have key {have_key}') - if type(want_value) != type(have_value): - log.error('powerdns: comparison of values with different instance types is not supported') - return False + if have_key in want_data: + want_value = want_data[have_key] + log.debug(f'powerdns: key {have_key} is wanted with value {want_value}') - if isinstance(want_value, list): - if key == 'nameservers': - want_value.sort() - have_value.sort() + payload.update( + { + have_key: deepcopy(want_value) + } + ) - if key == 'rrsets': - for i, rrset in enumerate(want_value): - want_value[i-1]['name'] = __salt__['powerdns.canonicalize_recname'](zone, rrset['name']) + if isinstance(have_value, str) or isinstance(have_value, int): + if have_value == want_value: + log.debug(f'powerdns: str/int {have_value} already matches') - payload.update( - { - key: [ - { - 'changetype': 'REPLACE', - **rrset - } for rrset in want_value - ], - } - ) + else: + ret['changes']['old'][have_key] = have_value + ret['changes']['new'][have_key] = want_value - if not 'rrsets' in ret['changes']: - ret['changes']['rrsets'] = {} + elif isinstance(have_value, list): # rrset/nameserver list? + if have_key == 'nameserver': + have_value = have_value.sort() + want_value = want_value.sort() - for want_rrset in want_value: # for dict in list of rrsets - log.debug(f'powerdns: want rrset {want_rrset}') - rrset_name = f'{want_rrset["name"]}_{want_rrset["type"]}' + if have_value == want_value: + log.debug(f'powerdns: list {have_value} already matches') + else: + # maybe make a diff of the lists here + ret['changes']['old'][have_key] = have_value + ret['changes']['new'][have_key] = want_value + + elif have_key == 'rrsets': + processed_rrsets = [] + processed_wanted_rrsets = [] + + # 1. preprocess rrsets + for i_rrs, rrset in enumerate(want_value): + rrset_name = __salt__['powerdns.canonicalize_recname'](zone, rrset['name']) + for i_rec, record in enumerate(rrset.get('records', [])): + if 'disabled' not in record: + payload[have_key][i_rrs]['records'][i_rec]['disabled'] = False + if '.' not in rrset['name']: + payload[have_key][i_rrs]['name'] = rrset_name + if not rrset.get('comments', []): + payload[have_key][i_rrs]['comments'] = [] + + # 2. process changes to existing rrsets for have_rrset in have_value: - log.debug(f'powerdns: have rrset {have_rrset}') - - if want_rrset['name'] == have_rrset['name'] and want_rrset['type'] == have_rrset['type']: - log.debug('powerdns: match!') + processed_rrsets.append((have_rrset['name'], have_rrset['type'])) - for rrset_key in want_rrset.keys(): - log.debug(f'powerdns: parsing key "{rrset_key}"') + for i, want_rrset in enumerate(payload[have_key]): + if have_rrset['name'] == want_rrset['name'] and have_rrset['type'] == want_rrset['type']: + log.debug(f'powerdns: want_rrset {want_rrset}') + log.debug('comparing') + log.debug(have_rrset) + log.debug(want_rrset) + diff = list(dictdiff(have_rrset, want_rrset)) + log.debug(f'powerdns: have diff') + for x in diff: + log.debug(x) - if rrset_key == 'name': - continue + if diff: + if not 'rrset_diffs' in ret['changes']: + ret['changes']['rrset_diffs'] = {} - if rrset_key in have_rrset: - log.debug('powerdns: found wanted key in existing keys') + if not name in ret['changes']['rrset_diffs']: + ret['changes']['rrset_diffs'][name] = [] - if isinstance(want_rrset[rrset_key], str) or isinstance(want_rrset[rrset_key], int): - if want_rrset[rrset_key] != have_rrset[rrset_key]: - if rrset_name not in ret['changes']['rrsets']: - ret['changes']['rrsets'][rrset_name] = { - 'old': {}, - 'new': {}, - } + ret['changes']['rrset_diffs'][name].append(diff) - ret['changes']['rrsets'][rrset_name]['old'] = have_rrset[rrset_key] - ret['changes']['rrsets'][rrset_name]['new'] = want_rrset[rrset_key] - - elif isinstance(want_rrset[rrset_key], list): - if rrset_key == 'comments': - log.error('powerdns: comments in rrsets are not supported by the formula') - - have_rrset_records = sorted(have_rrset[rrset_key], key=lambda record: record['content']) - want_rrset_records = sorted(want_rrset[rrset_key], key=lambda record: record['content']) - - if have_rrset_records == want_rrset_records: - log.debug('powerdns: records match exactly') - continue - - new_rrset_records = [record for record in want_rrset_records if record['content'] not in [record['content'] for record in have_rrset_records]] - - for i, want_rrset_record in enumerate(want_rrset_records): - i = i-1 - want_rrset_record_disabled = want_rrset_record.get('disabled', False) - if want_rrset_record['content'] == have_rrset_records[i]['content']: - if want_rrset_record_disabled != have_rrset_records[i]['disabled']: - if rrset_name not in ret['changes']['rrsets']: - ret['changes']['rrsets'][rrset_name] = { - 'old': {}, - 'new': {}, - } - - for x in ['old', 'new']: - if 'records' not in ret['changes']['rrsets'][rrset_name][x]: - ret['changes']['rrsets'][rrset_name][x]['records'] = [] + else: + payload[have_key].pop(i) - ret['changes']['rrsets'][rrset_name]['old']['records'].append({have_rrset_records[i]['name']: not have_rrset_records[i]['disabled']}) - ret['changes']['rrsets'][rrset_name]['new']['records'].append({want_rrset_record['name']: want_rrset_record_disabled}) + wrrsettup = (want_rrset['name'], want_rrset['type']) + processed_rrsets.remove(wrrsettup) + processed_wanted_rrsets.append(wrrsettup) - if new_rrset_records: - log.debug(f'powerdns: new rrset records: {new_rrset_records}') + break - for new_rrset_record in new_rrset_records: - if rrset_name not in ret['changes']['rrsets']: - ret['changes']['rrsets'][rrset_name] = { - 'old': {}, - 'new': {}, - } + # 3. process new rrsets + for want_rrset in payload[have_key]: + for wrrsettup in processed_wanted_rrsets: + if want_rrset['name'] == wrrsettup[0] and want_rrset['type'] == wrrsettup[1]: + break - for x in ['old', 'new']: - if 'records' not in ret['changes']['rrsets'][rrset_name][x]: - ret['changes']['rrsets'][rrset_name][x]['records'] = [] + else: + if not 'rrset_additions' in ret['changes']: + ret['changes']['rrset_additions'] = [] - ret['changes']['rrsets'][rrset_name]['old']['records'].append({new_rrset_record['content']: None}) - ret['changes']['rrsets'][rrset_name]['new']['records'].append({new_rrset_record['content']: not new_rrset_record.get('disabled', False)}) + ret['changes']['rrset_additions'].append(rrset) - else: - if rrset_name not in ret['changes']['rrsets']: - ret['changes']['rrsets'][rrset_name] = { - 'old': {}, - 'new': {}, - } + # 4. process rrset removals + for rrset in processed_rrsets: + if rrset[1] == 'SOA': + continue - ret['changes']['rrsets'][rrset_name]['old'][rrset_key] = None - ret['changes']['rrsets'][rrset_name]['new'][rrset_key] = want_rrset[rrset_key] + payload[have_key].append( + { + 'name': rrset[0], + 'type': rrset[1], + 'changetype': 'DELETE', + } + ) - log.debug('powerdns: breaking after analyzing match') - break + if not 'rrset_deletions' in ret['changes']: + ret['changes']['rrset_deletions'] = [] - # no existing rrset found with the given name - else: - log.debug(f'powerdns: rrset "{rrset_name}" not found') - ret['changes']['rrsets'][rrset_name] = { - 'old': {}, - 'new': want_rrset, - } - - if not ret['changes']['rrsets']: - del ret['changes']['rrsets'] - - if isinstance(want_value, str) or isinstance(want_value, list) and key != 'rrsets': - if want_value != have_value: - log.debug('powerdns: value differs') - ret['changes'][key] = { - 'old': have_value, - 'new': want_value, - } + ret['changes']['rrset_deletions'].append(rrset) - payload.update( - { - key: want_value, - } - ) + if 'rrset_diffs' in ret['changes']: + for i, rrset in enumerate(payload['rrsets']): + payload['rrsets'][i]['changetype'] = 'REPLACE' log.debug(f'powerdns: payload: {payload}') + for x in ['old', 'new']: + if not ret['changes'][x]: + del ret['changes'][x] + if not ret['changes']: ret['result'] = True ret['comment'] = 'Zone is already in the correct state.' @@ -231,7 +199,6 @@ def zone_present(name, kind=None, rrsets=None, masters=None, dnssec=None, nsec3p ret['comment'] = 'Zone would be created.' return ret - payload = want_data ok, status, output = __salt__['powerdns.post_zone'](zone, payload, session) if ok: @@ -243,168 +210,3 @@ def zone_present(name, kind=None, rrsets=None, masters=None, dnssec=None, nsec3p ret['comment'] = f'Zone creation failed: {status} - {output}' return ret - -def rrsets_present(name, rrsets): - zone = __salt__['powerdns.canonicalize_name'](name) - ret = {'name': zone, 'changes': {}, 'result': False, 'comment': ''} - - session = __salt__['powerdns.new_session'](True) - log.debug('powerdns: got session') - - have_rrsets = __salt__['powerdns.get_zone_rrsets'](zone, session) - - want_rrsets = [] - payload = [] - - for rrset in rrsets: - this_rrset = { - 'name': __salt__['powerdns.canonicalize_recname'](zone, rrset['name']), - 'type': rrset['type'], - } - - if 'ttl' in rrset: - this_rrset['ttl'] = rrset['ttl'] - - this_rrset['records'] = [ - { - 'content': record, - 'disabled': False, - } - for record in rrset['records'] - ] - - want_rrsets.append(this_rrset) - - up_to_date_rrsets = [] - only_ttl_rrsets = [] - only_records_rrsets = [] - - log.debug('powerdns: rrset payload BEFORE mangling') - log.debug(want_rrsets) - - # TODO: this needs a second iteration over have_rrsets to remove records which are no longer in Salt - - for i, want_rrset in enumerate(want_rrsets): - log.debug(want_rrset) - for have_rrset in have_rrsets: - log.debug(have_rrset) - if want_rrset['name'] == have_rrset['name'] and have_rrset['type'] == want_rrset['type']: - ttl_ok = False - records_ok = [] - - if 'ttl' in want_rrset and want_rrset['ttl'] == have_rrset['ttl'] or not 'ttl' in want_rrset: - ttl_ok = True - - for ir, want_record in enumerate(want_rrset['records']): - log.debug(f'powerdns: reading wanted record {want_record}') - - for have_record in have_rrset['records']: - log.debug(f'powernds: comparing against {have_record}') - - if have_record == want_record: - log.debug('powerdns: matches') - records_ok.append(ir) - break - - elif have_record['content'] == want_record['content'] and have_record['disabled'] != want_record['disabled']: - log.debug('powerdns: status mismatch') - - if not want_rrset['name'] in ret['changes']: - ret['changes'][want_rrset['name']] = [{}] - - if not 'old' in ret['changes'][want_rrset['name']][i] and not 'new' in ret['changes'][want_rrset['name']][i]: - ret['changes'][want_rrset['name']][i]['old'] = {} - ret['changes'][want_rrset['name']][i]['new'] = {} - - ret['changes'][want_rrset['name']][i]['old'].update({have_record['content']: not have_record['disabled']}) - ret['changes'][want_rrset['name']][i]['new'].update({want_record['content']: not want_record['disabled']}) - - break - - else: - log.debug(f'powerdns: not found') - - if not want_rrset['name'] in ret['changes']: - ret['changes'][want_rrset['name']] = [{}] - - if not 'old' in ret['changes'][want_rrset['name']][i] and not 'new' in ret['changes'][want_rrset['name']][i]: - ret['changes'][want_rrset['name']][i]['old'] = {} - ret['changes'][want_rrset['name']][i]['new'] = {} - - ret['changes'][want_rrset['name']][i]['old'].update({want_record['content']: None}) - ret['changes'][want_rrset['name']][i]['new'].update({want_record['content']: not want_record['disabled']}) - - all_records_ok = len(records_ok) == len(want_rrset['records']) - if ttl_ok and all_records_ok: - up_to_date_rrsets.append(i) - - elif not ttl_ok and all_records_ok: - only_ttl_rrsets.append(i) - if not want_rrset['name'] in ret['changes']: - ret['changes'][want_rrset['name']] = [{}] - ret['changes'][want_rrset['name']][i] = { - 'old': {'ttl': have_rrset['ttl']}, - 'new': {'ttl': want_rrset['ttl']}, - } - - elif ttl_ok and not all_records_ok: - only_records_rrsets.append(i) - - # update of individual records not possible / this might be ok to drop ; just always write all records in the set - #for ir in records_ok: - # want_rrset['records'].pop(ir) - - break - - else: - ret['changes'][want_rrset['name']] = [ - { - 'old': {}, - 'new': { - 'ttl': want_rrset['ttl'], - 'records': { - record['content']: not record['disabled'] - for record in want_rrset.get('records', []) - } - }, - }, - ] - - for i in up_to_date_rrsets: - want_rrsets.pop(i-1) - - # API throws "No change for RRset" if attempting to patch only the TTL without records, bug? - #for i in only_ttl_rrsets: - # del want_rrsets[i]['records'] - - # API requires ttl for record updates, this might be ok to drop - #for i in only_records_rrsets: - # if 'ttl' in want_rrsets[i]: - # del want_rrsets[i]['ttl'] - - log.debug('powerdns: rrset changes') - log.debug(ret['changes']) - log.debug('powerdns: rrset payload AFTER mangling') - log.debug(want_rrsets) - - if want_rrsets: - if __opts__['test']: - ret['result'] = None - ret['comment'] = 'Resource sets would be updated.' - return ret - - ok, status, output = __salt__['powerdns.patch_rrsets'](zone, 'REPLACE', session, rrsets=want_rrsets) - - if ok: - ret['result'] = True - ret['comment'] = f'Resource sets updated: {status} - {output}' - - else: - ret['result'] = False - ret['comment'] = f'Resource set update failed: {status} - {output}' - - return ret - - ret['result'] = True - ret['comment'] = 'Resource sets are already up to date.' - return ret