From d27ce4cef30b87defaccdaaa0039ee18a3f4cce2 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 25 May 2021 10:35:17 -0500 Subject: [PATCH] Remove deprecated features from ansible.playbook.helpers (#74809) * Remove deprecated features from ansible.playbook.helpers. Fixes #74135 --- .../74135-remove-include-deprecations.yml | 3 + lib/ansible/config/base.yml | 27 ------- lib/ansible/playbook/helpers.py | 78 ++++--------------- lib/ansible/playbook/included_file.py | 2 - lib/ansible/playbook/task_include.py | 5 -- .../include_import/undefined_var/playbook.yml | 3 +- .../test_includes/tasks/branch_toplevel.yml | 10 ++- .../roles/test_includes/tasks/main.yml | 24 ++++-- test/sanity/ignore.txt | 1 - test/units/playbook/test_helpers.py | 30 +------ 10 files changed, 43 insertions(+), 140 deletions(-) create mode 100644 changelogs/fragments/74135-remove-include-deprecations.yml diff --git a/changelogs/fragments/74135-remove-include-deprecations.yml b/changelogs/fragments/74135-remove-include-deprecations.yml new file mode 100644 index 00000000000..6dbe97fb9a5 --- /dev/null +++ b/changelogs/fragments/74135-remove-include-deprecations.yml @@ -0,0 +1,3 @@ +bugfixes: +- include - Remove deprecated ``static`` argument for ``include`` + (https://github.com/ansible/ansible/issues/74135) diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index 121f1b173fd..949afab57cc 100644 --- a/lib/ansible/config/base.yml +++ b/lib/ansible/config/base.yml @@ -697,19 +697,6 @@ DEFAULT_GATHER_TIMEOUT: - {key: gather_timeout, section: defaults} type: integer yaml: {key: defaults.gather_timeout} -DEFAULT_HANDLER_INCLUDES_STATIC: - name: Make handler M(ansible.builtin.include) static - default: False - description: - - "Since 2.0 M(ansible.builtin.include) can be 'dynamic', this setting (if True) forces that if the include appears in a ``handlers`` section to be 'static'." - env: [{name: ANSIBLE_HANDLER_INCLUDES_STATIC}] - ini: - - {key: handler_includes_static, section: defaults} - type: boolean - deprecated: - why: include itself is deprecated and this setting will not matter in the future - version: "2.12" - alternatives: none as its already built into the decision between include_tasks and import_tasks DEFAULT_HASH_BEHAVIOUR: name: Hash merge behaviour default: replace @@ -1095,20 +1082,6 @@ DEFAULT_SYSLOG_FACILITY: env: [{name: ANSIBLE_SYSLOG_FACILITY}] ini: - {key: syslog_facility, section: defaults} -DEFAULT_TASK_INCLUDES_STATIC: - name: Task include static - default: False - description: - - The `include` tasks can be static or dynamic, this toggles the default expected behaviour if autodetection fails and it is not explicitly set in task. - env: [{name: ANSIBLE_TASK_INCLUDES_STATIC}] - ini: - - {key: task_includes_static, section: defaults} - type: boolean - version_added: "2.1" - deprecated: - why: include itself is deprecated and this setting will not matter in the future - version: "2.12" - alternatives: None, as its already built into the decision between include_tasks and import_tasks DEFAULT_TERMINAL_PLUGIN_PATH: name: Terminal Plugins Path default: ~/.ansible/plugins/terminal:/usr/share/ansible/plugins/terminal diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index a31f9104b49..16873375f17 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -154,15 +154,9 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h is_static = False elif action in C._ACTION_IMPORT_TASKS: is_static = True - elif t.static is not None: - display.deprecated("The use of 'static' has been deprecated. " - "Use 'import_tasks' for static inclusion, or 'include_tasks' for dynamic inclusion", - version='2.12', collection_name='ansible.builtin') - is_static = t.static else: - is_static = C.DEFAULT_TASK_INCLUDES_STATIC or \ - (use_handlers and C.DEFAULT_HANDLER_INCLUDES_STATIC) or \ - (not templar.is_template(t.args['_raw_params']) and t.all_parents_static() and not t.loop) + display.deprecated('"include" is deprecated, use include_tasks/import_tasks instead', "2.16") + is_static = not templar.is_template(t.args['_raw_params']) and t.all_parents_static() and not t.loop if is_static: if t.loop is not None: @@ -234,35 +228,18 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h else: include_file = loader.path_dwim(include_target) - try: - data = loader.load_from_file(include_file) - if not data: - display.warning('file %s is empty and had no tasks to include' % include_file) - continue - elif not isinstance(data, list): - raise AnsibleParserError("included task files must contain a list of tasks", obj=data) - - # since we can't send callbacks here, we display a message directly in - # the same fashion used by the on_include callback. We also do it here, - # because the recursive nature of helper methods means we may be loading - # nested includes, and we want the include order printed correctly - display.vv("statically imported: %s" % include_file) - except AnsibleFileNotFound: - if action not in C._ACTION_INCLUDE or t.static or \ - C.DEFAULT_TASK_INCLUDES_STATIC or \ - C.DEFAULT_HANDLER_INCLUDES_STATIC and use_handlers: - raise - display.deprecated( - "Included file '%s' not found, however since this include is not " - "explicitly marked as 'static: yes', we will try and include it dynamically " - "later. In the future, this will be an error unless 'static: no' is used " - "on the include task. If you do not want missing includes to be considered " - "dynamic, use 'static: yes' on the include or set the global ansible.cfg " - "options to make all includes static for tasks and/or handlers" % include_file, - version="2.12", collection_name='ansible.builtin' - ) - task_list.append(t) + data = loader.load_from_file(include_file) + if not data: + display.warning('file %s is empty and had no tasks to include' % include_file) continue + elif not isinstance(data, list): + raise AnsibleParserError("included task files must contain a list of tasks", obj=data) + + # since we can't send callbacks here, we display a message directly in + # the same fashion used by the on_include callback. We also do it here, + # because the recursive nature of helper methods means we may be loading + # nested includes, and we want the include order printed correctly + display.vv("statically imported: %s" % include_file) ti_copy = t.copy(exclude_parent=True) ti_copy._parent = block @@ -277,28 +254,7 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h variable_manager=variable_manager, ) - # FIXME: remove once 'include' is removed - # pop tags out of the include args, if they were specified there, and assign - # them to the include. If the include already had tags specified, we raise an - # error so that users know not to specify them both ways - tags = ti_copy.vars.pop('tags', []) - if isinstance(tags, string_types): - tags = tags.split(',') - - if len(tags) > 0: - if action in C._ACTION_ALL_PROPER_INCLUDE_IMPORT_TASKS: - raise AnsibleParserError('You cannot specify "tags" inline to the task, it is a task keyword') - if len(ti_copy.tags) > 0: - raise AnsibleParserError( - "Include tasks should not specify tags in more than one way (both via args and directly on the task). " - "Mixing styles in which tags are specified is prohibited for whole import hierarchy, not only for single import statement", - obj=task_ds, - suppress_extended_error=True, - ) - display.deprecated("You should not specify tags in the include parameters. All tags should be specified using the task-level option", - version="2.12", collection_name='ansible.builtin') - else: - tags = ti_copy.tags[:] + tags = ti_copy.tags[:] # now we extend the tags on each of the included blocks for b in included_blocks: @@ -332,12 +288,6 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h if action in C._ACTION_IMPORT_ROLE: is_static = True - elif ir.static is not None: - display.deprecated("The use of 'static' for 'include_role' has been deprecated. " - "Use 'import_role' for static inclusion, or 'include_role' for dynamic inclusion", - version='2.12', collection_name='ansible.builtin') - is_static = ir.static - if is_static: if ir.loop is not None: if action in C._ACTION_IMPORT_ROLE: diff --git a/lib/ansible/playbook/included_file.py b/lib/ansible/playbook/included_file.py index c7ca3a94a23..7d334e82bc5 100644 --- a/lib/ansible/playbook/included_file.py +++ b/lib/ansible/playbook/included_file.py @@ -118,8 +118,6 @@ class IncludedFile: if original_task.action in C._ACTION_ALL_INCLUDE_TASKS: include_file = None - if original_task.static: - continue if original_task._parent: # handle relative includes by walking up the list of parent include diff --git a/lib/ansible/playbook/task_include.py b/lib/ansible/playbook/task_include.py index 48eee18803a..edfc54ea575 100644 --- a/lib/ansible/playbook/task_include.py +++ b/lib/ansible/playbook/task_include.py @@ -46,11 +46,6 @@ class TaskInclude(Task): 'loop_with', 'name', 'no_log', 'register', 'run_once', 'tags', 'timeout', 'vars', 'when')) - # ================================================================================= - # ATTRIBUTES - - _static = FieldAttribute(isa='bool', default=None) - def __init__(self, block=None, role=None, task_include=None): super(TaskInclude, self).__init__(block=block, role=role, task_include=task_include) self.statically_loaded = False diff --git a/test/integration/targets/include_import/undefined_var/playbook.yml b/test/integration/targets/include_import/undefined_var/playbook.yml index 0584fa8a681..6576d50ae96 100644 --- a/test/integration/targets/include_import/undefined_var/playbook.yml +++ b/test/integration/targets/include_import/undefined_var/playbook.yml @@ -26,8 +26,7 @@ - "_include_role_result is failed" msg: "'include_role' did not evaluate it's attached condition and failed" - - include: include_that_defines_var.yml - static: yes + - import_tasks: include_that_defines_var.yml when: - "_undefined == 'yes'" diff --git a/test/integration/targets/includes/roles/test_includes/tasks/branch_toplevel.yml b/test/integration/targets/includes/roles/test_includes/tasks/branch_toplevel.yml index 624167057b3..30cd6f28788 100644 --- a/test/integration/targets/includes/roles/test_includes/tasks/branch_toplevel.yml +++ b/test/integration/targets/includes/roles/test_includes/tasks/branch_toplevel.yml @@ -1,9 +1,11 @@ # 'canary2' used instead of 'canary', otherwise a "recursive loop detected in # template string" occurs when both includes use static=yes -- include: 'leaf_sublevel.yml canary2={{ canary }}' - static: yes +- import_tasks: leaf_sublevel.yml + vars: + canary2: '{{ canary }}' when: 'nested_include_static|bool' # value for 'static' can not be a variable, hence use 'when' -- include: 'leaf_sublevel.yml canary2={{ canary }}' - static: no +- include_tasks: leaf_sublevel.yml + vars: + canary2: '{{ canary }}' when: 'not nested_include_static|bool' diff --git a/test/integration/targets/includes/roles/test_includes/tasks/main.yml b/test/integration/targets/includes/roles/test_includes/tasks/main.yml index 6fcac9ebe51..83ca468baff 100644 --- a/test/integration/targets/includes/roles/test_includes/tasks/main.yml +++ b/test/integration/targets/includes/roles/test_includes/tasks/main.yml @@ -81,26 +81,34 @@ - included_handler - verify_handler -- include: branch_toplevel.yml canary=value1 nested_include_static=no - static: no +- include_tasks: branch_toplevel.yml + vars: + canary: value1 + nested_include_static: 'no' - assert: that: - 'canary_fact == "value1"' -- include: branch_toplevel.yml canary=value2 nested_include_static=yes - static: no +- include_tasks: branch_toplevel.yml + vars: + canary: value2 + nested_include_static: 'yes' - assert: that: - 'canary_fact == "value2"' -- include: branch_toplevel.yml canary=value3 nested_include_static=no - static: yes +- import_tasks: branch_toplevel.yml + vars: + canary: value3 + nested_include_static: 'no' - assert: that: - 'canary_fact == "value3"' -- include: branch_toplevel.yml canary=value4 nested_include_static=yes - static: yes +- import_tasks: branch_toplevel.yml + vars: + canary: value4 + nested_include_static: 'yes' - assert: that: - 'canary_fact == "value4"' diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 6eafb0b554f..df57c961a7b 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -114,7 +114,6 @@ lib/ansible/modules/yum_repository.py validate-modules:undocumented-parameter lib/ansible/parsing/vault/__init__.py pylint:blacklisted-name lib/ansible/playbook/base.py pylint:blacklisted-name lib/ansible/playbook/collectionsearch.py required-and-default-attributes # https://github.com/ansible/ansible/issues/61460 -lib/ansible/playbook/helpers.py pylint:ansible-deprecated-version lib/ansible/playbook/helpers.py pylint:blacklisted-name lib/ansible/plugins/action/__init__.py pylint:ansible-deprecated-version lib/ansible/plugins/action/async_status.py pylint:ansible-deprecated-version diff --git a/test/units/playbook/test_helpers.py b/test/units/playbook/test_helpers.py index a4ed6178d4e..8574cb4c5b9 100644 --- a/test/units/playbook/test_helpers.py +++ b/test/units/playbook/test_helpers.py @@ -193,8 +193,7 @@ class TestLoadListOfTasks(unittest.TestCase, MixinForMocks): self.assertEqual(len(res), 0) def test_one_bogus_include_static(self): - ds = [{'include': 'somefile.yml', - 'static': 'true'}] + ds = [{'import_tasks': 'somefile.yml'}] res = helpers.load_list_of_tasks(ds, play=self.mock_play, variable_manager=self.mock_variable_manager, loader=self.fake_loader) self.assertIsInstance(res, list) @@ -241,28 +240,6 @@ class TestLoadListOfTasks(unittest.TestCase, MixinForMocks): self.assertIn('test_one_parent_include_tags_tag1', res[0].tags) self.assertIn('and_another_tag2', res[0].tags) - # It would be useful to be able to tell what kind of deprecation we encountered and where we encountered it. - def test_one_include_tags_deprecated_mixed(self): - ds = [{'include': "/dev/null/includes/other_test_include.yml", - 'vars': {'tags': "['tag_on_include1', 'tag_on_include2']"}, - 'tags': 'mixed_tag1, mixed_tag2' - }] - self.assertRaisesRegexp(errors.AnsibleParserError, 'Mixing styles', - helpers.load_list_of_tasks, - ds, play=self.mock_play, - variable_manager=self.mock_variable_manager, loader=self.fake_include_loader) - - def test_one_include_tags_deprecated_include(self): - ds = [{'include': '/dev/null/includes/other_test_include.yml', - 'vars': {'tags': ['include_tag1_deprecated', 'and_another_tagB_deprecated']} - }] - res = helpers.load_list_of_tasks(ds, play=self.mock_play, - variable_manager=self.mock_variable_manager, loader=self.fake_include_loader) - self._assert_is_task_list_or_blocks(res) - self.assertIsInstance(res[0], Block) - self.assertIn('include_tag1_deprecated', res[0].tags) - self.assertIn('and_another_tagB_deprecated', res[0].tags) - def test_one_include_use_handlers(self): ds = [{'include': '/dev/null/includes/other_test_include.yml'}] res = helpers.load_list_of_tasks(ds, play=self.mock_play, @@ -286,11 +263,10 @@ class TestLoadListOfTasks(unittest.TestCase, MixinForMocks): # figure out how to get the non-static errors to be raised, this seems to just ignore everything def test_one_include_not_static(self): ds = [{ - 'include': '/dev/null/includes/static_test_include.yml', - 'static': False + 'include_tasks': '/dev/null/includes/static_test_include.yml', }] # a_block = Block() - ti_ds = {'include': '/dev/null/includes/ssdftatic_test_include.yml'} + ti_ds = {'include_tasks': '/dev/null/includes/ssdftatic_test_include.yml'} a_task_include = TaskInclude() ti = a_task_include.load(ti_ds) res = helpers.load_list_of_tasks(ds, play=self.mock_play,