Merge pull request #2116 from bacongobbler/apiv1

API v1 fixes

Author: Matthew Fisher

builder/templates/builder

@@ -92,8 +92,8 @@ RESPONSE=$(curl -s -XPOST \
      --data "{\"receive_user\":\"$USER\",\"receive_repo\":\"$APP_NAME\"}" \
      $URL)
 
-# massage response for the environment variables in form HELLO=world
-CONFIG=$(echo $RESPONSE | python -c 'import json,sys;obj=json.load(sys.stdin);print obj["values"]' | jq -c -M "to_entries|map(\"\(.key)=\(.value|tostring)\")|.[]" 2> /dev/null || echo $RESPONSE)
+# massage response for the environment variables in the form "-e HELLO=world"
+CONFIG=$(echo $RESPONSE | jq -r ".values|to_entries|map(\"-e \(.key)=\(.value|tostring)\")|.[]" | sed -e 's/^"//' -e 's/"$//' 2> /dev/null || echo $RESPONSE)
 
 if [ "$CONFIG" == "$RESPONSE" ];
 then
@@ -102,32 +102,25 @@ then
     exit 1
 fi
 
-# build option string to send to slugbuilder
-BUILD_OPTS=""
-echo $CONFIG > tmp
-while read envvar; do
-    if [ "$envvar" != "" ]; then
-        BUILD_OPTS+="-e $envvar "
-    fi
-done < tmp
-rm tmp
-
 # if no Dockerfile is present, use slugbuilder to compile a heroku slug
 # and write out a Dockerfile to use that slug
 if [ ! -f Dockerfile ]; then
-    if [ -f /buildpacks ]; then
-        BUILD_OPTS+="-v /buildpacks:/tmp/buildpacks:rw "
-        # give non-root slubuilder user R/W perms for docker volumes
-        chown -R 2000:2000 /buildpacks
-    fi
+    # build option string to send to slugbuilder
+    BUILD_OPTS=("$CONFIG")
 
     # run in the background, we'll attach to it to retrieve logs
-    BUILD_OPTS+="-d "
-    BUILD_OPTS+="-v $TMP_DIR:/tmp/app "
-    BUILD_OPTS+="-v $CACHE_DIR:/tmp/cache:rw "
-    # give non-root slubuilder user R/W perms for docker volumes
+    BUILD_OPTS+=" -d"
+    BUILD_OPTS+=" -v $TMP_DIR:/tmp/app"
+    BUILD_OPTS+=" -v $CACHE_DIR:/tmp/cache:rw"
+    # give non-root slugbuilder user R/W perms for docker volumes
     chown -R 2000:2000 $TMP_DIR $CACHE_DIR
 
+    if [ -f /buildpacks ]; then
+        BUILD_OPTS+=" -v /buildpacks:/tmp/buildpacks:rw"
+        # give non-root slugbuilder user R/W perms for docker volumes
+        chown -R 2000:2000 /buildpacks
+    fi
+
     # build the application and attach to the process
     JOB=$(docker run $BUILD_OPTS deis/slugbuilder)
     docker attach $JOB

client/deis.py

@@ -621,7 +621,7 @@ def apps_logs(self, args):
         app = args.get('--app')
         if not app:
             app = self._session.app
-        response = self._dispatch('post',
+        response = self._dispatch('get',
                                   "/api/apps/{}/logs".format(app))
         if response.status_code == requests.codes.ok:  # @UndefinedVariable
             # strip the last newline character
@@ -1125,7 +1125,7 @@ def config_list(self, args):
         response = self._dispatch('get', "/api/apps/{}/config".format(app))
         if response.status_code == requests.codes.ok:  # @UndefinedVariable
             config = response.json()
-            values = json.loads(config['values'])
+            values = config['values']
             self._logger.info("=== {} Config".format(app))
             items = values.items()
             if len(items) == 0:
@@ -1180,7 +1180,7 @@ def config_set(self, args):
             version = response.headers['x-deis-release']
             self._logger.info("done, v{}\n".format(version))
             config = response.json()
-            values = json.loads(config['values'])
+            values = config['values']
             self._logger.info("=== {}".format(app))
             items = values.items()
             if len(items) == 0:
@@ -1226,7 +1226,7 @@ def config_unset(self, args):
             version = response.headers['x-deis-release']
             self._logger.info("done, v{}\n".format(version))
             config = response.json()
-            values = json.loads(config['values'])
+            values = config['values']
             self._logger.info("=== {}".format(app))
             items = values.items()
             if len(items) == 0:
@@ -1270,7 +1270,7 @@ def config_pull(self, args):
                 pass
         response = self._dispatch('get', "/api/apps/{}/config".format(app))
         if response.status_code == requests.codes.ok:  # @UndefinedVariable
-            config = json.loads(response.json()['values'])
+            config = response.json()['values']
             for k, v in config.items():
                 if interactive and raw_input("overwrite {} with {}? (y/N) ".format(k, v)) == 'y':
                     env_dict[k] = v
@@ -1545,9 +1545,9 @@ def write(d):
                 self._logger.info(("{k:<" + str(width) + "} {v}").format(**locals()))
 
         self._logger.info("\n--- Memory")
-        write(json.loads(config.get('memory', '{}')))
+        write(config.get('memory', '{}'))
         self._logger.info("\n--- CPU")
-        write(json.loads(config.get('cpu', '{}')))
+        write(config.get('cpu', '{}'))
 
     def ps(self, args):
         """
@@ -1743,7 +1743,7 @@ def tags_unset(self, args):
             raise ResponseError(response)
 
     def _print_tags(self, app, config):
-        items = json.loads(config['tags'])
+        items = config['tags']
         self._logger.info("=== {} Tags".format(app))
         if len(items) == 0:
             self._logger.info('No tags defined')

controller/api/serializers.py

@@ -4,6 +4,7 @@
 
 from __future__ import unicode_literals
 
+import json
 import re
 
 from django.conf import settings
@@ -32,6 +33,18 @@ def from_native(self, data):
         return serializers.SlugRelatedField.from_native(self, data)
 
 
+class JSONFieldSerializer(serializers.WritableField):
+    def to_native(self, obj):
+        return obj
+
+    def from_native(self, value):
+        try:
+            val = json.loads(value)
+        except TypeError:
+            val = value
+        return val
+
+
 class UserSerializer(serializers.ModelSerializer):
     """Serialize a User model."""
 
@@ -64,6 +77,7 @@ class ClusterSerializer(serializers.ModelSerializer):
     """Serialize a :class:`~api.models.Cluster` model."""
 
     owner = serializers.Field(source='owner.username')
+    options = JSONFieldSerializer(source='options', required=False)
 
     class Meta:
         """Metadata options for a :class:`ClusterSerializer`."""
@@ -98,6 +112,7 @@ class BuildSerializer(serializers.ModelSerializer):
 
     owner = serializers.Field(source='owner.username')
     app = serializers.SlugRelatedField(slug_field='id')
+    procfile = JSONFieldSerializer(source='procfile', required=False)
 
     class Meta:
         """Metadata options for a :class:`BuildSerializer`."""
@@ -110,14 +125,10 @@ class ConfigSerializer(serializers.ModelSerializer):
 
     owner = serializers.Field(source='owner.username')
     app = serializers.SlugRelatedField(slug_field='id')
-    values = serializers.ModelField(
-        model_field=models.Config()._meta.get_field('values'), required=False)
-    memory = serializers.ModelField(
-        model_field=models.Config()._meta.get_field('memory'), required=False)
-    cpu = serializers.ModelField(
-        model_field=models.Config()._meta.get_field('cpu'), required=False)
-    tags = serializers.ModelField(
-        model_field=models.Config()._meta.get_field('tags'), required=False)
+    values = JSONFieldSerializer(source='values', required=False)
+    memory = JSONFieldSerializer(source='memory', required=False)
+    cpu = JSONFieldSerializer(source='cpu', required=False)
+    tags = JSONFieldSerializer(source='tags', required=False)
 
     class Meta:
         """Metadata options for a :class:`ConfigSerializer`."""
@@ -185,6 +196,7 @@ class AppSerializer(serializers.ModelSerializer):
     id = serializers.SlugField(default=utils.generate_app_name)
     cluster = serializers.SlugRelatedField(slug_field='id')
     url = serializers.Field(source='url')
+    structure = JSONFieldSerializer(source='structure', required=False)
 
     class Meta:
         """Metadata options for a :class:`AppSerializer`."""
@@ -203,14 +215,6 @@ 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_app.py

@@ -10,12 +10,12 @@
 import os.path
 
 from django.test import TestCase
-
 from django.conf import settings
 
+from api.models import App
 
-class AppTest(TestCase):
 
+class AppTest(TestCase):
     """Tests creation of applications"""
 
     fixtures = ['tests.json']
@@ -79,13 +79,13 @@ def test_app_actions(self):
         if os.path.exists(path):
             os.remove(path)
         url = '/api/apps/{app_id}/logs'.format(**locals())
-        response = self.client.post(url)
+        response = self.client.get(url)
         self.assertEqual(response.status_code, 204)
         self.assertEqual(response.data, 'No logs for {}'.format(app_id))
         # write out some fake log data and try again
         with open(path, 'a') as f:
             f.write(FAKE_LOG_DATA)
-        response = self.client.post(url)
+        response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, FAKE_LOG_DATA)
         os.remove(path)
@@ -107,7 +107,7 @@ def test_app_release_notes_in_logs(self):
         app_id = response.data['id']  # noqa
         path = os.path.join(settings.DEIS_LOG_DIR, app_id + '.log')
         url = '/api/apps/{app_id}/logs'.format(**locals())
-        response = self.client.post(url)
+        response = self.client.get(url)
         self.assertIn('autotest created initial release', response.data)
         self.assertEqual(response.status_code, 200)
         # delete file for future runs
@@ -135,6 +135,23 @@ def test_app_errors(self):
             response = self.client.get(url)
             self.assertEquals(response.status_code, 404)
 
+    def test_app_structure_is_valid_json(self):
+        """Application structures should be valid JSON objects."""
+        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']
+        self.assertIn('structure', response.data)
+        self.assertEqual(response.data['structure'], {})
+        app = App.objects.get(id=app_id)
+        app.structure = {'web': 1}
+        app.save()
+        url = '/api/apps/{}'.format(app_id)
+        response = self.client.get(url)
+        self.assertIn('structure', response.data)
+        self.assertEqual(response.data['structure'], {"web": 1})
+
     def test_admin_can_manage_other_apps(self):
         """Administrators of Deis should be able to manage all applications.
         """
@@ -153,7 +170,7 @@ def test_admin_can_manage_other_apps(self):
         self.assertEqual(response.status_code, 200)
         # check app logs
         url = '/api/apps/{app_id}/logs'.format(**locals())
-        response = self.client.post(url)
+        response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
         self.assertIn('autotest2 created initial release', response.data)
         # run one-off commands

controller/api/tests/test_cluster.py

@@ -42,7 +42,7 @@ def test_cluster(self):
         self.assertIn('auth', response.data)
         self.assertIn('options', response.data)
         self.assertEqual(response.data['hosts'], 'host1,host2')
-        self.assertEqual(json.loads(response.data['options']), {'key': 'val'})
+        self.assertEqual(response.data['options'], {'key': 'val'})
         response = self.client.get('/api/clusters')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data['results']), 1)
@@ -68,7 +68,7 @@ def test_cluster(self):
         response = self.client.patch(url, json.dumps(body), content_type='application/json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['hosts'], new_hosts)
-        self.assertEqual(json.loads(response.data['options']), new_options)
+        self.assertEqual(response.data['options'], new_options)
         response = self.client.delete(url)
         self.assertEqual(response.status_code, 204)