Release v1.1.1

Add a name_slug field to Circuit, make it the natural key (#259)

This new `name_slug` field is a URL-friendly version of the existing
`name` field, which will allow us to have a friendly natural key without
having to worry about conflicts with things such as slashes in the URL.

The slugification is done by our own function rather than Django's
built-in `slugify()`. The built-in is too agressive and would cause
conflicts with certain interface names. For example, since it simply
drops slashes, the two following interfaces would slugify out to be the
same thing, causing a uniqueness conflict:

  * `Ethernet1/2/3`
  * `Ethernet12/3`

Our slugify function simply replaces `/` with `_`, which is the bare
minimum to make these names URL-friendly.

The purpose of this is the same as #258, where this fixes a
bug that keeps clients from working with circuits by their name if the
name contains a slash.

CHANGELOG.rst

@@ -5,6 +5,14 @@ Changelog
 Version History
 ===============
 
+.. _v1.1.1:
+
+1.1 (2017-01-27)
+----------------
+
+* Add `name_slug` field to Circuit, make it the natural key to fix a bug with
+  Circuit names that contain slashes
+
 .. _v1.1:
 
 1.1 (2017-01-23)

docker/Dockerfile

@@ -37,7 +37,7 @@ RUN pip install psycopg2
 
 # Try to run this as late as possible for layer caching - this version will be
 # updated every update so let the build not take longer than necessary
-RUN pip install nsot==1.1
+RUN pip install nsot==1.1.1
 COPY conf /etc/nsot
 
 ENTRYPOINT ["nsot-server", "--config=/etc/nsot/nsot.conf.py"]

nsot/api/views.py

@@ -706,7 +706,7 @@ class CircuitViewSet(ResourceViewSet):
     queryset = models.Circuit.objects.all()
     serializer_class = serializers.CircuitSerializer
     filter_class = filters.CircuitFilter
-    natural_key = 'name'
+    natural_key = 'name_slug'
 
     # TODO(jathan): Revisit this if and when we upgrade or replace
     # django-rest-framework-bulk==0.2.1

nsot/migrations/0030_add_circuit_name_slug.py

@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('nsot', '0029_auto__add_circuit'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='circuit',
+            name='name_slug',
+            field=models.CharField(db_index=True, editable=False, max_length=255, help_text='Slugified version of the name field, used for the natural key', null=True, unique=True),
+        ),
+    ]

nsot/migrations/0031_populate_circuit_name_slug.py

@@ -0,0 +1,34 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+from nsot.util import slugify
+
+
+def add_name_slug(apps, schema_editor):
+    """ Add a name_slug for every Circuit that doesn't already have one """
+
+    Circuit = apps.get_model('nsot', 'Circuit')
+    for c in Circuit.objects.all():
+        if not c.name_slug:
+            c.name_slug = slugify(c.name)
+            c.save()
+
+
+def remove_name_slug(apps, schema_editor):
+    Circuit = apps.get_model('nsot', 'Circuit')
+    for c in Circuit.objects.all():
+        c.name_slug = None
+        c.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('nsot', '0030_add_circuit_name_slug'),
+    ]
+
+    operations = [
+        migrations.RunPython(add_name_slug, remove_name_slug)
+    ]

nsot/models.py

@@ -20,7 +20,8 @@
 from . import exc
 from . import fields
 from . import validators
-from .util import cidr_to_dict, generate_secret_key, parse_set_query, stats
+from .util import (cidr_to_dict, generate_secret_key, parse_set_query, slugify,
+                   stats)
 
 
 log = logging.getLogger(__name__)
@@ -1547,6 +1548,19 @@ class Circuit(Resource):
         )
     )
 
+    # This doesn't use the built-in SlugField since we're doing our own
+    # slugification (django.utils.text.slugify() is too agressive)
+    name_slug = models.CharField(
+        db_index=True,
+        editable=False,
+        max_length=255,
+        null=True,
+        unique=True,
+        help_text=(
+            'Slugified version of the name field, used for the natural key'
+        )
+    )
+
     def __unicode__(self):
         return u'%s' % self.name
 
@@ -1558,7 +1572,7 @@ class Meta:
         '''
         index_together = [
             ('endpoint_a', 'endpoint_z'),
-            ('site', 'name'),
+            ('site', 'name_slug'),
         ]
         '''
 
@@ -1616,6 +1630,8 @@ def clean_fields(self, exclude=None):
         self.endpoint_z = self.clean_endpoint_z(self.endpoint_z_id)
         self.name = self.clean_name(self.name)
 
+        self.name_slug = slugify(self.name)
+
     def save(self, *args, **kwargs):
         self.full_clean()
         super(Circuit, self).save(*args, **kwargs)
@@ -1624,6 +1640,7 @@ def to_dict(self):
         return {
             'id': self.id,
             'name': self.name,
+            'name_slug': self.name_slug,
             'endpoint_a': self.endpoint_a_id,
             'endpoint_z': self.endpoint_z_id,
             'attributes': self.get_attributes(),

nsot/util/core.py

@@ -21,7 +21,7 @@
 __all__ = (
     'qpbool', 'normalize_auth_header', 'generate_secret_key', 'get_field_attr',
     'SetQuery', 'parse_set_query', 'generate_settings', 'initialize_app',
-    'main', 'cidr_to_dict'
+    'main', 'cidr_to_dict', 'slugify'
 )
 
 
@@ -90,6 +90,32 @@ def cidr_to_dict(cidr):
     }
 
 
+def slugify(s):
+    """
+    Slugify a string.
+
+    This works in a less-agressive manner than Django's slugify, which simply
+    drops most drops most non-alphanumeric characters and lowercases the entire
+    string. It would likely to cause uniqueness conflicts for things like
+    interface names, such as Eth1/2/3 and Eth12/3, which would slugify to be
+    the same.
+
+    >>> slugify('switch-foo01:Ethernet1/2')
+    'switch-foo01:Ethernet1_2'
+
+    :param s:
+        String to slugify
+    """
+
+    disallowed_chars = ['/']
+    replacement = '_'
+
+    for char in disallowed_chars:
+        s = s.replace(char, replacement)
+
+    return s
+
+
 #: Namedtuple for resultant items from ``parse_set_query()``
 SetQuery = collections.namedtuple('SetQuery', 'action name value')
 

nsot/version.py

@@ -1 +1 @@
-__version__ = '1.1'
+__version__ = '1.1.1'

tests/api_tests/test_circuits.py

@@ -147,6 +147,7 @@ def test_bulk_operations(site, client):
     updated = copy.deepcopy(expected)
     for item in updated:
         item['name'] = item['name'].replace('foo', 'wtf')
+        item['name_slug'] = item['name_slug'].replace('foo', 'wtf')
     updated_resp = client.put(cir_uri, data=json.dumps(updated))
     expected = updated_resp.json()
 
@@ -205,6 +206,9 @@ def test_update(site, client):
     params['name'] = 'foo-bar1:eth0_None'
     payload.update(params)
 
+    # Read-only computed parameters
+    payload['name_slug'] = 'foo-bar1:eth0_None'
+
     assert_success(
         client.update(cir_obj_uri, **params),
         payload
@@ -289,6 +293,8 @@ def test_partial_update(site, client):
     # Update only name
     payload = copy.deepcopy(cir)
     payload.update(params)
+    payload['name_slug'] = 'mycircuit'
+
     assert_success(
         client.partial_update(cir_obj_uri, **params),
         payload

tests/api_tests/test_util.py

@@ -7,7 +7,7 @@
 import pytest
 
 from nsot.util import SetQuery, parse_set_query
-from nsot.util import stats
+from nsot.util import slugify, stats
 
 
 def test_parse_set_query():
@@ -111,3 +111,18 @@ def test_stats_get_utilization(parent=PARENT, hosts=HOSTS):
     output = stats.calculate_network_utilization(parent, hosts, as_string=True)
 
     assert output == expected
+
+
+def test_slugify():
+    cases = [
+        ('/', '_'),
+        ('my cool string', 'my cool string'),
+        ('Ethernet1/2', 'Ethernet1_2'),
+        (
+            'foo-bar1:xe-0/0/0.0_foo-bar2:xe-0/0/0.0',
+            'foo-bar1:xe-0_0_0.0_foo-bar2:xe-0_0_0.0'
+        ),
+    ]
+
+    for case, expected in cases:
+        assert slugify(case) == expected

tests/model_tests/test_circuits.py

@@ -29,15 +29,15 @@ def test_creation(device):
     # A-side device/interface
     device_a = device
     iface_a = models.Interface.objects.create(
-        device=device_a, name='eth0', addresses=['10.32.0.1/32']
+        device=device_a, name='eth0/1', addresses=['10.32.0.1/32']
     )
 
     # Z-side device/interface
     device_z = models.Device.objects.create(
         hostname='foo-bar2', site=site
     )
     iface_z = models.Interface.objects.create(
-        device=device_z, name='eth0', addresses=['10.32.0.2/32']
+        device=device_z, name='eth0/1', addresses=['10.32.0.2/32']
     )
 
     # Create the circuit
@@ -55,6 +55,9 @@ def test_creation(device):
     )
     assert circuit.name == expected_name
 
+    # Name slug should be the slugified version of the name
+    assert circuit.name_slug == expected_name.replace('/', '_')
+
     # Assert property values
     assert circuit.interfaces == [iface_a, iface_z]
     assert [str(a) for a in circuit.addresses] == ['10.32.0.1/32', '10.32.0.2/32']
@@ -108,4 +111,3 @@ def test_attributes(circuit):
 
     with pytest.raises(exc.ValidationError):
         circuit.set_attributes({'made_up': 'value'})
-