From e40832df847bc7dcc94f41c491618de665c0b9e5 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 12 Apr 2019 12:13:55 -0400 Subject: [PATCH] become mixin is no more (#54002) * become mixin is no more since sudo/su keywords are removed in 2.9 .. no need to keep this code around * also don't need test for code that is removed * made preprocess_data on base noop its not used by anything anymore, but kept for backwards compat since other methods of same name are used --- changelogs/fragments/bye_become_mixin.yml | 2 + lib/ansible/playbook/base.py | 19 ++-- lib/ansible/playbook/become.py | 93 -------------------- lib/ansible/playbook/block.py | 3 +- lib/ansible/playbook/play.py | 3 +- lib/ansible/playbook/role/__init__.py | 3 +- lib/ansible/playbook/role/definition.py | 3 +- lib/ansible/playbook/task.py | 3 +- test/units/playbook/test_base.py | 6 -- test/units/playbook/test_become.py | 100 ---------------------- test/units/playbook/test_play_context.py | 7 ++ 11 files changed, 23 insertions(+), 219 deletions(-) create mode 100644 changelogs/fragments/bye_become_mixin.yml delete mode 100644 lib/ansible/playbook/become.py delete mode 100644 test/units/playbook/test_become.py diff --git a/changelogs/fragments/bye_become_mixin.yml b/changelogs/fragments/bye_become_mixin.yml new file mode 100644 index 00000000000..076d05601a8 --- /dev/null +++ b/changelogs/fragments/bye_become_mixin.yml @@ -0,0 +1,2 @@ +bugfixes: + - remove obsolete become mixin diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index 55530938252..74357e39f96 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -195,11 +195,6 @@ class FieldAttributeBase(with_metaclass(BaseMeta, object)): def preprocess_data(self, ds): ''' infrequently used method to do some pre-processing of legacy terms ''' - - for base_class in self.__class__.mro(): - method = getattr(self, "_preprocess_data_%s" % base_class.__name__.lower(), None) - if method: - return method(ds) return ds def load_data(self, ds, variable_manager=None, loader=None): @@ -615,8 +610,12 @@ class Base(FieldAttributeBase): # explicitly invoke a debugger on tasks _debugger = FieldAttribute(isa='string') - # param names which have been deprecated/removed - DEPRECATED_ATTRIBUTES = [ - 'sudo', 'sudo_user', 'sudo_pass', 'sudo_exe', 'sudo_flags', - 'su', 'su_user', 'su_pass', 'su_exe', 'su_flags', - ] + # Privilege escalation + _become = FieldAttribute(isa='bool', default=context.cliargs_deferred_get('become')) + _become_method = FieldAttribute(isa='string', default=context.cliargs_deferred_get('become_method')) + _become_user = FieldAttribute(isa='string', default=context.cliargs_deferred_get('become_user')) + _become_flags = FieldAttribute(isa='string', default=context.cliargs_deferred_get('become_flags')) + _become_exe = FieldAttribute(isa='string', default=context.cliargs_deferred_get('become_exe')) + + # used to hold sudo/su stuff + DEPRECATED_ATTRIBUTES = [] diff --git a/lib/ansible/playbook/become.py b/lib/ansible/playbook/become.py deleted file mode 100644 index cdaa0ce8a44..00000000000 --- a/lib/ansible/playbook/become.py +++ /dev/null @@ -1,93 +0,0 @@ -# (c) 2012-2014, Michael DeHaan -# -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -# Make coding more python3-ish -from __future__ import (absolute_import, division, print_function) -__metaclass__ = type - -from ansible import constants as C -from ansible import context -from ansible.errors import AnsibleParserError -from ansible.playbook.attribute import FieldAttribute -from ansible.utils.display import Display - -display = Display() - - -class Become: - - # Privilege escalation - _become = FieldAttribute(isa='bool', default=context.cliargs_deferred_get('become')) - _become_method = FieldAttribute(isa='string', default=context.cliargs_deferred_get('become_method')) - _become_user = FieldAttribute(isa='string', default=context.cliargs_deferred_get('become_user')) - _become_flags = FieldAttribute(isa='string') - - def __init__(self): - super(Become, self).__init__() - - def _detect_privilege_escalation_conflict(self, ds): - - # Fail out if user specifies conflicting privilege escalations - has_become = 'become' in ds or 'become_user'in ds - has_sudo = 'sudo' in ds or 'sudo_user' in ds - has_su = 'su' in ds or 'su_user' in ds - - if has_become: - msg = 'The become params ("become", "become_user") and' - if has_sudo: - raise AnsibleParserError('%s sudo params ("sudo", "sudo_user") cannot be used together' % msg) - elif has_su: - raise AnsibleParserError('%s su params ("su", "su_user") cannot be used together' % msg) - elif has_sudo and has_su: - raise AnsibleParserError('sudo params ("sudo", "sudo_user") and su params ("su", "su_user") cannot be used together') - - def _preprocess_data_become(self, ds): - """Preprocess the playbook data for become attributes - - This is called from the Base object's preprocess_data() method which - in turn is called pretty much anytime any sort of playbook object - (plays, tasks, blocks, etc) is created. - """ - - self._detect_privilege_escalation_conflict(ds) - - # Privilege escalation, backwards compatibility for sudo/su - if 'sudo' in ds or 'sudo_user' in ds: - ds['become_method'] = 'sudo' - if 'sudo' in ds: - ds['become'] = ds['sudo'] - del ds['sudo'] - - if 'sudo_user' in ds: - ds['become_user'] = ds['sudo_user'] - del ds['sudo_user'] - - display.deprecated("Instead of sudo/sudo_user, use become/become_user and make sure become_method is 'sudo' (default)", '2.9') - - elif 'su' in ds or 'su_user' in ds: - ds['become_method'] = 'su' - if 'su' in ds: - ds['become'] = ds['su'] - del ds['su'] - - if 'su_user' in ds: - ds['become_user'] = ds['su_user'] - del ds['su_user'] - - display.deprecated("Instead of su/su_user, use become/become_user and set become_method to 'su' (default is sudo)", '2.9') - - return ds diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py index 103b7e4b1f7..2bd7bd8a41e 100644 --- a/lib/ansible/playbook/block.py +++ b/lib/ansible/playbook/block.py @@ -22,7 +22,6 @@ __metaclass__ = type from ansible.errors import AnsibleParserError from ansible.playbook.attribute import FieldAttribute from ansible.playbook.base import Base -from ansible.playbook.become import Become from ansible.playbook.conditional import Conditional from ansible.playbook.collectionsearch import CollectionSearch from ansible.playbook.helpers import load_list_of_tasks @@ -31,7 +30,7 @@ from ansible.playbook.taggable import Taggable from ansible.utils.sentinel import Sentinel -class Block(Base, Become, Conditional, CollectionSearch, Taggable): +class Block(Base, Conditional, CollectionSearch, Taggable): # main block fields containing the task lists _block = FieldAttribute(isa='list', default=list, inherit=False) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 386f5871c0a..706d3962224 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -25,7 +25,6 @@ from ansible.errors import AnsibleParserError, AnsibleAssertionError from ansible.module_utils.six import string_types from ansible.playbook.attribute import FieldAttribute from ansible.playbook.base import Base -from ansible.playbook.become import Become from ansible.playbook.block import Block from ansible.playbook.collectionsearch import CollectionSearch from ansible.playbook.helpers import load_list_of_blocks, load_list_of_roles @@ -40,7 +39,7 @@ display = Display() __all__ = ['Play'] -class Play(Base, Taggable, Become, CollectionSearch): +class Play(Base, Taggable, CollectionSearch): """ A play is a language feature that represents a list of roles and/or diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index a00db31b59a..bb304fa755f 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -26,7 +26,6 @@ from ansible.module_utils.six import iteritems, binary_type, text_type from ansible.module_utils.common._collections_compat import Container, Mapping, Set, Sequence from ansible.playbook.attribute import FieldAttribute from ansible.playbook.base import Base -from ansible.playbook.become import Become from ansible.playbook.collectionsearch import CollectionSearch from ansible.playbook.conditional import Conditional from ansible.playbook.helpers import load_list_of_blocks @@ -92,7 +91,7 @@ def hash_params(params): return frozenset((params,)) -class Role(Base, Become, Conditional, Taggable, CollectionSearch): +class Role(Base, Conditional, Taggable, CollectionSearch): _delegate_to = FieldAttribute(isa='string') _delegate_facts = FieldAttribute(isa='bool') diff --git a/lib/ansible/playbook/role/definition.py b/lib/ansible/playbook/role/definition.py index c5999bb1b33..b18f1d16525 100644 --- a/lib/ansible/playbook/role/definition.py +++ b/lib/ansible/playbook/role/definition.py @@ -27,7 +27,6 @@ from ansible.module_utils.six import iteritems, string_types from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject, AnsibleMapping from ansible.playbook.attribute import Attribute, FieldAttribute from ansible.playbook.base import Base -from ansible.playbook.become import Become from ansible.playbook.collectionsearch import CollectionSearch from ansible.playbook.conditional import Conditional from ansible.playbook.taggable import Taggable @@ -41,7 +40,7 @@ __all__ = ['RoleDefinition'] display = Display() -class RoleDefinition(Base, Become, Conditional, Taggable, CollectionSearch): +class RoleDefinition(Base, Conditional, Taggable, CollectionSearch): _role = FieldAttribute(isa='string') diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 68204b49684..4854801dc6d 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -30,7 +30,6 @@ from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject, AnsibleMapping from ansible.plugins.loader import lookup_loader from ansible.playbook.attribute import FieldAttribute from ansible.playbook.base import Base -from ansible.playbook.become import Become from ansible.playbook.block import Block from ansible.playbook.collectionsearch import CollectionSearch from ansible.playbook.conditional import Conditional @@ -45,7 +44,7 @@ __all__ = ['Task'] display = Display() -class Task(Base, Conditional, Taggable, Become, CollectionSearch): +class Task(Base, Conditional, Taggable, CollectionSearch): """ A task is a language feature that represents a call to a module, with given arguments and other parameters. diff --git a/test/units/playbook/test_base.py b/test/units/playbook/test_base.py index a8dcf848744..dc3509577e1 100644 --- a/test/units/playbook/test_base.py +++ b/test/units/playbook/test_base.py @@ -367,12 +367,6 @@ class BaseSubClass(base.Base): _test_attr_method_missing = FieldAttribute(isa='string', default='some attr with a missing getter', always_post_validate=True) - def _preprocess_data_basesubclass(self, ds): - return ds - - def preprocess_data(self, ds): - return super(BaseSubClass, self).preprocess_data(ds) - def _get_attr_test_attr_method(self): return 'foo bar' diff --git a/test/units/playbook/test_become.py b/test/units/playbook/test_become.py deleted file mode 100644 index 3501dc856c3..00000000000 --- a/test/units/playbook/test_become.py +++ /dev/null @@ -1,100 +0,0 @@ -# -*- coding: utf-8 -*- -# (c) 2018 Matt Martz -# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) - -from __future__ import absolute_import, division, print_function -__metaclass__ = type - -import re - -from ansible.errors import AnsibleParserError -from ansible.playbook.become import Become -from ansible.module_utils._text import to_native - -import pytest - - -class InString(str): - def __eq__(self, other): - return self in other - - -@pytest.mark.parametrize("ds", [ - {}, - {'become': True}, - {'become_user': 'root'}, - {'sudo': True}, - {'sudo_user': 'root'}, - {'su': True}, - {'su_user': 'root'} -]) -def test_detect_privilege_escalation_conflict_valid(ds): - become = Become() - become._detect_privilege_escalation_conflict(ds) - - -@pytest.mark.parametrize("ds,message", [ - ({'become': True, 'sudo': True}, re.compile('"become".*"sudo"')), - ({'become': True, 'su': True}, re.compile('"become".*"su"')), - ({'sudo': True, 'su': True}, re.compile('"sudo".*"su"')), - ({'become_user': 'root', 'sudo': True}, re.compile('"become".*"sudo"')), - ({'sudo_user': 'root', 'su': True}, re.compile('"sudo".*"su"')), -]) -def test_detect_privilege_escalation_conflict_invalid(ds, message): - become = Become() - with pytest.raises(AnsibleParserError) as excinfo: - become._detect_privilege_escalation_conflict(ds) - assert message.search(excinfo.value.message) is not None - - -def test_preprocess_data_become(mocker): - display_mock = mocker.patch('ansible.playbook.become.display') - - become = Become() - ds = {} - assert become._preprocess_data_become(ds) == {} - - display_mock.reset_mock() - ds = {'sudo': True} - out = become._preprocess_data_become(ds) - assert 'sudo' not in out - assert out.get('become_method') == 'sudo' - display_mock.deprecated.assert_called_once_with( - "Instead of sudo/sudo_user, use become/become_user and make sure become_method is 'sudo' (default)", - '2.9' - ) - - ds = {'sudo_user': 'root'} - out = become._preprocess_data_become(ds) - assert 'sudo_user' not in out - assert out.get('become_user') == 'root' - - ds = {'sudo': True, 'sudo_user': 'root'} - out = become._preprocess_data_become(ds) - assert 'sudo' not in out - assert 'sudo_user' not in out - assert out.get('become_method') == 'sudo' - assert out.get('become_user') == 'root' - - display_mock.reset_mock() - ds = {'su': True} - out = become._preprocess_data_become(ds) - assert 'su' not in out - assert out.get('become_method') == 'su' - display_mock.deprecated.assert_called_once_with( - "Instead of su/su_user, use become/become_user and set become_method to 'su' (default is sudo)", - '2.9' - ) - display_mock.reset_mock() - - ds = {'su_user': 'root'} - out = become._preprocess_data_become(ds) - assert 'su_user' not in out - assert out.get('become_user') == 'root' - - ds = {'su': True, 'su_user': 'root'} - out = become._preprocess_data_become(ds) - assert 'su' not in out - assert 'su_user' not in out - assert out.get('become_method') == 'su' - assert out.get('become_user') == 'root' diff --git a/test/units/playbook/test_play_context.py b/test/units/playbook/test_play_context.py index 9bb3285973c..4deabf93b05 100644 --- a/test/units/playbook/test_play_context.py +++ b/test/units/playbook/test_play_context.py @@ -140,6 +140,7 @@ def test_play_context_make_become_cmd(mocker, parser, reset_cli_args): default_exe, success, default_cmd), cmd) is not None) play_context.become_pass = None + play_context.become_method = 'su' play_context.set_become_plugin(become_loader.get('su')) play_context.become_flags = su_flags cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash") @@ -147,33 +148,39 @@ def test_play_context_make_become_cmd(mocker, parser, reset_cli_args): success, default_cmd), cmd) is not None) play_context.set_become_plugin(become_loader.get('pbrun')) + play_context.become_method = 'pbrun' play_context.become_flags = pbrun_flags cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash") assert re.match("""%s %s -u %s 'echo %s; %s'""" % (pbrun_exe, pbrun_flags, play_context.become_user, success, default_cmd), cmd) is not None play_context.set_become_plugin(become_loader.get('pfexec')) + play_context.become_method = 'pfexec' play_context.become_flags = pfexec_flags cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash") assert re.match('''%s %s "'echo %s; %s'"''' % (pfexec_exe, pfexec_flags, success, default_cmd), cmd) is not None play_context.set_become_plugin(become_loader.get('doas')) + play_context.become_method = 'doas' play_context.become_flags = doas_flags cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash") assert (re.match("""%s %s -u %s %s -c 'echo %s; %s'""" % (doas_exe, doas_flags, play_context.become_user, default_exe, success, default_cmd), cmd) is not None) play_context.set_become_plugin(become_loader.get('ksu')) + play_context.become_method = 'ksu' play_context.become_flags = ksu_flags cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash") assert (re.match("""%s %s %s -e %s -c 'echo %s; %s'""" % (ksu_exe, play_context.become_user, ksu_flags, default_exe, success, default_cmd), cmd) is not None) play_context.set_become_plugin(become_loader.get('bad')) + play_context.become_method = 'bad' with pytest.raises(AnsibleError): play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash") play_context.set_become_plugin(become_loader.get('dzdo')) + play_context.become_method = 'dzdo' play_context.become_flags = dzdo_flags cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash") assert re.match("""%s %s -u %s %s -c 'echo %s; %s'""" % (dzdo_exe, dzdo_flags, play_context.become_user, default_exe,