diff --git a/AUTHORS.rst b/AUTHORS.rst index e9b6bf8cfe..e8f8fef389 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -44,6 +44,10 @@ Other contributors and previous maintainers Active from 2017, until he left Uninett in 2019. An experienced Python developer who, among other things, rewrote the ipdevpoll multiprocess mode. +* Joar Heimonen + Contributed Palo Alto ARP plugin to enable ipdevpoll fetch ARP data from Palo + Alto firewalls. + * Ragnhild Bodsberg Contributed various bugfixes to NAV as an intern at Sikt, during the summer of 2022. diff --git a/CHANGELOG.md b/CHANGELOG.md index 59ddc5a272..88c35ed8b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,20 @@ This project uses [*towncrier*](https://towncrier.readthedocs.io/) and the chang +## [5.10.1] - 2024-05-27 + + +### Fixed + +- Fix Machine Tracker DNS search crashing from exhausting all available file + descriptors ([#2669](https://github.com/Uninett/nav/issues/2669)) +- ARP records of unreachable devices are now closed by `navclean` cron job at + configurable expiry intervals, rather than immediately as a response to a + short ICMP packet loss ([#2913](https://github.com/Uninett/nav/issues/2913)) +- Palo Alto API XML responses are now parsed based on keys instead of indexes + ([#2924](https://github.com/Uninett/nav/issues/2924)) + + ## [5.10.0] - 2024-05-16 diff --git a/doc/reference/backend-processes.rst b/doc/reference/backend-processes.rst index 15d6078949..c9c94c944b 100644 --- a/doc/reference/backend-processes.rst +++ b/doc/reference/backend-processes.rst @@ -126,7 +126,9 @@ dbclean Regularly cleans out old data from the NAV database, using the :program:`navclean` program. The standard cleanup routine removes old web user interface sessions, and deletes IP devices that have been scheduled for -deletion through either SeedDB or the API. +deletion through either SeedDB or the API. Additionally, it closes open ARP +records that have been collected from routers that have been unreachable for +more than 30 minutes (adjustable by modifying the `dbclean` cron fragment). :Dependencies: None diff --git a/python/nav/asyncdns.py b/python/nav/asyncdns.py index 4edd5dcfa0..fb1d9d1148 100644 --- a/python/nav/asyncdns.py +++ b/python/nav/asyncdns.py @@ -33,7 +33,7 @@ from IPy import IP from twisted.names import dns from twisted.names import client -from twisted.internet import defer +from twisted.internet import defer, task # pylint: disable=E1101 from twisted.internet import reactor @@ -46,6 +46,9 @@ from twisted.names.error import DNSNotImplementedError, DNSQueryRefusedError +BATCH_SIZE = 100 + + def reverse_lookup(addresses): """Runs parallel reverse DNS lookups for addresses. @@ -75,21 +78,31 @@ def __init__(self): ) self.results = defaultdict(list) self._finished = False + self._errors = [] def resolve(self, names): """Resolves DNS names in parallel""" - self._finished = False self.results = defaultdict(list) - - deferred_list = [] - for name in names: - for deferred in self.lookup(name): - deferred.addCallback(self._extract_records, name) - deferred.addErrback(self._errback, name) - deferred_list.append(deferred) - - deferred_list = defer.DeferredList(deferred_list) - deferred_list.addCallback(self._parse_result) + self._finished = False + self._errors = [] + + def lookup_names(): + for name in names: + for deferred in self.lookup(name): + deferred.addCallback(self._extract_records, name) + deferred.addErrback(self._errback, name) + deferred.addCallback(self._save_result) + yield deferred + + # Limits the number of parallel requests to BATCH_SIZE + coop = task.Cooperator() + work = lookup_names() + deferred_list = defer.DeferredList( + [ + coop.coiterate(work).addErrback(self._save_error) + for _ in range(BATCH_SIZE) + ] + ) deferred_list.addCallback(self._finish) while not self._finished: @@ -98,6 +111,10 @@ def resolve(self, names): # iteration to ensure the resolver library closes its UDP sockets reactor.iterate() + # raise first error if any occurred + for error in self._errors: + raise error + return dict(self.results) def lookup(self, name): @@ -108,19 +125,22 @@ def lookup(self, name): def _extract_records(result, name): raise NotImplementedError - def _parse_result(self, result): - """Parses the result to the correct format""" - for _success, (name, response) in result: - if isinstance(response, Exception): - self.results[name] = response - else: - self.results[name].extend(response) + def _save_result(self, result): + name, response = result + if isinstance(response, Exception): + self.results[name] = response + else: + self.results[name].extend(response) @staticmethod def _errback(failure, host): """Errback""" return host, failure.value + def _save_error(self, failure): + """Errback for coiterator. Saves error so it can be raised later""" + self._errors.append(failure.value) + def _finish(self, _): self._finished = True diff --git a/python/nav/bin/navclean.py b/python/nav/bin/navclean.py index f32f980dcc..5e5f60b7bf 100755 --- a/python/nav/bin/navclean.py +++ b/python/nav/bin/navclean.py @@ -1,8 +1,10 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- # -*- testargs: --arp -*- +# -*- testargs: --close-arp -*- # # Copyright (C) 2017 Uninett AS +# Copyright (C) 2024 Sikt # # This file is part of Network Administration Visualized (NAV). # @@ -16,11 +18,10 @@ # details. You should have received a copy of the GNU General Public License # along with NAV. If not, see . # -"""Deletes old data from the NAV database""" -from __future__ import print_function - -import sys +"""Cleans old data from the NAV database""" import argparse +import sys +from typing import List from nav.bootstrap import bootstrap_django @@ -45,19 +46,20 @@ def main(): expiry = args.datetime connection = nav.db.getConnection('default', 'manage') - deleters = get_selected_deleters(args, connection) + cleaners = get_selected_cleaners(args, connection) - deleted = False - for deleter in deleters: + cleaned = False + for cleaner in cleaners: try: - count = deleter.delete(expiry, dry_run=not args.force) + count = cleaner.clean(expiry, dry_run=not args.force) if not args.quiet: print( - "Expired records in {table}: {count}".format( - table=deleter.table, count=count if count is not None else "N/A" + "Expired {expiry_type} records: {count}".format( + expiry_type=cleaner.expiry_type, + count=count if count is not None else "N/A", ) ) - deleted = True + cleaned = True except psycopg2.Error as error: print("The PostgreSQL backend produced an error", file=sys.stderr) @@ -67,15 +69,15 @@ def main(): if not args.force: connection.rollback() - deleted = False + cleaned = False else: connection.commit() if not args.quiet: - if deleted: - print("Expired records were deleted.") + if cleaned: + print("Expired records were updated/deleted.") else: - print("Nothing deleted.") + print("Nothing changed.") connection.close() @@ -88,39 +90,56 @@ def main(): def make_argparser(): """Makes this program's ArgumentParser""" parser = argparse.ArgumentParser( - description="Deletes old data from the NAV database", - epilog="Unless options are given, the number of expired records will " - "be printed. The default expiry limit is 6 months. The -e and -E" - " options set a common expiry date for all selected tables. If " - "you want different expiry dates for each table, you need to " - "run navclean more than once. To actually delete the expired " - "records, add the -f option.", + description="Cleans old data from the NAV database", + epilog="Cleaning old data means either deleting old records, or updating " + "expired records. Use options to select which types of data to clean. " + "The -e and -E options set an expiry date that applies to all " + "selected data types. Every run is a dry-run by default, unless the " + "-f option is given, in order to avoid accidental data deletion.", ) arg = parser.add_argument - arg("-q", "--quiet", action="store_true", help="be quiet") - arg("-f", "--force", action="store_true", help="force deletion of expired records") + arg("-q", "--quiet", action="store_true", help="Be quiet") + arg("-f", "--force", action="store_true", help="Force actual database updates") arg( "-e", "--datetime", type=postgresql_datetime, - help="set an explicit expiry date on ISO format", + help="Set an explicit expiry date on ISO format", ) arg( "-E", "--interval", type=postgresql_interval, default="6 months", - help="set an expiry interval using PostgreSQL interval syntax, e.g. " - "'30 days', '4 weeks', '6 months'", + help="Set an expiry interval using PostgreSQL interval syntax, e.g. " + "'30 days', '4 weeks', '6 months', '30 minutes'", ) - arg("--arp", action="store_true", help="delete from ARP table") - arg("--cam", action="store_true", help="delete from CAM table") - arg("--radiusacct", action="store_true", help="delete from Radius accounting table") - arg("--radiuslog", action="store_true", help="delete from Radius error log table") - arg("--netbox", action="store_true", help="delete from NETBOX table") - arg("--websessions", action="store_true", help="delete expired web sessions") + arg("--arp", action="store_true", help="Delete old records from ARP table") + arg( + "--close-arp", + action="store_true", + help="Close expired ARP records. Expired records are those where the netbox " + "has been down for longer than the set expiry limit", + ) + arg("--cam", action="store_true", help="Delete old records from CAM table") + arg( + "--radiusacct", + action="store_true", + help="Delete old records from Radius accounting table", + ) + arg( + "--radiuslog", + action="store_true", + help="Delete old records from Radius error log table", + ) + arg( + "--netbox", + action="store_true", + help="Delete netboxes that have been marked for deletion by Seed Database", + ) + arg("--websessions", action="store_true", help="Delete expired web sessions") return parser @@ -150,26 +169,28 @@ def postgresql_interval(value): return value -def get_selected_deleters(args, connection): - """Returns a list of RecordDeleter instances for each of the tables +def get_selected_cleaners( + args: argparse.Namespace, connection +) -> List["RecordCleaner"]: + """Returns a list of RecordCleaner instances for each of the tables selected in the supplied ArgumentParser. """ return [ - deleter(connection) - for deleter in RecordDeleter.__subclasses__() - if getattr(args, deleter.table, False) + cleaner(connection) + for cleaner in RecordCleaner.__subclasses__() + if getattr(args, cleaner.expiry_type, False) ] # -# Deleter implementations +# Cleaner implementations # -class RecordDeleter(object): - """Base class for record deletion""" +class RecordCleaner: + """Base class for record cleaning""" - table = None + expiry_type = None selector = "" def __init__(self, connection): @@ -180,12 +201,16 @@ def filter(self, expiry): return self.selector.format(expiry=expiry) def sql(self, expiry): - """Returns the full DELETE statement based on the expiry date""" + """Returns the full DELETE statement based on the expiry date. Override this + method if a different kind of update statement is needed. + """ where = self.filter(expiry) - return 'DELETE FROM {table} {filter}'.format(table=self.table, filter=where) + return 'DELETE FROM {table} {filter}'.format( + table=self.expiry_type, filter=where + ) - def delete(self, expiry, dry_run=False): - """Deletes the records selected by the expiry spec""" + def clean(self, expiry: str, dry_run: bool = False): + """Cleans the records selected by the expiry spec""" cursor = self.connection.cursor() sql = self.sql(expiry) cursor.execute(sql) @@ -195,18 +220,54 @@ def delete(self, expiry, dry_run=False): # pylint: disable=missing-docstring -class ArpDeleter(RecordDeleter): - table = "arp" +class ArpDeleter(RecordCleaner): + expiry_type = "arp" selector = "WHERE end_time < {expiry}" -class CamDeleter(RecordDeleter): - table = "cam" +class ArpCloser(RecordCleaner): + """Closes ARP records that have "expired", i.e. they're open, but the netbox they + were collected from has been down for too long. + """ + + expiry_type = "close_arp" + + def sql(self, expiry): + """Returns the full UPDATE statement based on the expiry date""" + return f""" + WITH unreachable_devices AS ( + SELECT + netboxid, + start_time AS downsince + FROM + alerthist + WHERE + eventtypeid = 'boxState' + AND end_time >= 'infinity' + ) + UPDATE + arp + SET + end_time = NOW() + WHERE + netboxid IN ( + SELECT + netboxid + FROM + unreachable_devices + WHERE + downsince < {expiry}) + AND end_time >= 'infinity' + """ + + +class CamDeleter(RecordCleaner): + expiry_type = "cam" selector = "WHERE end_time < {expiry}" -class RadiusAcctDeleter(RecordDeleter): - table = "radiusacct" +class RadiusAcctDeleter(RecordCleaner): + expiry_type = "radiusacct" selector = """ WHERE (acctstoptime < {expiry}) OR ((acctstarttime + (acctsessiontime * interval '1 sec')) < {expiry}) @@ -215,23 +276,23 @@ class RadiusAcctDeleter(RecordDeleter): """ -class RadiusLogDeleter(RecordDeleter): - table = "radiuslog" +class RadiusLogDeleter(RecordCleaner): + expiry_type = "radiuslog" selector = "WHERE time < {expiry}" -class NetboxDeleter(RecordDeleter): - table = "netbox" +class NetboxDeleter(RecordCleaner): + expiry_type = "netbox" selector = "WHERE pg_try_advisory_lock(netboxid) and deleted_at IS NOT NULL" -class SessionDeleter(RecordDeleter): +class SessionDeleter(RecordCleaner): """Special case deleter for Django Sessions""" - table = "websessions" + expiry_type = "websessions" @atomic - def delete(self, expiry, dry_run=False): + def clean(self, expiry, dry_run=False): """Deletes all expired django sessions if not a dry_run. Expiry spec is ignored, as sessions have a different expiry mechanism. diff --git a/python/nav/etc/cron.d/dbclean b/python/nav/etc/cron.d/dbclean index c5165c9f2a..bbcb7817d6 100644 --- a/python/nav/etc/cron.d/dbclean +++ b/python/nav/etc/cron.d/dbclean @@ -1,2 +1,3 @@ ## info: Clean database with navclean */5 * * * * navclean -f -q --netbox --websessions +*/5 * * * * navclean -f -q --close-arp --interval '30 minutes' diff --git a/python/nav/ipdevpoll/plugins/paloaltoarp.py b/python/nav/ipdevpoll/plugins/paloaltoarp.py index 5d5bd0dc06..fc5ee484df 100644 --- a/python/nav/ipdevpoll/plugins/paloaltoarp.py +++ b/python/nav/ipdevpoll/plugins/paloaltoarp.py @@ -138,11 +138,11 @@ def parse_arp(arp): arps = [] root = ET.fromstring(arp) - entries = root[0][4] + entries = root.find("result").find("entries") for entry in entries: - status = entry[0].text - ip = entry[1].text - mac = entry[2].text + status = entry.find("status").text + ip = entry.find("ip").text + mac = entry.find("mac").text if status.strip() != "i": if mac != "(incomplete)": arps.append(('ifindex', IP(ip), mac)) diff --git a/python/nav/models/sql/changes/sc.05.10.0001.sql b/python/nav/models/sql/changes/sc.05.10.0001.sql new file mode 100644 index 0000000000..dae97f6cb7 --- /dev/null +++ b/python/nav/models/sql/changes/sc.05.10.0001.sql @@ -0,0 +1,2 @@ +-- Remove malfunctioning ARP record closing rule, see #2910 +DROP RULE IF EXISTS netbox_close_arp ON netbox; diff --git a/python/nav/web/templates/webfront/about.html b/python/nav/web/templates/webfront/about.html index 786f313351..92ca58bcab 100644 --- a/python/nav/web/templates/webfront/about.html +++ b/python/nav/web/templates/webfront/about.html @@ -93,6 +93,7 @@

Contributors

  • Ilona Podliashanyk
  • John-Magne Bredal
  • Sigmund Augdal
    • +
  • Joar Heimonen
  • Ragnhild Bodsberg
  • Philipp Petermann
  • Leigh Murray