Add option "options" to snap module (#3943)

* Add functionality proposed in https://github.com/ansible-collections/community.general/issues/666

* Fix pylint errors mentioned in CI pipeline

* Fix pylint errors mentioned in CI pipeline (continued)

* Update plugins/modules/packaging/os/snap.py

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

* Apply suggestions from code review

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

* Added tests
Fixed error occurring when called without options
Added changelog snippet

* Remove changelog entry as suggested in review

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

* Apply suggestions from code review

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

* rewrite `if len(overall_options_changed) > 0` in a more Pythonic way
un-indent `if len(overall_options_changed) > 0` to only be executed after the options of all snaps have been checked

* better placement of local variable `overall_options_changed`

* Re-arrange code to reduce indentation level (suggested by reviewer)

* Re-arrange code to reduce indentation level (suggested by reviewer, continued)

* Re-arrange code to reduce indentation level (suggested by reviewer, continued)
Raise exception if option map returned by `snap set` contains list container (suggested by reviewer)
Handle Python2 type `long` correctly (suggested by reviewer)

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

changelogs/fragments/3943-add-option-options-to-snap-module.yml

@@ -0,0 +1,2 @@
+minor_changes:
+   - snap - add option ``options`` permitting to set options using the ``snap set`` command (https://github.com/ansible-collections/community.general/pull/3943).

plugins/modules/packaging/os/snap.py

@@ -2,6 +2,7 @@
 # -*- coding: utf-8 -*-
 
 # Copyright: (c) 2021, Alexei Znamensky (russoz) <russoz@gmail.com>
+# Copyright: (c) 2021, Marcus Rickert <marcus.rickert@web.de>
 # Copyright: (c) 2018, Stanislas Lange (angristan) <angristan@pm.me>
 # Copyright: (c) 2018, Victor Carceler <vcarceler@iespuigcastellar.xeill.net>
 
@@ -46,6 +47,15 @@ options:
         type: str
         required: false
         default: stable
+    options:
+        description:
+            - Set options with pattern C(key=value) or C(snap:key=value). If a snap name is given, the option will be applied
+              to that snap only. If the snap name is omitted, the options will be applied to all snaps listed in I(name). Options will
+              only be applied to active snaps.
+        required: false
+        type: list
+        elements: str
+        version_added: 4.3.0
 
 author:
     - Victor Carceler (@vcarceler) <vcarceler@iespuigcastellar.xeill.net>
@@ -63,6 +73,26 @@ EXAMPLES = '''
       - foo
       - bar
 
+# Install "foo" snap with options par1=A and par2=B
+- name: Install "foo" with options
+  community.general.snap:
+    name:
+      - foo
+    options:
+      - par1=A
+      - par2=B
+
+# Install "foo" and "bar" snaps with common option com=A and specific options fooPar=X and barPar=Y
+- name: Install "foo" and "bar" with options
+  community.general.snap:
+    name:
+      - foo
+      - bar
+    options:
+      - com=A
+      - foo:fooPar=X
+      - bar:barPar=Y
+
 # Remove "foo" snap
 - name: Remove foo
   community.general.snap:
@@ -103,9 +133,15 @@ snaps_removed:
     description: The list of actually removed snaps
     type: list
     returned: When any snaps have been removed
+options_changed:
+    description: The list of options set/changed in format C(snap:key=value).
+    type: list
+    returned: When any options have been changed/set
 '''
 
 import re
+import json
+import numbers
 
 from ansible.module_utils.common.text.converters import to_native
 
@@ -121,6 +157,8 @@ __state_map = dict(
     disabled='disable',
     info='info',  # not public
     list='list',  # not public
+    set='set',  # not public
+    get='get',  # not public
 )
 
 
@@ -130,6 +168,7 @@ def _state_map(value):
 
 class Snap(CmdStateModuleHelper):
     __disable_re = re.compile(r'(?:\S+\s+){5}(?P<notes>\S+)')
+    __set_param_re = re.compile(r'(?P<snap_prefix>\S+:)?(?P<key>\S+)=(?P<value>\S+)')
     module = dict(
         argument_spec={
             'name': dict(type='list', elements='str', required=True),
@@ -137,6 +176,7 @@ class Snap(CmdStateModuleHelper):
                           choices=['absent', 'present', 'enabled', 'disabled']),
             'classic': dict(type='bool', default=False),
             'channel': dict(type='str', default='stable'),
+            'options': dict(type='list', elements='str'),
         },
         supports_check_mode=True,
     )
@@ -146,6 +186,8 @@ class Snap(CmdStateModuleHelper):
         state=dict(fmt=_state_map),
         classic=dict(fmt="--classic", style=ArgFormat.BOOLEAN),
         channel=dict(fmt=lambda v: [] if v == 'stable' else ['--channel', '{0}'.format(v)]),
+        options=dict(fmt=list),
+        json_format=dict(fmt="-d", style=ArgFormat.BOOLEAN),
     )
     check_rc = False
 
@@ -171,6 +213,49 @@ class Snap(CmdStateModuleHelper):
             '\n'.join(results[3]),
         ]
 
+    def convert_json_subtree_to_map(self, json_subtree, prefix=None):
+        option_map = {}
+
+        if not isinstance(json_subtree, dict):
+            raise ModuleHelperException("Non-dict non-leaf element encountered while parsing option map. "
+                                        "The output format of 'snap set' may have changed. Aborting!")
+
+        for key, value in json_subtree.items():
+            full_key = key if prefix is None else prefix + "." + key
+
+            if isinstance(value, (str, float, bool, numbers.Integral)):
+                option_map[full_key] = str(value)
+
+            else:
+                option_map.update(self.convert_json_subtree_to_map(json_subtree=value, prefix=full_key))
+
+        return option_map
+
+    def convert_json_to_map(self, json_string):
+        json_object = json.loads(json_string)
+        return self.convert_json_subtree_to_map(json_object)
+
+    def retrieve_option_map(self, snap_name):
+        params = [{'state': 'get'}, {'name': snap_name}, {'json_format': True}]
+        rc, out, err = self.run_command(params=params)
+
+        if rc != 0:
+            return {}
+
+        result = out.splitlines()
+
+        if "has no configuration" in result[0]:
+            return {}
+
+        try:
+            option_map = self.convert_json_to_map(out)
+
+        except Exception as e:
+            raise ModuleHelperException(
+                msg="Parsing option map returned by 'snap get {0}' triggers exception '{1}', output:\n'{2}'".format(snap_name, str(e), out))
+
+        return option_map
+
     def is_snap_installed(self, snap_name):
         return 0 == self.run_command(params=[{'state': 'list'}, {'name': snap_name}])[0]
 
@@ -185,24 +270,23 @@ class Snap(CmdStateModuleHelper):
         notes = match.group('notes')
         return "disabled" not in notes.split(',')
 
-    def state_present(self):
+    def process_actionable_snaps(self, actionable_snaps):
-        self.vars.meta('classic').set(output=True)
-        self.vars.meta('channel').set(output=True)
-        actionable_snaps = [s for s in self.vars.name if not self.is_snap_installed(s)]
-        if not actionable_snaps:
-            return
         self.changed = True
         self.vars.snaps_installed = actionable_snaps
+
         if self.module.check_mode:
             return
+
         params = ['state', 'classic', 'channel']  # get base cmd parts
         has_one_pkg_params = bool(self.vars.classic) or self.vars.channel != 'stable'
         has_multiple_snaps = len(actionable_snaps) > 1
+
         if has_one_pkg_params and has_multiple_snaps:
             commands = [params + [{'actionable_snaps': [s]}] for s in actionable_snaps]
         else:
             commands = [params + [{'actionable_snaps': actionable_snaps}]]
         self.vars.cmd, rc, out, err = self._run_multiple_commands(commands)
+
         if rc == 0:
             return
 
@@ -217,6 +301,73 @@ class Snap(CmdStateModuleHelper):
                   "error output for more details.".format(cmd=self.vars.cmd)
         raise ModuleHelperException(msg=msg)
 
+    def state_present(self):
+
+        self.vars.meta('classic').set(output=True)
+        self.vars.meta('channel').set(output=True)
+        actionable_snaps = [s for s in self.vars.name if not self.is_snap_installed(s)]
+
+        if actionable_snaps:
+            self.process_actionable_snaps(actionable_snaps)
+
+        self.set_options()
+
+    def set_options(self):
+        if self.vars.options is None:
+            return
+
+        actionable_snaps = [s for s in self.vars.name if self.is_snap_installed(s)]
+        overall_options_changed = []
+
+        for snap_name in actionable_snaps:
+            option_map = self.retrieve_option_map(snap_name=snap_name)
+
+            options_changed = []
+
+            for option_string in self.vars.options:
+                match = self.__set_param_re.match(option_string)
+
+                if not match:
+                    msg = "Cannot parse set option '{option_string}'".format(option_string=option_string)
+                    raise ModuleHelperException(msg)
+
+                snap_prefix = match.group("snap_prefix")
+                selected_snap_name = snap_prefix[:-1] if snap_prefix else None
+
+                if selected_snap_name is not None and selected_snap_name not in self.vars.name:
+                    msg = "Snap option '{option_string}' refers to snap which is not in the list of snap names".format(option_string=option_string)
+                    raise ModuleHelperException(msg)
+
+                if selected_snap_name is None or (snap_name is not None and snap_name == selected_snap_name):
+                    key = match.group("key")
+                    value = match.group("value")
+
+                    if key not in option_map or key in option_map and option_map[key] != value:
+                        option_without_prefix = key + "=" + value
+                        option_with_prefix = option_string if selected_snap_name is not None else snap_name + ":" + option_string
+                        options_changed.append(option_without_prefix)
+                        overall_options_changed.append(option_with_prefix)
+
+            if options_changed:
+                self.changed = True
+
+                if not self.module.check_mode:
+                    params = [{'state': 'set'}, {'name': snap_name}, {'options': options_changed}]
+
+                    rc, out, err = self.run_command(params=params)
+
+                    if rc != 0:
+                        if 'has no "configure" hook' in err:
+                            msg = "Snap '{snap}' does not have any configurable options".format(snap=snap_name)
+                            raise ModuleHelperException(msg)
+
+                        msg = "Cannot set options '{options}' for snap '{snap}': error={error}".format(
+                            options=" ".join(options_changed), snap=snap_name, error=err)
+                        raise ModuleHelperException(msg)
+
+        if overall_options_changed:
+            self.vars.options_changed = overall_options_changed
+
     def _generic_state_action(self, actionable_func, actionable_var, params=None):
         actionable_snaps = [s for s in self.vars.name if actionable_func(s)]
         if not actionable_snaps:

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

@@ -5,38 +5,38 @@
 ####################################################################
 
 - block:
-    - name: Make sure package is not installed
+    - name: Make sure package is not installed (hello-world)
       community.general.snap:
         name: hello-world
         state: absent
 
-    - name: Install package (check mode)
+    - name: Install package (hello-world) (check mode)
       community.general.snap:
         name: hello-world
         state: present
       register: install_check
       check_mode: true
 
-    - name: Install package
+    - name: Install package (hello-world)
       community.general.snap:
         name: hello-world
         state: present
       register: install
 
-    - name: Install package again (check mode)
+    - name: Install package again (hello-world) (check mode)
       community.general.snap:
         name: hello-world
         state: present
       register: install_again_check
       check_mode: true
 
-    - name: Install package again
+    - name: Install package again (hello-world)
       community.general.snap:
         name: hello-world
         state: present
       register: install_again
 
-    - name: Assert package has been installed just once
+    - name: Assert package has been installed just once (hello-world)
       assert:
         that:
           - install is changed
@@ -44,38 +44,38 @@
           - install_again is not changed
           - install_again_check is not changed
 
-    - name: Check package has been installed correctly
+    - name: Check package has been installed correctly (hello-world)
       command: hello-world
       environment:
         PATH: /var/lib/snapd/snap/bin/
 
-    - name: Remove package (check mode)
+    - name: Remove package (hello-world) (check mode)
       community.general.snap:
         name: hello-world
         state: absent
       register: remove_check
       check_mode: true
 
-    - name: Remove package
+    - name: Remove package (hello-world)
       community.general.snap:
         name: hello-world
         state: absent
       register: remove
 
-    - name: Remove package again (check mode)
+    - name: Remove package again (hello-world) (check mode)
       community.general.snap:
         name: hello-world
         state: absent
       register: remove_again_check
       check_mode: true
 
-    - name: Remove package again
+    - name: Remove package again (hello-world)
       community.general.snap:
         name: hello-world
         state: absent
       register: remove_again
 
-    - name: Assert package has been removed just once
+    - name: Assert package has been removed just once (hello-world)
       assert:
         that:
           - remove is changed
@@ -83,12 +83,12 @@
           - remove_again is not changed
           - remove_again_check is not changed
 
-    - name: Make sure package from classic snap is not installed
+    - name: Make sure package from classic snap is not installed (nvim)
       community.general.snap:
         name: nvim
         state: absent
 
-    - name: Install package from classic snap
+    - name: Install package from classic snap (nvim)
       community.general.snap:
         name: nvim
         state: present
@@ -96,14 +96,14 @@
       register: classic_install
 
     # testing classic idempotency
-    - name: Install package from classic snap again
+    - name: Install package from classic snap again (nvim)
       community.general.snap:
         name: nvim
         state: present
         classic: true
       register: classic_install_again
 
-    - name: Assert package has been installed just once
+    - name: Assert package has been installed just once (nvim)
       assert:
         that:
           - classic_install is changed
@@ -111,22 +111,79 @@
 
     # this is just testing if a package which has been installed
     # with true classic can be removed without setting classic to true
-    - name: Remove package from classic snap without setting classic to true
+    - name: Remove package from classic snap without setting classic to true (nvim)
       community.general.snap:
         name: nvim
         state: absent
       register: classic_remove_without_true_classic
 
-    - name: Remove package from classic snap with setting classic to true
+    - name: Remove package from classic snap with setting classic to true (nvim)
       community.general.snap:
         name: nvim
         state: absent
         classic: true
       register: classic_remove_with_true_classic
 
-    - name: Assert package has been removed without setting classic to true
+    - name: Assert package has been removed without setting classic to true (nvim)
       assert:
         that:
           - classic_remove_without_true_classic is changed
           - classic_remove_with_true_classic is not changed
+
+
+    - name: Make sure package is not installed (uhttpd)
+      community.general.snap:
+        name: uhttpd
+        state: absent
+
+    - name: Install package (uhttpd)
+      community.general.snap:
+        name: uhttpd
+        state: present
+      register: install
+
+    - name: Install package (uhttpd)
+      community.general.snap:
+        name: uhttpd
+        state: present
+        options:
+          - "listening-port=8080"
+      register: install_with_option
+
+    - name: Install package again with option (uhttpd)
+      community.general.snap:
+        name: uhttpd
+        state: present
+        options:
+          - "listening-port=8080"
+      register: install_with_option_again
+
+    - name: Install package again with different options (uhttpd)
+      community.general.snap:
+        name: uhttpd
+        state: present
+        options:
+          - "listening-port=8088"
+          - "document-root-dir=/tmp"
+      register: install_with_option_changed
+
+    - name: Remove package (uhttpd)
+      community.general.snap:
+        name: uhttpd
+        state: absent
+      register: remove
+
+    - name: Assert package has been installed with options just once and only changed options trigger a change (uhttpd)
+      assert:
+        that:
+          - install is changed
+          - install_with_option is changed
+          - "install_with_option.options_changed[0] == 'uhttpd:listening-port=8080'"
+          - install_with_option_again is not changed
+          - install_with_option_changed is changed
+          - "'uhttpd:listening-port=8088' in install_with_option_changed.options_changed"
+          - "'uhttpd:document-root-dir=/tmp' in install_with_option_changed.options_changed"
+          - "'uhttpd:listening-port=8080' not in install_with_option_changed.options_changed"
+          - remove is changed
+
   when: has_snap