diff --git a/README.rst b/README.rst index cc4dc070c..0be7fe2ff 100644 --- a/README.rst +++ b/README.rst @@ -531,7 +531,24 @@ with templates, this feature is also known as *configuration context*, think of it like a dictionary which is passed to the function which renders the configuration, so that it can fill variables according to the passed context. -The different ways in which variables are defined are described below. +The different ways in which variables are defined are described below in +the order (high to low) of their precedence: + +1. `User defined device variables <#user-defined-device-variables>`_ +2. `Predefined device variables <#predefined-device-variables>`_ +3. `Group variables <#group-variables>`_ +4. `Global variables <#global-variables>`_ +5. `Template default values <#template-default-values>`_ + +User defined device variables +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In the device configuration section you can find a section named +"Configuration variables" where it is possible to define the configuration +variables and their values, as shown in the example below: + +.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/device-context.png + :alt: context Predefined device variables ~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -543,15 +560,19 @@ Each device gets the following attributes passed as configuration variables: * ``name`` * ``mac_address`` -User defined device variables -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Group variables +~~~~~~~~~~~~~~~ -In the device configuration section you can find a section named -"Configuration variables" where it is possible to define the configuration -variables and their values, as shown in the example below: +Variables can also be defined in `Device groups <#device-groups>`__. -.. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/device-context.png - :alt: context +Refer the `Group configuration variables `_ +section for detailed information. + +Global variables +~~~~~~~~~~~~~~~~ + +Variables can also be defined globally using the +`OPENWISP_CONTROLLER_CONTEXT <#openwisp-controller-context>`_ setting. Template default values ~~~~~~~~~~~~~~~~~~~~~~~ @@ -574,12 +595,6 @@ The default values of variables can be manipulated from the section .. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/template-default-values.png :alt: default values -Global variables -~~~~~~~~~~~~~~~~ - -Variables can also be defined globally using the -`OPENWISP_CONTROLLER_CONTEXT <#openwisp-controller-context>`_ setting. - System defined variables ~~~~~~~~~~~~~~~~~~~~~~~~ @@ -591,6 +606,9 @@ in read-only mode. .. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/system-defined-variables.png :alt: system defined variables +**Note:** `Group configuration variables <#group-configuration-variables>`__ +are also added to the **System Defined Variables** of the device. + Example usage of variables ~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -935,12 +953,9 @@ Device Groups provide features aimed at adding specific management rules for the devices of an organization: - Group similar devices by having dedicated groups for access points, routers, etc. -- Store additional information regarding a group in the structured metadata field - (which can be accessed via the REST API). -- Customize structure and validation of metadata field of DeviceGroup to standardize - information across all groups using `"OPENWISP_CONTROLLER_DEVICE_GROUP_SCHEMA" <#openwisp-controller-device-group-schema>`_ - setting. +- Define `group metadata <#group-metadata>`_. - Define `group configuration templates <#group-templates>`_. +- Define `group configuration variables <#group-configuration-variables>`__. .. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/1.1/device-groups.png :alt: Device Group example @@ -983,6 +998,33 @@ to new devices. This feature works also when editing group templates or the group assigned to a device via the `REST API <#change-device-group-detail>`__. +Group Configuration Variables +############################# + +Groups allow to define configuration variables which are automatically +added to the device's context in the **System Defined Variables**. +Check the `"How to use configuration variables" section <#how-to-use-configuration-variables>`_ +to learn about precedence of different configuration variables. + +This feature works also when editing group templates or the group assigned +to a device via the `REST API <#change-device-group-detail>`__. + +Group Metadata +############## + +Groups allow to store additional information regarding a group in the +structured metadata field (which can be accessed via the REST API). + +The metadata field allows custom structure and validation to standardize +information across all groups using the +`"OPENWISP_CONTROLLER_DEVICE_GROUP_SCHEMA" <#openwisp-controller-device-group-schema>`_ +setting. + +**Note:** *Group configuration variables* and *Group metadata* serves different purposes. +The group configuration variables should be used when the device configuration is required +to be changed for particular group of devices. Group metadata should be used to store +additional data for the devices. Group metadata is not used for configuration generation. + Export/Import Device data ~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -3063,7 +3105,7 @@ while all the other organizations will have all command types enabled. | **default**: | ``{'type': 'object', 'properties': {}}`` | +--------------+------------------------------------------+ -Allows specifying JSONSchema used for validating meta-data of `Device Group <#device-groups>`_. +Allows specifying JSONSchema used for validating meta-data of `Device Group <#device-groups>`__. ``OPENWISP_CONTROLLER_SHARED_MANAGEMENT_IP_ADDRESS_SPACE`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index 791e1c843..df3f6e4af 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -26,10 +26,7 @@ from openwisp_ipam.filters import SubnetFilter from swapper import load_model -from openwisp_controller.config.views import ( - get_relevant_templates, - get_template_default_values, -) +from openwisp_controller.config.views import get_default_values, get_relevant_templates from openwisp_users.admin import OrganizationAdmin from openwisp_users.multitenancy import MultitenantOrgFilter from openwisp_utils.admin import ( @@ -665,6 +662,7 @@ def _get_preview_instance(self, request): c.device.name = request.POST.get('name') c.device.mac_address = request.POST.get('mac_address') c.device.key = request.POST.get('key') + c.device.group_id = request.POST.get('group') or None if 'hardware_id' in request.POST: c.device.hardware_id = request.POST.get('hardware_id') return c @@ -677,9 +675,9 @@ def get_urls(self): name='get_relevant_templates', ), path( - 'get-template-default-values/', - self.admin_site.admin_view(get_template_default_values), - name='get_template_default_values', + 'get-default-values/', + self.admin_site.admin_view(get_default_values), + name='get_default_values', ), ] + super().get_urls() for inline in self.inlines + self.conditional_inlines: @@ -1002,14 +1000,7 @@ def save(self, *args, **kwargs): class Meta(BaseForm.Meta): model = DeviceGroup - widgets = {'meta_data': DeviceGroupJsonSchemaWidget} - labels = {'meta_data': _('Metadata')} - help_texts = { - 'meta_data': _( - 'Group meta data, use this field to store data which is related' - 'to this group and can be retrieved via the REST API.' - ) - } + widgets = {'meta_data': DeviceGroupJsonSchemaWidget, 'context': FlatJsonWidget} class DeviceGroupAdmin(MultitenantAdminMixin, BaseAdmin): @@ -1026,6 +1017,7 @@ class DeviceGroupAdmin(MultitenantAdminMixin, BaseAdmin): 'organization', 'description', 'templates', + 'context', 'meta_data', 'created', 'modified', diff --git a/openwisp_controller/config/api/serializers.py b/openwisp_controller/config/api/serializers.py index 9822f9289..701dfb943 100644 --- a/openwisp_controller/config/api/serializers.py +++ b/openwisp_controller/config/api/serializers.py @@ -304,6 +304,7 @@ def get_queryset(self): class DeviceGroupSerializer(BaseSerializer): + context = serializers.JSONField(required=False, initial={}) meta_data = serializers.JSONField(required=False, initial={}) templates = FilterGroupTemplates(many=True) _templates = None @@ -316,6 +317,7 @@ class Meta(BaseMeta): 'organization', 'description', 'templates', + 'context', 'meta_data', 'created', 'modified', diff --git a/openwisp_controller/config/base/config.py b/openwisp_controller/config/base/config.py index 23ad7b3a5..d3f149915 100644 --- a/openwisp_controller/config/base/config.py +++ b/openwisp_controller/config/base/config.py @@ -558,7 +558,7 @@ def _has_device(self): return hasattr(self, 'device') def get_vpn_context(self): - context = super().get_context() + context = {} for vpnclient in self.vpnclient_set.all().select_related('vpn', 'cert'): vpn = vpnclient.vpn vpn_id = vpn.pk.hex @@ -599,8 +599,11 @@ def get_context(self, system=False): additional context passed to netjsonconfig """ c = collections.OrderedDict() - extra = {} + # Add global variables + context = super().get_context() if self._has_device(): + # These pre-defined variables are needed at the start of OrderedDict. + # Hence, they are added separately. c.update( [ ('name', self.name), @@ -609,14 +612,20 @@ def get_context(self, system=False): ('key', self.key), ] ) - if self.context and not system: - extra.update(self.context) - extra.update(self.get_vpn_context()) - for func in self._config_context_functions: - extra.update(func(config=self)) - if app_settings.HARDWARE_ID_ENABLED and self._has_device(): - extra.update({'hardware_id': str(self.device.hardware_id)}) - c.update(sorted(extra.items())) + if self.device._get_group(): + # Add device group variables + context.update(self.device._get_group().get_context()) + # Add predefined variables + context.update(self.get_vpn_context()) + for func in self._config_context_functions: + context.update(func(config=self)) + if app_settings.HARDWARE_ID_ENABLED: + context.update({'hardware_id': str(self.device.hardware_id)}) + + if self.context and not system: + context.update(self.context) + + c.update(sorted(context.items())) return c def get_system_context(self): diff --git a/openwisp_controller/config/base/device.py b/openwisp_controller/config/base/device.py index 054b0ac53..cfe0ef7a1 100644 --- a/openwisp_controller/config/base/device.py +++ b/openwisp_controller/config/base/device.py @@ -127,6 +127,9 @@ def __str__(self): def _has_config(self): return hasattr(self, 'config') + def _has_group(self): + return hasattr(self, 'group') + def _get_config_attr(self, attr): """ gets property or calls method of related config object @@ -143,6 +146,11 @@ def _get_config(self): else: return self.get_config_model()(device=self) + def _get_group(self): + if self._has_group(): + return self.group + return self.get_group_model()(device=self) + def get_context(self): config = self._get_config() return config.get_context() diff --git a/openwisp_controller/config/base/device_group.py b/openwisp_controller/config/base/device_group.py index 5378a8265..95e8d3042 100644 --- a/openwisp_controller/config/base/device_group.py +++ b/openwisp_controller/config/base/device_group.py @@ -1,4 +1,5 @@ import collections +from copy import deepcopy import jsonschema from django.core.exceptions import ValidationError @@ -39,6 +40,22 @@ class AbstractDeviceGroup(OrgMixin, TimeStampedEditableModel): default=dict, load_kwargs={'object_pairs_hook': collections.OrderedDict}, dump_kwargs={'indent': 4}, + help_text=_( + 'Group meta data, use this field to store data which is related' + ' to this group and can be retrieved via the REST API.' + ), + verbose_name=_('Metadata'), + ) + context = JSONField( + blank=True, + default=dict, + load_kwargs={'object_pairs_hook': collections.OrderedDict}, + dump_kwargs={'indent': 4}, + help_text=_( + 'This field can be used to add meta data for the group' + ' or to add "Configuration Variables" to the devices.' + ), + verbose_name=_('Configuration Variables'), ) def __str__(self): @@ -58,6 +75,9 @@ def clean(self): except SchemaError as e: raise ValidationError({'input': e.message}) + def get_context(self): + return deepcopy(self.context) + @classmethod def templates_changed(cls, instance, old_templates, templates, *args, **kwargs): group_templates_changed.send( diff --git a/openwisp_controller/config/migrations/0036_device_group.py b/openwisp_controller/config/migrations/0036_device_group.py index 10c12ea2c..1bb00482d 100644 --- a/openwisp_controller/config/migrations/0036_device_group.py +++ b/openwisp_controller/config/migrations/0036_device_group.py @@ -62,6 +62,12 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={'ensure_ascii': False, 'indent': 4}, load_kwargs={'object_pairs_hook': collections.OrderedDict}, + help_text=( + 'Group meta data, use this field to store data which is' + ' related to this group and can be retrieved via the' + ' REST API.' + ), + verbose_name='Metadata', ), ), ( diff --git a/openwisp_controller/config/migrations/0049_devicegroup_context.py b/openwisp_controller/config/migrations/0049_devicegroup_context.py new file mode 100644 index 000000000..c54c41024 --- /dev/null +++ b/openwisp_controller/config/migrations/0049_devicegroup_context.py @@ -0,0 +1,31 @@ +# Generated by Django 3.2.19 on 2023-06-27 14:45 + +import collections + +import jsonfield.fields +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('config', '0048_wifi_radio_band_migration'), + ] + + operations = [ + migrations.AddField( + model_name='devicegroup', + name='context', + field=jsonfield.fields.JSONField( + blank=True, + default=dict, + dump_kwargs={'ensure_ascii': False, 'indent': 4}, + help_text=( + 'This field can be used to add meta data for the group' + ' or to add "Configuration Variables" to the devices.' + ), + load_kwargs={'object_pairs_hook': collections.OrderedDict}, + verbose_name='Configuration Variables', + ), + ), + ] diff --git a/openwisp_controller/config/static/config/js/widget.js b/openwisp_controller/config/static/config/js/widget.js index e8d647021..66fd155a2 100644 --- a/openwisp_controller/config/static/config/js/widget.js +++ b/openwisp_controller/config/static/config/js/widget.js @@ -4,7 +4,7 @@ django._jsonEditors = new Map(); var inFullScreenMode = false, prevDefaultValues = {}, - defaultValuesUrl = window.location.origin + '/admin/config/device/get-template-default-values/', + defaultValuesUrl = window.location.origin + '/admin/config/device/get-default-values/', removeDefaultValues = function(contextValue, defaultValues) { // remove default values when template is removed. Object.keys(prevDefaultValues).forEach(function (key) { @@ -32,7 +32,20 @@ systemContextValue = JSON.parse(systemContextField.text()); // add default values to contextValue Object.keys(defaultValues).forEach(function (key) { - if (!contextValue.hasOwnProperty(key) && !systemContextValue.hasOwnProperty(key)) { + if ( + // Handles the case when different templates and group contains the keys. + // If the contextValue was set by a template or group, then + // override the value. + (prevDefaultValues.hasOwnProperty(key) && prevDefaultValues[key] !== defaultValues[key]) || + // Gives precedence to device's context (saved in database) + (!contextValue.hasOwnProperty(key) && + // Gives precedence to systemContextValue. + // But if the default value is equal to the system context value, + // then add the variable in the contextValue. This allows users + // to override the value. + (!systemContextValue.hasOwnProperty(key) || systemContextValue[key] === defaultValues[key]) + ) + ) { contextValue[key] = defaultValues[key]; } }); @@ -55,9 +68,14 @@ } }, getDefaultValues = function (isLoading=false) { - var pks = $('input[name="config-0-templates"]').attr('value'); - if (pks) { - $.get(defaultValuesUrl, {pks: pks}) + var templatePks = $('input[name="config-0-templates"]').attr('value'), + groupPk = $('#id_group').val(); + if (templatePks) { + var payload = {pks: templatePks}; + if (groupPk) { + payload.group = groupPk; + } + $.get(defaultValuesUrl, payload) .done( function (data) { updateContext(isLoading, data.default_values); }) @@ -430,6 +448,12 @@ $('.sortedm2m-items').on('change', function() { getDefaultValues(); }); + $('.sortedm2m-items').on('sortstop', function() { + getDefaultValues(); + }); + $('#id_group').on('change', function() { + getDefaultValues(); + }); }); // deletes maxLength on ip address schema if address contains variable diff --git a/openwisp_controller/config/tests/test_admin.py b/openwisp_controller/config/tests/test_admin.py index be3e0aaaf..e50e143a3 100644 --- a/openwisp_controller/config/tests/test_admin.py +++ b/openwisp_controller/config/tests/test_admin.py @@ -2,6 +2,7 @@ import json import os from unittest.mock import patch +from uuid import uuid4 from django.contrib.admin.models import LogEntry from django.contrib.auth import get_user_model @@ -1426,9 +1427,12 @@ def test_clone_template(self): self.assertIn('test-template (Clone)', str(response.content)) self.assertEqual(LogEntry.objects.all().count(), 3) - def test_get_template_default_values(self): + def test_get_default_values(self): t1 = self._create_template(name='t1', default_values={'name1': 'test1'}) - path = reverse('admin:get_template_default_values') + group = self._create_device_group( + context={'name1': 'device group', 'name2': 'should not appear'} + ) + path = reverse('admin:get_default_values') with self.subTest('get default values for one template'): with self.assertNumQueries(3): @@ -1445,19 +1449,38 @@ def test_get_template_default_values(self): expected = {'default_values': {'name1': 'test1', 'name2': 'test2'}} self.assertEqual(r.json(), expected) - def test_get_template_default_values_invalid_pks(self): - path = reverse('admin:get_template_default_values') - expected = {'error': 'invalid template pks were received'} + with self.subTest('get default values conflicting with device group'): + with self.assertNumQueries(4): + response = self.client.get( + path, {'pks': f'{t1.pk}', 'group': str(group.pk)} + ) + self.assertEqual(response.status_code, 200) + expected = {'default_values': {'name1': 'device group'}} + response_data = response.json() + self.assertEqual(response_data, expected) + self.assertNotIn('name2', response_data) + + def test_get_default_values_invalid_pks(self): + path = reverse('admin:get_default_values') + expected = { + 'template': {'error': 'invalid template pks were received'}, + 'group': {'error': 'invalid group pk were received'}, + } - with self.subTest('test with invalid pk'): + with self.subTest('test with invalid template pk'): r = self.client.get(path, {'pks': 'invalid'}) self.assertEqual(r.status_code, 400) - self.assertEqual(r.json(), expected) + self.assertEqual(r.json(), expected['template']) with self.subTest('test with absent pk'): r = self.client.get(path) self.assertEqual(r.status_code, 400) - self.assertEqual(r.json(), expected) + self.assertEqual(r.json(), expected['template']) + + with self.subTest('test with invalid group pk'): + r = self.client.get(path, {'group': 'invalid', 'pks': str(uuid4())}) + self.assertEqual(r.status_code, 400) + self.assertEqual(r.json(), expected['group']) def _test_system_context_field_helper(self, path): r = self.client.get(path) @@ -1496,16 +1519,15 @@ def test_system_context(self): path = reverse(f'admin:{self.app_label}_device_change', args=[d.pk]) self._test_system_context_field_helper(path) - def test_no_system_context(self): + @patch.dict(app_settings.CONTEXT, {}, clear=True) + def test_no_system_context(self, *args): self._create_template() - old_context = app_settings.CONTEXT.copy() - app_settings.CONTEXT = {} path = reverse(f'admin:{self.app_label}_template_add') r = self.client.get(path) + print(app_settings.CONTEXT) self.assertContains( r, 'There are no system defined variables available right now' ) - app_settings.CONTEXT = old_context def test_config_form_old_templates(self): config = self._create_config(organization=self._get_org()) diff --git a/openwisp_controller/config/tests/test_api.py b/openwisp_controller/config/tests/test_api.py index 139a0b462..a246775ec 100644 --- a/openwisp_controller/config/tests/test_api.py +++ b/openwisp_controller/config/tests/test_api.py @@ -81,6 +81,7 @@ class ApiTestMixin: 'description': 'Group for APs of default organization', 'organization': 'None', 'meta_data': {'captive_portal_url': 'https://example.com'}, + 'context': {'SSID': 'OpenWISP'}, 'templates': [], } @@ -900,6 +901,7 @@ def test_devicegroup_create_api(self): self.assertEqual(response.data['name'], data['name']) self.assertEqual(response.data['description'], data['description']) self.assertEqual(response.data['meta_data'], data['meta_data']) + self.assertEqual(response.data['context'], data['context']) self.assertEqual(response.data['organization'], org.pk) self.assertEqual(response.data['templates'], [template.pk]) @@ -936,7 +938,7 @@ def _assert_devicegroup_list_filter(response=None, device_group=None): self.assertEqual(response.status_code, 200) data = response.data self.assertEqual(data['count'], 1) - self.assertEqual(len(data['results'][0]), 8) + self.assertEqual(len(data['results'][0]), 9) self.assertEqual(data['results'][0]['id'], str(device_group.pk)) self.assertEqual(data['results'][0]['name'], str(device_group.name)) self.assertEqual( @@ -972,6 +974,7 @@ def test_devicegroup_detail_api(self): self.assertEqual(response.data['name'], device_group.name) self.assertEqual(response.data['description'], device_group.description) self.assertDictEqual(response.data['meta_data'], device_group.meta_data) + self.assertDictEqual(response.data['context'], device_group.context) self.assertEqual( response.data['organization'], device_group.organization.pk ) @@ -1013,6 +1016,7 @@ def test_devicegroup_commonname(self): self.assertEqual(response.data['name'], device_group.name) self.assertEqual(response.data['description'], device_group.description) self.assertDictEqual(response.data['meta_data'], device_group.meta_data) + self.assertDictEqual(response.data['context'], device_group.context) self.assertEqual( response.data['organization'], device_group.organization.pk ) @@ -1026,6 +1030,7 @@ def test_devicegroup_commonname(self): self.assertEqual(response.data['name'], device_group.name) self.assertEqual(response.data['description'], device_group.description) self.assertDictEqual(response.data['meta_data'], device_group.meta_data) + self.assertDictEqual(response.data['context'], device_group.context) self.assertEqual( response.data['organization'], device_group.organization.pk ) diff --git a/openwisp_controller/config/tests/test_device.py b/openwisp_controller/config/tests/test_device.py index d07cd931a..5b5222c5b 100644 --- a/openwisp_controller/config/tests/test_device.py +++ b/openwisp_controller/config/tests/test_device.py @@ -310,12 +310,51 @@ def test_change_org(self): self.assertEqual(config.device.organization_id, org2.pk) def test_device_get_system_context(self): - d = self._create_device(organization=self._get_org()) + org = self._get_org() + device_group = self._create_device_group( + organization=org, context={'SSID': 'OpenWISP'} + ) + d = self._create_device(organization=org, group=device_group) self._create_config(context={'test': 'name'}, device=d) d.refresh_from_db() system_context = d.get_system_context() + self.assertEqual(system_context['SSID'], 'OpenWISP') self.assertNotIn('test', system_context.keys()) + @mock.patch.dict(app_settings.CONTEXT, {'variable_type': 'global-context-variable'}) + def test_configuration_variable_priority(self, *args): + org = self._get_org() + device_group = self._create_device_group( + organization=org, context={'variable_type': 'device-group-variable'} + ) + device = self._create_device(organization=org, group=device_group) + config = self._create_config( + device=device, + context={ + 'id': 'user-defined-variable', + 'variable_type': 'user-defined-variable', + }, + ) + + # User defined variable has highest precedence + self.assertEqual(config.get_context()['id'], 'user-defined-variable') + self.assertEqual(config.get_context()['variable_type'], 'user-defined-variable') + # Second precedence is given to predefined device variables + config.context = {} + self.assertEqual( + config.get_context()['id'], + str(device.pk), + ) + # It is not possible to overwrite the pre-defined device variables. + # Therefore, for further tests, "tesT" variable is used. + # Third precedence is given to the device group variable' + self.assertEqual(config.get_context()['variable_type'], 'device-group-variable') + # Fourth precedence is given to global variables + device.group = None + self.assertEqual( + config.get_context()['variable_type'], 'global-context-variable' + ) + def test_management_ip_changed_not_emitted_on_creation(self): with catch_signal(management_ip_changed) as handler: self._create_device(organization=self._get_org()) diff --git a/openwisp_controller/config/tests/test_views.py b/openwisp_controller/config/tests/test_views.py index 14b7d0915..e79271f39 100644 --- a/openwisp_controller/config/tests/test_views.py +++ b/openwisp_controller/config/tests/test_views.py @@ -6,14 +6,18 @@ from openwisp_users.tests.utils import TestOrganizationMixin from ...tests.utils import TestAdminMixin -from .utils import CreateConfigTemplateMixin +from .utils import CreateConfigTemplateMixin, CreateDeviceGroupMixin Template = load_model('config', 'Template') User = get_user_model() class TestViews( - CreateConfigTemplateMixin, TestAdminMixin, TestOrganizationMixin, TestCase + CreateConfigTemplateMixin, + CreateDeviceGroupMixin, + TestAdminMixin, + TestOrganizationMixin, + TestCase, ): """ tests for config.views @@ -241,7 +245,7 @@ def test_get_relevant_templates_400(self): ) self.assertEqual(response.status_code, 404) - def get_template_default_values_authorization(self): + def test_get_default_values_authorization(self): org1 = self._get_org() org1_template = self._create_template( organization=org1, default_values={'org1': 'secret1'} @@ -253,8 +257,11 @@ def get_template_default_values_authorization(self): shared_template = self._create_template( name='shared-template', default_values={'key': 'value'} ) + org2_device_group = self._create_device_group( + organization=org2, context={'key': 'org2 device group'} + ) url = ( - reverse('admin:get_template_default_values') + reverse('admin:get_default_values') + f'?pks={org1_template.pk},{org2_template.pk},{shared_template.pk}' ) @@ -281,8 +288,9 @@ def get_template_default_values_authorization(self): org1_user.is_staff = True org1_user.save() self.client.force_login(org1_user) + response = self.client.get(url + f'&group={org2_device_group.pk}') + # Do not override default value from device group of another org expected_response = {'default_values': {'org1': 'secret1', 'key': 'value'}} - response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertJSONEqual(response.content, expected_response) @@ -295,7 +303,7 @@ def get_template_default_values_authorization(self): self.assertEqual(response.status_code, 200) self.assertJSONEqual(response.content, expected_response) - def get_template_default_values_same_keys(self): + def test_get_default_values_same_keys(self): self._login() # Atleast 4 templates are required to create enough entropy in database # to make the test fail consistently without patch @@ -312,7 +320,7 @@ def get_template_default_values_same_keys(self): template5 = self._create_template( name='VNI 5', default_values={'vn1': '5', 'vn2': '50', 'vn3': '500'} ) - url = reverse('admin:get_template_default_values') + url = reverse('admin:get_default_values') templates = [template5, template4, template3, template2, template1] template_pks = ','.join([str(template.pk) for template in templates]) response = self.client.get(url, {'pks': template_pks}) diff --git a/openwisp_controller/config/tests/utils.py b/openwisp_controller/config/tests/utils.py index 581e049f0..d09b5cc33 100644 --- a/openwisp_controller/config/tests/utils.py +++ b/openwisp_controller/config/tests/utils.py @@ -233,7 +233,7 @@ def _create_device_group(self, **kwargs): options = { 'name': 'Routers', 'description': 'Group for all routers', - 'meta_data': {}, + 'context': {}, } options.update(kwargs) if 'organization' not in options: diff --git a/openwisp_controller/config/views.py b/openwisp_controller/config/views.py index e0cf25c11..a98cd8734 100644 --- a/openwisp_controller/config/views.py +++ b/openwisp_controller/config/views.py @@ -15,6 +15,7 @@ Organization = load_model('openwisp_users', 'Organization') Template = load_model('config', 'Template') +DeviceGroup = load_model('config', 'DeviceGroup') def get_relevant_templates(request, organization_id): @@ -92,36 +93,73 @@ def schema(request): return HttpResponse(c, status=status, content_type='application/json') -def get_template_default_values(request): +def get_default_values(request): """ - returns default_values for one or more templates + The view returns default values from the templates + and group specified in the URL's query parameters. + + URL query parameters: + pks (required): Comma separated primary keys of Template + group (optional): Primary key of the DeviceGroup + + The conflicting keys between the templates' default_values + and DeviceGroup's context are overridden by the value present + in DeviceGroup's context, i.e. DeviceGroup takes precedence. + + NOTE: Not all key-value pair of DeviceGroup.context are added + in the response. Only the conflicting keys are altered. """ + + def _clean_pk(pks): + pk_list = [] + for pk in pks: + UUID(pk, version=4) + pk_list.append(pk) + return pk_list + user = request.user - pk_list = [] - for pk in request.GET.get('pks', '').split(','): + try: + templates_pk_list = _clean_pk(request.GET.get('pks', '').split(',')) + except ValueError: + return JsonResponse({'error': 'invalid template pks were received'}, status=400) + group_pk = request.GET.get('group', None) + if group_pk: try: - UUID(pk, version=4) + group_pk = _clean_pk([group_pk])[0] except ValueError: - return JsonResponse( - {'error': 'invalid template pks were received'}, status=400 - ) + return JsonResponse({'error': 'invalid group pk were received'}, status=400) else: - pk_list.append(pk) - where = Q(pk__in=pk_list) + group_where = Q(pk=group_pk) + if not request.user.is_superuser: + group_where &= Q(organization__in=user.organizations_managed) + + templates_where = Q(pk__in=templates_pk_list) if not user.is_superuser: - where = where & ( + templates_where = templates_where & ( Q(organization=None) | Q(organization__in=user.organizations_managed) ) - qs = Template.objects.filter(where).values('id', 'default_values') - qs_dict = {} - # Create a mapping of UUID to default values of the templates in qs_dict. - # Iterate over received pk_list and retrieve default_values for corresponding - # template from qs_dict. + templates_qs = Template.objects.filter(templates_where).values( + 'id', 'default_values' + ) + templates_qs_dict = {} + # Create a mapping of UUID to default values of the templates in templates_ + # qs_dict. Iterate over received templates_pk_list and retrieve default_values for + # corresponding template from templates_qs_dict. # This ensures that default_values of templates that come later in the order # will override default_values of any previous template if same keys are present. - for template in qs: - qs_dict[str(template['id'])] = template['default_values'] + for template in templates_qs: + templates_qs_dict[str(template['id'])] = template['default_values'] default_values = {} - for pk in pk_list: - default_values.update(qs_dict[pk]) + for pk in templates_pk_list: + default_values.update(templates_qs_dict.get(pk, {})) + # Check for conflicting key's in DeviceGroup.context + if group_pk: + try: + group = DeviceGroup.objects.only('context').get(group_where) + except DeviceGroup.DoesNotExist: + pass + else: + for key, value in group.get_context().items(): + if key in default_values: + default_values[key] = value return JsonResponse({'default_values': default_values}) diff --git a/tests/openwisp2/sample_config/migrations/0001_initial.py b/tests/openwisp2/sample_config/migrations/0001_initial.py index 8bb0ea91d..77ca53f35 100644 --- a/tests/openwisp2/sample_config/migrations/0001_initial.py +++ b/tests/openwisp2/sample_config/migrations/0001_initial.py @@ -734,6 +734,26 @@ class Migration(migrations.Migration): default=dict, dump_kwargs={'ensure_ascii': False, 'indent': 4}, load_kwargs={'object_pairs_hook': collections.OrderedDict}, + help_text=( + 'Group meta data, use this field to store data which is' + ' related to this group and can be retrieved via the' + ' REST API.' + ), + verbose_name='Metadata', + ), + ), + ( + 'context', + jsonfield.fields.JSONField( + blank=True, + default=dict, + dump_kwargs={'ensure_ascii': False, 'indent': 4}, + help_text=( + 'This field can be used to add meta data for the group' + ' or to add "Configuration Variables" to the devices.' + ), + load_kwargs={'object_pairs_hook': collections.OrderedDict}, + verbose_name='Configuration Variables', ), ), (