Only change mine data if using new allow_tgt feature

doc/topics/releases/3000.rst

@@ -592,6 +592,19 @@ Enhancements to chroot
   :py:func:`highstate<salt.modules.chroot.highstate>` that allow executing
   states in sls files or running apply/highstate inside of a chroot.
 
+Minion-side ACL
+---------------
+
+Salt has had master-side ACL for the salt mine for some time, where the master
+configuration contained `mine_get` that specified which minions could request
+which functions. However, now you can specify which minions can access a function
+in the salt mine function definition itself (or when calling :py:func:`mine.send <salt.modules.mine.send>`).
+This targeting works the same as the generic minion targeting as specified
+:ref:`here <targeting>`. The parameters used are ``allow_tgt`` and ``allow_tgt_type``.
+See also :ref:`the documentation of the Salt Mine <mine_minion-side-acl>`. Please
+note that if you want to use this new feature both your minion and masters will need
+to be on atleast version 3000.
+
 Deprecations
 ============
 

doc/topics/releases/sodium.rst

@@ -16,14 +16,3 @@ also support the syntax used in :py:mod:`module.run <salt.states.module.run>`.
 The old syntax for the mine_function - as a dict, or as a list with dicts that
 contain more than exactly one key - is still supported but discouraged in favor
 of the more uniform syntax of module.run.
-
-Minion-side ACL
----------------
-
-Salt has had master-side ACL for the salt mine for some time, where the master
-configuration contained `mine_get` that specified which minions could request
-which functions. However, now you can specify which minions can access a function
-in the salt mine function definition itself (or when calling :py:func:`mine.send <salt.modules.mine.send>`).
-This targeting works the same as the generic minion targeting as specified
-:ref:`here <targeting>`. The parameters used are ``allow_tgt`` and ``allow_tgt_type``.
-See also :ref:`the documentation of the Salt Mine <mine_minion-side-acl>`.

salt/daemons/masterapi.py

@@ -617,14 +617,17 @@ def _mine_get(self, load, skip_verify=False):
                         if 'allow_tgt' in mine_entry:
                             # Only determine allowed targets if any have been specified.
                             # This prevents having to add a list of all minions as allowed targets.
+                            get_minion = checker.check_minions(
+                                         mine_entry['allow_tgt'],
+                                         mine_entry.get('allow_tgt_type', 'glob'))['minions']
+                            # the minion in allow_tgt does not exist
+                            if not get_minion:
+                                continue
                             salt.utils.dictupdate.set_dict_key_value(
                                 minion_side_acl,
                                 '{}:{}'.format(minion, function),
-                                checker.check_minions(
-                                    mine_entry['allow_tgt'],
-                                    mine_entry.get('allow_tgt_type', 'glob')
-                                )['minions']
-                            )
+                                get_minion
+                           )
                 if salt.utils.mine.minion_side_acl_denied(minion_side_acl, minion, function, load['id']):
                     continue
                 if _ret_dict:

salt/modules/mine.py

@@ -194,10 +194,13 @@ def update(clear=False, mine_functions=None):
             log.error('Function %s in mine.update failed to execute', function_name or function_alias)
             log.debug('Error: %s', trace)
             continue
-        mine_data[function_alias] = salt.utils.mine.wrap_acl_structure(
-            res,
-            **minion_acl
-        )
+        if minion_acl.get('allow_tgt'):
+            mine_data[function_alias] = salt.utils.mine.wrap_acl_structure(
+                res,
+                **minion_acl
+            )
+        else:
+            mine_data[function_alias] = res
     return _mine_store(mine_data, clear)
 
 
@@ -213,9 +216,13 @@ def send(name, *args, **kwargs):
     :param str mine_function: The name of the execution_module.function to run
         and whose value will be stored in the salt mine. Defaults to ``name``.
     :param str allow_tgt: Targeting specification for ACL. Specifies which minions
-        are allowed to access this function.
+        are allowed to access this function. Please note both your master and
+        minion need to be on atleast version 3000 for this to work properly.
+
     :param str allow_tgt_type: Type of the targeting specification. This value will
-        be ignored if ``allow_tgt`` is not specified.
+        be ignored if ``allow_tgt`` is not specified. Please note both your
+        master and minion need to be on atleast version 3000 for this to work
+        properly.
 
     Remaining args and kwargs will be passed on to the function to run.
 
@@ -252,11 +259,15 @@ def send(name, *args, **kwargs):
         log.error('Function %s in mine.send failed to execute', mine_function or name)
         log.debug('Error: %s', trace)
         return False
-    mine_data[name] = salt.utils.mine.wrap_acl_structure(
-        res,
-        allow_tgt=allow_tgt,
-        allow_tgt_type=allow_tgt_type
-    )
+
+    if allow_tgt:
+        mine_data[name] = salt.utils.mine.wrap_acl_structure(
+            res,
+            allow_tgt=allow_tgt,
+            allow_tgt_type=allow_tgt_type
+        )
+    else:
+        mine_data[name] = res
     return _mine_store(mine_data)
 
 

tests/integration/files/conf/minion

@@ -95,6 +95,9 @@ config_test:
 
 mine_functions:
   test.ping: []
+  test.arg:
+    - isn't
+    - allow_tgt: 'sub_minion'
 
 # sdb env module
 osenv:

tests/integration/files/conf/sub_minion

@@ -62,3 +62,8 @@ grains:
     keystone.password: demopass
     keystone.tenant: demo
     keystone.auth_url: http://127.0.0.1:5000/v3/
+
+mine_functions:
+  test.arg:
+    - isn't
+    - allow_tgt: 'sub_minion'

tests/integration/modules/test_mine.py

@@ -8,10 +8,11 @@
 import pprint
 
 # Import Salt Testing libs
-from tests.support.case import ModuleCase
+from tests.support.case import ModuleCase, ShellCase
+from tests.support.runtests import RUNTIME_VARS
 
 
-class MineTest(ModuleCase):
+class MineTest(ModuleCase, ShellCase):
     '''
     Test the mine system
     '''
@@ -22,15 +23,8 @@ def test_get(self):
         '''
         test mine.get and mine.update
         '''
-        self.assertTrue(self.run_function('mine.update', minion_tgt='minion'))
-        # The sub_minion does not have mine_functions defined in its configuration
-        # In this case, mine.update returns None
-        self.assertIsNone(
-            self.run_function(
-                'mine.update',
-                minion_tgt='sub_minion'
-            )
-        )
+        assert self.run_function('mine.update', minion_tgt='minion')
+        assert self.run_function('mine.update', minion_tgt='sub_minion')
         # Since the minion has mine_functions defined in its configuration,
         # mine.update will return True
         self.assertTrue(
@@ -40,6 +34,78 @@ def test_get(self):
             )
         )
 
+    def test_get_allow_tgt(self):
+        '''
+        test mine.get and mine.update using allow_tgt
+        '''
+        assert self.run_function('mine.update', minion_tgt='minion')
+        assert self.run_function('mine.update', minion_tgt='sub_minion')
+
+        # sub_minion should be able to view test.arg data
+        sub_min_ret = self.run_call(r'mine.get \* test.arg', config_dir=RUNTIME_VARS.TMP_SUB_MINION_CONF_DIR)
+        assert "            - isn't" in sub_min_ret
+
+        # minion should not be able to view test.arg data
+        min_ret = self.run_call(r'mine.get \* test.arg')
+        assert "            - isn't" not in min_ret
+
+    def test_send_allow_tgt(self):
+        '''
+        test mine.send with allow_tgt set
+        '''
+        mine_name = 'test_this'
+        for minion in ['sub_minion', 'minion']:
+            assert self.run_function('mine.send', [mine_name,
+                'mine_function=test.arg_clean', 'one'], allow_tgt='sub_minion',
+                minion_tgt=minion)
+        min_ret = self.run_call(r'mine.get \* ' + mine_name)
+        sub_ret = self.run_call(r'mine.get \* ' + mine_name,
+                config_dir=RUNTIME_VARS.TMP_SUB_MINION_CONF_DIR)
+
+        # ensure we did get the mine_name mine function for sub_minion
+        assert '            - one' in sub_ret
+        # ensure we did not get the mine_name mine function for minion
+        assert '            - one' not in min_ret
+
+    def test_send_allow_tgt_compound(self):
+        '''
+        test mine.send with allow_tgt set
+        and using compound targeting
+        '''
+        mine_name = 'test_this_comp'
+        for minion in ['sub_minion', 'minion']:
+            assert self.run_function('mine.send', [mine_name,
+                'mine_function=test.arg_clean', 'one'],
+                allow_tgt='L@minion,sub_minion',
+                allow_tgt_type='compound',
+                minion_tgt=minion)
+        min_ret = self.run_call(r'mine.get \* ' + mine_name)
+        sub_ret = self.run_call(r'mine.get \* ' + mine_name,
+                config_dir=RUNTIME_VARS.TMP_SUB_MINION_CONF_DIR)
+
+        # ensure we get the mine_name mine function for both minions
+        for ret in [min_ret, sub_ret]:
+            assert '            - one' in ret
+
+    def test_send_allow_tgt_doesnotexist(self):
+        '''
+        test mine.send with allow_tgt set when
+        the minion defined in allow_tgt does
+        not exist
+        '''
+        mine_name = 'mine_doesnotexist'
+        for minion in ['sub_minion', 'minion']:
+            assert self.run_function('mine.send', [mine_name,
+                'mine_function=test.arg_clean', 'one'], allow_tgt='doesnotexist',
+                minion_tgt=minion)
+        min_ret = self.run_call(r'mine.get \* ' + mine_name)
+        sub_ret = self.run_call(r'mine.get \* ' + mine_name,
+                config_dir=RUNTIME_VARS.TMP_SUB_MINION_CONF_DIR)
+
+        # ensure we did not get the mine_name mine function for both minions
+        for ret in [sub_ret, min_ret]:
+            assert '            - one' not in ret
+
     def test_send(self):
         '''
         test mine.send

tests/support/case.py

@@ -175,9 +175,12 @@ def run_cp(self, arg_str, with_retcode=False, catch_stderr=False):
         arg_str = '--config-dir {0} {1}'.format(self.config_dir, arg_str)
         return self.run_script('salt-cp', arg_str, with_retcode=with_retcode, catch_stderr=catch_stderr)
 
-    def run_call(self, arg_str, with_retcode=False, catch_stderr=False, local=False, timeout=15):
+    def run_call(self, arg_str, with_retcode=False, catch_stderr=False,
+            local=False, timeout=15, config_dir=None):
+        if not config_dir:
+            config_dir = self.config_dir
         arg_str = '{0} --config-dir {1} {2}'.format('--local' if local else '',
-                                                    self.config_dir, arg_str)
+                                                    config_dir, arg_str)
 
         return self.run_script('salt-call',
                                arg_str,
@@ -582,12 +585,14 @@ def run_cp(self, arg_str, with_retcode=False, catch_stderr=False,  # pylint: dis
                                timeout=timeout)
 
     def run_call(self, arg_str, with_retcode=False, catch_stderr=False,  # pylint: disable=W0221
-            local=False, timeout=RUN_TIMEOUT):
+            local=False, timeout=RUN_TIMEOUT, config_dir=None):
         '''
         Execute salt-call.
         '''
+        if not config_dir:
+            config_dir = self.config_dir
         arg_str = '{0} --config-dir {1} {2}'.format('--local' if local else '',
-                                                    self.config_dir, arg_str)
+                                                    config_dir, arg_str)
         ret = self.run_script('salt-call',
                               arg_str,
                               with_retcode=with_retcode,
@@ -772,8 +777,6 @@ def run_function(self, function, arg=(), minion_tgt='minion', timeout=300, maste
             'ssh.recv_known_host_entries',
             'time.sleep'
         )
-        if minion_tgt == 'sub_minion':
-            known_to_return_none += ('mine.update',)
         if 'f_arg' in kwargs:
             kwargs['arg'] = kwargs.pop('f_arg')
         if 'f_timeout' in kwargs: