Merge pull request #1658 from mboersma/1557-validate-ps-scale

fix(controller): validate app structure values

Author: mboersma

controller/api/models.py

@@ -61,6 +61,16 @@ def validate_comma_separated(value):
             "{} should be a comma-separated list".format(value))
 
 
+def validate_app_structure(value):
+    """Error if the dict values aren't ints >= 0."""
+    try:
+        for k, v in value.iteritems():
+            if int(v) < 0:
+                raise ValueError("Must be greater than or equal to zero")
+    except ValueError, err:
+        raise ValidationError(err)
+
+
 class AuditedModel(models.Model):
     """Add created and updated fields to a model."""
 
@@ -134,7 +144,7 @@ class App(UuidAuditedModel):
     owner = models.ForeignKey(settings.AUTH_USER_MODEL)
     id = models.SlugField(max_length=64, unique=True)
     cluster = models.ForeignKey('Cluster')
-    structure = JSONField(default={}, blank=True)
+    structure = JSONField(default={}, blank=True, validators=[validate_app_structure])
 
     class Meta:
         permissions = (('use_app', 'Can use app'),)

controller/api/serializers.py

@@ -184,6 +184,14 @@ def validate_id(self, attrs, source):
             raise serializers.ValidationError("App IDs cannot be 'deis'")
         return attrs
 
+    def validate_structure(self, attrs, source):
+        """
+        Check that the structure JSON dict has non-negative ints as its values.
+        """
+        value = attrs[source]
+        models.validate_app_structure(value)
+        return attrs
+
 
 class ContainerSerializer(serializers.ModelSerializer):
     """Serialize a :class:`~api.models.Container` model."""

controller/api/tests/test_container.py

@@ -362,3 +362,41 @@ def test_container_command_format(self):
         uuid = response.data['results'][0]['uuid']
         container = Container.objects.get(uuid=uuid)
         self.assertNotIn('{c_type}', container._command)
+
+    def test_container_scale_errors(self):
+        url = '/api/apps'
+        body = {'cluster': 'autotest'}
+        response = self.client.post(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 201)
+        app_id = response.data['id']
+        # should start with zero
+        url = "/api/apps/{app_id}/containers".format(**locals())
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data['results']), 0)
+        # post a new build
+        url = "/api/apps/{app_id}/builds".format(**locals())
+        body = {'image': 'autotest/example', 'sha': 'a'*40,
+                'procfile': json.dumps({'web': 'node server.js', 'worker': 'node worker.js'})}
+        response = self.client.post(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 201)
+        # scale to a negative number
+        url = "/api/apps/{app_id}/scale".format(**locals())
+        body = {'web': -1}
+        response = self.client.post(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 400)
+        # scale to something other than a number
+        url = "/api/apps/{app_id}/scale".format(**locals())
+        body = {'web': 'one'}
+        response = self.client.post(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 400)
+        # scale to something other than a number
+        url = "/api/apps/{app_id}/scale".format(**locals())
+        body = {'web': [1]}
+        response = self.client.post(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 400)
+        # scale up to an integer as a sanity check
+        url = "/api/apps/{app_id}/scale".format(**locals())
+        body = {'web': 1}
+        response = self.client.post(url, json.dumps(body), content_type='application/json')
+        self.assertEqual(response.status_code, 204)

controller/api/views.py

@@ -8,6 +8,7 @@
 
 from django.contrib.auth.models import AnonymousUser
 from django.contrib.auth.models import User
+from django.core.exceptions import ValidationError
 from django.http import Http404
 from django.utils import timezone
 from guardian.shortcuts import assign_perm
@@ -282,14 +283,15 @@ def scale(self, request, **kwargs):
         try:
             for target, count in request.DATA.items():
                 new_structure[target] = int(count)
-        except ValueError:
+        except (TypeError, ValueError):
             return Response('Invalid scaling format',
                             status=status.HTTP_400_BAD_REQUEST)
         app = self.get_object()
         try:
+            models.validate_app_structure(new_structure)
             app.structure = new_structure
             app.scale()
-        except EnvironmentError as e:
+        except (EnvironmentError, ValidationError) as e:
             return Response(str(e), status=status.HTTP_400_BAD_REQUEST)
         return Response(status=status.HTTP_204_NO_CONTENT,
                         content_type='application/json')