Repository: saltstack/salt
Pull request: Only change mine data if using new allow_tgt feature
### What does this PR do?
With the new `allow_tgt` feature in the mine.get the data structure was changed. This PR ensures we only send this new data structure if we are using the new `allow_tgt` feature. 

Fixes the issue in https://github.com/saltstack/salt/issues/56118

The initial issue made it so a master on <3000 would not  work with a >=3000 minion with mine data. With this PR we only change the data structure if we are using the new feature, so we assume if they are using the new feature they want to use 3000. Also documented that both the master and minion need to be on atleast versions 3000 if using this feature.

This PR also fixes another issue I found. If you set a minion in `allow_tgt` that is a minion that does not actually exist it will return the data regardless. Without this PR given the following config:

```
mine_functions:
  cpu:
    - mine_function: grains.get
    - cpuarch
    - allow_tgt: doesnotexist
```

As you can see above we have defined a minion that does not exist in `allow_tgt`, but I can still gather the data for any minion:

```
[root@localhost ~]# salt-call mine.get \* cpu
local:
    ----------
    master_min:
        x86_64
    thecakeisalie:
        x86_64
```

With this patch, we check to see if the minion even exists, if it does not we do not allow the minion to view the data:

```
[root@localhost ~]# salt-call mine.get \* cpu
local:
    ----------
```


### What issues does this PR fix or reference?
Fixes https://github.com/saltstack/salt/issues/56118

### Previous Behavior
Data returned on a master (<3000) and minion (>=3000), even if not using the new `allow_tgt` feature:

```
    minion_name:
        ----------
        __data__:
            - 10.1.2.3
        __saltmine_acl__:
            1
```

### New Behavior
Data returned on a master (<3000) and minion (>=3000), even if not using the new `allow_tgt` feature:

```
    minion_name:
        - 10.1.2.3
```

To note if one is using the `allow_tgt` feature and they have a master <3000 they will see the wrong data (ex, `__data__`), this feature requires both master and minion are upgraded.

### Tests written?
Yes

### Commits signed with GPG?

Yes

Problem statement:
mine.get returns different dictionary format in version 3000
I created couple new servers today and noticed that the output of the mine function I use for getting  the ip address of the minion is returning in a different format.    I can find documentation about the change in format for defining a mine function, but nothing noting the difference in output.

Am I overlooking something?

### Set
mine function definition (via pillar):
```
mine_functions:
  internal_ip:
    - mine_function: grains.get
    - fqdn_ip4
  internal_ips:
    - mine_function: grains.get
    - ipv4
```

command to get mine data:
```
salt saltmaster mine.get '*' internal_ip
```
expected output:
```
    minion_name:
        - 10.1.2.3
```
received output:
```
    minion_name:
        ----------
        __data__:
            - 10.1.2.3
        __saltmine_acl__:
            1
```

When used in a jinja template I receive an error that there is not an element 0
```
{% for server, addr in salt['mine.get'](match, 'internal_ip', tgt_type='compound').items() | sort(case_sensitive=False) -%}
{{ host }}     IN    A   {{ addr[0] }}
{% endfor -%}
```
```
          ID: zone_file
    Function: file.managed
        Name: /var/named/chroot/etc/named/name.zone
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python2.7/site-packages/salt/state.py", line 1933, in call
                  **cdata['kwargs'])
                File "/usr/lib/python2.7/site-packages/salt/loader.py", line 1951, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python2.7/site-packages/salt/states/file.py", line 2769, in managed
                  **kwargs
                File "/usr/lib/python2.7/site-packages/salt/modules/file.py", line 4824, in check_managed_changes
                  **kwargs)
                File "/usr/lib/python2.7/site-packages/salt/modules/file.py", line 4265, in get_managed
                  **kwargs)
                File "/usr/lib/python2.7/site-packages/salt/utils/templates.py", line 169, in render_tmpl
                  output = render_str(tmplstr, context, tmplpath)
                File "/usr/lib/python2.7/site-packages/salt/utils/templates.py", line 404, in render_jinja_tmpl
                  buf=tmplstr)
              SaltRenderError: Jinja variable dict object has no element 0
     Started: 21:07:23.444192
    Duration: 1053.174 ms
     Changes:
```

### Versions Report
```
Salt Version:
           Salt: 3000

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.7.2
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.6.2
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Aug  7 2019, 00:51:29)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.7.1908 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-1062.12.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7.7.1908 Core
```

Hints from issue discussion:
Probably introduced in https://github.com/saltstack/salt/pull/55760

CC: @github-abcde 
>  I can find documentation about the change in format for defining a mine function

Where? It's not mentioned in the release notes.
Interesting. The underscore-variables returned are for internal use only and should not be returned. The code at https://github.com/saltstack/salt/pull/55760/files#diff-b63dd508d9b7489d1f22527bda3f300dR613 detects the `__saltmine_acl__`-key and determines that the mine item is in the "new" (with minion-side ACL) format, and then only returns the contents of the `__data__`-key (line 614). Except that this apparently does not happen in your case. I will look into this.
Edit: Correct spelling error
@ryanmilne-digicert Just a thought, is your salt-master running on version 3000 as well? Because if the versions are mismatched, the minion will send the new datastructure (including minion-side-acl metadata), but the master will not expect this and just return all the data the minion has provided.
> @ryanmilne-digicert Just a thought, is your salt-master running on version 3000 as well? Because if the versions are mismatched, the minion will send the new datastructure (including minion-side-acl metadata), but the master will not expect this and just return all the data the minion has provided.

I ran into this as well - updating the master to 3000 allows the mine to display as expected.
I just checked and my saltmaster is still on 2019.2.2.  However, I am using the docker images and there does not appear to be anything newer published to docker hub.

https://hub.docker.com/r/saltstack/salt/tags
> > I can find documentation about the change in format for defining a mine function
> 
> Where? It's not mentioned in the release notes.
I was only pointing out that there was documentation of changed around mine functions, but nothing about a change to the output.

https://docs.saltstack.com/en/latest/topics/mine/
```
Changed in version 3000.

The format to define mine_functions has been changed to allow the same format as used for module.run. The old format (above) will still be supported.
```
https://docs.saltstack.com/en/latest/faq.html#can-i-run-different-versions-of-salt-on-my-master-and-minion

> When upgrading Salt, the master(s) should always be upgraded first. Backwards compatibility for minions running newer versions of salt than their masters is not guaranteed.

However, it looks like it is quite easy to temporarily patch an older master:
 
```diff
diff --git a/salt/daemons/masterapi.py b/salt/daemons/masterapi.py
index 8fc5bdc0af..61067bef56 100644
--- a/salt/daemons/masterapi.py
+++ b/salt/daemons/masterapi.py
@@ -591,7 +591,10 @@ class RemoteFuncs(object):
             if isinstance(fdata, dict):
                 fdata = fdata.get(load['fun'])
                 if fdata:
-                    ret[minion] = fdata
+                    if isinstance(fdata, dict) and '__data__' in fdata:
+                        ret[minion] = fdata['__data__']
+                    else:
+                        ret[minion] = fdata
         return ret

     def _mine(self, load, skip_verify=False):
diff --git a/salt/utils/minions.py b/salt/utils/minions.py
index a24f293701..21bea126e8 100644
--- a/salt/utils/minions.py
+++ b/salt/utils/minions.py
@@ -1157,5 +1157,8 @@ def mine_get(tgt, fun, tgt_type='glob', opts=None):
             continue
         fdata = mdata.get(fun)
         if fdata:
-            ret[minion] = fdata
+            if isinstance(fdata, dict) and '__data__' in fdata:
+                ret[minion] = fdata['__data__']
+            else:
+                ret[minion] = fdata
     return ret
```
@max-arnold is completely right that you should always upgrade your master first, so this is definitely the recommended approach. Although we do not guarantee backwards compatibility of newer minions with older masters we want to try to make a best effort to keep that compatibility. Diving in now to see if its even possible.
It makes sense to upgrade the master first, however I ran into this as I created some new servers with Salt-Cloud and did not see that a new version of salt had been release.

Another problem though is I am using the docker images on docker hub and those have not been updated for 2 months.  
https://hub.docker.com/r/saltstack/salt/tags
Is there a timeframe when a new image will be published on dockerhub?

Commit message: Use different targeting for windows/linux

Patch to review:
diff --git a/doc/topics/releases/3000.rst b/doc/topics/releases/3000.rst
index b844fe163230..9affa1549c24 100644
--- a/doc/topics/releases/3000.rst
+++ b/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
 ============
 
diff --git a/doc/topics/releases/sodium.rst b/doc/topics/releases/sodium.rst
index 2bd871e84644..407a12d78bcb 100644
--- a/doc/topics/releases/sodium.rst
+++ b/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>`.
diff --git a/salt/daemons/masterapi.py b/salt/daemons/masterapi.py
index d96db3dd31a1..073765b8fa13 100644
--- a/salt/daemons/masterapi.py
+++ b/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:
diff --git a/salt/modules/mine.py b/salt/modules/mine.py
index 91e36f8197ee..9f91992dd4c9 100644
--- a/salt/modules/mine.py
+++ b/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)
 
 
diff --git a/tests/integration/files/conf/minion b/tests/integration/files/conf/minion
index 29fdd43e6c86..920ce4b41de6 100644
--- a/tests/integration/files/conf/minion
+++ b/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:
diff --git a/tests/integration/files/conf/sub_minion b/tests/integration/files/conf/sub_minion
index 4ebe93c359aa..2574e17c5dae 100644
--- a/tests/integration/files/conf/sub_minion
+++ b/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'
diff --git a/tests/integration/modules/test_mine.py b/tests/integration/modules/test_mine.py
index 844536b0c92f..ac8f27970eb3 100644
--- a/tests/integration/modules/test_mine.py
+++ b/tests/integration/modules/test_mine.py
@@ -8,29 +8,29 @@
 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
 
+# Import Salt libs
+import salt.utils.platform
 
-class MineTest(ModuleCase):
+
+class MineTest(ModuleCase, ShellCase):
     '''
     Test the mine system
     '''
     def setUp(self):
+        self.tgt = r'\*'
+        if salt.utils.platform.is_windows():
+            self.tgt = '*'
         self.wait_for_all_jobs()
 
     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 +40,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('mine.get {0} test.arg'.format(self.tgt), 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('mine.get {0} test.arg'.format(self.tgt))
+        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('mine.get {0} {1}'.format(self.tgt, mine_name))
+        sub_ret = self.run_call('mine.get {0} {1}'.format(self.tgt, 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('mine.get {0} {1}'.format(self.tgt, mine_name))
+        sub_ret = self.run_call('mine.get {0} {1}'.format(self.tgt, 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('mine.get {0} {1}'.format(self.tgt, mine_name))
+        sub_ret = self.run_call('mine.get {0} {1}'.format(self.tgt, 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
diff --git a/tests/support/case.py b/tests/support/case.py
index ccb7161fe406..c0e3186b8f2e 100644
--- a/tests/support/case.py
+++ b/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:
diff --git a/tests/unit/modules/test_mine.py b/tests/unit/modules/test_mine.py
index 300ff2c33851..30de6f4b2ff1 100644
--- a/tests/unit/modules/test_mine.py
+++ b/tests/unit/modules/test_mine.py
@@ -43,6 +43,9 @@ class MineTestCase(TestCase, LoaderModuleMockMixin):
     Test cases for salt.modules.mine
     '''
     def setUp(self):
+        self.kernel_ret = 'Linux!'
+        self.foo_ret = 'baz'
+        self.ip_ret = '2001:db8::1:3'
         self.cache = FakeCache()
 
     def setup_loader_modules(self):
@@ -94,15 +97,16 @@ def test_send_get_local(self):
         '''
         Tests sending an item to the mine in the minion's local cache,
         and then immediately fetching it again (since tests are executed unordered).
-        Also verify that the stored mine cache has the correct structure (with ACL).
+        Also verify that the stored mine cache does not use ACL data structure
+        without allow_tgt passed.
         '''
         with patch.dict(mine.__opts__, {
                     'file_client': 'local',
                     'id': 'webserver',
                 }), \
                 patch.dict(mine.__salt__, {
-                    'network.ip_addrs': MagicMock(return_value='2001:db8::1:3'),
-                    'foo.bar': MagicMock(return_value='baz'),
+                    'network.ip_addrs': MagicMock(return_value=self.ip_ret),
+                    'foo.bar': MagicMock(return_value=self.foo_ret),
                 }):
             ret = mine.send('ip_addr', mine_function='network.ip_addrs')
             mine.send('foo.bar')
@@ -110,14 +114,8 @@ def test_send_get_local(self):
         self.assertEqual(
             self.cache.fetch('minions/webserver', 'mine_cache'),
             {
-                'ip_addr': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: '2001:db8::1:3',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
-                'foo.bar': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: 'baz',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
+                'ip_addr': self.ip_ret,
+                'foo.bar': self.foo_ret,
             }
         )
         with patch.dict(mine.__opts__, {
@@ -128,9 +126,9 @@ def test_send_get_local(self):
             ret_single_dict = mine.get('*', ['ip_addr'])
             ret_multi = mine.get('*', 'ip_addr,foo.bar')
             ret_multi2 = mine.get('*', ['ip_addr', 'foo.bar'])
-        self.assertEqual(ret_single, {'webserver': '2001:db8::1:3'})
-        self.assertEqual(ret_single_dict, {'ip_addr': {'webserver': '2001:db8::1:3'}})
-        self.assertEqual(ret_multi, {'ip_addr': {'webserver': '2001:db8::1:3'}, 'foo.bar': {'webserver': 'baz'}})
+        self.assertEqual(ret_single, {'webserver': self.ip_ret})
+        self.assertEqual(ret_single_dict, {'ip_addr': {'webserver': self.ip_ret}})
+        self.assertEqual(ret_multi, {'ip_addr': {'webserver': self.ip_ret}, 'foo.bar': {'webserver': self.foo_ret}})
         self.assertEqual(ret_multi, ret_multi2)
 
     def test_send_get_acl_local(self):
@@ -138,15 +136,16 @@ def test_send_get_acl_local(self):
         Tests sending an item to the mine in the minion's local cache,
         including ACL information (useless when only working locally, but hey),
         and then immediately fetching it again (since tests are executed unordered).
-        Also verify that the stored mine cache has the correct structure (with ACL).
+        Also verify that the stored mine cache has the correct structure (with ACL)
+        when using allow_tgt and no ACL without allow_tgt.
         '''
         with patch.dict(mine.__opts__, {
                     'file_client': 'local',
                     'id': 'webserver',
                 }), \
                 patch.dict(mine.__salt__, {
-                    'network.ip_addrs': MagicMock(return_value='2001:db8::1:3'),
-                    'foo.bar': MagicMock(return_value='baz'),
+                    'network.ip_addrs': MagicMock(return_value=self.ip_ret),
+                    'foo.bar': MagicMock(return_value=self.foo_ret),
                 }):
             ret = mine.send('ip_addr', mine_function='network.ip_addrs', allow_tgt='web*', allow_tgt_type='glob')
             mine.send('foo.bar')
@@ -155,15 +154,12 @@ def test_send_get_acl_local(self):
             self.cache.fetch('minions/webserver', 'mine_cache'),
             {
                 'ip_addr': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: '2001:db8::1:3',
+                    salt.utils.mine.MINE_ITEM_ACL_DATA: self.ip_ret,
                     salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
                     'allow_tgt': 'web*',
                     'allow_tgt_type': 'glob',
                 },
-                'foo.bar': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: 'baz',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
+                'foo.bar': self.foo_ret,
             }
         )
         with patch.dict(mine.__opts__, {
@@ -171,7 +167,7 @@ def test_send_get_acl_local(self):
                     'id': 'webserver',
                 }):
             ret_single = mine.get('*', 'ip_addr')
-        self.assertEqual(ret_single, {'webserver': '2001:db8::1:3'})
+        self.assertEqual(ret_single, {'webserver': self.ip_ret})
 
     def test_send_master(self):
         '''
@@ -180,7 +176,7 @@ def test_send_master(self):
         '''
         with patch.object(mine, '_mine_send', MagicMock(side_effect=lambda x, y: x)),\
                 patch.dict(mine.__salt__, {
-                    'foo.bar': MagicMock(return_value='baz'),
+                    'foo.bar': MagicMock(return_value=self.foo_ret),
                 }), \
                 patch.dict(mine.__opts__, {
                     'file_client': 'remote',
@@ -192,12 +188,7 @@ def test_send_master(self):
             {
                 'id': 'foo',
                 'cmd': '_mine',
-                'data': {
-                    'foo.bar': {
-                        salt.utils.mine.MINE_ITEM_ACL_DATA: 'baz',
-                        salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                    },
-                },
+                'data': {'foo.bar': self.foo_ret},
                 'clear': False,
             }
         )
@@ -209,7 +200,7 @@ def test_send_master_acl(self):
         '''
         with patch.object(mine, '_mine_send', MagicMock(side_effect=lambda x, y: x)),\
                 patch.dict(mine.__salt__, {
-                    'foo.bar': MagicMock(return_value='baz'),
+                    'foo.bar': MagicMock(return_value=self.foo_ret),
                 }), \
                 patch.dict(mine.__opts__, {
                     'file_client': 'remote',
@@ -223,7 +214,7 @@ def test_send_master_acl(self):
                 'cmd': '_mine',
                 'data': {
                     'foo.bar': {
-                        salt.utils.mine.MINE_ITEM_ACL_DATA: 'baz',
+                        salt.utils.mine.MINE_ITEM_ACL_DATA: self.foo_ret,
                         salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
                         'allow_tgt': 'roles:web',
                         'allow_tgt_type': 'grains',
@@ -239,7 +230,7 @@ def test_get_master(self):
         '''
         mock_load = {
             'tgt_type': 'qux',
-            'tgt': 'baz',
+            'tgt': self.foo_ret,
             'cmd': '_mine_get',
             'fun': 'foo.bar',
             'id': 'foo'
@@ -292,9 +283,9 @@ def test_update_local(self):
                 }), \
                 patch.dict(mine.__salt__, {
                     'config.merge': MagicMock(return_value=config_mine_functions),
-                    'grains.get': lambda x: 'Linux!',
-                    'network.ip_addrs': MagicMock(return_value='2001:db8::1:3'),
-                    'foo.bar': MagicMock(return_value='baz'),
+                    'grains.get': lambda x: self.kernel_ret,
+                    'network.ip_addrs': MagicMock(return_value=self.ip_ret),
+                    'foo.bar': MagicMock(return_value=self.foo_ret),
                 }):
             ret = mine.update()
         self.assertEqual(ret, 'FakeCache:StoreSuccess!')
@@ -302,22 +293,16 @@ def test_update_local(self):
         self.assertEqual(
             self.cache.fetch('minions/webserver', 'mine_cache'),
             {
-                'ip_addr': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: '2001:db8::1:3',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
-                'network.ip_addrs': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: '2001:db8::1:3',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
+                'ip_addr': self.ip_ret,
+                'network.ip_addrs': self.ip_ret,
                 'foo.bar': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: 'baz',
+                    salt.utils.mine.MINE_ITEM_ACL_DATA: self.foo_ret,
                     salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
                     'allow_tgt': 'G@roles:webserver',
                     'allow_tgt_type': 'compound',
                 },
                 'kernel': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: 'Linux!',
+                    salt.utils.mine.MINE_ITEM_ACL_DATA: self.kernel_ret,
                     salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
                     'allow_tgt': 'web*',
                 },
@@ -343,8 +328,8 @@ def test_update_local_specific(self):
                 patch.dict(mine.__salt__, {
                     'config.merge': MagicMock(return_value={}),
                     'grains.get': lambda x: 'Linux!!',
-                    'network.ip_addrs': MagicMock(return_value='2001:db8::1:4'),
-                    'foo.bar': MagicMock(return_value='baz'),
+                    'network.ip_addrs': MagicMock(return_value=self.ip_ret),
+                    'foo.bar': MagicMock(return_value=self.foo_ret),
                 }):
             ret = mine.update(mine_functions=manual_mine_functions)
         self.assertEqual(ret, 'FakeCache:StoreSuccess!')
@@ -352,16 +337,10 @@ def test_update_local_specific(self):
         self.assertEqual(
             self.cache.fetch('minions/webserver', 'mine_cache'),
             {
-                'ip_addr': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: '2001:db8::1:4',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
-                'network.ip_addrs': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: '2001:db8::1:4',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
+                'ip_addr': self.ip_ret,
+                'network.ip_addrs': self.ip_ret,
                 'foo.bar': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: 'baz',
+                    salt.utils.mine.MINE_ITEM_ACL_DATA: self.foo_ret,
                     salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
                     'allow_tgt': 'G@roles:webserver',
                     'allow_tgt_type': 'compound',
@@ -388,22 +367,10 @@ def test_update_master(self):
             'id': 'webserver',
             'cmd': '_mine',
             'data': {
-                'ip_addr': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: '2001:db8::1:3',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
-                'network.ip_addrs': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: '2001:db8::1:3',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
-                'foo.bar': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: 'baz',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
-                'kernel': {
-                    salt.utils.mine.MINE_ITEM_ACL_DATA: 'Linux!',
-                    salt.utils.mine.MINE_ITEM_ACL_ID: salt.utils.mine.MINE_ITEM_ACL_VERSION,
-                },
+                'ip_addr': self.ip_ret,
+                'network.ip_addrs': self.ip_ret,
+                'foo.bar': self.foo_ret,
+                'kernel': self.kernel_ret,
             },
             'clear': False,
         }
@@ -415,9 +382,9 @@ def test_update_master(self):
                 }), \
                 patch.dict(mine.__salt__, {
                     'config.merge': MagicMock(return_value=config_mine_functions),
-                    'grains.get': lambda x: 'Linux!',
-                    'network.ip_addrs': MagicMock(return_value='2001:db8::1:3'),
-                    'foo.bar': MagicMock(return_value='baz'),
+                    'grains.get': lambda x: self.kernel_ret,
+                    'network.ip_addrs': MagicMock(return_value=self.ip_ret),
+                    'foo.bar': MagicMock(return_value=self.foo_ret),
                 }):
             # Verify the correct load
             self.assertEqual(


Code review comments:
diff_hunk: @@ -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.
path: salt/modules/mine.py
text: @user1:
`be on, at least, version`?