Merge pull request #47793 from slivik/ini_manage-dry-run

Fixes dry run in ini_manage + Fixes related bug - when working with o…

salt/modules/ini_manage.py

@@ -35,9 +35,9 @@ def __virtual__():
     return __virtualname__
 
 
-ini_regx = re.compile(r'^\s*\[(.+?)\]\s*$', flags=re.M)
-com_regx = re.compile(r'^\s*(#|;)\s*(.*)')
-indented_regx = re.compile(r'(\s+)(.*)')
+INI_REGX = re.compile(r'^\s*\[(.+?)\]\s*$', flags=re.M)
+COM_REGX = re.compile(r'^\s*(#|;)\s*(.*)')
+INDENTED_REGX = re.compile(r'(\s+)(.*)')
 
 
 def set_option(file_name, sections=None, separator='='):
@@ -105,7 +105,13 @@ def get_option(file_name, section, option, separator='='):
         salt '*' ini.get_option /path/to/ini section_name option_name
     '''
     inifile = _Ini.get_ini_file(file_name, separator=separator)
-    return inifile.get(section, {}).get(option, None)
+    if section:
+        try:
+            return inifile.get(section, {}).get(option, None)
+        except AttributeError:
+            return None
+    else:
+        return inifile.get(option, None)
 
 
 def remove_option(file_name, section, option, separator='='):
@@ -129,7 +135,10 @@ def remove_option(file_name, section, option, separator='='):
         salt '*' ini.remove_option /path/to/ini section_name option_name
     '''
     inifile = _Ini.get_ini_file(file_name, separator=separator)
-    value = inifile.get(section, {}).pop(option, None)
+    if isinstance(inifile.get(section), (dict, OrderedDict)):
+        value = inifile.get(section, {}).pop(option, None)
+    else:
+        value = inifile.pop(option, None)
     inifile.flush()
     return value
 
@@ -182,15 +191,53 @@ def remove_section(file_name, section, separator='='):
 
         salt '*' ini.remove_section /path/to/ini section_name
     '''
+    inifile = _Ini.get_ini_file(file_name, separator=separator)
+    if section in inifile:
+        section = inifile.pop(section)
+        inifile.flush()
+        ret = {}
+        for key, value in six.iteritems(section):
+            if key[0] != '#':
+                ret.update({key: value})
+        return ret
+
+
+def get_ini(file_name, separator='='):
+    '''
+    Retrieve whole structure from an ini file and return it as dictionary.
+
+    API Example:
+
+    .. code-block:: python
+
+        import salt
+        sc = salt.client.get_local_client()
+        sc.cmd('target', 'ini.get_ini',
+               [path_to_ini_file])
+
+    CLI Example:
+
+    .. code-block:: bash
+
+        salt '*' ini.get_ini /path/to/ini
+    '''
+    def ini_odict2dict(odict):
+        '''
+        Transform OrderedDict to regular dict recursively
+        :param odict: OrderedDict
+        :return: regular dict
+        '''
+        ret = {}
+        for key, val in six.iteritems(odict):
+            if key[0] != '#':
+                if isinstance(val, (dict, OrderedDict)):
+                    ret.update({key: ini_odict2dict(val)})
+                else:
+                    ret.update({key: val})
+        return ret
 
     inifile = _Ini.get_ini_file(file_name, separator=separator)
-    section = inifile.pop(section, {})
-    inifile.flush()
-    ret = {}
-    for key, value in six.iteritems(section):
-        if key[0] != '#':
-            ret.update({key: value})
-    return ret
+    return ini_odict2dict(inifile)
 
 
 class _Section(OrderedDict):
@@ -221,7 +268,7 @@ class _Section(OrderedDict):
             self.pop(opt)
         for opt_str in inicontents.split(os.linesep):
             # Match comments
-            com_match = com_regx.match(opt_str)
+            com_match = COM_REGX.match(opt_str)
             if com_match:
                 name = '#comment{0}'.format(comment_count)
                 self.com = com_match.group(1)
@@ -229,7 +276,7 @@ class _Section(OrderedDict):
                 self.update({name: opt_str})
                 continue
             # Add indented lines to the value of the previous entry.
-            indented_match = indented_regx.match(opt_str)
+            indented_match = INDENTED_REGX.match(opt_str)
             if indented_match:
                 indent = indented_match.group(1).replace('\t', '    ')
                 if indent > curr_indent:
@@ -318,7 +365,7 @@ class _Section(OrderedDict):
         sections_dict = OrderedDict()
         for name, value in six.iteritems(self):
             # Handle Comment Lines
-            if com_regx.match(name):
+            if COM_REGX.match(name):
                 yield '{0}{1}'.format(value, os.linesep)
             # Handle Sections
             elif isinstance(value, _Section):
@@ -363,9 +410,6 @@ class _Section(OrderedDict):
 
 
 class _Ini(_Section):
-    def __init__(self, name, inicontents='', separator='=', commenter='#'):
-        super(_Ini, self).__init__(name, inicontents, separator, commenter)
-
     def refresh(self, inicontents=None):
         if inicontents is None:
             try:
@@ -382,7 +426,7 @@ class _Ini(_Section):
         # Remove anything left behind from a previous run.
         self.clear()
 
-        inicontents = ini_regx.split(inicontents)
+        inicontents = INI_REGX.split(inicontents)
         inicontents.reverse()
         # Pop anything defined outside of a section (ie. at the top of
         # the ini file).

salt/states/ini_manage.py

@@ -15,6 +15,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 # Import Salt libs
 from salt.ext import six
+from salt.utils.odict import OrderedDict
 
 __virtualname__ = 'ini'
 
@@ -53,46 +54,78 @@ def options_present(name, sections=None, separator='=', strict=False):
            'comment': 'No anomaly detected'
            }
     if __opts__['test']:
-        ret['result'] = True
         ret['comment'] = ''
-        for section in sections or {}:
-            section_name = ' in section ' + section if section != 'DEFAULT_IMPLICIT' else ''
-            try:
-                cur_section = __salt__['ini.get_section'](name, section, separator)
-            except IOError as err:
-                ret['comment'] = "{0}".format(err)
-                ret['result'] = False
-                return ret
-            for key in sections[section]:
-                cur_value = cur_section.get(key)
-                if cur_value == six.text_type(sections[section][key]):
-                    ret['comment'] += 'Key {0}{1} unchanged.\n'.format(key, section_name)
-                    continue
-                ret['comment'] += 'Changed key {0}{1}.\n'.format(key, section_name)
-                ret['result'] = None
-        if ret['comment'] == '':
-            ret['comment'] = 'No changes detected.'
-        return ret
+    # pylint: disable=too-many-nested-blocks
     try:
         changes = {}
         if sections:
-            for section_name, section_body in sections.items():
+            options = {}
+            for sname, sbody in sections.items():
+                if not isinstance(sbody, (dict, OrderedDict)):
+                    options.update({sname: sbody})
+            cur_ini = __salt__['ini.get_ini'](name, separator)
+            original_top_level_opts = {}
+            original_sections = {}
+            for key, val in cur_ini.items():
+                if isinstance(val, (dict, OrderedDict)):
+                    original_sections.update({key: val})
+                else:
+                    original_top_level_opts.update({key: val})
+            if __opts__['test']:
+                for option in options:
+                    if option in original_top_level_opts:
+                        if six.text_type(original_top_level_opts[option]) == six.text_type(options[option]):
+                            ret['comment'] += 'Unchanged key {0}.\n'.format(option)
+                        else:
+                            ret['comment'] += 'Changed key {0}.\n'.format(option)
+                            ret['result'] = None
+                    else:
+                        ret['comment'] += 'Changed key {0}.\n'.format(option)
+                        ret['result'] = None
+            else:
+                options_updated = __salt__['ini.set_option'](name, options, separator)
+                changes.update(options_updated)
+            if strict:
+                for opt_to_remove in set(original_top_level_opts).difference(options):
+                    if __opts__['test']:
+                        ret['comment'] += 'Removed key {0}.\n'.format(opt_to_remove)
+                        ret['result'] = None
+                    else:
+                        __salt__['ini.remove_option'](name, None, opt_to_remove, separator)
+                        changes.update({opt_to_remove: {'before': original_top_level_opts[opt_to_remove],
+                                                        'after': None}})
+            for section_name, section_body in [(sname, sbody) for sname, sbody in sections.items()
+                                               if isinstance(sbody, (dict, OrderedDict))]:
                 changes[section_name] = {}
                 if strict:
-                    original = __salt__['ini.get_section'](name, section_name, separator)
+                    original = cur_ini.get(section_name, {})
                     for key_to_remove in set(original.keys()).difference(section_body.keys()):
-                        orig_value = __salt__['ini.get_option'](name, section_name, key_to_remove, separator)
-                        __salt__['ini.remove_option'](name, section_name, key_to_remove, separator)
-                        changes[section_name].update({key_to_remove: ''})
-                        changes[section_name].update({key_to_remove: {'before': orig_value,
-                                                                      'after': None}})
-                options_updated = __salt__['ini.set_option'](name, {section_name: section_body}, separator)
-                if options_updated:
-                    changes[section_name].update(options_updated[section_name])
-                if not changes[section_name]:
-                    del changes[section_name]
+                        orig_value = original_sections.get(section_name, {}).get(key_to_remove, '#-#-')
+                        if __opts__['test']:
+                            ret['comment'] += 'Deleted key {0} in section {1}.\n'.format(key_to_remove, section_name)
+                            ret['result'] = None
+                        else:
+                            __salt__['ini.remove_option'](name, section_name, key_to_remove, separator)
+                            changes[section_name].update({key_to_remove: ''})
+                            changes[section_name].update({key_to_remove: {'before': orig_value,
+                                                                          'after': None}})
+                if __opts__['test']:
+                    for option in section_body:
+                        if six.text_type(section_body[option]) == \
+                                six.text_type(original_sections.get(section_name, {}).get(option, '#-#-')):
+                            ret['comment'] += 'Unchanged key {0} in section {1}.\n'.format(option, section_name)
+                        else:
+                            ret['comment'] += 'Changed key {0} in section {1}.\n'.format(option, section_name)
+                            ret['result'] = None
+                else:
+                    options_updated = __salt__['ini.set_option'](name, {section_name: section_body}, separator)
+                    if options_updated:
+                        changes[section_name].update(options_updated[section_name])
+                    if not changes[section_name]:
+                        del changes[section_name]
         else:
-            changes = __salt__['ini.set_option'](name, sections, separator)
+            if not __opts__['test']:
+                changes = __salt__['ini.set_option'](name, sections, separator)
     except (IOError, KeyError) as err:
         ret['comment'] = "{0}".format(err)
         ret['result'] = False
@@ -102,10 +135,10 @@ def options_present(name, sections=None, separator='=', strict=False):
         ret['comment'] = 'Errors encountered. {0}'.format(changes['error'])
         ret['changes'] = {}
     else:
-        for name, body in changes.items():
+        for ciname, body in changes.items():
             if body:
                 ret['comment'] = 'Changes take effect'
-                ret['changes'].update({name: changes[name]})
+                ret['changes'].update({ciname: changes[ciname]})
     return ret
 
 
@@ -144,13 +177,24 @@ def options_absent(name, sections=None, separator='='):
                 ret['comment'] = "{0}".format(err)
                 ret['result'] = False
                 return ret
-            for key in sections[section]:
-                cur_value = cur_section.get(key)
-                if not cur_value:
-                    ret['comment'] += 'Key {0}{1} does not exist.\n'.format(key, section_name)
+            except AttributeError:
+                cur_section = section
+            if isinstance(sections[section], (dict, OrderedDict)):
+                for key in sections[section]:
+                    cur_value = cur_section.get(key)
+                    if not cur_value:
+                        ret['comment'] += 'Key {0}{1} does not exist.\n'.format(key, section_name)
+                        continue
+                    ret['comment'] += 'Deleted key {0}{1}.\n'.format(key, section_name)
+                    ret['result'] = None
+            else:
+                option = section
+                if not __salt__['ini.get_option'](name, None, option, separator):
+                    ret['comment'] += 'Key {0} does not exist.\n'.format(option)
                     continue
-                ret['comment'] += 'Deleted key {0}{1}.\n'.format(key, section_name)
+                ret['comment'] += 'Deleted key {0}.\n'.format(option)
                 ret['result'] = None
+
         if ret['comment'] == '':
             ret['comment'] = 'No changes detected.'
         return ret
@@ -168,6 +212,9 @@ def options_absent(name, sections=None, separator='='):
             if section not in ret['changes']:
                 ret['changes'].update({section: {}})
             ret['changes'][section].update({key: current_value})
+            if not isinstance(sections[section], (dict, OrderedDict)):
+                ret['changes'].update({section: current_value})
+                # break
             ret['comment'] = 'Changes take effect'
     return ret
 
@@ -197,18 +244,16 @@ def sections_present(name, sections=None, separator='='):
     if __opts__['test']:
         ret['result'] = True
         ret['comment'] = ''
+        try:
+            cur_ini = __salt__['ini.get_ini'](name, separator)
+        except IOError as err:
+            ret['result'] = False
+            ret['comment'] = "{0}".format(err)
+            return ret
         for section in sections or {}:
-            try:
-                cur_section = __salt__['ini.get_section'](name, section, separator)
-            except IOError as err:
-                ret['result'] = False
-                ret['comment'] = "{0}".format(err)
-                return ret
-            if dict(sections[section]) == cur_section:
+            if section in cur_ini:
                 ret['comment'] += 'Section unchanged {0}.\n'.format(section)
                 continue
-            elif cur_section:
-                ret['comment'] += 'Changed existing section {0}.\n'.format(section)
             else:
                 ret['comment'] += 'Created new section {0}.\n'.format(section)
             ret['result'] = None
@@ -255,14 +300,14 @@ def sections_absent(name, sections=None, separator='='):
     if __opts__['test']:
         ret['result'] = True
         ret['comment'] = ''
+        try:
+            cur_ini = __salt__['ini.get_ini'](name, separator)
+        except IOError as err:
+            ret['result'] = False
+            ret['comment'] = "{0}".format(err)
+            return ret
         for section in sections or []:
-            try:
-                cur_section = __salt__['ini.get_section'](name, section, separator)
-            except IOError as err:
-                ret['result'] = False
-                ret['comment'] = "{0}".format(err)
-                return ret
-            if not cur_section:
+            if section not in cur_ini:
                 ret['comment'] += 'Section {0} does not exist.\n'.format(section)
                 continue
             ret['comment'] += 'Deleted section {0}.\n'.format(section)

tests/unit/modules/test_ini_manage.py

@@ -1,5 +1,7 @@
 # -*- coding: utf-8 -*-
-
+'''
+Testing ini_manage exec module.
+'''
 # Import python libs
 from __future__ import absolute_import, print_function, unicode_literals
 import os
@@ -15,6 +17,9 @@ import salt.modules.ini_manage as ini
 
 
 class IniManageTestCase(TestCase):
+    '''
+    Testing ini_manage exec module.
+    '''
 
     TEST_FILE_CONTENT = os.linesep.join([
         '# Comment on the first line',
@@ -54,6 +59,9 @@ class IniManageTestCase(TestCase):
         os.remove(self.tfile.name)
 
     def test_get_option(self):
+        '''
+        Test get_option method.
+        '''
         self.assertEqual(
             ini.get_option(self.tfile.name, 'main', 'test1'),
             'value 1')
@@ -71,23 +79,46 @@ class IniManageTestCase(TestCase):
             '')
 
     def test_get_section(self):
+        '''
+        Test get_section method.
+        '''
         self.assertEqual(
             ini.get_section(self.tfile.name, 'SectionB'),
             {'test1': 'value 1B', 'test3': 'value 3B'})
 
     def test_remove_option(self):
+        '''
+        Test remove_option method.
+        '''
         self.assertEqual(
             ini.remove_option(self.tfile.name, 'SectionB', 'test1'),
             'value 1B')
         self.assertIsNone(ini.get_option(self.tfile.name, 'SectionB', 'test1'))
 
     def test_remove_section(self):
+        '''
+        Test remove_section method.
+        '''
         self.assertEqual(
             ini.remove_section(self.tfile.name, 'SectionB'),
             {'test1': 'value 1B', 'test3': 'value 3B'})
         self.assertEqual(ini.get_section(self.tfile.name, 'SectionB'), {})
 
+    def test_get_ini(self):
+        '''
+        Test get_ini method.
+        '''
+        self.assertEqual(
+            dict(ini.get_ini(self.tfile.name)), {
+                'SectionC': {'empty_option': ''},
+                'SectionB': {'test1': 'value 1B', 'test3': 'value 3B'},
+                'main': {'test1': 'value 1', 'test2': 'value 2'},
+                'option2': 'main2', 'option1': 'main1'})
+
     def test_set_option(self):
+        '''
+        Test set_option method.
+        '''
         result = ini.set_option(self.tfile.name, {
             'SectionB': {
                 'test3': 'new value 3B',
@@ -101,12 +132,9 @@ class IniManageTestCase(TestCase):
             'SectionB': {'test3': {'after': 'new value 3B',
                                    'before': 'value 3B'},
                          'test_set_option': {'after': 'test_set_value',
-                                             'before': None}
-            },
+                                             'before': None}},
             'SectionD': {'after': {'test_set_option2': 'test_set_value1'},
-                         'before': None
-            }
-        })
+                         'before': None}})
         # Check existing option updated
         self.assertEqual(
             ini.get_option(self.tfile.name, 'SectionB', 'test3'),
@@ -116,16 +144,22 @@ class IniManageTestCase(TestCase):
             ini.get_option(self.tfile.name, 'SectionD', 'test_set_option2'),
             'test_set_value1')
 
-    def test_empty_value_preserved_after_edit(self):
+    def test_empty_value(self):
+        '''
+        Test empty value preserved after edit
+        '''
         ini.set_option(self.tfile.name, {
             'SectionB': {'test3': 'new value 3B'},
         })
-        with salt.utils.files.fopen(self.tfile.name, 'r') as fp:
-            file_content = salt.utils.stringutils.to_unicode(fp.read())
+        with salt.utils.files.fopen(self.tfile.name, 'r') as fp_:
+            file_content = salt.utils.stringutils.to_unicode(fp_.read())
         expected = '{0}{1}{0}'.format(os.linesep, 'empty_option = ')
         self.assertIn(expected, file_content, 'empty_option was not preserved')
 
-    def test_empty_lines_preserved_after_edit(self):
+    def test_empty_lines(self):
+        '''
+        Test empty lines preserved after edit
+        '''
         ini.set_option(self.tfile.name, {
             'SectionB': {'test3': 'new value 3B'},
         })
@@ -155,12 +189,15 @@ class IniManageTestCase(TestCase):
             'empty_option = ',
             ''
         ])
-        with salt.utils.files.fopen(self.tfile.name, 'r') as fp:
-            file_content = salt.utils.stringutils.to_unicode(fp.read())
+        with salt.utils.files.fopen(self.tfile.name, 'r') as fp_:
+            file_content = salt.utils.stringutils.to_unicode(fp_.read())
         self.assertEqual(expected, file_content)
 
-    def test_empty_lines_preserved_after_multiple_edits(self):
+    def test_empty_lines_multiple_edits(self):
+        '''
+        Test empty lines preserved after multiple edits
+        '''
         ini.set_option(self.tfile.name, {
             'SectionB': {'test3': 'this value will be edited two times'},
         })
-        self.test_empty_lines_preserved_after_edit()
+        self.test_empty_lines()

tests/unit/states/test_ini_manage.py

@@ -17,6 +17,8 @@ from tests.support.mock import (
 # Import Salt Libs
 import salt.states.ini_manage as ini_manage
 
+# pylint: disable=no-member
+
 
 @skipIf(NO_MOCK, NO_MOCK_REASON)
 class IniManageTestCase(TestCase, LoaderModuleMockMixin):
@@ -40,7 +42,7 @@ class IniManageTestCase(TestCase, LoaderModuleMockMixin):
                'changes': {}}
 
         with patch.dict(ini_manage.__opts__, {'test': True}):
-            comt = 'No changes detected.'
+            comt = ''
             ret.update({'comment': comt, 'result': True})
             self.assertDictEqual(ini_manage.options_present(name), ret)
 
@@ -61,7 +63,7 @@ class IniManageTestCase(TestCase, LoaderModuleMockMixin):
         changes = {'mysection': {'first': 'who is on',
                                  'second': 'what is on',
                                  'third': {'after': None, 'before': "I don't know"}}}
-        with patch.dict(ini_manage.__salt__, {'ini.get_section': MagicMock(return_value=original['mysection'])}):
+        with patch.dict(ini_manage.__salt__, {'ini.get_ini': MagicMock(return_value=original)}):
             with patch.dict(ini_manage.__salt__, {'ini.remove_option': MagicMock(return_value='third')}):
                 with patch.dict(ini_manage.__salt__, {'ini.get_option': MagicMock(return_value="I don't know")}):
                     with patch.dict(ini_manage.__salt__, {'ini.set_option': MagicMock(return_value=desired)}):
@@ -107,9 +109,10 @@ class IniManageTestCase(TestCase, LoaderModuleMockMixin):
                'changes': {}}
 
         with patch.dict(ini_manage.__opts__, {'test': True}):
-            comt = 'No changes detected.'
-            ret.update({'comment': comt, 'result': True})
-            self.assertDictEqual(ini_manage.sections_present(name), ret)
+            with patch.dict(ini_manage.__salt__, {'ini.get_ini': MagicMock(return_value=None)}):
+                comt = 'No changes detected.'
+                ret.update({'comment': comt, 'result': True})
+                self.assertDictEqual(ini_manage.sections_present(name), ret)
 
         changes = {'first': 'who is on',
                    'second': 'what is on',
@@ -134,9 +137,10 @@ class IniManageTestCase(TestCase, LoaderModuleMockMixin):
                'changes': {}}
 
         with patch.dict(ini_manage.__opts__, {'test': True}):
-            comt = 'No changes detected.'
-            ret.update({'comment': comt, 'result': True})
-            self.assertDictEqual(ini_manage.sections_absent(name), ret)
+            with patch.dict(ini_manage.__salt__, {'ini.get_ini': MagicMock(return_value=None)}):
+                comt = 'No changes detected.'
+                ret.update({'comment': comt, 'result': True})
+                self.assertDictEqual(ini_manage.sections_absent(name), ret)
 
         with patch.dict(ini_manage.__opts__, {'test': False}):
             comt = ('No anomaly detected')