Repository: saltstack/salt
Pull request: Fix to exsi grains
### What does this PR do?
Moving import salt.modules.vsphere into virtual so we have access to test proxytype in opts.  Gating the check with salt.utils.proxy.is_proxytype.  Adding tests.

### What issues does this PR fix or reference?
Fixes: #57811

### Merge requirements satisfied?
**[NOTICE] Bug fixes or features added to Salt require tests.**
<!-- Please review the [test documentation](https://docs.saltstack.com/en/master/topics/tutorials/writing_tests.html) for details on how to implement tests into Salt's test suite. -->
- [ ] Docs
- [x] Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
- [x] Tests written/updated

### Commits signed with GPG?
Yes

Please review [Salt's Contributing Guide](https://docs.saltstack.com/en/master/topics/development/contributing.html) for best practices.

See GitHub's [page on GPG signing](https://help.github.com/articles/signing-commits-using-gpg/) for more information about signing commits with GPG.


Problem statement:
[BUG] Traceback on any Proxy Minion startup due to ESXI Grain module missing __opts__
**Description**

Start any Proxy Minion with 3001, and see the following error multiple times:

```
[ERROR   ] Failed to import grains esxi, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/salt/loader.py", line 1685, in _load_module
    mod = spec.loader.load_module()
  File "<frozen importlib._bootstrap_external>", line 399, in _check_name_wrapper
  File "<frozen importlib._bootstrap_external>", line 823, in load_module
  File "<frozen importlib._bootstrap_external>", line 682, in load_module
  File "<frozen importlib._bootstrap>", line 265, in _load_module_shim
  File "<frozen importlib._bootstrap>", line 684, in _load
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/local/lib/python3.6/site-packages/salt/grains/esxi.py", line 18, in <module>
    if salt.utils.platform.is_proxy() and __opts__["proxy"]["proxytype"] == "esxi":
NameError: name '__opts__' is not defined
```

**Setup**

Proxy config:

```yaml
open_mode: true
master: localhost
```

Master config:

```yaml
open_mode: true

pillar_roots:
  base:
    - /tmp/pillar
```

Pillar top ``/tmp/pillar/top.sls``

```yaml
base: 
  b0rk:
    - dummy
```

``/tmp/pillar/dummy.sls``

```yaml
proxy:
  proxytype: dummy
```

**Steps to Reproduce the behavior**

Using the files above, start the Proxy Minion and should see the error:

```bash
root@b6c49910bacb:/# salt-proxy -c /tmp --proxyid b0rk
[ERROR   ] Failed to import grains esxi, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/salt/loader.py", line 1685, in _load_module
    mod = spec.loader.load_module()
  File "<frozen importlib._bootstrap_external>", line 399, in _check_name_wrapper
  File "<frozen importlib._bootstrap_external>", line 823, in load_module
  File "<frozen importlib._bootstrap_external>", line 682, in load_module
  File "<frozen importlib._bootstrap>", line 265, in _load_module_shim
  File "<frozen importlib._bootstrap>", line 684, in _load
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/local/lib/python3.6/site-packages/salt/grains/esxi.py", line 18, in <module>
    if salt.utils.platform.is_proxy() and __opts__["proxy"]["proxytype"] == "esxi":
NameError: name '__opts__' is not defined
[ERROR   ] Failed to import grains esxi, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/site-packages/salt/loader.py", line 1685, in _load_module
    mod = spec.loader.load_module()
  File "<frozen importlib._bootstrap_external>", line 399, in _check_name_wrapper
  File "<frozen importlib._bootstrap_external>", line 823, in load_module
  File "<frozen importlib._bootstrap_external>", line 682, in load_module
  File "<frozen importlib._bootstrap>", line 265, in _load_module_shim
  File "<frozen importlib._bootstrap>", line 684, in _load
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/usr/local/lib/python3.6/site-packages/salt/grains/esxi.py", line 18, in <module>
    if salt.utils.platform.is_proxy() and __opts__["proxy"]["proxytype"] == "esxi":
NameError: name '__opts__' is not defined
```

**Versions Report**
<details><summary>salt --versions-report</summary>
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) 

```
root@b6c49910bacb:/# salt -V
Salt Version:
           Salt: 3001
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.11.1
        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: 3.9.8
         pygit2: Not Installed
         Python: 3.6.10 (default, Feb 26 2020, 16:40:36)
   python-gnupg: Not Installed
         PyYAML: 5.3.1
          PyZMQ: 19.0.0
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.2
 
System Versions:
           dist: debian 9 stretch
         locale: UTF-8
        machine: x86_64
        release: 5.3.0-59-generic
         system: Linux
        version: Debian GNU/Linux 9 stretch
```
</details>

Hints from issue discussion:

Commit message: Adding changelog file.

Patch to review:
diff --git a/changelog/57743.fixed b/changelog/57743.fixed
new file mode 100644
index 000000000000..80266bc89db6
--- /dev/null
+++ b/changelog/57743.fixed
@@ -0,0 +1,4 @@
+Moving import salt.modules.vsphere into virtual so we have access to test proxytype in opts,
+previously this was causing a traceback when run on proxy minion as __opts__ does not exist
+outside of any functions. Introducing a new utils function, is_proxytype, to check that the
+device is a proxy minion and also that the proxy type matches.
diff --git a/salt/grains/esxi.py b/salt/grains/esxi.py
index 4b60bdfb2468..66617995c247 100644
--- a/salt/grains/esxi.py
+++ b/salt/grains/esxi.py
@@ -12,13 +12,9 @@
 import logging
 
 # Import Salt Libs
-import salt.utils.platform
+import salt.utils.proxy
 from salt.exceptions import SaltSystemExit
 
-if salt.utils.platform.is_proxy() and __opts__["proxy"]["proxytype"] == "esxi":
-    import salt.modules.vsphere
-
-
 __proxyenabled__ = ["esxi"]
 __virtualname__ = "esxi"
 
@@ -29,8 +25,14 @@
 
 def __virtual__():
 
+    # import salt.utils.proxy again
+    # so it is available for tests.
+    import salt.utils.proxy
+
     try:
-        if salt.utils.platform.is_proxy() and __opts__["proxy"]["proxytype"] == "esxi":
+        if salt.utils.proxy.is_proxytype(__opts__, "esxi"):
+            import salt.modules.vsphere
+
             return __virtualname__
     except KeyError:
         pass
@@ -104,7 +106,7 @@ def _grains():
                 port=port,
             )
             GRAINS_CACHE.update(ret)
-    except KeyError:
+    except KeyError as e:
         pass
 
     return GRAINS_CACHE
diff --git a/salt/utils/proxy.py b/salt/utils/proxy.py
new file mode 100644
index 000000000000..948fa9570cd9
--- /dev/null
+++ b/salt/utils/proxy.py
@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+"""
+Utils for proxy.
+"""
+
+# Import Python libs
+from __future__ import absolute_import, print_function, unicode_literals
+
+import logging
+
+import salt.utils.platform
+
+log = logging.getLogger(__file__)
+
+
+def is_proxytype(opts, proxytype):
+    """
+    Is this a proxy minion of type proxytype
+    """
+    return (
+        salt.utils.platform.is_proxy()
+        and opts.get("proxy", None).get("proxytype", None) == proxytype
+    )
diff --git a/tests/unit/grains/test_esxi.py b/tests/unit/grains/test_esxi.py
new file mode 100644
index 000000000000..557677160eb0
--- /dev/null
+++ b/tests/unit/grains/test_esxi.py
@@ -0,0 +1,111 @@
+# -*- coding: utf-8 -*-
+"""
+    Unit tests for salt.utils.proxy
+
+    :codeauthor: :email:`Gareth J. Greenaway <gareth@saltstack.com>`
+"""
+
+# Import Python Libs
+from __future__ import absolute_import, print_function, unicode_literals
+
+import logging
+
+import salt.grains.esxi as esxi_grains
+
+# Import Salt Testing Libs
+from tests.support.mixins import LoaderModuleMockMixin
+from tests.support.mock import patch
+from tests.support.unit import TestCase
+
+log = logging.getLogger(__name__)
+
+
+class EsxiGrainsTestCase(TestCase, LoaderModuleMockMixin):
+    def setup_loader_modules(self):
+        module_globals = {
+            "__salt__": {},
+            "__opts__": {
+                "proxy": {
+                    "proxytype": "esxi",
+                    "host": "esxi.domain.com",
+                    "username": "username",
+                    "passwords": ["password1"],
+                }
+            },
+            "__pillar__": {
+                "proxy": {
+                    "proxytype": "esxi",
+                    "host": "esxi.domain.com",
+                    "username": "username",
+                    "passwords": ["password1"],
+                }
+            },
+        }
+
+        return {esxi_grains: module_globals}
+
+    def test_virtual(self):
+        with patch("salt.utils.proxy.is_proxytype", return_value=True, autospec=True):
+            ret = esxi_grains.__virtual__()
+            self.assertEqual(ret, "esxi")
+
+    def test_kernel(self):
+        with patch("salt.utils.proxy.is_proxytype", return_value=True, autospec=True):
+            ret = esxi_grains.kernel()
+            self.assertEqual(ret, {"kernel": "proxy"})
+
+    def test_osfamily(self):
+        with patch("salt.utils.proxy.is_proxytype", return_value=True, autospec=True):
+            ret = esxi_grains.os_family()
+            self.assertEqual(ret, {"os_family": "proxy"})
+
+    def test_os(self):
+        grain_cache_return = {
+            "name": "VMware vCenter Server",
+            "fullName": "VMware vCenter Server 6.7.0 build-15679289",
+            "vendor": "VMware, Inc.",
+            "version": "6.7.0",
+            "build": "15679289",
+            "localeVersion": "INTL",
+            "localeBuild": "000",
+            "osType": "linux-x64",
+            "productLineId": "vpx",
+            "apiType": "VirtualCenter",
+            "apiVersion": "6.7.3",
+            "instanceUuid": "058ed113-1820-41dc-a8a9-8a5dd48632a4",
+            "licenseProductName": "VMware VirtualCenter Server",
+            "licenseProductVersion": "6.0",
+        }
+
+        expected = {"os": "VMware vCenter Server 6.7.0 build-15679289"}
+        with patch("salt.utils.proxy.is_proxytype", return_value=True, autospec=True):
+            with patch(
+                "salt.modules.vsphere.system_info", return_value=grain_cache_return
+            ):
+                ret = esxi_grains.os()
+                self.assertEqual(ret, expected)
+
+    def test_esxi(self):
+        grain_cache_return = {
+            "name": "VMware vCenter Server",
+            "fullName": "VMware vCenter Server 6.7.0 build-15679289",
+            "vendor": "VMware, Inc.",
+            "version": "6.7.0",
+            "build": "15679289",
+            "localeVersion": "INTL",
+            "localeBuild": "000",
+            "osType": "linux-x64",
+            "productLineId": "vpx",
+            "apiType": "VirtualCenter",
+            "apiVersion": "6.7.3",
+            "instanceUuid": "058ed113-1820-41dc-a8a9-8a5dd48632a4",
+            "licenseProductName": "VMware VirtualCenter Server",
+            "licenseProductVersion": "6.0",
+        }
+
+        with patch("salt.utils.proxy.is_proxytype", return_value=True, autospec=True):
+            with patch(
+                "salt.modules.vsphere.system_info", return_value=grain_cache_return
+            ):
+                ret = esxi_grains.esxi()
+                self.assertEqual(ret, grain_cache_return)
diff --git a/tests/unit/utils/test_proxy.py b/tests/unit/utils/test_proxy.py
new file mode 100644
index 000000000000..f00f145326de
--- /dev/null
+++ b/tests/unit/utils/test_proxy.py
@@ -0,0 +1,31 @@
+# -*- coding: utf-8 -*-
+"""
+    Unit tests for salt.utils.proxy
+    :codeauthor: :email:`Gareth J. Greenaway <gareth@saltstack.com>`
+"""
+
+# Import python libs
+from __future__ import absolute_import, print_function, unicode_literals
+
+# Import Salt libs
+import salt.utils.proxy
+from tests.support.mock import patch
+
+# Import Salt Testing libs
+from tests.support.unit import TestCase
+
+
+class ProxyUtilsTestCase(TestCase):
+    def test_is_proxytype(self):
+        opts = {
+            "proxy": {
+                "proxytype": "esxi",
+                "host": "esxi.domain.com",
+                "username": "username",
+                "passwords": ["password1"],
+            }
+        }
+
+        with patch("salt.utils.platform.is_proxy", return_value=True, autospec=True):
+            ret = salt.utils.proxy.is_proxytype(opts, "esxi")
+            self.assertTrue(ret)


Code review comments:
diff_hunk: @@ -104,7 +106,7 @@ def _grains():
                 port=port,
             )
             GRAINS_CACHE.update(ret)
-    except KeyError:
+    except KeyError as e:
path: salt/grains/esxi.py
text: @user1:
If we're not going to use `e` let's remove `as e` please.

@author:
Done.

diff_hunk: @@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+"""
+Utils for proxy.
+"""
+
+# Import Python libs
path: salt/utils/proxy.py
text: @user1:
Let's also drop the import comment headers.
Our automated formatting sometimes moves stuff around and those headers stop making sense.
Also, isort, these days maintains a constant order that everyone will, eventually, get accustomed to.

@author:
Done.