diff --git a/docs/configuration/index.md b/docs/configuration/index.md index e3e2d061c..0ae37b134 100644 --- a/docs/configuration/index.md +++ b/docs/configuration/index.md @@ -21,6 +21,7 @@ Some configuration parameters are primarily controlled via NetBox's admin interf * [`BANNER_BOTTOM`](./miscellaneous.md#banner_bottom) * [`BANNER_LOGIN`](./miscellaneous.md#banner_login) * [`BANNER_TOP`](./miscellaneous.md#banner_top) +* [`CHANGELOG_RETAIN_CREATE_LAST_UPDATE`](./miscellaneous.md#changelog_retain_create_last_update) * [`CHANGELOG_RETENTION`](./miscellaneous.md#changelog_retention) * [`CUSTOM_VALIDATORS`](./data-validation.md#custom_validators) * [`DEFAULT_USER_PREFERENCES`](./default-values.md#default_user_preferences) diff --git a/docs/configuration/miscellaneous.md b/docs/configuration/miscellaneous.md index 4ad3ba6ac..2de8a861d 100644 --- a/docs/configuration/miscellaneous.md +++ b/docs/configuration/miscellaneous.md @@ -73,6 +73,27 @@ This data enables the project maintainers to estimate how many NetBox deployment --- +## CHANGELOG_RETAIN_CREATE_LAST_UPDATE + +!!! tip "Dynamic Configuration Parameter" + +Default: `True` + +When pruning expired changelog entries (per `CHANGELOG_RETENTION`), retain each non-deleted object's original `create` +change record and its most recent `update` change record. If an object has a `delete` change record, its changelog +entries are pruned normally according to `CHANGELOG_RETENTION`. + +!!! note + For objects without a `delete` change record, the original `create` record and most recent `update` record are + exempt from pruning. All other changelog records (including intermediate `update` records and all `delete` records) + remain subject to pruning per `CHANGELOG_RETENTION`. + +!!! warning + This setting is enabled by default. Upgrading deployments that rely on complete pruning of expired changelog entries + should explicitly set `CHANGELOG_RETAIN_CREATE_LAST_UPDATE = False` to preserve the previous behavior. + +--- + ## CHANGELOG_RETENTION !!! tip "Dynamic Configuration Parameter" diff --git a/netbox/core/forms/model_forms.py b/netbox/core/forms/model_forms.py index 4f26d6024..c840827b5 100644 --- a/netbox/core/forms/model_forms.py +++ b/netbox/core/forms/model_forms.py @@ -165,9 +165,10 @@ class ConfigRevisionForm(forms.ModelForm, metaclass=ConfigFormMetaclass): FieldSet('PAGINATE_COUNT', 'MAX_PAGE_SIZE', name=_('Pagination')), FieldSet('CUSTOM_VALIDATORS', 'PROTECTION_RULES', name=_('Validation')), FieldSet('DEFAULT_USER_PREFERENCES', name=_('User Preferences')), + FieldSet('CHANGELOG_RETENTION', 'CHANGELOG_RETAIN_CREATE_LAST_UPDATE', name=_('Change Log')), FieldSet( - 'MAINTENANCE_MODE', 'COPILOT_ENABLED', 'GRAPHQL_ENABLED', 'CHANGELOG_RETENTION', 'JOB_RETENTION', - 'MAPS_URL', name=_('Miscellaneous'), + 'MAINTENANCE_MODE', 'COPILOT_ENABLED', 'GRAPHQL_ENABLED', 'JOB_RETENTION', 'MAPS_URL', + name=_('Miscellaneous'), ), FieldSet('comment', name=_('Config Revision')) ) diff --git a/netbox/core/jobs.py b/netbox/core/jobs.py index 5859a9989..46dc10d1c 100644 --- a/netbox/core/jobs.py +++ b/netbox/core/jobs.py @@ -5,6 +5,7 @@ from importlib import import_module import requests from django.conf import settings from django.core.cache import cache +from django.db.models import Exists, OuterRef, Subquery from django.utils import timezone from packaging import version @@ -14,7 +15,7 @@ from netbox.jobs import JobRunner, system_job from netbox.search.backends import search_backend from utilities.proxy import resolve_proxies -from .choices import DataSourceStatusChoices, JobIntervalChoices +from .choices import DataSourceStatusChoices, JobIntervalChoices, ObjectChangeActionChoices from .models import DataSource @@ -126,19 +127,51 @@ class SystemHousekeepingJob(JobRunner): """ Delete any ObjectChange records older than the configured changelog retention time (if any). """ - self.logger.info("Pruning old changelog entries...") + self.logger.info('Pruning old changelog entries...') config = Config() if not config.CHANGELOG_RETENTION: - self.logger.info("No retention period specified; skipping.") + self.logger.info('No retention period specified; skipping.') return cutoff = timezone.now() - timedelta(days=config.CHANGELOG_RETENTION) - self.logger.debug( - f"Changelog retention period: {config.CHANGELOG_RETENTION} days ({cutoff:%Y-%m-%d %H:%M:%S})" - ) + self.logger.debug(f'Changelog retention period: {config.CHANGELOG_RETENTION} days ({cutoff:%Y-%m-%d %H:%M:%S})') - count = ObjectChange.objects.filter(time__lt=cutoff).delete()[0] - self.logger.info(f"Deleted {count} expired changelog records") + expired_qs = ObjectChange.objects.filter(time__lt=cutoff) + + # When enabled, retain each object's original create record and most recent update record while pruning expired + # changelog entries. This applies only to objects without a delete record. + if config.CHANGELOG_RETAIN_CREATE_LAST_UPDATE: + self.logger.debug('Retaining changelog create records and last update records (excluding deleted objects)') + + deleted_exists = ObjectChange.objects.filter( + action=ObjectChangeActionChoices.ACTION_DELETE, + changed_object_type_id=OuterRef('changed_object_type_id'), + changed_object_id=OuterRef('changed_object_id'), + ) + + # Keep create records only where no delete exists for that object + create_pks_to_keep = ( + ObjectChange.objects.filter(action=ObjectChangeActionChoices.ACTION_CREATE) + .annotate(has_delete=Exists(deleted_exists)) + .filter(has_delete=False) + .values('pk') + ) + + # Keep the most recent update per object only where no delete exists for the object + latest_update_pks_to_keep = ( + ObjectChange.objects.filter(action=ObjectChangeActionChoices.ACTION_UPDATE) + .annotate(has_delete=Exists(deleted_exists)) + .filter(has_delete=False) + .order_by('changed_object_type_id', 'changed_object_id', '-time', '-pk') + .distinct('changed_object_type_id', 'changed_object_id') + .values('pk') + ) + + expired_qs = expired_qs.exclude(pk__in=Subquery(create_pks_to_keep)) + expired_qs = expired_qs.exclude(pk__in=Subquery(latest_update_pks_to_keep)) + + count = expired_qs.delete()[0] + self.logger.info(f'Deleted {count} expired changelog records') def delete_expired_jobs(self): """ diff --git a/netbox/core/tests/test_changelog.py b/netbox/core/tests/test_changelog.py index c3924bccb..dc9dbb2dc 100644 --- a/netbox/core/tests/test_changelog.py +++ b/netbox/core/tests/test_changelog.py @@ -1,9 +1,16 @@ +import logging +import uuid +from datetime import timedelta +from unittest.mock import patch + from django.contrib.contenttypes.models import ContentType -from django.test import override_settings +from django.test import TestCase, override_settings from django.urls import reverse +from django.utils import timezone from rest_framework import status from core.choices import ObjectChangeActionChoices +from core.jobs import SystemHousekeepingJob from core.models import ObjectChange, ObjectType from dcim.choices import InterfaceTypeChoices, ModuleStatusChoices, SiteStatusChoices from dcim.models import ( @@ -694,3 +701,99 @@ class ChangeLogAPITest(APITestCase): self.assertEqual(changes[3].changed_object_type, ContentType.objects.get_for_model(Module)) self.assertEqual(changes[3].changed_object_id, module.pk) self.assertEqual(changes[3].action, ObjectChangeActionChoices.ACTION_DELETE) + + +class ChangelogPruneRetentionTest(TestCase): + """Test suite for Changelog pruning retention settings.""" + + @staticmethod + def _make_oc(*, ct, obj_id, action, ts): + oc = ObjectChange.objects.create( + changed_object_type=ct, + changed_object_id=obj_id, + action=action, + user_name='test', + request_id=uuid.uuid4(), + object_repr=f'Object {obj_id}', + ) + ObjectChange.objects.filter(pk=oc.pk).update(time=ts) + return oc.pk + + @staticmethod + def _run_prune(*, retention_days, retain_create_last_update): + job = SystemHousekeepingJob.__new__(SystemHousekeepingJob) + job.logger = logging.getLogger('netbox.tests.changelog_prune') + + with patch('core.jobs.Config') as MockConfig: + cfg = MockConfig.return_value + cfg.CHANGELOG_RETENTION = retention_days + cfg.CHANGELOG_RETAIN_CREATE_LAST_UPDATE = retain_create_last_update + job.prune_changelog() + + def test_prune_retain_create_last_update_excludes_deleted_objects(self): + ct = ContentType.objects.get_for_model(Site) + + retention_days = 90 + now = timezone.now() + cutoff = now - timedelta(days=retention_days) + + expired_old = cutoff - timedelta(days=10) + expired_newer = cutoff - timedelta(days=1) + not_expired = cutoff + timedelta(days=1) + + # A) Not deleted: should keep CREATE + latest UPDATE, prune intermediate UPDATEs + a_create = self._make_oc(ct=ct, obj_id=1, action=ObjectChangeActionChoices.ACTION_CREATE, ts=expired_old) + a_update1 = self._make_oc(ct=ct, obj_id=1, action=ObjectChangeActionChoices.ACTION_UPDATE, ts=expired_old) + a_update2 = self._make_oc(ct=ct, obj_id=1, action=ObjectChangeActionChoices.ACTION_UPDATE, ts=expired_newer) + + # B) Deleted (all expired): should keep NOTHING + b_create = self._make_oc(ct=ct, obj_id=2, action=ObjectChangeActionChoices.ACTION_CREATE, ts=expired_old) + b_update = self._make_oc(ct=ct, obj_id=2, action=ObjectChangeActionChoices.ACTION_UPDATE, ts=expired_newer) + b_delete = self._make_oc(ct=ct, obj_id=2, action=ObjectChangeActionChoices.ACTION_DELETE, ts=expired_newer) + + # C) Deleted but delete is not expired: create/update expired should be pruned; delete remains + c_create = self._make_oc(ct=ct, obj_id=3, action=ObjectChangeActionChoices.ACTION_CREATE, ts=expired_old) + c_update = self._make_oc(ct=ct, obj_id=3, action=ObjectChangeActionChoices.ACTION_UPDATE, ts=expired_newer) + c_delete = self._make_oc(ct=ct, obj_id=3, action=ObjectChangeActionChoices.ACTION_DELETE, ts=not_expired) + + self._run_prune(retention_days=retention_days, retain_create_last_update=True) + + remaining = set(ObjectChange.objects.values_list('pk', flat=True)) + + # A) Not deleted -> create + latest update remain + self.assertIn(a_create, remaining) + self.assertIn(a_update2, remaining) + self.assertNotIn(a_update1, remaining) + + # B) Deleted (all expired) -> nothing remains + self.assertNotIn(b_create, remaining) + self.assertNotIn(b_update, remaining) + self.assertNotIn(b_delete, remaining) + + # C) Deleted, delete not expired -> delete remains, but create/update are pruned + self.assertNotIn(c_create, remaining) + self.assertNotIn(c_update, remaining) + self.assertIn(c_delete, remaining) + + def test_prune_disabled_deletes_all_expired(self): + ct = ContentType.objects.get_for_model(Site) + + retention_days = 90 + now = timezone.now() + cutoff = now - timedelta(days=retention_days) + expired = cutoff - timedelta(days=1) + not_expired = cutoff + timedelta(days=1) + + # expired create/update should be deleted when feature disabled + x_create = self._make_oc(ct=ct, obj_id=10, action=ObjectChangeActionChoices.ACTION_CREATE, ts=expired) + x_update = self._make_oc(ct=ct, obj_id=10, action=ObjectChangeActionChoices.ACTION_UPDATE, ts=expired) + + # non-expired delete should remain regardless + y_delete = self._make_oc(ct=ct, obj_id=11, action=ObjectChangeActionChoices.ACTION_DELETE, ts=not_expired) + + self._run_prune(retention_days=retention_days, retain_create_last_update=False) + + remaining = set(ObjectChange.objects.values_list('pk', flat=True)) + self.assertNotIn(x_create, remaining) + self.assertNotIn(x_update, remaining) + self.assertIn(y_delete, remaining) diff --git a/netbox/extras/management/commands/housekeeping.py b/netbox/extras/management/commands/housekeeping.py index 63a5e5b61..e68142ac3 100644 --- a/netbox/extras/management/commands/housekeeping.py +++ b/netbox/extras/management/commands/housekeeping.py @@ -6,9 +6,11 @@ import requests from django.conf import settings from django.core.cache import cache from django.core.management.base import BaseCommand +from django.db.models import Exists, OuterRef, Subquery from django.utils import timezone from packaging import version +from core.choices import ObjectChangeActionChoices from core.models import Job, ObjectChange from netbox.config import Config from utilities.proxy import resolve_proxies @@ -47,29 +49,63 @@ class Command(BaseCommand): # Delete expired ObjectChanges if options['verbosity']: - self.stdout.write("[*] Checking for expired changelog records") + self.stdout.write('[*] Checking for expired changelog records') if config.CHANGELOG_RETENTION: cutoff = timezone.now() - timedelta(days=config.CHANGELOG_RETENTION) if options['verbosity'] >= 2: - self.stdout.write(f"\tRetention period: {config.CHANGELOG_RETENTION} days") - self.stdout.write(f"\tCut-off time: {cutoff}") - expired_records = ObjectChange.objects.filter(time__lt=cutoff).count() + self.stdout.write(f'\tRetention period: {config.CHANGELOG_RETENTION} days') + self.stdout.write(f'\tCut-off time: {cutoff}') + + expired_qs = ObjectChange.objects.filter(time__lt=cutoff) + + # When enabled, retain each object's original create and most recent update record while pruning expired + # changelog entries. This applies only to objects without a delete record. + if config.CHANGELOG_RETAIN_CREATE_LAST_UPDATE: + if options['verbosity'] >= 2: + self.stdout.write('\tRetaining create & last update records for non-deleted objects') + + deleted_exists = ObjectChange.objects.filter( + action=ObjectChangeActionChoices.ACTION_DELETE, + changed_object_type_id=OuterRef('changed_object_type_id'), + changed_object_id=OuterRef('changed_object_id'), + ) + + # Keep create records only where no delete exists for that object + create_pks_to_keep = ( + ObjectChange.objects.filter(action=ObjectChangeActionChoices.ACTION_CREATE) + .annotate(has_delete=Exists(deleted_exists)) + .filter(has_delete=False) + .values('pk') + ) + + # Keep the most recent update per object only where no delete exists for the object + latest_update_pks_to_keep = ( + ObjectChange.objects.filter(action=ObjectChangeActionChoices.ACTION_UPDATE) + .annotate(has_delete=Exists(deleted_exists)) + .filter(has_delete=False) + .order_by('changed_object_type_id', 'changed_object_id', '-time', '-pk') + .distinct('changed_object_type_id', 'changed_object_id') + .values('pk') + ) + + expired_qs = expired_qs.exclude(pk__in=Subquery(create_pks_to_keep)) + expired_qs = expired_qs.exclude(pk__in=Subquery(latest_update_pks_to_keep)) + + expired_records = expired_qs.count() if expired_records: if options['verbosity']: self.stdout.write( - f"\tDeleting {expired_records} expired records... ", - self.style.WARNING, - ending="" + f'\tDeleting {expired_records} expired records... ', self.style.WARNING, ending='' ) self.stdout.flush() - ObjectChange.objects.filter(time__lt=cutoff).delete() + expired_qs.delete() if options['verbosity']: - self.stdout.write("Done.", self.style.SUCCESS) + self.stdout.write('Done.', self.style.SUCCESS) elif options['verbosity']: - self.stdout.write("\tNo expired records found.", self.style.SUCCESS) + self.stdout.write('\tNo expired records found.', self.style.SUCCESS) elif options['verbosity']: self.stdout.write( - f"\tSkipping: No retention period specified (CHANGELOG_RETENTION = {config.CHANGELOG_RETENTION})" + f'\tSkipping: No retention period specified (CHANGELOG_RETENTION = {config.CHANGELOG_RETENTION})' ) # Delete expired Jobs diff --git a/netbox/netbox/config/__init__.py b/netbox/netbox/config/__init__.py index 3c084949a..b6ca4f90c 100644 --- a/netbox/netbox/config/__init__.py +++ b/netbox/netbox/config/__init__.py @@ -10,6 +10,7 @@ from .parameters import PARAMS __all__ = ( 'PARAMS', + 'Config', 'ConfigItem', 'clear_config', 'get_config', diff --git a/netbox/netbox/config/parameters.py b/netbox/netbox/config/parameters.py index cc87b58bb..80081509a 100644 --- a/netbox/netbox/config/parameters.py +++ b/netbox/netbox/config/parameters.py @@ -175,6 +175,25 @@ PARAMS = ( field=forms.JSONField ), + # Change log + ConfigParam( + name='CHANGELOG_RETENTION', + label=_('Changelog retention'), + default=90, + description=_("Days to retain changelog history (set to zero for unlimited)"), + field=forms.IntegerField, + ), + ConfigParam( + name='CHANGELOG_RETAIN_CREATE_LAST_UPDATE', + label=_('Retain create & last update changelog records'), + default=True, + description=_( + "Retain each object's create record and most recent update record when pruning expired changelog entries " + "(excluding objects with a delete record)." + ), + field=forms.BooleanField, + ), + # Miscellaneous ConfigParam( name='MAINTENANCE_MODE', @@ -199,13 +218,6 @@ PARAMS = ( description=_("Enable the GraphQL API"), field=forms.BooleanField ), - ConfigParam( - name='CHANGELOG_RETENTION', - label=_('Changelog retention'), - default=90, - description=_("Days to retain changelog history (set to zero for unlimited)"), - field=forms.IntegerField - ), ConfigParam( name='JOB_RETENTION', label=_('Job result retention'), diff --git a/netbox/templates/core/inc/config_data.html b/netbox/templates/core/inc/config_data.html index cb32d7a88..964f9cee8 100644 --- a/netbox/templates/core/inc/config_data.html +++ b/netbox/templates/core/inc/config_data.html @@ -122,6 +122,19 @@ {% endif %} + {# Changelog #} + + {% trans "Change log" %} + + + {% trans "Changelog retention" %} + {{ config.CHANGELOG_RETENTION }} + + + {% trans "Changelog retain create & last update records" %} + {% checkmark config.CHANGELOG_RETAIN_CREATE_LAST_UPDATE %} + + {# Miscellaneous #} {% trans "Miscellaneous" %} @@ -137,10 +150,6 @@ {% trans "GraphQL enabled" %} {% checkmark config.GRAPHQL_ENABLED %} - - {% trans "Changelog retention" %} - {{ config.CHANGELOG_RETENTION }} - {% trans "Job retention" %} {{ config.JOB_RETENTION }} diff --git a/netbox/templates/core/objectchange_list.html b/netbox/templates/core/objectchange_list.html index ef11c0ef1..6231eb875 100644 --- a/netbox/templates/core/objectchange_list.html +++ b/netbox/templates/core/objectchange_list.html @@ -6,6 +6,6 @@ {% block content %} {{ block.super }}
- {% trans "Change log retention" %}: {% if config.CHANGELOG_RETENTION %}{{ config.CHANGELOG_RETENTION }} {% trans "days" %}{% else %}{% trans "Indefinite" %}{% endif %} + {% trans "Change log retention" %}: {% if config.CHANGELOG_RETENTION %}{{ config.CHANGELOG_RETENTION }} {% trans "days" %}{% if config.CHANGELOG_RETAIN_CREATE_LAST_UPDATE %} ({% trans "retaining create & last update records for non-deleted objects" %}){% endif %}{% else %}{% trans "Indefinite" %}{% endif %}
{% endblock content %} diff --git a/netbox/templates/extras/object_changelog.html b/netbox/templates/extras/object_changelog.html index 90855a750..30c822083 100644 --- a/netbox/templates/extras/object_changelog.html +++ b/netbox/templates/extras/object_changelog.html @@ -12,7 +12,7 @@
- {% trans "Change log retention" %}: {% if config.CHANGELOG_RETENTION %}{{ config.CHANGELOG_RETENTION }} {% trans "days" %}{% else %}{% trans "Indefinite" %}{% endif %} + {% trans "Change log retention" %}: {% if config.CHANGELOG_RETENTION %}{{ config.CHANGELOG_RETENTION }} {% trans "days" %}{% if config.CHANGELOG_RETAIN_CREATE_LAST_UPDATE %} ({% trans "retaining create & last update records for non-deleted objects" %}){% endif %}{% else %}{% trans "Indefinite" %}{% endif %}