From 694499d70e6c80a45a4e75d137641fa044d2dde7 Mon Sep 17 00:00:00 2001
From: Madhuri Kumari <madhuri.kumari@nectechnologies.in>
Date: Tue, 10 Mar 2015 05:56:36 +0000
Subject: [PATCH] Make resource creation return 400 with empty manifest

When resource pod/rc/service is created with no
manifest, 500 status is returned to user due to
unhandled exception. This patch make this resource
creation return 400 status to user when no manifest
is passed. Also added test for empty manifest.

Change-Id: I7f6edc5bc7f8d46ea0eb0b71e22d2b88b3482154
Closes-bug:#1429767
Closes-bug:#1430064
---
 magnum/api/controllers/v1/base.py                         | 2 ++
 magnum/api/controllers/v1/service.py                      | 2 --
 magnum/common/k8s_manifest.py                             | 3 +++
 magnum/tests/api/controllers/v1/test_pod.py               | 8 ++++++++
 .../api/controllers/v1/test_replicationcontroller.py      | 8 ++++++++
 5 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/magnum/api/controllers/v1/base.py b/magnum/api/controllers/v1/base.py
index b26f90a697..46ad1028e3 100644
--- a/magnum/api/controllers/v1/base.py
+++ b/magnum/api/controllers/v1/base.py
@@ -72,6 +72,8 @@ class K8sResourceBase(base.APIBase):
     """Labels of this k8s resource"""
 
     def _get_manifest(self):
+        if not self.manifest and not self.manifest_url:
+            return None
         if self.manifest is not wsme.Unset and self.manifest is not None:
             return self.manifest
         if (self.manifest_url is not wsme.Unset
diff --git a/magnum/api/controllers/v1/service.py b/magnum/api/controllers/v1/service.py
index ac55a0296e..caa0dbbdbc 100644
--- a/magnum/api/controllers/v1/service.py
+++ b/magnum/api/controllers/v1/service.py
@@ -114,8 +114,6 @@ class Service(v1_base.K8sResourceBase):
         return cls._convert_with_links(sample, 'http://localhost:9511', expand)
 
     def parse_manifest(self):
-        if not self.manifest and not self.manifest_url:
-            raise exception.InvalidParameterValue("'manifest' can't be empty")
         try:
             manifest = k8s_manifest.parse(self._get_manifest())
         except ValueError as e:
diff --git a/magnum/common/k8s_manifest.py b/magnum/common/k8s_manifest.py
index bd33590ebf..c9e7197d4c 100644
--- a/magnum/common/k8s_manifest.py
+++ b/magnum/common/k8s_manifest.py
@@ -48,6 +48,9 @@ def parse(manifest_str):
     This includes determination of whether the string is using the
     JSON or YAML format.
     '''
+    if not manifest_str:
+        msg = _("'manifest' can't be empty")
+        raise ValueError(msg)
     if manifest_str.startswith('{'):
         manifest = json.loads(manifest_str)
     else:
diff --git a/magnum/tests/api/controllers/v1/test_pod.py b/magnum/tests/api/controllers/v1/test_pod.py
index 672bd0f0bb..f95986502b 100644
--- a/magnum/tests/api/controllers/v1/test_pod.py
+++ b/magnum/tests/api/controllers/v1/test_pod.py
@@ -355,6 +355,14 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual(400, response.status_int)
         self.assertTrue(response.json['error_message'])
 
+    def test_create_pod_no_manifest(self):
+        pdict = apiutils.pod_post_data()
+        del pdict['manifest']
+        response = self.post_json('/pods', pdict, expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(400, response.status_int)
+        self.assertTrue(response.json['error_message'])
+
 
 class TestDelete(api_base.FunctionalTest):
 
diff --git a/magnum/tests/api/controllers/v1/test_replicationcontroller.py b/magnum/tests/api/controllers/v1/test_replicationcontroller.py
index 0d9b464db1..f7f51ab8e7 100644
--- a/magnum/tests/api/controllers/v1/test_replicationcontroller.py
+++ b/magnum/tests/api/controllers/v1/test_replicationcontroller.py
@@ -331,6 +331,14 @@ class TestPost(api_base.FunctionalTest):
         self.assertEqual(400, response.status_int)
         self.assertTrue(response.json['error_message'])
 
+    def test_create_rc_no_manifest(self):
+        rc_dict = apiutils.rc_post_data()
+        del rc_dict['manifest']
+        response = self.post_json('/rcs', rc_dict, expect_errors=True)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(400, response.status_int)
+        self.assertTrue(response.json['error_message'])
+
 
 class TestDelete(api_base.FunctionalTest):