Merge pull request #1651 from mboersma/1552-validate-hosts

fix(controller): validate Cluster.hosts field as comma-separated

Author: mboersma

controller/api/models.py

@@ -9,11 +9,13 @@
 import importlib
 import logging
 import os
+import re
 import subprocess
 
 from celery.canvas import group
 from django.conf import settings
 from django.contrib.auth.models import User
+from django.core.exceptions import ValidationError
 from django.db import models, connections
 from django.db.models import Max
 from django.db.models.signals import post_delete
@@ -50,6 +52,15 @@ def _inner(*args, **kwargs):
     return _inner
 
 
+def validate_comma_separated(value):
+    """Error if the value doesn't look like a list of hostnames or IP addresses
+    separated by commas.
+    """
+    if not re.search(r'^[a-zA-Z0-9-,\.]+$', value):
+        raise ValidationError(
+            "{} should be a comma-separated list".format(value))
+
+
 class AuditedModel(models.Model):
     """Add created and updated fields to a model."""
 
@@ -86,7 +97,7 @@ class Cluster(UuidAuditedModel):
     type = models.CharField(max_length=16, choices=CLUSTER_TYPES, default='coreos')
 
     domain = models.CharField(max_length=128)
-    hosts = models.CharField(max_length=256)
+    hosts = models.CharField(max_length=256, validators=[validate_comma_separated])
     auth = models.TextField()
     options = JSONField(default={}, blank=True)
 

controller/api/serializers.py

@@ -68,6 +68,11 @@ class Meta:
         model = models.Cluster
         read_only_fields = ('created', 'updated')
 
+    def validate_hosts(self, attrs, source):
+        value = attrs[source]
+        models.validate_comma_separated(value)
+        return attrs
+
 
 class PushSerializer(serializers.ModelSerializer):
     """Serialize a :class:`~api.models.Push` model."""

controller/api/tests/test_cluster.py

@@ -30,7 +30,10 @@ def test_cluster(self):
         url = '/api/clusters'
         options = {'key': 'val'}
         body = {'id': 'autotest', 'domain': 'autotest.local', 'type': 'mock',
-                'hosts': 'host1,host2', 'auth': 'base64string', 'options': options}
+                'hosts': 'host1;host2', 'auth': 'base64string', 'options': options}
+        response = self.client.post(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 400)
+        body['hosts'] = 'host1,host2'
         response = self.client.post(url, json.dumps(body), content_type='application/json')
         self.assertEqual(response.status_code, 201)
         cluster_id = response.data['id']  # noqa
@@ -54,7 +57,15 @@ def test_cluster(self):
         url = '/api/clusters/{cluster_id}'.format(**locals())
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        new_hosts, new_options = 'host2,host3', {'key': 'val2'}
+        # regression test for https://github.com/deis/deis/issues/1552
+        body = {'hosts': 'host2 host3'}
+        response = self.client.patch(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 400)
+        body = {'hosts': 'host2;host3'}
+        response = self.client.patch(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 400)
+        # update cluster hosts
+        new_hosts, new_options = 'host2.domain,host3.sub.domain,127.0.0.1', {'key': 'val2'}
         body = {'hosts': new_hosts, 'options': new_options}
         response = self.client.patch(url, json.dumps(body), content_type='application/json')
         self.assertEqual(response.status_code, 200)