Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Allow updating templates with invalid configurations #948

Merged
merged 4 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ Other popular building blocks that are part of the OpenWISP ecosystem are:
provides device status monitoring, collection of metrics, charts,
alerts, possibility to define custom checks
- `openwisp-firmware-upgrader
<https://openwisp.io/docs/stable/firmware-upgrader/>`_: automated firmware
upgrades (single devices or mass network upgrades)
<https://openwisp.io/docs/stable/firmware-upgrader/>`_: automated
firmware upgrades (single devices or mass network upgrades)
- `openwisp-radius <https://openwisp.io/docs/stable/user/radius.html>`_:
based on FreeRADIUS, allows to implement network access authentication
systems like 802.1x WPA2 Enterprise, captive portal authentication,
Expand All @@ -65,10 +65,11 @@ Other popular building blocks that are part of the OpenWISP ecosystem are:
daemons or other network software (e.g.: OpenVPN); it can be used in
conjunction with openwisp-monitoring to get a better idea of the state
of the network
- `openwisp-ipam <https://openwisp.io/docs/stable/ipam/>`_: allows to manage
the assignment of IP addresses used in the network
- `openwisp-notifications <https://openwisp.io/docs/stable/notifications/>`_:
allows users to be aware of important events happening in the network.
- `openwisp-ipam <https://openwisp.io/docs/stable/ipam/>`_: allows to
manage the assignment of IP addresses used in the network
- `openwisp-notifications
<https://openwisp.io/docs/stable/notifications/>`_: allows users to be
aware of important events happening in the network.

**For a more complete overview of the OpenWISP modules and architecture**,
see the `OpenWISP Architecture Overview
Expand Down
2 changes: 1 addition & 1 deletion openwisp_controller/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
VERSION = (1, 1, 0, 'final')
VERSION = (1, 2, 0, 'alpha')
__version__ = VERSION # alias


Expand Down
10 changes: 9 additions & 1 deletion openwisp_controller/config/base/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.db import models, transaction
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField
from netjsonconfig.exceptions import ValidationError as NetjsonconfigValidationError
from swapper import get_model_name
from taggit.managers import TaggableManager

Expand Down Expand Up @@ -115,7 +116,14 @@ def save(self, *args, **kwargs):
if hasattr(self, 'backend_instance'):
del self.backend_instance
current = self.__class__.objects.get(pk=self.pk)
update_related_config_status = self.checksum != current.checksum
try:
current_checksum = current.checksum
except NetjsonconfigValidationError:
# If the Netjsonconfig library upgrade changes the schema,
# the old configuration may become invalid, raising an exception.
# Setting the checksum to None forces related configurations to update.
current_checksum = None
update_related_config_status = self.checksum != current_checksum
# save current changes
super().save(*args, **kwargs)
# update relations
Expand Down
15 changes: 15 additions & 0 deletions openwisp_controller/config/tests/test_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from django.db import transaction
from django.test import TestCase, TransactionTestCase
from netjsonconfig import OpenWrt
from netjsonconfig.exceptions import ValidationError as NetjsonconfigValidationError
from swapper import load_model

from openwisp_utils.tests import catch_signal
Expand Down Expand Up @@ -500,6 +501,20 @@ def test_required_vpn_template_corner_case(self):
# {'__all__': ['VPN client with this Config and Vpn already exists.']}
self.assertIsNotNone(vpn_client)

def test_regression_preventing_from_fixing_invalid_conf(self):
template = self._create_template()
# create a template with an invalid configuration
Template.objects.update(config={'interfaces': [{'name': 'eth0', 'type': ''}]})
# ensure the configuration raises ValidationError
with self.assertRaises(NetjsonconfigValidationError):
template.refresh_from_db()
del template.backend_instance
template.checksum
del template.backend_instance
template.config = {'interfaces': [{'name': 'eth0', 'type': 'ethernet'}]}
template.full_clean()
template.save()


class TestTemplateTransaction(
TransactionTestMixin,
Expand Down
Loading