Refactored DictField + Added value size limit to metadata field (#321)

* Refactored DictField + Added value size limit to metadata field Closes #321 * Removed obsolete string conversion in metadata and array fields * Changed metadata field limit to 2 MB
erigones · Dec 18, 2017 · 84b0e1c · 84b0e1c
1 parent af5581a
commit 84b0e1c
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 37 deletions.
diff --git a/api/dc/base/serializers.py b/api/dc/base/serializers.py
@@ -303,7 +303,7 @@ class DcSettingsSerializer(s.InstanceSerializer):
     VMS_VM_SSH_KEYS_DEFAULT = s.ArrayField(label='VMS_VM_SSH_KEYS_DEFAULT', max_items=32, required=False,
                                            help_text=_('List of public SSH keys added to every virtual machine '
                                                        'in this virtual datacenter.'))
-    VMS_VM_MDATA_DEFAULT = s.MetadataField(label='VMS_VM_MDATA_DEFAULT', max_items=32, required=False,
+    VMS_VM_MDATA_DEFAULT = s.MetadataField(label='VMS_VM_MDATA_DEFAULT', required=False,
                                            validators=(validate_mdata(Vm.RESERVED_MDATA_KEYS),),
                                            help_text=_('Default VM metadata (key=value string pairs).'))
     VMS_DISK_MODEL_DEFAULT = s.ChoiceField(label='VMS_DISK_MODEL_DEFAULT', choices=Vm.DISK_MODEL,

diff --git a/api/fields.py b/api/fields.py
@@ -51,6 +51,7 @@
 from django.utils.translation import ugettext_lazy as _
 from django.utils.dateparse import parse_date, parse_datetime, parse_time
 from taggit.utils import parse_tags, split_strip
+from frozendict import frozendict
 
 from api import settings as api_settings
 from api.settings import ISO_8601
@@ -1378,12 +1379,18 @@ class BaseDictField(WritableField):
     """
     Dictionary - object.
     """
-    _key_field = NotImplemented
-    _val_field = NotImplemented
+    _key_field_class = NotImplemented
+    _key_field_params = frozendict()
+    _val_field_class = NotImplemented
+    _val_field_params = frozendict()
+    _default_max_size = None
 
     def __init__(self, *args, **kwargs):
         self.max_items = kwargs.pop('max_items', None)
+        self.max_size = kwargs.pop('max_size', self._default_max_size)  # bytes
         super(BaseDictField, self).__init__(*args, **kwargs)
+        self._key_field = self._key_field_class(**self._key_field_params)
+        self._val_field = self._val_field_class(**self._val_field_params)
 
     def to_native(self, value):
         ret = {}
@@ -1423,6 +1430,12 @@ def validate(self, value):
                 raise ValidationError(_('Ensure this value has at most %(max)d items (it has %(count)d).') %
                                       {'max': self.max_items, 'count': count})
 
+        if self.max_size:
+            size = sum(len(val) for key, val in items)
+            if size > self.max_size:
+                raise ValidationError(_('Ensure this value has maximum size of %(max)d bytes '
+                                        '(it has %(size)d bytes).') % {'max': self.max_size, 'size': size})
+
         for k, v in items:
             self._key_field.validate(k)
             self._val_field.validate(v)
@@ -1441,18 +1454,21 @@ class RoutesField(BaseDictField):
          "10.3.0.1": "nics[1]"
      }
     """
-    _key_field = CIDRField()
-    _val_field = IPNICField()
+    _key_field_class = CIDRField
+    _val_field_class = IPNICField
 
 
 class MetadataField(BaseDictField):
-    _key_field = RegexField(r'^[A-Za-z0-9\.\-_:]+$', max_length=128)
-    _val_field = CharField(max_length=65536)
+    _key_field_class = RegexField
+    _key_field_params = frozendict(regex=r'^[A-Za-z0-9\.\-_:]+$', max_length=128)
+    _val_field_class = CharField
+    _default_max_size = 1024 * 1024 * 2  # 2 MB (mentioned in user guide)
 
 
 class URLDictField(BaseDictField):
-    _key_field = RegexField(r'^[a-z0-9][a-z0-9\.\-_]*$', max_length=32)
-    _val_field = URLField()
+    _key_field_class = RegexField
+    _key_field_params = frozendict(regex=r'^[a-z0-9][a-z0-9\.\-_]*$', max_length=32)
+    _val_field_class = URLField
 
 
 class SafeCharField(RegexField):

diff --git a/api/vm/define/serializers.py b/api/vm/define/serializers.py
@@ -100,7 +100,7 @@ class VmDefineSerializer(VmBaseSerializer):
     maintain_resolvers = s.BooleanField(default=True)  # OS only
     routes = s.RoutesField(default={})  # OS only
     vga = s.ChoiceField(choices=Vm.VGA_MODEL, default=settings.VMS_VGA_MODEL_DEFAULT)  # KVM only
-    mdata = s.MetadataField(max_items=32, default=settings.VMS_VM_MDATA_DEFAULT,
+    mdata = s.MetadataField(default=settings.VMS_VM_MDATA_DEFAULT,
                             validators=(validate_mdata(Vm.RESERVED_MDATA_KEYS),))
     locked = s.BooleanField(read_only=True, required=False)
     created = s.DateTimeField(read_only=True, required=False)

diff --git a/api/vm/migrate/serializers.py b/api/vm/migrate/serializers.py
@@ -1,5 +1,6 @@
 from django.utils.translation import ugettext_lazy as _
 from django.core.exceptions import ObjectDoesNotExist
+from frozendict import frozendict
 
 from api import serializers as s
 from api.serializers import APIValidationError
@@ -17,8 +18,10 @@
 
 
 class DiskPoolDictField(s.BaseDictField):
-    _key_field = s.IntegerField(min_value=DISK_ID_MIN + 1, max_value=DISK_ID_MAX + 1)
-    _val_field = s.CharField(max_length=64)
+    _key_field_class = s.IntegerField
+    _key_field_params = frozendict(min_value=DISK_ID_MIN + 1, max_value=DISK_ID_MAX + 1)
+    _val_field_class = s.CharField
+    _val_field_params = frozendict(max_length=64)
 
 
 class VmMigrateSerializer(s.Serializer):

diff --git a/doc/changelog.rst b/doc/changelog.rst
@@ -16,6 +16,7 @@ Features
 - Added automatic synchronization of overlay ARP files - `#286 <https://github.com/erigones/esdc-ce/issues/286>`__
 - Added script for automating platform upgrade - `#289 <https://github.com/erigones/esdc-ce/issues/289>`__
 - Added experimental support for live migration - `#306 <https://github.com/erigones/esdc-ce/issues/306>`__
+- Added value size limit to metadata - `#321 <https://github.com/erigones/esdc-ce/issues/321>`__
 
 Bugs
 ----

diff --git a/gui/static/gui/js/helpers.js b/gui/static/gui/js/helpers.js
@@ -35,27 +35,6 @@ var ERIGONES_SUPPORTED_BROWSER = (function() {
   );
 })();
 
-function repr(str) {
-  var x = JSON.stringify(str);
-
-  if (x.substring(0, 1) === '"' && x.substring(x.length-1) === '"') {
-    return x.slice(1, -1);
-  } else {
-    return x;
-  }
-}
-
-function load_repr(str) {
-  try {
-    if (!(str.substring(0, 1) === '"' && str.substring(str.length-1) === '"')) {
-      str = '"' + str + '"';
-    }
-    return JSON.parse(str);
-  } catch (e) {
-    return str;
-  }
-}
-
 function is_value_in_field(field, value) {
   return Boolean(field.find('option[value="'+ value +'"]').length);
 }
@@ -754,7 +733,7 @@ function mdata_display(input_field) {
     }
 
     $.each(value, function(key, val) {
-      row = $(row_template({'key': key, 'value': repr(val), 'sign': 'minus icon-link-enabled'}));
+      row = $(row_template({'key': key, 'value': val, 'sign': 'minus icon-link-enabled'}));
       input_group.append(row);
       input = row.children().get(1);
       input.scrollLeft = input.scrollWidth;
@@ -804,7 +783,7 @@ function mdata_handler(input_field) {
     $.each(rows, function() {
       var children = this.children;
       if (children[0].value || children[1].value) {
-        result[children[0].value] = load_repr(children[1].value);
+        result[children[0].value] = children[1].value;
       }
     });
 
@@ -867,7 +846,7 @@ function array_display(input_field) {
     }
 
     $.each(value, function(i, val) {
-      row = $(row_template({'value': repr(val), 'sign': 'minus icon-link-enabled'}));
+      row = $(row_template({'value': val, 'sign': 'minus icon-link-enabled'}));
       input_group.append(row);
       input = row.children().get(0);
       input.scrollLeft = input.scrollWidth;
@@ -912,7 +891,7 @@ function array_handler(input_field) {
     $.each(rows, function() {
       var children = this.children;
       if (children[0].value) {
-        result.push(load_repr(children[0].value));
+        result.push(children[0].value);
       }
     });