ini_file: fixed case of empty section + parameter validation (#1533)

* Fixed case of empty section + parameter validation

* Adjusted code for integration test, and added test case for empty section

* Update changelogs/fragments/479-ini_file-empty-section.yaml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelogs/fragments/479-ini_file-empty-section.yaml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Using "is changed" in integration tests, per PR review

Co-authored-by: Felix Fontein <felix@fontein.de>

changelogs/fragments/479-ini_file-empty-section.yaml

@@ -0,0 +1,4 @@
+minor_changes:
+  - ini_file - module now can create an empty section (https://github.com/ansible-collections/community.general/issues/479).
+bugfixes:
+  - ini_file - check for parameter ``value`` if ``state`` is ``present`` and ``allow_no_value`` is ``false`` (https://github.com/ansible-collections/community.general/issues/479).

plugins/modules/files/ini_file.py

@@ -143,11 +143,8 @@ def do_ini(module, filename, section=None, option=None, value=None,
             os.makedirs(destpath)
         ini_lines = []
     else:
-        ini_file = open(filename, 'r')
-        try:
+        with open(filename, 'r') as ini_file:
             ini_lines = ini_file.readlines()
-        finally:
-            ini_file.close()
 
     if module._diff:
         diff['before'] = ''.join(ini_lines)
@@ -172,7 +169,7 @@ def do_ini(module, filename, section=None, option=None, value=None,
     # Insert it at the beginning
     ini_lines.insert(0, '[%s]' % fake_section_name)
 
-    # At botton:
+    # At bottom:
     ini_lines.append('[')
 
     # If no section is defined, fake section is used
@@ -198,12 +195,14 @@ def do_ini(module, filename, section=None, option=None, value=None,
                     for i in range(index, 0, -1):
                         # search backwards for previous non-blank or non-comment line
                         if not re.match(r'^[ \t]*([#;].*)?$', ini_lines[i - 1]):
-                            if not value and allow_no_value:
-                                ini_lines.insert(i, '%s\n' % option)
-                            else:
+                            if option and value:
                                 ini_lines.insert(i, assignment_format % (option, value))
                                 msg = 'option added'
                                 changed = True
+                            elif option and not value and allow_no_value:
+                                ini_lines.insert(i, '%s\n' % option)
+                                msg = 'option added'
+                                changed = True
                             break
                 elif state == 'absent' and not option:
                     # remove the entire section
@@ -249,14 +248,16 @@ def do_ini(module, filename, section=None, option=None, value=None,
     del ini_lines[0]
     del ini_lines[-1:]
 
-    if not within_section and option and state == 'present':
+    if not within_section and state == 'present':
         ini_lines.append('[%s]\n' % section)
-        if not value and allow_no_value:
+        msg = 'section and option added'
+        if option and value:
+            ini_lines.append(assignment_format % (option, value))
+        elif option and not value and allow_no_value:
             ini_lines.append('%s\n' % option)
         else:
-            ini_lines.append(assignment_format % (option, value))
+            msg = 'only section added'
         changed = True
-        msg = 'section and option added'
 
     if module._diff:
         diff['after'] = ''.join(ini_lines)
@@ -311,6 +312,9 @@ def main():
     allow_no_value = module.params['allow_no_value']
     create = module.params['create']
 
+    if state == 'present' and not allow_no_value and not value:
+        module.fail_json("Parameter 'value' must not be empty if state=present and allow_no_value=False")
+
     (changed, backup_file, diff, msg) = do_ini(module, path, section, option, value, state, backup, no_extra_spaces, create, allow_no_value)
 
     if not module.check_mode and os.path.exists(path):

tests/integration/targets/ini_file/tasks/main.yml

@@ -36,7 +36,7 @@
 - name: verify ini_file 'changed' is true
   assert:
     that:
-      - result1.changed == True
+      - result1 is changed
       - result1.msg == 'section and option added'
 
 - name: read content from output file
@@ -68,7 +68,7 @@
 - name: Ensure unchanged
   assert:
     that:
-      - result2.changed == False
+      - result2 is not changed
       - result2.msg == 'OK'
 
 - name: Ensure "beverage=coke" is in section "[drinks]"
@@ -96,7 +96,7 @@
 - name: assert 'changed' is true and content is OK
   assert:
     that:
-      - result3.changed == True
+      - result3 is changed
       - result3.msg == 'option added'
       - content3 == expected3
 
@@ -120,7 +120,7 @@
 - name: assert changed and content is as expected
   assert:
     that:
-      - result4.changed == True
+      - result4 is changed
       - result4.msg == 'option changed'
       - content4 == expected1
 
@@ -143,7 +143,7 @@
 - name: assert changed and content is empty
   assert:
     that:
-      - result5.changed == True
+      - result5 is changed
       - result5.msg == 'section removed'
       - content5 == "\n"
 
@@ -160,7 +160,7 @@
 - name: assert section and option added
   assert:
     that:
-      - result6.changed == True
+      - result6 is changed
       - result6.msg == 'section and option added'
 
 - name: test allow_no_value idempotency
@@ -174,9 +174,42 @@
 - name: assert 'changed' false
   assert:
     that:
-      - result6.changed == False
+      - result6 is not changed
       - result6.msg == 'OK'
 
+- name: test create empty section
+  ini_file:
+    path: "{{ output_file }}"
+    section: new_empty_section
+    allow_no_value: yes
+  register: result6a
+
+- name: assert section added
+  assert:
+    that:
+      - result6a is changed
+      - result6a.msg == 'only section added'
+
+- name: test create empty section idempotency
+  ini_file:
+    path: "{{ output_file }}"
+    section: new_empty_section
+    allow_no_value: yes
+  register: result6a
+
+- name: assert 'changed' false
+  assert:
+    that:
+      - result6a is not changed
+      - result6a.msg == 'OK'
+
+- name: test remove empty section
+  ini_file:
+    state: absent
+    path: "{{ output_file }}"
+    section: new_empty_section
+    allow_no_value: yes
+
 - name: test allow_no_value with loop
   ini_file:
     path: "{{ output_file }}"
@@ -234,7 +267,7 @@
 - name: assert 'changed' and msg 'option changed' and content is as expected
   assert:
     that:
-      - result8.changed == True
+      - result8 is changed
       - result8.msg == 'option changed'
       - content8 == expected8
 
@@ -264,7 +297,7 @@
 - name: assert 'changed' and msg 'option changed' and content is as expected
   assert:
     that:
-      - result9.changed == True
+      - result9 is changed
       - result9.msg == 'option changed'
       - content9 == expected9
 
@@ -293,7 +326,7 @@
 - name: assert 'changed' and msg 'option changed' and content is as expected
   assert:
     that:
-      - result10.changed == True
+      - result10 is changed
       - result10.msg == 'option changed'
       - content10 == expected10