From 12c0220c5990f6d25756c24886cdf5a4f55c713c Mon Sep 17 00:00:00 2001 From: marcus67 Date: Mon, 24 Jan 2022 06:39:37 +0100 Subject: [PATCH] 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 * Apply suggestions from code review Co-authored-by: Felix Fontein * Added tests Fixed error occurring when called without options Added changelog snippet * Remove changelog entry as suggested in review Co-authored-by: Felix Fontein * Apply suggestions from code review Co-authored-by: Felix Fontein * 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 --- ...3943-add-option-options-to-snap-module.yml | 2 + plugins/modules/packaging/os/snap.py | 163 +++++++++++++++++- tests/integration/targets/snap/tasks/main.yml | 95 ++++++++-- 3 files changed, 235 insertions(+), 25 deletions(-) create mode 100644 changelogs/fragments/3943-add-option-options-to-snap-module.yml diff --git a/changelogs/fragments/3943-add-option-options-to-snap-module.yml b/changelogs/fragments/3943-add-option-options-to-snap-module.yml new file mode 100644 index 0000000000..021d0ec6d7 --- /dev/null +++ b/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). diff --git a/plugins/modules/packaging/os/snap.py b/plugins/modules/packaging/os/snap.py index 578abe215c..8406135389 100644 --- a/plugins/modules/packaging/os/snap.py +++ b/plugins/modules/packaging/os/snap.py @@ -2,6 +2,7 @@ # -*- coding: utf-8 -*- # Copyright: (c) 2021, Alexei Znamensky (russoz) +# Copyright: (c) 2021, Marcus Rickert # Copyright: (c) 2018, Stanislas Lange (angristan) # Copyright: (c) 2018, Victor Carceler @@ -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) @@ -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\S+)') + __set_param_re = re.compile(r'(?P\S+:)?(?P\S+)=(?P\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): - 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 + def process_actionable_snaps(self, actionable_snaps): 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: diff --git a/tests/integration/targets/snap/tasks/main.yml b/tests/integration/targets/snap/tasks/main.yml index 9ca431bafe..502a82d914 100644 --- a/tests/integration/targets/snap/tasks/main.yml +++ b/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