Fix: Error Handling for YAML Load

Introduce yaml_safe_load, a util wrapper to invoke
yaml.safe_load and correctly handle malformed YAML files.
This wrapper catches exceptions thrown (e.g., incorrect syntax)
while loading/parsing a yaml file. Currently, there's no error
reported back to the user on such failures, rather an ambiguous
python object string is reported and the user is unaware of the
cause of failure. All calls for reading yaml files being uploaded
by the user have been replaced with the wrapper for correct error
reporting.

Test:
  1. PASS: Ensure that the following error message is reported when
           a malformed file is passed to a command:
           "Error: Unable to load file contents.
           (problem on line: #)."
            - Add an entry in the bootstrap yaml file without a
              space after the colon:
                "key:val"
            - Run dcmanager subcloud add with the
              malformed bootstrap file
            - Check the cmdline reported error
  2. PASS: Repeat test #1 with common YAML issues, for example:
            - Incorrect indentation
            - Missing new line between entries
  3. PASS: Repeat test #1 with an empty file. Ensure that the
           following error message is reported:
           "Error: Unable to load file contents.
           (empty file provided)."

Closes-Bug: 2041679

Change-Id: I33210a45038d6d4e98b69809ffa8d2d431b43190
Signed-off-by: Salman Rana <salman.rana@windriver.com>

distributedcloud/dcmanager/api/controllers/v1/phased_subcloud_deploy.py

@@ -10,7 +10,6 @@ import os
 from oslo_log import log as logging
 from oslo_messaging import RemoteError
 import pecan
-import yaml
 
 from dcmanager.api.controllers import restcomm
 from dcmanager.api.policies import phased_subcloud_deploy as \
@@ -142,7 +141,7 @@ def get_create_payload(request: pecan.Request) -> dict:
         if f in request.POST:
             file_item = request.POST[f]
             file_item.file.seek(0, os.SEEK_SET)
-            data = yaml.safe_load(file_item.file.read().decode('utf8'))
+            data = utils.yaml_safe_load(file_item.file.read().decode('utf8'), f)
             if f == consts.BOOTSTRAP_VALUES:
                 payload.update(data)
             else:

distributedcloud/dcmanager/api/controllers/v1/subcloud_backup.py

@@ -15,7 +15,6 @@ import pecan
 from pecan import expose
 from pecan import request as pecan_request
 from pecan import response
-import yaml
 
 from dcmanager.api.controllers import restcomm
 from dcmanager.api.policies import subcloud_backup as subcloud_backup_policy
@@ -99,7 +98,7 @@ class SubcloudBackupController(object):
             if param in request.POST:
                 file_item = request.POST[param]
                 file_item.file.seek(0, os.SEEK_SET)
-                data = yaml.safe_load(file_item.file.read().decode('utf8'))
+                data = utils.yaml_safe_load(file_item.file.read().decode('utf8'), param)
                 payload.update({param: data})
                 del request.POST[param]
 

distributedcloud/dcmanager/api/controllers/v1/subclouds.py

@@ -28,7 +28,6 @@ from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_messaging import RemoteError
 import re
-import yaml
 
 import pecan
 from pecan import expose
@@ -115,7 +114,7 @@ class SubcloudsController(object):
 
             # only the install_values field is yaml, force should be bool
             if field_name in [consts.INSTALL_VALUES, 'force']:
-                field_content = yaml.safe_load(field_content)
+                field_content = utils.yaml_safe_load(field_content, field_name)
 
             payload[field_name] = field_content
 

distributedcloud/dcmanager/common/phased_subcloud_deploy.py

@@ -14,7 +14,6 @@ from oslo_log import log as logging
 from oslo_utils import uuidutils
 import pecan
 import tsconfig.tsconfig as tsc
-import yaml
 
 from dccommon import consts as dccommon_consts
 from dccommon.drivers.openstack import patching_v1
@@ -907,7 +906,7 @@ def get_request_data(request: pecan.Request,
                 upload_config_file(contents, fn, f)
                 payload.update({f: fn})
             else:
-                data = yaml.safe_load(contents.decode('utf8'))
+                data = utils.yaml_safe_load(contents.decode('utf8'), f)
                 if f == consts.BOOTSTRAP_VALUES:
                     payload.update(data)
                 else:
@@ -1059,7 +1058,7 @@ def get_bootstrap_subcloud_name(request: pecan.Request):
     if bootstrap_values is not None:
         bootstrap_values.file.seek(0, os.SEEK_SET)
         contents = bootstrap_values.file.read()
-        data = yaml.safe_load(contents.decode('utf8'))
+        data = utils.yaml_safe_load(contents.decode('utf8'), consts.BOOTSTRAP_VALUES)
         bootstrap_sc_name = data.get('name')
 
     return bootstrap_sc_name

distributedcloud/dcmanager/common/utils.py

@@ -1382,3 +1382,35 @@ def is_req_from_cert_mon_agent(request):
         return True
     else:
         return False
+
+
+def yaml_safe_load(contents, content_type):
+    """Wrapper for yaml.safe_load with error logging and reporting.
+
+    :param contents: decoded contents to load
+    :param content_type: values being loaded
+    :returns dict constructed from parsed contents
+    """
+    error = False
+    msg = "Error: Unable to load " + content_type + " file contents ({})."
+
+    try:
+        data = yaml.safe_load(contents)
+        if data is None:
+            error = True
+            msg = msg.format("empty file provided")
+    except yaml.YAMLError as e:
+        error = True
+        if hasattr(e, 'problem_mark'):
+            mark = e.problem_mark
+            msg = msg.format("problem on line: " + str(mark.line))
+        else:
+            msg = msg.format("please see logs for more details")
+
+        LOG.exception(e)
+
+    if error:
+        LOG.error(msg)
+        pecan.abort(400, _(msg))
+
+    return data

distributedcloud/dcmanager/tests/unit/api/v1/controllers/test_subclouds.py

@@ -27,6 +27,7 @@ import six
 from six.moves import http_client
 from tsconfig.tsconfig import SW_VERSION
 import webtest
+import yaml
 
 from dccommon import consts as dccommon_consts
 from dcmanager.api.controllers.v1 import phased_subcloud_deploy as psd
@@ -340,6 +341,40 @@ class TestSubcloudPost(testroot.DCManagerApiTest,
                               params={},
                               headers=self.get_api_headers())
 
+    @mock.patch.object(cutils, 'LOG')
+    def test_post_subcloud_boostrap_file_malformed(self, mock_logging):
+        """Test POST operation with malformed bootstrap file contents."""
+
+        params = self.get_post_params()
+
+        valid_keyval = "key: val"
+        invalid_keyval = "key:val"  # A space is required after the colon
+        fake_name = consts.BOOTSTRAP_VALUES + "_fake"
+
+        corrupt_fake_content = \
+            (yaml.dump(self.FAKE_BOOTSTRAP_DATA) + invalid_keyval).encode("utf-8")
+        upload_files = list()
+        upload_files.append((consts.BOOTSTRAP_VALUES, fake_name, corrupt_fake_content))
+        response = self.app.post(self.get_api_prefix(),
+                                 params=params,
+                                 upload_files=upload_files,
+                                 headers=self.get_api_headers(),
+                                 expect_errors=True)
+        self.assertEqual(mock_logging.error.call_count, 1)
+        log_string = mock_logging.error.call_args[0][0]
+        self.assertIn('Error: Unable to load bootstrap_values file contents', log_string)
+        self.assertEqual(http_client.BAD_REQUEST, response.status_code)
+
+        # try with valid new entry and verify it works
+        valid_content = (yaml.dump(self.FAKE_BOOTSTRAP_DATA) + valid_keyval).encode("utf-8")
+        upload_files = list()
+        upload_files.append((consts.BOOTSTRAP_VALUES, fake_name, valid_content))
+        response = self.app.post(self.get_api_prefix(),
+                                 params=params,
+                                 upload_files=upload_files,
+                                 headers=self.get_api_headers())
+        self._verify_post_success(response)
+
     def test_post_subcloud_boostrap_entries_missing(self):
         """Test POST operation with some mandatory boostrap fields missing.