Adding a few more test cases. Fixed a bug when key was empty. Safeguard against yedit module being passed an empty key

5 files changed, 132 insertions, 19 deletions

diff --git a/roles/lib_utils/library/yedit.py b/roles/lib_utils/library/yedit.py
index b311354da..48d5a411b 100644
--- a/roles/lib_utils/library/yedit.py
+++ b/roles/lib_utils/library/yedit.py
@@ -201,8 +201,6 @@ EXAMPLES = '''
 # -*- -*- -*- End included fragment: doc/yedit -*- -*- -*-
 
 # -*- -*- -*- Begin included fragment: class/yedit.py -*- -*- -*-
-# pylint: disable=undefined-variable,missing-docstring
-# noqa: E301,E302
 
 
 class YeditException(Exception):
@@ -236,13 +234,13 @@ class Yedit(object):
 
     @property
     def separator(self):
-        ''' getter method for yaml_dict '''
+        ''' getter method for separator '''
         return self._separator
 
     @separator.setter
-    def separator(self):
-        ''' getter method for yaml_dict '''
-        return self._separator
+    def separator(self, inc_sep):
+        ''' setter method for separator '''
+        self._separator = inc_sep
 
     @property
     def yaml_dict(self):
@@ -656,7 +654,17 @@ class Yedit(object):
             pass
 
         result = Yedit.add_entry(tmp_copy, path, value, self.separator)
-        if not result:
+        if result is None:
+            return (False, self.yaml_dict)
+
+        # When path equals "" it is a special case.
+        # "" refers to the root of the document
+        # Only update the root path (entire document) when its a list or dict
+        if path == '':
+            if isinstance(result, list) or isinstance(result, dict):
+                self.yaml_dict = result
+                return (True, self.yaml_dict)
+
             return (False, self.yaml_dict)
 
         self.yaml_dict = tmp_copy
@@ -682,7 +690,7 @@ class Yedit(object):
                 pass
 
             result = Yedit.add_entry(tmp_copy, path, value, self.separator)
-            if result:
+            if result is not None:
                 self.yaml_dict = tmp_copy
                 return (True, self.yaml_dict)
 
@@ -724,7 +732,7 @@ class Yedit(object):
         # If vtype is not str then go ahead and attempt to yaml load it.
         elif isinstance(inc_value, str) and 'str' not in vtype:
             try:
-                inc_value = yaml.load(inc_value)
+                inc_value = yaml.safe_load(inc_value)
             except Exception:
                 raise YeditException('Could not determine type of incoming ' +
                                      'value. value=[%s] vtype=[%s]'
@@ -856,6 +864,8 @@ class Yedit(object):
                         'result': rval[1],
                         'state': state}
 
+            # We were passed content but no src, key or value, or edits.  Return contents in memory
+            return {'changed': False, 'result': yamlfile.yaml_dict, 'state': state}
         return {'failed': True, 'msg': 'Unkown state passed'}
 
 # -*- -*- -*- End included fragment: class/yedit.py -*- -*- -*-
@@ -893,6 +903,9 @@ def main():
         required_one_of=[["content", "src"]],
     )
 
+    if module.params['src'] is not None and module.params['key'] in [None, '']:
+        module.fail_json(failed=True, msg='Empty value for parameter key not allowed.')
+
     rval = Yedit.run_ansible(module.params)
     if 'failed' in rval and rval['failed']:
         module.fail_json(**rval)
diff --git a/roles/lib_utils/src/ansible/yedit.py b/roles/lib_utils/src/ansible/yedit.py
index ea112ac83..bdb9915d6 100644
--- a/roles/lib_utils/src/ansible/yedit.py
+++ b/roles/lib_utils/src/ansible/yedit.py
@@ -32,6 +32,9 @@ def main():
         required_one_of=[["content", "src"]],
     )
 
+    if module.params['src'] is not None and module.params['key'] in [None, '']:
+        module.fail_json(failed=True, msg='Empty value for parameter key not allowed.')
+
     rval = Yedit.run_ansible(module.params)
     if 'failed' in rval and rval['failed']:
         module.fail_json(**rval)
diff --git a/roles/lib_utils/src/class/yedit.py b/roles/lib_utils/src/class/yedit.py
index 9f37a9244..65472afc0 100644
--- a/roles/lib_utils/src/class/yedit.py
+++ b/roles/lib_utils/src/class/yedit.py
@@ -1,6 +1,5 @@
 # flake8: noqa
-# pylint: disable=undefined-variable,missing-docstring
-# noqa: E301,E302
+# pylint: skip-file
 
 
 class YeditException(Exception):
@@ -34,13 +33,13 @@ class Yedit(object):
 
     @property
     def separator(self):
-        ''' getter method for yaml_dict '''
+        ''' getter method for separator '''
         return self._separator
 
     @separator.setter
-    def separator(self):
-        ''' getter method for yaml_dict '''
-        return self._separator
+    def separator(self, inc_sep):
+        ''' setter method for separator '''
+        self._separator = inc_sep
 
     @property
     def yaml_dict(self):
@@ -454,7 +453,17 @@ class Yedit(object):
             pass
 
         result = Yedit.add_entry(tmp_copy, path, value, self.separator)
-        if not result:
+        if result is None:
+            return (False, self.yaml_dict)
+
+        # When path equals "" it is a special case.
+        # "" refers to the root of the document
+        # Only update the root path (entire document) when its a list or dict
+        if path == '':
+            if isinstance(result, list) or isinstance(result, dict):
+                self.yaml_dict = result
+                return (True, self.yaml_dict)
+
             return (False, self.yaml_dict)
 
         self.yaml_dict = tmp_copy
@@ -480,7 +489,7 @@ class Yedit(object):
                 pass
 
             result = Yedit.add_entry(tmp_copy, path, value, self.separator)
-            if result:
+            if result is not None:
                 self.yaml_dict = tmp_copy
                 return (True, self.yaml_dict)
 
@@ -522,7 +531,7 @@ class Yedit(object):
         # If vtype is not str then go ahead and attempt to yaml load it.
         elif isinstance(inc_value, str) and 'str' not in vtype:
             try:
-                inc_value = yaml.load(inc_value)
+                inc_value = yaml.safe_load(inc_value)
             except Exception:
                 raise YeditException('Could not determine type of incoming ' +
                                      'value. value=[%s] vtype=[%s]'
@@ -654,4 +663,6 @@ class Yedit(object):
                         'result': rval[1],
                         'state': state}
 
+            # We were passed content but no src, key or value, or edits.  Return contents in memory
+            return {'changed': False, 'result': yamlfile.yaml_dict, 'state': state}
         return {'failed': True, 'msg': 'Unkown state passed'}
diff --git a/roles/lib_utils/src/test/integration/yedit.yml b/roles/lib_utils/src/test/integration/yedit.yml
index c960c9856..65209bade 100755
--- a/roles/lib_utils/src/test/integration/yedit.yml
+++ b/roles/lib_utils/src/test/integration/yedit.yml
@@ -248,4 +248,4 @@
     assert:
       that: results.result == [1, 2, 3, 4]
       msg: "Test: '[1, 2, 3, 4]' != [{{ results.result }}]"
-  ###### end test create multiple list value #####
+      ###### end test create multiple list value #####
diff --git a/roles/lib_utils/src/test/unit/test_yedit.py b/roles/lib_utils/src/test/unit/test_yedit.py
index 23a3f7353..f9f42843a 100755
--- a/roles/lib_utils/src/test/unit/test_yedit.py
+++ b/roles/lib_utils/src/test/unit/test_yedit.py
@@ -5,6 +5,7 @@
 import os
 import sys
 import unittest
+import mock
 
 # Removing invalid variable names for tests so that I can
 # keep them brief
@@ -277,6 +278,91 @@ class YeditTest(unittest.TestCase):
         with self.assertRaises(YeditException):
             yed.put('new.stuff.here[0]', 'item')
 
+    def test_empty_key_with_int_value(self):
+        '''test editing top level with not list or dict'''
+        yed = Yedit(content={'a': {'b': 12}})
+        result = yed.put('', 'b')
+        self.assertFalse(result[0])
+
+    def test_setting_separator(self):
+        '''test editing top level with not list or dict'''
+        yed = Yedit(content={'a': {'b': 12}})
+        yed.separator = ':'
+        self.assertEqual(yed.separator, ':')
+
+    def test_remove_all(self):
+        '''test removing all data'''
+        data = Yedit.remove_entry({'a': {'b': 12}}, '')
+        self.assertTrue(data)
+
+    def test_remove_list_entry(self):
+        '''test removing list entry'''
+        data = {'a': {'b': [{'c': 3}]}}
+        results = Yedit.remove_entry(data, 'a.b[0]')
+        self.assertTrue(results)
+        self.assertTrue(data, {'a': {'b': []}})
+
+    def test_parse_value_string_true(self):
+        '''test parse_value'''
+        results = Yedit.parse_value('true', 'str')
+        self.assertEqual(results, 'true')
+
+    def test_parse_value_bool_true(self):
+        '''test parse_value'''
+        results = Yedit.parse_value('true', 'bool')
+        self.assertTrue(results)
+
+    def test_parse_value_bool_exception(self):
+        '''test parse_value'''
+        with self.assertRaises(YeditException):
+            Yedit.parse_value('TTT', 'bool')
+
+    @mock.patch('yedit.Yedit.write')
+    def test_run_ansible_basic(self, mock_write):
+        '''test parse_value'''
+        params = {
+            'src': None,
+            'backup': False,
+            'separator': '.',
+            'state': 'present',
+            'edits': [],
+            'value': None,
+            'key': None,
+            'content': {'a': {'b': {'c': 1}}},
+            'content_type': '',
+        }
+
+        results = Yedit.run_ansible(params)
+
+        mock_write.side_effect = [
+            (True, params['content']),
+        ]
+
+        self.assertFalse(results['changed'])
+
+    @mock.patch('yedit.Yedit.write')
+    def test_run_ansible_and_write(self, mock_write):
+        '''test parse_value'''
+        params = {
+            'src': '/tmp/test',
+            'backup': False,
+            'separator': '.',
+            'state': 'present',
+            'edits': [],
+            'value': None,
+            'key': None,
+            'content': {'a': {'b': {'c': 1}}},
+            'content_type': '',
+        }
+
+        results = Yedit.run_ansible(params)
+
+        mock_write.side_effect = [
+            (True, params['content']),
+        ]
+
+        self.assertTrue(results['changed'])
+
     def tearDown(self):
         '''TearDown method'''
         os.unlink(YeditTest.filename)