From e3854c85695849fca336de478265823487763f77 Mon Sep 17 00:00:00 2001 From: Peter Tripp Date: Fri, 18 Mar 2016 13:38:49 -0700 Subject: [PATCH 01/16] D2 and G2 EC2 instance types. --- salt/cloud/clouds/ec2.py | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/salt/cloud/clouds/ec2.py b/salt/cloud/clouds/ec2.py index e34f52101c..8c61b8b7d3 100644 --- a/salt/cloud/clouds/ec2.py +++ b/salt/cloud/clouds/ec2.py @@ -658,6 +658,46 @@ def avail_sizes(call=None): 'ram': '60 GiB' } }, + 'Dense Storage': { + 'd2.xlarge': { + 'id': 'd2.xlarge', + 'cores': '4', + 'disk': '6 TiB (3 x 2 TiB hard disk drives)', + 'ram': '30.5 GiB' + }, + 'd2.2xlarge': { + 'id': 'd2.2xlarge', + 'cores': '8', + 'disk': '12 TiB (6 x 2 TiB hard disk drives)', + 'ram': '61 GiB' + }, + 'd2.4xlarge': { + 'id': 'd2.4xlarge', + 'cores': '16', + 'disk': '24 TiB (12 x 2 TiB hard disk drives)', + 'ram': '122 GiB' + }, + 'd2.8xlarge': { + 'id': 'd2.8xlarge', + 'cores': '36', + 'disk': '24 TiB (24 x 2 TiB hard disk drives)', + 'ram': '244 GiB' + }, + }, + 'GPU': { + 'g2.2xlarge': { + 'id': 'g2.2xlarge', + 'cores': '8', + 'disk': '60 GiB (1 x 60 GiB SSD)', + 'ram': '15 GiB' + }, + 'g2.8xlarge': { + 'id': 'g2.8xlarge', + 'cores': '32', + 'disk': '240 GiB (2 x 120 GiB SSD)', + 'ram': '60 GiB' + }, + }, 'High I/O': { 'i2.xlarge': { 'id': 'i2.xlarge', From 31bb573abc4f1bb666340dddc847711f24bf31d8 Mon Sep 17 00:00:00 2001 From: Jacob Hammons Date: Fri, 18 Mar 2016 17:43:36 -0600 Subject: [PATCH 02/16] Fixes a doc build exception caused by missing mocks for modules.win_dacl --- doc/conf.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/conf.py b/doc/conf.py index 5346243ad1..be2192faf9 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -118,6 +118,9 @@ MOCK_MODULES = [ 'yum', 'OpenSSL', 'zfs', + 'salt.ext.six.moves.winreg', + 'win32security', + 'ntsecuritycon', ] for mod_name in MOCK_MODULES: @@ -126,6 +129,8 @@ for mod_name in MOCK_MODULES: # Define a fake version attribute for the following libs. sys.modules['libcloud'].__version__ = '0.0.0' sys.modules['pymongo'].version = '0.0.0' +sys.modules['ntsecuritycon'].STANDARD_RIGHTS_REQUIRED = 0 +sys.modules['ntsecuritycon'].SYNCHRONIZE = 0 # -- Add paths to PYTHONPATH --------------------------------------------------- From e511864a553d34d5e7e07a474c2127ca96264e8a Mon Sep 17 00:00:00 2001 From: Nicolas Delaby Date: Mon, 21 Mar 2016 10:44:22 +0100 Subject: [PATCH 03/16] Fix ports exposition when protocol is passed. fixes #32013 --- salt/states/dockerng.py | 29 +++++++++++++-- tests/unit/states/dockerng_test.py | 58 ++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/salt/states/dockerng.py b/salt/states/dockerng.py index f39abf6033..3516eda3d1 100644 --- a/salt/states/dockerng.py +++ b/salt/states/dockerng.py @@ -87,6 +87,30 @@ def _format_comments(comments): return ret +def _map_port_from_yaml_to_docker(port): + ''' + docker-py interface is not very nice: + While for ``port_bindings`` they support: + + .. code-block:: python + + '8888/tcp' + + For ``ports``, it has to be transformed into: + + .. code-block:: python + + (8888, 'tcp') + + ''' + if isinstance(port, six.string_types): + port, sep, protocol = port.partition('/') + if protocol: + return int(port), protocol + return int(port) + return port + + def _prep_input(kwargs): ''' Repack (if necessary) data that should be in a dict but is easier to @@ -197,7 +221,7 @@ def _compare(actual, create_kwargs, defaults_from_image): for port_def in data: if isinstance(port_def, six.integer_types): port_def = str(port_def) - if isinstance(port_def, tuple): + if isinstance(port_def, (tuple, list)): desired_ports.append('{0}/{1}'.format(*port_def)) elif '/' not in port_def: desired_ports.append('{0}/tcp'.format(port_def)) @@ -1534,7 +1558,8 @@ def running(name, if create_kwargs.get('port_bindings') is not None: # Be smart and try to provide `ports` argument derived from # the "port_bindings" configuration. - auto_ports = list(create_kwargs['port_bindings']) + auto_ports = [_map_port_from_yaml_to_docker(port) + for port in create_kwargs['port_bindings']] actual_ports = create_kwargs.setdefault('ports', []) actual_ports.extend([p for p in auto_ports if p not in actual_ports]) diff --git a/tests/unit/states/dockerng_test.py b/tests/unit/states/dockerng_test.py index 4ef692b4bb..314f096d7a 100644 --- a/tests/unit/states/dockerng_test.py +++ b/tests/unit/states/dockerng_test.py @@ -200,6 +200,64 @@ class DockerngTestCase(TestCase): client_timeout=60) dockerng_start.assert_called_with('cont') + def test_running_with_udp_bindings(self): + ''' + Check that `ports` contains ports defined from `port_bindings` with + protocol declaration passed as tuple. As stated by docker-py + documentation + + https://docker-py.readthedocs.org/en/latest/port-bindings/ + + In sls: + + .. code-block:: yaml + + container: + dockerng.running: + - port_bindings: + - '9090:9797/udp' + + is equivalent of: + + .. code-block:: yaml + + container: + dockerng.running: + - ports: + - 9797/udp + - port_bindings: + - '9090:9797/udp' + ''' + dockerng_create = Mock() + dockerng_start = Mock() + dockerng_inspect_image = Mock(return_value={ + 'Id': 'abcd', + 'Config': {'ExposedPorts': {}} + }) + __salt__ = {'dockerng.list_containers': MagicMock(), + 'dockerng.list_tags': MagicMock(), + 'dockerng.pull': MagicMock(), + 'dockerng.state': MagicMock(), + 'dockerng.inspect_image': dockerng_inspect_image, + 'dockerng.create': dockerng_create, + 'dockerng.start': dockerng_start, + } + with patch.dict(dockerng_state.__dict__, + {'__salt__': __salt__}): + dockerng_state.running( + 'cont', + image='image:latest', + port_bindings=['9090:9797/udp']) + dockerng_create.assert_called_with( + 'image:latest', + validate_input=False, + name='cont', + ports=[(9797, 'udp')], + port_bindings={'9797/udp': [9090]}, + validate_ip_addrs=False, + client_timeout=60) + dockerng_start.assert_called_with('cont') + def test_running_compare_images_by_id(self): ''' Make sure the container is running From 26eee1505fbcb523e54832839e3aea7be8d315a9 Mon Sep 17 00:00:00 2001 From: Josh Fraser Date: Sun, 20 Mar 2016 21:55:29 -0700 Subject: [PATCH 04/16] There were two identical blocks concerning Windows Deploy Timeouts. This pull request removes the extra block of text. --- doc/topics/cloud/aws.rst | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/doc/topics/cloud/aws.rst b/doc/topics/cloud/aws.rst index bdee883867..926caab636 100644 --- a/doc/topics/cloud/aws.rst +++ b/doc/topics/cloud/aws.rst @@ -186,16 +186,6 @@ These retries and timeouts relate to validating the Administrator password once AWS provides the credentials via the AWS API. -Windows Deploy Timeouts -======================= -For Windows instances, it may take longer than normal for the instance to be -ready. In these circumstances, the provider configuration can be configured -with a ``win_deploy_auth_retries`` and/or a ``win_deploy_auth_retry_delay`` -setting, which default to 10 retries and a one second delay between retries. -These retries and timeouts relate to validating the Administrator password -once AWS provides the credentials via the AWS API. - - Key Pairs ========= In order to create an instance with Salt installed and configured, a key pair From 711a0a9844081eb9f84f437c9e2ebd44138894e7 Mon Sep 17 00:00:00 2001 From: Skip Breidbach Date: Mon, 21 Mar 2016 09:37:57 -0700 Subject: [PATCH 05/16] Move constant declaration into member variable to avoid issues when modules can't be loaded. --- salt/modules/win_dacl.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/salt/modules/win_dacl.py b/salt/modules/win_dacl.py index d57bb7b02f..d9ee27a527 100644 --- a/salt/modules/win_dacl.py +++ b/salt/modules/win_dacl.py @@ -44,9 +44,10 @@ class daclConstants(object): # in ntsecuritycon has the extra bits 0x200 enabled. # Note that you when you set this permission what you'll generally get back is it # ORed with 0x200 (SI_NO_ACL_PROTECT), which is what ntsecuritycon incorrectly defines. - FILE_ALL_ACCESS = (ntsecuritycon.STANDARD_RIGHTS_REQUIRED | ntsecuritycon.SYNCHRONIZE | 0x1ff) def __init__(self): + self.FILE_ALL_ACCESS = (ntsecuritycon.STANDARD_RIGHTS_REQUIRED | ntsecuritycon.SYNCHRONIZE | 0x1ff) + self.hkeys_security = { 'HKEY_LOCAL_MACHINE': 'MACHINE', 'HKEY_USERS': 'USERS', @@ -88,7 +89,7 @@ class daclConstants(object): ntsecuritycon.DELETE, 'TEXT': 'modify'}, 'FULLCONTROL': { - 'BITS': daclConstants.FILE_ALL_ACCESS, + 'BITS': self.FILE_ALL_ACCESS, 'TEXT': 'full control'} } } @@ -368,7 +369,7 @@ def add_ace(path, objectType, user, permission, acetype, propagation): path: path to the object (i.e. c:\\temp\\file, HKEY_LOCAL_MACHINE\\SOFTWARE\\KEY, etc) user: user to add permission: permissions for the user - acetypes: either allow/deny for each user/permission (ALLOW, DENY) + acetype: either allow/deny for each user/permission (ALLOW, DENY) propagation: how the ACE applies to children for Registry Keys and Directories(KEY, KEY&SUBKEYS, SUBKEYS) CLI Example: From f27da41b714a833fcd601eccd2c8d84d308991f9 Mon Sep 17 00:00:00 2001 From: Joseph Hall Date: Mon, 21 Mar 2016 13:15:31 -0600 Subject: [PATCH 06/16] Don't require the decode_out file to already exist --- salt/utils/http.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/utils/http.py b/salt/utils/http.py index 7182b4a3fc..a3c127ba1f 100644 --- a/salt/utils/http.py +++ b/salt/utils/http.py @@ -552,7 +552,7 @@ def query(url, else: text = True - if decode_out and os.path.exists(decode_out): + if decode_out: with salt.utils.fopen(decode_out, 'w') as dof: dof.write(result_text) From 3b6d25b4e90a94a846cae3f33c6502785ec8dca4 Mon Sep 17 00:00:00 2001 From: Colton Myers Date: Tue, 22 Mar 2016 11:03:57 -0600 Subject: [PATCH 07/16] Remove leading whitespace on tags Was converting '[admin, monitoring]' to ['admin', ' monitoring'] Now converts to ['admin', 'monitoring'] --- salt/modules/rabbitmq.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/rabbitmq.py b/salt/modules/rabbitmq.py index 28e7730c68..150a24a385 100644 --- a/salt/modules/rabbitmq.py +++ b/salt/modules/rabbitmq.py @@ -126,7 +126,7 @@ def list_users(runas=None): runas=runas) # func to get tags from string such as "[admin, monitoring]" - func = lambda string: set(string[1:-1].split(',')) + func = lambda string: set([x.strip() for x in string[1:-1].split(',')]) return _output_to_dict(res, func) From 08868cb32aa038068d711433d31697f279928097 Mon Sep 17 00:00:00 2001 From: Colton Myers Date: Tue, 22 Mar 2016 11:16:17 -0600 Subject: [PATCH 08/16] Fix tag handling code for rabbitmq_user.present --- salt/states/rabbitmq_user.py | 43 ++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/salt/states/rabbitmq_user.py b/salt/states/rabbitmq_user.py index efdb21b0bf..20eaea9f3c 100644 --- a/salt/states/rabbitmq_user.py +++ b/salt/states/rabbitmq_user.py @@ -73,20 +73,14 @@ def _check_perms_changes(name, newperms, runas=None, existing=None): return perm_need_change -def _check_tags_changes(name, new_tags, runas=None): +def _get_current_tags(name, runas=None): ''' Whether Rabbitmq user's tags need to be changed ''' - if new_tags: - if isinstance(new_tags, str): - new_tags = new_tags.split() - try: - users = __salt__['rabbitmq.list_users'](runas=runas)[name] - set(new_tags) - except CommandExecutionError as err: - log.error('Error: {0}'.format(err)) - return [] - return list(users) - else: + try: + return list(__salt__['rabbitmq.list_users'](runas=runas)[name]) + except CommandExecutionError as err: + log.error('Error: {0}'.format(err)) return [] @@ -165,17 +159,22 @@ def present(name, {'old': 'Removed password.', 'new': ''}}) - new_tags = _check_tags_changes(name, tags, runas=runas) - if new_tags: - if not __opts__['test']: - try: - __salt__['rabbitmq.set_user_tags'](name, tags, runas=runas) - except CommandExecutionError as err: - ret['comment'] = 'Error: {0}'.format(err) - return ret - ret['changes'].update({'tags': - {'old': tags, - 'new': list(new_tags)}}) + if tags is not None: + current_tags = _get_current_tags(name, tags, runas=runas) + if isinstance(tags, str): + tags = tags.split() + # Diff the tags sets. Symmetric difference operator ^ will give us + # any element in one set, but not both + if set(tags) ^ set(current_tags): + if not __opts__['test']: + try: + __salt__['rabbitmq.set_user_tags'](name, tags, runas=runas) + except CommandExecutionError as err: + ret['comment'] = 'Error: {0}'.format(err) + return ret + ret['changes'].update({'tags': + {'old': current_tags, + 'new': tags}}) try: existing_perms = __salt__['rabbitmq.list_user_permissions'](name, runas=runas) except CommandExecutionError as err: From bed048e1e78089e136840bd6ec4e4a4922ef258b Mon Sep 17 00:00:00 2001 From: Colton Myers Date: Tue, 22 Mar 2016 11:50:38 -0600 Subject: [PATCH 09/16] Remove leftover arg (lint) --- salt/states/rabbitmq_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/states/rabbitmq_user.py b/salt/states/rabbitmq_user.py index 20eaea9f3c..589e672601 100644 --- a/salt/states/rabbitmq_user.py +++ b/salt/states/rabbitmq_user.py @@ -160,7 +160,7 @@ def present(name, 'new': ''}}) if tags is not None: - current_tags = _get_current_tags(name, tags, runas=runas) + current_tags = _get_current_tags(name, runas=runas) if isinstance(tags, str): tags = tags.split() # Diff the tags sets. Symmetric difference operator ^ will give us From 95c08f55e9caffe9da3b729c86bf2f2e272dd91a Mon Sep 17 00:00:00 2001 From: Colton Myers Date: Tue, 22 Mar 2016 13:46:06 -0600 Subject: [PATCH 10/16] Tear out useless unit test --- tests/unit/states/rabbitmq_user_test.py | 63 ------------------------- 1 file changed, 63 deletions(-) diff --git a/tests/unit/states/rabbitmq_user_test.py b/tests/unit/states/rabbitmq_user_test.py index cbd6710a01..2d8d6c60eb 100644 --- a/tests/unit/states/rabbitmq_user_test.py +++ b/tests/unit/states/rabbitmq_user_test.py @@ -30,69 +30,6 @@ class RabbitmqUserTestCase(TestCase): ''' Test cases for salt.states.rabbitmq_user ''' - # 'present' function tests: 1 - - def test_present(self): - ''' - Test to ensure the RabbitMQ user exists. - ''' - name = 'foo' - passwd = 'password' - tag = 'user' - existing_perms = {'/': ['.*', '.*']} - perms = [existing_perms] - - ret = {'name': name, - 'changes': {}, - 'result': True, - 'comment': ''} - - mock = MagicMock(side_effect=[True, False, True, True, - True, True, True]) - mock_dct = MagicMock(return_value={name: set(tag)}) - mock_pr = MagicMock(return_value=existing_perms) - mock_add = MagicMock(return_value={'Added': name}) - with patch.dict(rabbitmq_user.__salt__, - {'rabbitmq.user_exists': mock, - 'rabbitmq.list_users': mock_dct, - 'rabbitmq.list_user_permissions': mock_pr, - 'rabbitmq.set_user_tags': mock_add}): - comment = 'User \'foo\' is already present.' - ret.update({'comment': comment}) - self.assertDictEqual(rabbitmq_user.present(name), ret) - - with patch.dict(rabbitmq_user.__opts__, {'test': True}): - comment = 'User \'foo\' is set to be created.' - changes = {'user': {'new': 'foo', 'old': ''}} - ret.update({'comment': comment, 'result': None, 'changes': changes}) - self.assertDictEqual(rabbitmq_user.present(name), ret) - - comment = 'Configuration for \'foo\' will change.' - changes = {'password': {'new': 'Set password.', 'old': ''}} - ret.update({'comment': comment, 'changes': changes}) - self.assertDictEqual(rabbitmq_user.present(name, - password=passwd, - force=True), ret) - - changes = {'password': {'new': '', 'old': 'Removed password.'}} - ret.update({'changes': changes}) - self.assertDictEqual(rabbitmq_user.present(name, force=True), - ret) - - changes = {'tags': {'new': ['u', 's', 'r', 'e'], 'old': 'user'}} - ret.update({'changes': changes}) - self.assertDictEqual(rabbitmq_user.present(name, tags=tag), ret) - - comment = '\'foo\' is already in the desired state.' - ret.update({'changes': {}, 'comment': comment, 'result': True}) - self.assertDictEqual(rabbitmq_user.present(name, perms=perms), - ret) - - with patch.dict(rabbitmq_user.__opts__, {'test': False}): - ret.update({'comment': '\'foo\' was configured.', 'result': True, - 'changes': {'tags': {'new': ['u', 's', 'r', 'e'], 'old': 'user'}}}) - self.assertDictEqual(rabbitmq_user.present(name, tags=tag), ret) - # 'absent' function tests: 1 def test_absent(self): From aae3af7e497b0e03c9d4e2ef20279c56f15c3470 Mon Sep 17 00:00:00 2001 From: Xiami Date: Wed, 23 Mar 2016 18:19:18 +0800 Subject: [PATCH 11/16] Fix code for proto args in modules.iptables In modules.iptables.build_rule: Fix incorrect output when providing proto='!tcp' Make salt cleverer when both 'protocol' and 'proto' in kwargs. NOTE: Conflicts in merge commit 000de95 are wrong dealed with. --- salt/modules/iptables.py | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/salt/modules/iptables.py b/salt/modules/iptables.py index 979052c83c..aab84f45f1 100644 --- a/salt/modules/iptables.py +++ b/salt/modules/iptables.py @@ -195,22 +195,12 @@ def build_rule(table='filter', chain=None, command=None, position='', full=None, rule.append('{0}-o {1}'.format(maybe_add_negation('of'), kwargs['of'])) del kwargs['of'] - if 'protocol' in kwargs: - proto = kwargs['protocol'] - proto_negation = maybe_add_negation('protocol') - del kwargs['protocol'] - elif 'proto' in kwargs: - proto = kwargs['proto'] - proto_negation = maybe_add_negation('proto') - del kwargs['proto'] - - if proto: - if proto.startswith('!') or proto.startswith('not'): - proto = re.sub(bang_not_pat, '', proto) - rule += '! ' - - rule.append('{0}-p {1}'.format(proto_negation, proto)) - proto = True + for proto_arg in ('protocol', 'proto'): + if proto_arg in kwargs: + if not proto: + rule.append('{0}-p {1}'.format(maybe_add_negation(proto_arg), kwargs[proto_arg])) + proto = True + del kwargs[proto_arg] if 'match' in kwargs: match_value = kwargs['match'] From 4af4280d04aa4cb3d7807a94540293983665db25 Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Sat, 19 Mar 2016 14:21:00 -0600 Subject: [PATCH 12/16] Fix issue causing automated tests to fail. --- salt/auth/ldap.py | 6 +++--- salt/master.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/salt/auth/ldap.py b/salt/auth/ldap.py index 9cb96c3121..8a1451cb57 100644 --- a/salt/auth/ldap.py +++ b/salt/auth/ldap.py @@ -392,7 +392,7 @@ def expand_ldap_entries(entries, opts=None): This function only gets called if auth.ldap.activedirectory = True ''' bind = _bind_for_search(opts=opts) - acl_tree = {} + acl_tree = [] for user_or_group_dict in entries: for minion_or_ou, matchers in six.iteritems(user_or_group_dict): permissions = matchers @@ -411,11 +411,11 @@ def expand_ldap_entries(entries, opts=None): retrieved_minion_ids.append(minion_id) for minion_id in retrieved_minion_ids: - acl_tree[minion_id] = permissions + acl_tree.append({minion_id: permissions}) except ldap.NO_SUCH_OBJECT: pass else: - acl_tree[minion_or_ou] = matchers + acl_tree.append({minion_or_ou: matchers}) log.trace('expand_ldap_entries: {0}'.format(acl_tree)) return acl_tree diff --git a/salt/master.py b/salt/master.py index 5c9a9d8dfe..c3a6263438 100644 --- a/salt/master.py +++ b/salt/master.py @@ -1969,7 +1969,6 @@ class ClearFuncs(object): # Add any add'l permissions allowed by group membership if group_auth_match: auth_list = self.ckminions.fill_auth_list_from_groups(eauth_config, token['groups'], auth_list) - auth_list = self.ckminions.fill_auth_list_from_ou(auth_list, self.opts) log.trace("Compiled auth_list: {0}".format(auth_list)) @@ -2066,7 +2065,8 @@ class ClearFuncs(object): self.opts['external_auth'][extra['eauth']], groups, auth_list) - auth_list = self.ckminions.fill_auth_list_from_ou(auth_list, self.opts) + if extra['eauth'] == 'ldap': + auth_list = self.ckminions.fill_auth_list_from_ou(auth_list, self.opts) good = self.ckminions.auth_check( auth_list, clear_load['fun'], From c57a232809bdf11c5ccfea54a582ae6d81d164b0 Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Tue, 22 Mar 2016 10:59:37 -0600 Subject: [PATCH 13/16] Catch traceback when LDAP returns something other than the nested tuple --- salt/auth/ldap.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/salt/auth/ldap.py b/salt/auth/ldap.py index 8a1451cb57..b5f628ea23 100644 --- a/salt/auth/ldap.py +++ b/salt/auth/ldap.py @@ -407,8 +407,14 @@ def expand_ldap_entries(entries, opts=None): search_string, ['cn']) for ldap_match in search_results: - minion_id = ldap_match[1]['cn'][0].lower() - retrieved_minion_ids.append(minion_id) + try: + minion_id = ldap_match[1]['cn'][0].lower() + retrieved_minion_ids.append(minion_id) + except TypeError: + # TypeError here just means that one of the returned + # entries didn't match the format we expected + # from LDAP. + pass for minion_id in retrieved_minion_ids: acl_tree.append({minion_id: permissions}) From 9d1ad2082ca93de5c943139df256c3b936ba5b6e Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Wed, 23 Mar 2016 13:41:18 -0600 Subject: [PATCH 14/16] Add comments for expanded auth checks. --- salt/utils/minions.py | 407 +++++++++++++++++++++++++++++++++--------- 1 file changed, 318 insertions(+), 89 deletions(-) diff --git a/salt/utils/minions.py b/salt/utils/minions.py index bb1cafaa99..fd8e3a8c83 100644 --- a/salt/utils/minions.py +++ b/salt/utils/minions.py @@ -698,6 +698,192 @@ class CkMinions(object): fun, form) + def auth_check_expanded(self, + auth_list, + funs, + args, + tgt, + tgt_type='glob', + groups=None, + publish_validate=False): + + # Here's my thinking + # 1. Retrieve anticipated targeted minions + # 2. Iterate through each entry in the auth_list + # 3. If it is a minion_id, check to see if any targeted minions match. + # If there is a match, check to make sure funs are permitted + # (if it's not a match we don't care about this auth entry and can + # move on) + # a. If funs are permitted, Add this minion_id to a new set of allowed minion_ids + # If funs are NOT permitted, can short-circuit and return FALSE + # b. At the end of the auth_list loop, make sure all targeted IDs + # are in the set of allowed minion_ids. If not, return FALSE + # 4. If it is a target (glob, pillar, etc), retrieve matching minions + # and make sure that ALL targeted minions are in the set. + # then check to see if the funs are permitted + # a. If ALL targeted minions are not in the set, then return FALSE + # b. If the desired fun doesn't mass the auth check with any + # auth_entry's fun, then return FALSE + + # NOTE we are not going to try to allow functions to run on partial + # sets of minions. If a user targets a group of minions and does not + # have access to run a job on ALL of these minions then the job will + # fail with 'Eauth Failed'. + + # The recommended workflow in that case will be for the user to narrow + # his target. + + # This should cover adding the AD LDAP lookup functionality while + # preserving the existing auth behavior. + + # Recommend we config-get this behind an entry called + # auth.enable_expanded_auth_matching + # and default to False + v_tgt_type = tgt_type + if tgt_type.lower() in ('pillar', 'pillar_pcre'): + v_tgt_type = 'pillar_exact' + elif tgt_type.lower() == 'compound': + v_tgt_type = 'compound_pillar_exact' + v_minions = set(self.check_minions(tgt, v_tgt_type)) + minions = set(self.check_minions(tgt, tgt_type)) + mismatch = bool(minions.difference(v_minions)) + # If the non-exact match gets more minions than the exact match + # then pillar globbing or PCRE is being used, and we have a + # problem + if publish_validate: + if mismatch: + return False + # compound commands will come in a list so treat everything as a list + if not isinstance(funs, list): + funs = [funs] + args = [args] + + # Take the auth list and get all the minion names inside it + allowed_minions_from_auth_list = set() + + auth_dictionary = {} + # Make a set, so we are guaranteed to have only one of each minion + # Also iterate through the entire auth_list and create a dictionary + # so it's easy to look up what functions are permitted + for auth_list_entry in auth_list: + if isinstance(auth_list_entry, six.string_types): + for fun in funs: + # represents toplevel auth entry is a function. + # so this fn is permitted by all minions + if self.match_check(auth_list_entry, fun): + return True + if isinstance(auth_list_entry, dict): + if len(auth_list_entry) != 1: + log.info('Malformed ACL: {0}'.format(auth_list_entry)) + continue + allowed_minions_from_auth_list.update(set(auth_list_entry.keys())) + for key in auth_list_entry.keys(): + if key in auth_dictionary: + auth_dictionary[key].concat(auth_list_entry[key]) + else: + auth_dictionary[key] = auth_list_entry[key] + + # 'minions' here are all the names of minions matched by the target + # if we take out all the allowed minions, and there are any left, then + # the target includes minions that are not allowed by eauth + # so we can give up here. + if len(minions - allowed_minions_from_auth_list) > 0: + return False + + try: + log.trace('######################################') + log.trace(funs) + for minion in minions: + results = [] + for num, fun in enumerate(auth_dictionary[minion]): + results.append(self.match_check(fun, funs)) + if not any(results): + return False + return True + + # for ind in auth_list: + # if isinstance(ind, six.string_types): + # # Allowed for all minions + # if self.match_check(ind, fun): + # return True + # elif isinstance(ind, dict): + # if len(ind) != 1: + # # Invalid argument + # continue + # valid = next(six.iterkeys(ind)) + # # Check if minions are allowed + # # if self.validate_tgt( + # # valid, + # # tgt, + # # tgt_type): + # # Minions are allowed, verify function in allowed list + # if isinstance(ind[valid], six.string_types): + # if self.match_check(ind[valid], fun): + # return True + # elif isinstance(ind[valid], list): + # for cond in ind[valid]: + # # Function name match + # if isinstance(cond, six.string_types): + # if self.match_check(cond, fun): + # return True + # # Function and args match + # elif isinstance(cond, dict): + # if len(cond) != 1: + # # Invalid argument + # continue + # fcond = next(six.iterkeys(cond)) + # # cond: { + # # 'mod.func': { + # # 'args': [ + # # 'one.*', 'two\\|three'], + # # 'kwargs': { + # # 'functioin': 'teach\\|feed', + # # 'user': 'mother\\|father' + # # } + # # } + # # } + # if self.match_check(fcond, fun): # check key that is function name match + # acond = cond[fcond] + # if not isinstance(acond, dict): + # # Invalid argument + # continue + # # whitelist args, kwargs + # arg_list = args[num] + # cond_args = acond.get('args', []) + # good = True + # for i, cond_arg in enumerate(cond_args): + # if len(arg_list) <= i: + # good = False + # break + # if cond_arg is None: # None == '.*' i.e. allow any + # continue + # if not self.match_check(cond_arg, arg_list[i]): + # good = False + # break + # if not good: + # continue + # # Check kwargs + # cond_kwargs = acond.get('kwargs', {}) + # arg_kwargs = {} + # for a in arg_list: + # if isinstance(a, dict) and '__kwarg__' in a: + # arg_kwargs = a + # break + # for k, v in six.iteritems(cond_kwargs): + # if k not in arg_kwargs: + # good = False + # break + # if v is None: # None == '.*' i.e. allow any + # continue + # if not self.match_check(v, arg_kwargs[k]): + # good = False + # break + # if good: + # return True + except TypeError: + return False + return False + def auth_check(self, auth_list, funs, @@ -711,105 +897,148 @@ class CkMinions(object): Used to evaluate the standard structure under external master authentication interfaces, like eauth, peer, peer_run, etc. ''' - if publish_validate: - v_tgt_type = tgt_type - if tgt_type.lower() in ('pillar', 'pillar_pcre'): - v_tgt_type = 'pillar_exact' - elif tgt_type.lower() == 'compound': - v_tgt_type = 'compound_pillar_exact' - v_minions = set(self.check_minions(tgt, v_tgt_type)) - minions = set(self.check_minions(tgt, tgt_type)) - mismatch = bool(minions.difference(v_minions)) + + v_tgt_type = tgt_type + if tgt_type.lower() in ('pillar', 'pillar_pcre'): + v_tgt_type = 'pillar_exact' + elif tgt_type.lower() == 'compound': + v_tgt_type = 'compound_pillar_exact' + v_minions = set(self.check_minions(tgt, v_tgt_type)) + minions = set(self.check_minions(tgt, tgt_type)) + mismatch = bool(minions.difference(v_minions)) # If the non-exact match gets more minions than the exact match # then pillar globbing or PCRE is being used, and we have a # problem + if publish_validate: if mismatch: return False # compound commands will come in a list so treat everything as a list if not isinstance(funs, list): funs = [funs] args = [args] + + # Take the auth list and get all the minion names inside it + allowed_minions_from_auth_list = set() + + auth_dictionary = {} + # Make a set, so we are guaranteed to have only one of each minion + # Also iterate through the entire auth_list and create a dictionary + # so it's easy to look up what functions are permitted + for auth_list_entry in auth_list: + if isinstance(auth_list_entry, six.string_types): + for fun in funs: + # represents toplevel auth entry is a function. + # so this fn is permitted by all minions + if self.match_check(auth_list_entry, fun): + return True + if isinstance(auth_list_entry, dict): + if len(auth_list_entry) != 1: + log.info('Malformed ACL: {0}'.format(auth_list_entry)) + continue + allowed_minions_from_auth_list.update(set(auth_list_entry.keys())) + for key in auth_list_entry.keys(): + if key in auth_dictionary: + auth_dictionary[key].concat(auth_list_entry[key]) + else: + auth_dictionary[key] = auth_list_entry[key] + + # 'minions' here are all the names of minions matched by the target + # if we take out all the allowed minions, and there are any left, then + # the target includes minions that are not allowed by eauth + # so we can give up here. + if len(minions - allowed_minions_from_auth_list) > 0: + return False + try: - for num, fun in enumerate(funs): - for ind in auth_list: - if isinstance(ind, six.string_types): - # Allowed for all minions - if self.match_check(ind, fun): - return True - elif isinstance(ind, dict): - if len(ind) != 1: - # Invalid argument - continue - valid = next(six.iterkeys(ind)) - # Check if minions are allowed - if self.validate_tgt( - valid, - tgt, - tgt_type): - # Minions are allowed, verify function in allowed list - if isinstance(ind[valid], six.string_types): - if self.match_check(ind[valid], fun): - return True - elif isinstance(ind[valid], list): - for cond in ind[valid]: - # Function name match - if isinstance(cond, six.string_types): - if self.match_check(cond, fun): - return True - # Function and args match - elif isinstance(cond, dict): - if len(cond) != 1: - # Invalid argument - continue - fcond = next(six.iterkeys(cond)) - # cond: { - # 'mod.func': { - # 'args': [ - # 'one.*', 'two\\|three'], - # 'kwargs': { - # 'functioin': 'teach\\|feed', - # 'user': 'mother\\|father' - # } - # } - # } - if self.match_check(fcond, fun): # check key that is function name match - acond = cond[fcond] - if not isinstance(acond, dict): - # Invalid argument - continue - # whitelist args, kwargs - arg_list = args[num] - cond_args = acond.get('args', []) - good = True - for i, cond_arg in enumerate(cond_args): - if len(arg_list) <= i: - good = False - break - if cond_arg is None: # None == '.*' i.e. allow any - continue - if not self.match_check(cond_arg, arg_list[i]): - good = False - break - if not good: - continue - # Check kwargs - cond_kwargs = acond.get('kwargs', {}) - arg_kwargs = {} - for a in arg_list: - if isinstance(a, dict) and '__kwarg__' in a: - arg_kwargs = a - break - for k, v in six.iteritems(cond_kwargs): - if k not in arg_kwargs: - good = False - break - if v is None: # None == '.*' i.e. allow any - continue - if not self.match_check(v, arg_kwargs[k]): - good = False - break - if good: - return True + log.trace('######################################') + log.trace(funs) + for minion in minions: + results = [] + for num, fun in enumerate(auth_dictionary[minion]): + results.append(self.match_check(fun, funs)) + if not any(results): + return False + return True + + # for ind in auth_list: + # if isinstance(ind, six.string_types): + # # Allowed for all minions + # if self.match_check(ind, fun): + # return True + # elif isinstance(ind, dict): + # if len(ind) != 1: + # # Invalid argument + # continue + # valid = next(six.iterkeys(ind)) + # # Check if minions are allowed + # # if self.validate_tgt( + # # valid, + # # tgt, + # # tgt_type): + # # Minions are allowed, verify function in allowed list + # if isinstance(ind[valid], six.string_types): + # if self.match_check(ind[valid], fun): + # return True + # elif isinstance(ind[valid], list): + # for cond in ind[valid]: + # # Function name match + # if isinstance(cond, six.string_types): + # if self.match_check(cond, fun): + # return True + # # Function and args match + # elif isinstance(cond, dict): + # if len(cond) != 1: + # # Invalid argument + # continue + # fcond = next(six.iterkeys(cond)) + # # cond: { + # # 'mod.func': { + # # 'args': [ + # # 'one.*', 'two\\|three'], + # # 'kwargs': { + # # 'functioin': 'teach\\|feed', + # # 'user': 'mother\\|father' + # # } + # # } + # # } + # if self.match_check(fcond, fun): # check key that is function name match + # acond = cond[fcond] + # if not isinstance(acond, dict): + # # Invalid argument + # continue + # # whitelist args, kwargs + # arg_list = args[num] + # cond_args = acond.get('args', []) + # good = True + # for i, cond_arg in enumerate(cond_args): + # if len(arg_list) <= i: + # good = False + # break + # if cond_arg is None: # None == '.*' i.e. allow any + # continue + # if not self.match_check(cond_arg, arg_list[i]): + # good = False + # break + # if not good: + # continue + # # Check kwargs + # cond_kwargs = acond.get('kwargs', {}) + # arg_kwargs = {} + # for a in arg_list: + # if isinstance(a, dict) and '__kwarg__' in a: + # arg_kwargs = a + # break + # for k, v in six.iteritems(cond_kwargs): + # if k not in arg_kwargs: + # good = False + # break + # if v is None: # None == '.*' i.e. allow any + # continue + # if not self.match_check(v, arg_kwargs[k]): + # good = False + # break + # if good: + # return True except TypeError: return False return False From f5bd3463aae2fa927de3d52570f2896ba804a391 Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Wed, 23 Mar 2016 14:45:31 -0600 Subject: [PATCH 15/16] Replace original auth_check fn. --- salt/utils/minions.py | 225 +++++++++++++++++------------------------- 1 file changed, 93 insertions(+), 132 deletions(-) diff --git a/salt/utils/minions.py b/salt/utils/minions.py index fd8e3a8c83..6d997c8dd7 100644 --- a/salt/utils/minions.py +++ b/salt/utils/minions.py @@ -897,148 +897,109 @@ class CkMinions(object): Used to evaluate the standard structure under external master authentication interfaces, like eauth, peer, peer_run, etc. ''' - - v_tgt_type = tgt_type - if tgt_type.lower() in ('pillar', 'pillar_pcre'): - v_tgt_type = 'pillar_exact' - elif tgt_type.lower() == 'compound': - v_tgt_type = 'compound_pillar_exact' - v_minions = set(self.check_minions(tgt, v_tgt_type)) - minions = set(self.check_minions(tgt, tgt_type)) - mismatch = bool(minions.difference(v_minions)) + if publish_validate: + v_tgt_type = tgt_type + if tgt_type.lower() in ('pillar', 'pillar_pcre'): + v_tgt_type = 'pillar_exact' + elif tgt_type.lower() == 'compound': + v_tgt_type = 'compound_pillar_exact' + v_minions = set(self.check_minions(tgt, v_tgt_type)) + minions = set(self.check_minions(tgt, tgt_type)) + mismatch = bool(minions.difference(v_minions)) # If the non-exact match gets more minions than the exact match # then pillar globbing or PCRE is being used, and we have a # problem - if publish_validate: if mismatch: return False # compound commands will come in a list so treat everything as a list if not isinstance(funs, list): funs = [funs] args = [args] - - # Take the auth list and get all the minion names inside it - allowed_minions_from_auth_list = set() - - auth_dictionary = {} - # Make a set, so we are guaranteed to have only one of each minion - # Also iterate through the entire auth_list and create a dictionary - # so it's easy to look up what functions are permitted - for auth_list_entry in auth_list: - if isinstance(auth_list_entry, six.string_types): - for fun in funs: - # represents toplevel auth entry is a function. - # so this fn is permitted by all minions - if self.match_check(auth_list_entry, fun): - return True - if isinstance(auth_list_entry, dict): - if len(auth_list_entry) != 1: - log.info('Malformed ACL: {0}'.format(auth_list_entry)) - continue - allowed_minions_from_auth_list.update(set(auth_list_entry.keys())) - for key in auth_list_entry.keys(): - if key in auth_dictionary: - auth_dictionary[key].concat(auth_list_entry[key]) - else: - auth_dictionary[key] = auth_list_entry[key] - - # 'minions' here are all the names of minions matched by the target - # if we take out all the allowed minions, and there are any left, then - # the target includes minions that are not allowed by eauth - # so we can give up here. - if len(minions - allowed_minions_from_auth_list) > 0: - return False - try: - log.trace('######################################') - log.trace(funs) - for minion in minions: - results = [] - for num, fun in enumerate(auth_dictionary[minion]): - results.append(self.match_check(fun, funs)) - if not any(results): - return False - return True - - # for ind in auth_list: - # if isinstance(ind, six.string_types): - # # Allowed for all minions - # if self.match_check(ind, fun): - # return True - # elif isinstance(ind, dict): - # if len(ind) != 1: - # # Invalid argument - # continue - # valid = next(six.iterkeys(ind)) - # # Check if minions are allowed - # # if self.validate_tgt( - # # valid, - # # tgt, - # # tgt_type): - # # Minions are allowed, verify function in allowed list - # if isinstance(ind[valid], six.string_types): - # if self.match_check(ind[valid], fun): - # return True - # elif isinstance(ind[valid], list): - # for cond in ind[valid]: - # # Function name match - # if isinstance(cond, six.string_types): - # if self.match_check(cond, fun): - # return True - # # Function and args match - # elif isinstance(cond, dict): - # if len(cond) != 1: - # # Invalid argument - # continue - # fcond = next(six.iterkeys(cond)) - # # cond: { - # # 'mod.func': { - # # 'args': [ - # # 'one.*', 'two\\|three'], - # # 'kwargs': { - # # 'functioin': 'teach\\|feed', - # # 'user': 'mother\\|father' - # # } - # # } - # # } - # if self.match_check(fcond, fun): # check key that is function name match - # acond = cond[fcond] - # if not isinstance(acond, dict): - # # Invalid argument - # continue - # # whitelist args, kwargs - # arg_list = args[num] - # cond_args = acond.get('args', []) - # good = True - # for i, cond_arg in enumerate(cond_args): - # if len(arg_list) <= i: - # good = False - # break - # if cond_arg is None: # None == '.*' i.e. allow any - # continue - # if not self.match_check(cond_arg, arg_list[i]): - # good = False - # break - # if not good: - # continue - # # Check kwargs - # cond_kwargs = acond.get('kwargs', {}) - # arg_kwargs = {} - # for a in arg_list: - # if isinstance(a, dict) and '__kwarg__' in a: - # arg_kwargs = a - # break - # for k, v in six.iteritems(cond_kwargs): - # if k not in arg_kwargs: - # good = False - # break - # if v is None: # None == '.*' i.e. allow any - # continue - # if not self.match_check(v, arg_kwargs[k]): - # good = False - # break - # if good: - # return True + for num, fun in enumerate(funs): + for ind in auth_list: + if isinstance(ind, six.string_types): + # Allowed for all minions + if self.match_check(ind, fun): + return True + elif isinstance(ind, dict): + if len(ind) != 1: + # Invalid argument + continue + valid = next(six.iterkeys(ind)) + # Check if minions are allowed + if self.validate_tgt( + valid, + tgt, + tgt_type): + # Minions are allowed, verify function in allowed list + if isinstance(ind[valid], six.string_types): + if self.match_check(ind[valid], fun): + return True + elif isinstance(ind[valid], list): + for cond in ind[valid]: + # Function name match + if isinstance(cond, six.string_types): + if self.match_check(cond, fun): + return True + # Function and args match + elif isinstance(cond, dict): + if len(cond) != 1: + # Invalid argument + continue + fcond = next(six.iterkeys(cond)) + # cond: { + # 'mod.func': { + # 'args': [ + # 'one.*', 'two\\|three'], + # 'kwargs': { + # 'functioin': 'teach\\|feed', + # 'user': 'mother\\|father' + # } + # } + # } + if self.match_check(fcond, + fun): # check key that is function name match + acond = cond[fcond] + if not isinstance(acond, dict): + # Invalid argument + continue + # whitelist args, kwargs + arg_list = args[num] + cond_args = acond.get('args', []) + good = True + for i, cond_arg in enumerate(cond_args): + if len(arg_list) <= i: + good = False + break + if cond_arg is None: # None == '.*' i.e. allow any + continue + if not self.match_check(cond_arg, + arg_list[i]): + good = False + break + if not good: + continue + # Check kwargs + cond_kwargs = acond.get('kwargs', {}) + arg_kwargs = {} + for a in arg_list: + if isinstance(a, + dict) and '__kwarg__' in a: + arg_kwargs = a + break + for k, v in six.iteritems(cond_kwargs): + if k not in arg_kwargs: + good = False + break + if v is None: # None == '.*' i.e. allow any + continue + if not self.match_check(v, + arg_kwargs[k]): + good = False + break + if good: + return True except TypeError: return False return False From 34300847ec90b14401c562dff54cfa4c87eee0c0 Mon Sep 17 00:00:00 2001 From: rallytime Date: Thu, 24 Mar 2016 09:48:54 -0600 Subject: [PATCH 16/16] Fix test failures --- tests/unit/modules/rabbitmq_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/modules/rabbitmq_test.py b/tests/unit/modules/rabbitmq_test.py index b5c467e4c5..eaabdec7bb 100644 --- a/tests/unit/modules/rabbitmq_test.py +++ b/tests/unit/modules/rabbitmq_test.py @@ -40,7 +40,7 @@ class RabbitmqTestCase(TestCase): ''' mock_run = MagicMock(return_value='Listing users ...\nguest\t[administrator]\n') with patch.dict(rabbitmq.__salt__, {'cmd.run': mock_run}): - self.assertDictEqual(rabbitmq.list_users(), {'guest': ['administrator']}) + self.assertDictEqual(rabbitmq.list_users(), {'guest': set(['administrator'])}) # 'list_users_with_warning' function tests: 1 @@ -55,7 +55,7 @@ class RabbitmqTestCase(TestCase): ]) mock_run = MagicMock(return_value=rtn_val) with patch.dict(rabbitmq.__salt__, {'cmd.run': mock_run}): - self.assertDictEqual(rabbitmq.list_users(), {'guest': ['administrator']}) + self.assertDictEqual(rabbitmq.list_users(), {'guest': set(['administrator'])}) # 'list_vhosts' function tests: 1