diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index 85397d1de..323f2fbba 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -410,7 +410,7 @@ class DeviceAdmin(MultitenantAdminMixin, BaseConfigAdmin, UUIDAdmin): 'system', 'devicelocation__location__address', ] - readonly_fields = ['last_ip', 'management_ip', 'uuid'] + readonly_fields = ['last_ip', 'uuid'] autocomplete_fields = ['group'] fields = [ 'name', @@ -421,6 +421,7 @@ class DeviceAdmin(MultitenantAdminMixin, BaseConfigAdmin, UUIDAdmin): 'group', 'last_ip', 'management_ip', + 'use_custom_ip', 'model', 'os', 'system', @@ -448,6 +449,14 @@ class Media(BaseConfigAdmin.Media): f'{prefix}js/relevant_templates.js', ] + def get_readonly_fields(self, request, obj=None): + res = set(self.readonly_fields) + if obj is None: + return res + if not obj.use_custom_ip: + res.add('management_ip') + return res + def get_fields(self, request, obj=None): """ Do not show readonly fields in add form diff --git a/openwisp_controller/config/base/device.py b/openwisp_controller/config/base/device.py index 5c82b25b5..c3eb21d2f 100644 --- a/openwisp_controller/config/base/device.py +++ b/openwisp_controller/config/base/device.py @@ -91,6 +91,13 @@ class AbstractDevice(OrgMixin, BaseModel): help_text=_('ip address of the management interface, if available'), ) hardware_id = models.CharField(**(app_settings.HARDWARE_ID_OPTIONS)) + use_custom_ip = models.BooleanField( + default=False, + help_text=_( + 'if enabled, user shall be able to change the ' + 'management ip of the device manually' + ), + ) class Meta: unique_together = ( diff --git a/openwisp_controller/config/migrations/0037_device_use_custom_ip.py b/openwisp_controller/config/migrations/0037_device_use_custom_ip.py new file mode 100644 index 000000000..0c8282f9f --- /dev/null +++ b/openwisp_controller/config/migrations/0037_device_use_custom_ip.py @@ -0,0 +1,22 @@ +# Generated by Django 3.1.13 on 2021-08-02 14:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('config', '0036_device_group'), + ] + + operations = [ + migrations.AddField( + model_name='device', + name='use_custom_ip', + field=models.BooleanField( + default=False, + help_text='if enabled, user shall be able to change the ' + 'management ip of the device manually', + ), + ), + ] diff --git a/openwisp_controller/config/tests/test_admin.py b/openwisp_controller/config/tests/test_admin.py index 0372337f3..eb2d4d0c1 100644 --- a/openwisp_controller/config/tests/test_admin.py +++ b/openwisp_controller/config/tests/test_admin.py @@ -1204,6 +1204,30 @@ def test_no_system_context(self): ) app_settings.CONTEXT = old_context + def test_editable_management_ip(self): + d = self._create_device() + d.use_custom_ip = False + d.save() + + with self.subTest('test auto management ip'): + r = self.client.get( + reverse(f'admin:{self.app_label}_device_change', args=[d.pk]) + ) + self.assertContains(r, '') + self.assertNotContains( + r, '' + ) + + with self.subTest('test manual management ip'): + d.use_custom_ip = True + d.save() + r = self.client.get( + reverse(f'admin:{self.app_label}_device_change', args=[d.pk]) + ) + self.assertContains( + r, '' + ) + @classmethod def tearDownClass(cls): super().tearDownClass() diff --git a/openwisp_controller/config/utils.py b/openwisp_controller/config/utils.py index 8be4a0546..a31c17f73 100644 --- a/openwisp_controller/config/utils.py +++ b/openwisp_controller/config/utils.py @@ -72,7 +72,8 @@ def update_last_ip(device, request): if device.last_ip != ip: device.last_ip = ip update_fields.append('last_ip') - if device.management_ip != management_ip: + if device.management_ip != management_ip and not device.use_custom_ip: + # do not update in case user uses a custom ip device.management_ip = management_ip update_fields.append('management_ip') if update_fields: diff --git a/openwisp_controller/connection/connectors/openwrt/snmp.py b/openwisp_controller/connection/connectors/openwrt/snmp.py deleted file mode 100644 index 41c5835bd..000000000 --- a/openwisp_controller/connection/connectors/openwrt/snmp.py +++ /dev/null @@ -1,9 +0,0 @@ -import logging - -from ..snmp import Snmp - -logger = logging.getLogger(__name__) - - -class OpenWrt(Snmp): - pass diff --git a/openwisp_controller/connection/connectors/snmp.py b/openwisp_controller/connection/connectors/snmp.py index c8f5aaf65..8347a7391 100644 --- a/openwisp_controller/connection/connectors/snmp.py +++ b/openwisp_controller/connection/connectors/snmp.py @@ -42,10 +42,8 @@ def validate(cls, params): @classmethod def custom_validation(cls, params): - if 'community' not in params: - raise SchemaError('Missing community') - if 'agent' not in params: - raise SchemaError('Missing agent') + if 'community' not in params or 'agent' not in params: + raise SchemaError('Missing community or agent') @cached_property def params(self): diff --git a/openwisp_controller/connection/tests/test_snmp.py b/openwisp_controller/connection/tests/test_snmp.py index 75c9c1229..7fcb0fa97 100644 --- a/openwisp_controller/connection/tests/test_snmp.py +++ b/openwisp_controller/connection/tests/test_snmp.py @@ -1,3 +1,4 @@ +from django.core.exceptions import ValidationError from django.test import TestCase from swapper import load_model @@ -16,7 +17,24 @@ def test_create_snmp_connector(self): ) obj.save() obj.connector_instance.validate(params) + self.assertEqual(params, obj.params) self.assertEqual(Credentials.objects.count(), init_credentials_count + 1) self.assertEqual( obj.connector, 'openwisp_controller.connection.connectors.snmp.Snmp' ) + + def test_validation(self): + with self.subTest('test validation unsuccessful'): + params = {'agent': 'my-agent', 'port': 161} + with self.assertRaises(ValidationError): + self._create_credentials( + params=params, connector=app_settings.CONNECTORS[1][0] + ) + + with self.subTest('test validation successful'): + params = {'community': 'public', 'agent': 'my-agent', 'port': 161} + obj = self._create_credentials( + params=params, connector=app_settings.CONNECTORS[1][0], auto_add=True + ) + obj.save() + obj.connector_instance.validate(params) diff --git a/tests/openwisp2/sample_config/migrations/0004_device_use_custom_ip.py b/tests/openwisp2/sample_config/migrations/0004_device_use_custom_ip.py new file mode 100644 index 000000000..c8cd813ad --- /dev/null +++ b/tests/openwisp2/sample_config/migrations/0004_device_use_custom_ip.py @@ -0,0 +1,21 @@ +# Generated by Django 3.1.13 on 2021-08-02 17:07 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('sample_config', '0003_name_unique_per_organization'), + ] + + operations = [ + migrations.AddField( + model_name='device', + name='use_custom_ip', + field=models.BooleanField( + default=False, + help_text='if enabled, user shall be able to change the management ip of the device manually', + ), + ), + ]