From 80beba88da1e828f43becf9e869a3cc7bcb87ee6 Mon Sep 17 00:00:00 2001 From: Kevin Carter Date: Sat, 6 Aug 2016 19:25:59 -0500 Subject: [PATCH] Add option to toggle list extensions By default a list item in a JSON or YAML format will extend if a its already defined in the target template and a config_override using a list is being set for the existing "key". This change allows that functionality to be toggled on or off. The boolean option to enable list extension is ``list_extend``. To maintain the API this feature will remain True by default. Co-Authored-By: Travis Truman Change-Id: I97e06aef2cc778f048f3d6863fe61d10eddb8602 Signed-off-by: Kevin Carter --- action/_v1_config_template.py | 23 ++-- action/_v2_config_template.py | 29 +++-- library/config_template | 10 ++ .../list-extend-toggle-46a75ded97b7ce02.yaml | 6 + tests/files/test_extend.yml.expected | 8 ++ tests/files/test_no_extend.yml.expected | 5 + tests/templates/test.ini | 6 + tests/templates/test.yml | 8 ++ tests/test-config_template.yml | 99 ++++++++++++++ tests/test-filters.yml | 122 ++++++++++++++++++ tests/test.yml | 105 +-------------- 11 files changed, 299 insertions(+), 122 deletions(-) create mode 100644 releasenotes/notes/list-extend-toggle-46a75ded97b7ce02.yaml create mode 100644 tests/files/test_extend.yml.expected create mode 100644 tests/files/test_no_extend.yml.expected create mode 100644 tests/templates/test.ini create mode 100644 tests/templates/test.yml create mode 100644 tests/test-config_template.yml create mode 100644 tests/test-filters.yml diff --git a/action/_v1_config_template.py b/action/_v1_config_template.py index 0afffb5..579c7f8 100644 --- a/action/_v1_config_template.py +++ b/action/_v1_config_template.py @@ -267,7 +267,7 @@ class ActionModule(object): else: config.set(str(section), str(key), str(value)) - def return_config_overrides_ini(self, config_overrides, resultant): + def return_config_overrides_ini(self, config_overrides, resultant, list_extend=True): """Returns string value from a modified config file. :param config_overrides: ``dict`` @@ -308,7 +308,7 @@ class ActionModule(object): finally: resultant_bytesio.close() - def return_config_overrides_json(self, config_overrides, resultant): + def return_config_overrides_json(self, config_overrides, resultant, list_extend=True): """Returns config json Its important to note that file ordering will not be preserved as the @@ -321,7 +321,8 @@ class ActionModule(object): original_resultant = json.loads(resultant) merged_resultant = self._merge_dict( base_items=original_resultant, - new_items=config_overrides + new_items=config_overrides, + list_extend=list_extend ) return json.dumps( merged_resultant, @@ -329,7 +330,7 @@ class ActionModule(object): sort_keys=True ) - def return_config_overrides_yaml(self, config_overrides, resultant): + def return_config_overrides_yaml(self, config_overrides, resultant, list_extend=True): """Return config yaml. :param config_overrides: ``dict`` @@ -339,7 +340,8 @@ class ActionModule(object): original_resultant = yaml.safe_load(resultant) merged_resultant = self._merge_dict( base_items=original_resultant, - new_items=config_overrides + new_items=config_overrides, + list_extend=list_extend ) return yaml.safe_dump( merged_resultant, @@ -347,7 +349,7 @@ class ActionModule(object): width=1000, ) - def _merge_dict(self, base_items, new_items): + def _merge_dict(self, base_items, new_items, list_extend=True): """Recursively merge new_items into base_items. :param base_items: ``dict`` @@ -364,7 +366,10 @@ class ActionModule(object): base_items[key] = re.split(', |,|\n', value) base_items[key] = [i.strip() for i in base_items[key] if i] elif isinstance(value, list): - base_items[key] = value + if isinstance(base_items.get(key), list) and list_extend: + base_items[key].extend(value) + else: + base_items[key] = value else: base_items[key] = new_items[key] return base_items @@ -418,7 +423,8 @@ class ActionModule(object): type_merger = getattr(self, CONFIG_TYPES.get(config_type)) resultant = type_merger( config_overrides=config_overrides, - resultant=resultant + resultant=resultant, + list_extend=options.get('list_extend', True) ) # Retemplate the resultant object as it may have new data within it @@ -446,6 +452,7 @@ class ActionModule(object): # Remove data types that are not available to the copy module complex_args.pop('config_overrides') complex_args.pop('config_type') + complex_args.pop('list_extend', None) # Return the copy module status. Access to protected method is # unavoidable in Ansible 1.x. diff --git a/action/_v2_config_template.py b/action/_v2_config_template.py index 6870f96..dd6301f 100644 --- a/action/_v2_config_template.py +++ b/action/_v2_config_template.py @@ -262,7 +262,7 @@ class ConfigTemplateParser(ConfigParser.RawConfigParser): class ActionModule(ActionBase): TRANSFERS_FILES = True - def return_config_overrides_ini(self, config_overrides, resultant): + def return_config_overrides_ini(self, config_overrides, resultant, list_extend=True): """Returns string value from a modified config file. :param config_overrides: ``dict`` @@ -338,7 +338,7 @@ class ActionModule(ActionBase): else: config.set(str(section), str(key), str(value)) - def return_config_overrides_json(self, config_overrides, resultant): + def return_config_overrides_json(self, config_overrides, resultant, list_extend=True): """Returns config json Its important to note that file ordering will not be preserved as the @@ -351,7 +351,8 @@ class ActionModule(ActionBase): original_resultant = json.loads(resultant) merged_resultant = self._merge_dict( base_items=original_resultant, - new_items=config_overrides + new_items=config_overrides, + list_extend=list_extend ) return json.dumps( merged_resultant, @@ -359,7 +360,7 @@ class ActionModule(ActionBase): sort_keys=True ) - def return_config_overrides_yaml(self, config_overrides, resultant): + def return_config_overrides_yaml(self, config_overrides, resultant, list_extend=True): """Return config yaml. :param config_overrides: ``dict`` @@ -369,7 +370,8 @@ class ActionModule(ActionBase): original_resultant = yaml.safe_load(resultant) merged_resultant = self._merge_dict( base_items=original_resultant, - new_items=config_overrides + new_items=config_overrides, + list_extend=list_extend ) return yaml.safe_dump( merged_resultant, @@ -377,7 +379,7 @@ class ActionModule(ActionBase): width=1000, ) - def _merge_dict(self, base_items, new_items): + def _merge_dict(self, base_items, new_items, list_extend=True): """Recursively merge new_items into base_items. :param base_items: ``dict`` @@ -387,14 +389,15 @@ class ActionModule(ActionBase): for key, value in new_items.iteritems(): if isinstance(value, dict): base_items[key] = self._merge_dict( - base_items.get(key, {}), - value + base_items=base_items.get(key, {}), + new_items=value, + list_extend=list_extend ) elif not isinstance(value, int) and (',' in value or '\n' in value): base_items[key] = re.split(',|\n', value) base_items[key] = [i.strip() for i in base_items[key] if i] elif isinstance(value, list): - if key in base_items and isinstance(base_items[key], list): + if isinstance(base_items.get(key), list) and list_extend: base_items[key].extend(value) else: base_items[key] = value @@ -448,6 +451,7 @@ class ActionModule(ActionBase): searchpath.insert(1, os.path.dirname(source)) _dest = self._task.args.get('dest') + list_extend = self._task.args.get('list_extend') if not _dest: return False, dict( failed=True, @@ -464,7 +468,8 @@ class ActionModule(ActionBase): dest=user_dest, config_overrides=self._task.args.get('config_overrides', dict()), config_type=config_type, - searchpath=searchpath + searchpath=searchpath, + list_extend=list_extend ) def run(self, tmp=None, task_vars=None): @@ -531,7 +536,8 @@ class ActionModule(ActionBase): type_merger = getattr(self, CONFIG_TYPES.get(_vars['config_type'])) resultant = type_merger( config_overrides=_vars['config_overrides'], - resultant=resultant + resultant=resultant, + list_extend=_vars.get('list_extend', True) ) # Re-template the resultant object as it may have new data within it @@ -562,6 +568,7 @@ class ActionModule(ActionBase): # Remove data types that are not available to the copy module new_module_args.pop('config_overrides', None) new_module_args.pop('config_type', None) + new_module_args.pop('list_extend', None) # Run the copy module return self._execute_module( diff --git a/library/config_template b/library/config_template index 0c343aa..f789b6c 100644 --- a/library/config_template +++ b/library/config_template @@ -39,6 +39,16 @@ options: - ini - json - yaml + list_extend: + description: + - By default a list item in a JSON or YAML format will extend if + its already defined in the target template and a config_override + using a list is being set for the existing "key". This functionality + can be toggled on or off using this option. If disabled an override + list will replace an existing "key". + choices: + - True + - False author: Kevin Carter """ diff --git a/releasenotes/notes/list-extend-toggle-46a75ded97b7ce02.yaml b/releasenotes/notes/list-extend-toggle-46a75ded97b7ce02.yaml new file mode 100644 index 0000000..f57c61f --- /dev/null +++ b/releasenotes/notes/list-extend-toggle-46a75ded97b7ce02.yaml @@ -0,0 +1,6 @@ +--- +features: + - The config_template action plugin now has a new option to + toggle list extension for JSON or YAML formats. The new + option is ``list_extend`` and is a boolean. The default + is True which maintains the existing API. diff --git a/tests/files/test_extend.yml.expected b/tests/files/test_extend.yml.expected new file mode 100644 index 0000000..b81b2e3 --- /dev/null +++ b/tests/files/test_extend.yml.expected @@ -0,0 +1,8 @@ +list_one: +- one +- two +- three +- four +list_two: +- one +- two diff --git a/tests/files/test_no_extend.yml.expected b/tests/files/test_no_extend.yml.expected new file mode 100644 index 0000000..b864299 --- /dev/null +++ b/tests/files/test_no_extend.yml.expected @@ -0,0 +1,5 @@ +list_one: +- four +list_two: +- one +- two diff --git a/tests/templates/test.ini b/tests/templates/test.ini new file mode 100644 index 0000000..c264f35 --- /dev/null +++ b/tests/templates/test.ini @@ -0,0 +1,6 @@ +[DEFAULT] + +[foo] +baz = baz + +[bar] \ No newline at end of file diff --git a/tests/templates/test.yml b/tests/templates/test.yml new file mode 100644 index 0000000..8fdb5d3 --- /dev/null +++ b/tests/templates/test.yml @@ -0,0 +1,8 @@ +list_one: + - one + - two + - three + +list_two: + - one + - two diff --git a/tests/test-config_template.yml b/tests/test-config_template.yml new file mode 100644 index 0000000..47dd59c --- /dev/null +++ b/tests/test-config_template.yml @@ -0,0 +1,99 @@ +--- +# Copyright 2016, Comcast Corp. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +- name: Test config_template + hosts: localhost + connection: local + gather_facts: no + tasks: + # Test basic function of config_template + - name: Template test INI template + config_template: + src: "{{ playbook_dir }}/templates/test.ini" + dest: "/tmp/test.ini" + config_overrides: "{{ test_config_ini_overrides }}" + config_type: "ini" + + - name: Read test.ini + slurp: + src: /tmp/test.ini + register: ini_file + - debug: + msg: "ini - {{ ini_file.content | b64decode }}" + - name: Validate output + assert: + that: + - "(lookup('ini', 'new_key section=DEFAULT file=/tmp/test.ini')) == 'new_value'" + - "(lookup('ini', 'baz section=foo file=/tmp/test.ini')) == 'bar'" + + # Test list additions in config_template + - name: Template test YML template + config_template: + src: "{{ playbook_dir }}/templates/test.yml" + dest: "/tmp/test_extend.yml" + config_overrides: "{{ test_config_yml_overrides }}" + config_type: "yaml" + list_extend: True + + - name: Read test_extend.yml + slurp: + src: /tmp/test_extend.yml + register: extend_file + - name: Read expected test_extend.yml + slurp: + src: "{{ playbook_dir }}/files/test_extend.yml.expected" + register: extend_file_expected + - debug: + msg: "extend - {{ extend_file.content | b64decode }}" + - debug: + msg: "extend.expected - {{ extend_file_expected.content | b64decode }}" + - name: Compare files + assert: + that: + - "(extend_file.content | b64decode) == (extend_file_expected.content | b64decode)" + + # Test list replacement in config_template + - name: Template test YML template + config_template: + src: "{{ playbook_dir }}/templates/test.yml" + dest: "/tmp/test_no_extend.yml" + config_overrides: "{{ test_config_yml_overrides }}" + config_type: "yaml" + list_extend: False + - name: Read test_no_extend.yml + slurp: + src: /tmp/test_no_extend.yml + register: no_extend_file + - name: Read expected test_no_extend.yml + slurp: + src: "{{ playbook_dir }}/files/test_no_extend.yml.expected" + register: no_extend_file_expected + - debug: + msg: "no_extend - {{ no_extend_file.content | b64decode }}" + - debug: + msg: "no_extend.expected - {{ no_extend_file_expected.content | b64decode }}" + - name: Compare files + assert: + that: + - "(no_extend_file.content | b64decode) == (no_extend_file_expected.content | b64decode)" + vars: + test_config_ini_overrides: + DEFAULT: + new_key: "new_value" + foo: + baz: "bar" + test_config_yml_overrides: + list_one: + - four \ No newline at end of file diff --git a/tests/test-filters.yml b/tests/test-filters.yml new file mode 100644 index 0000000..5ef9bf3 --- /dev/null +++ b/tests/test-filters.yml @@ -0,0 +1,122 @@ +--- +# Copyright 2016, @WalmartLabs +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + + +- name: Test filters + hosts: localhost + connection: local + gather_facts: no + tasks: + - name: Validate bit_length_power_of_2 filter + assert: + that: + - "{{ 1024 | bit_length_power_of_2 }} == 1024" + - "{{ 9600 | bit_length_power_of_2 }} == 16384" + + - name: Set net filter facts + set_fact: + url_netloc: "{{ 'http://review.openstack.org:29418/something' | netloc }}" + url_netloc_no_port: "{{ 'http://review.openstack.org:29418/something' | netloc_no_port }}" + url_netorigin: "{{ 'http://review.openstack.org:29418/something' | netorigin }}" + - name: Validate net filters + assert: + that: + - "url_netloc == 'review.openstack.org:29418'" + - "url_netloc_no_port == 'review.openstack.org'" + - "url_netorigin == 'http://review.openstack.org:29418'" + + - name: Validate string_2_int filter + assert: + that: + - "{{ 'test' | string_2_int }} == 3752" + + - name: Set pip package list facts + set_fact: + pip_package_list_1: + - pip==8.1.2 + - setuptools==25.1.0 + - wheel==0.29.0 + pip_package_list_1_names: + - pip + - setuptools + - wheel + pip_package_list_2: + - babel==2.3.4 + - pip==8.1.0 + pip_package_list_merged: + - babel==2.3.4 + - pip==8.1.0 + - setuptools==25.1.0 + - wheel==0.29.0 + - name: Set pip package filter facts + set_fact: + pip_package_list_1_names_filtered: "{{ pip_package_list_1 | pip_requirement_names }}" + pip_package_list_constraint_filtered: "{{ pip_package_list_1 | pip_constraint_update(pip_package_list_2) }}" + - name: Validate pip requirement filters + assert: + that: + - "pip_package_list_1_names_filtered == pip_package_list_1_names" + - "pip_package_list_constraint_filtered == pip_package_list_merged" + + - name: Set splitlines string facts + set_fact: + string_with_lines: | + this + is + a + test + string_split_lines: + - this + - is + - a + - test + - name: Set splitlines filter fact + set_fact: + string_split_lines_filtered: "{{ string_with_lines | splitlines }}" + - name: Validate splitlines filter + assert: + that: "string_split_lines_filtered == string_split_lines" + + - name: Set git repo facts + set_fact: + git_repo: "git+https://git.openstack.org/openstack/nova@2bc8128d7793cc72ca2e146de3a092e1fef5033b#egg=nova&gitname=nova" + git_repo_name: nova + git_repo_link_parts: + name: nova + version: 2bc8128d7793cc72ca2e146de3a092e1fef5033b + plugin_path: null + url: "https://git.openstack.org/openstack/nova" + original: "git+https://git.openstack.org/openstack/nova@2bc8128d7793cc72ca2e146de3a092e1fef5033b#egg=nova&gitname=nova" + - name: Set git link parse filter facts + set_fact: + git_repo_link_parse_filtered: "{{ git_repo | git_link_parse }}" + git_repo_link_parse_name_filtered: "{{ git_repo | git_link_parse_name }}" + - name: Validate git link parse filters + assert: + that: + - "git_repo_link_parse_filtered == git_repo_link_parts" + - "git_repo_link_parse_name_filtered == git_repo_name" + + - name: Set deprecation variable facts + set_fact: + new_var: new + old_var: old + - name: Set deprecated filter fact + set_fact: + deprecated_value: "{{ new_var | deprecated(old_var, 'old_var', 'new_var', 'Next Release', false) }}" + - name: Validate deprecated filter + assert: + that: "deprecated_value == old_var" diff --git a/tests/test.yml b/tests/test.yml index de3f186..f79b6cb 100644 --- a/tests/test.yml +++ b/tests/test.yml @@ -13,108 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -- name: Test filters - hosts: localhost - connection: local - gather_facts: no - tasks: - - name: Validate bit_length_power_of_2 filter - assert: - that: - - "{{ 1024 | bit_length_power_of_2 }} == 1024" - - "{{ 9600 | bit_length_power_of_2 }} == 16384" - - name: Set net filter facts - set_fact: - url_netloc: "{{ 'http://review.openstack.org:29418/something' | netloc }}" - url_netloc_no_port: "{{ 'http://review.openstack.org:29418/something' | netloc_no_port }}" - url_netorigin: "{{ 'http://review.openstack.org:29418/something' | netorigin }}" - - name: Validate net filters - assert: - that: - - "url_netloc == 'review.openstack.org:29418'" - - "url_netloc_no_port == 'review.openstack.org'" - - "url_netorigin == 'http://review.openstack.org:29418'" +- include: test-config_template.yml - - name: Validate string_2_int filter - assert: - that: - - "{{ 'test' | string_2_int }} == 3752" - - - name: Set pip package list facts - set_fact: - pip_package_list_1: - - pip==8.1.2 - - setuptools==25.1.0 - - wheel==0.29.0 - pip_package_list_1_names: - - pip - - setuptools - - wheel - pip_package_list_2: - - babel==2.3.4 - - pip==8.1.0 - pip_package_list_merged: - - babel==2.3.4 - - pip==8.1.0 - - setuptools==25.1.0 - - wheel==0.29.0 - - name: Set pip package filter facts - set_fact: - pip_package_list_1_names_filtered: "{{ pip_package_list_1 | pip_requirement_names }}" - pip_package_list_constraint_filtered: "{{ pip_package_list_1 | pip_constraint_update(pip_package_list_2) }}" - - name: Validate pip requirement filters - assert: - that: - - "pip_package_list_1_names_filtered == pip_package_list_1_names" - - "pip_package_list_constraint_filtered == pip_package_list_merged" - - - name: Set splitlines string facts - set_fact: - string_with_lines: | - this - is - a - test - string_split_lines: - - this - - is - - a - - test - - name: Set splitlines filter fact - set_fact: - string_split_lines_filtered: "{{ string_with_lines | splitlines }}" - - name: Validate splitlines filter - assert: - that: "string_split_lines_filtered == string_split_lines" - - - name: Set git repo facts - set_fact: - git_repo: "git+https://git.openstack.org/openstack/nova@2bc8128d7793cc72ca2e146de3a092e1fef5033b#egg=nova&gitname=nova" - git_repo_name: nova - git_repo_link_parts: - name: nova - version: 2bc8128d7793cc72ca2e146de3a092e1fef5033b - plugin_path: null - url: "https://git.openstack.org/openstack/nova" - original: "git+https://git.openstack.org/openstack/nova@2bc8128d7793cc72ca2e146de3a092e1fef5033b#egg=nova&gitname=nova" - - name: Set git link parse filter facts - set_fact: - git_repo_link_parse_filtered: "{{ git_repo | git_link_parse }}" - git_repo_link_parse_name_filtered: "{{ git_repo | git_link_parse_name }}" - - name: Validate git link parse filters - assert: - that: - - "git_repo_link_parse_filtered == git_repo_link_parts" - - "git_repo_link_parse_name_filtered == git_repo_name" - - - name: Set deprecation variable facts - set_fact: - new_var: new - old_var: old - - name: Set deprecated filter fact - set_fact: - deprecated_value: "{{ new_var | deprecated(old_var, 'old_var', 'new_var', 'Next Release', false) }}" - - name: Validate deprecated filter - assert: - that: "deprecated_value == old_var" +- include: test-filters.yml