Fixes commit a28eb94, which broke validate_certs = False for python <…

[MikeKlebolt]

SUMMARY

Fixes commit a28eb94, which broke validate_certs = False for python < 2.7.9

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

vsphere_guest

ANSIBLE VERSION

ansible 2.5.0 (devel 3950f5b9ce) last updated 2018/01/15 16:55:05 (GMT +000)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /data01/ansible/lib/ansible
  executable location = /data01/ansible/bin/ansible
  python version = 2.6.6 (r266:84292, Aug  9 2016, 06:11:56) [GCC 4.4.7 20120313 (Red Hat 4.4.7-17)]

ADDITIONAL INFORMATION

If the vsphere_guest module is used with validate_certs = False and your python version is < 2.7.9. The following line is evaluated in the module which results in the module failing.

    if not validate_certs:
        ssl._create_default_https_context = ssl._create_unverified_context

diff

-    if not validate_certs:
+    if not validate_certs and sys.version_info > (2,7,9):
        ssl._create_default_https_context = ssl._create_unverified_context

[MikeKlebolt]

I accidentally referenced the wrong commit in my commit name. Should I redo this?

18529a2
should have been
a28eb94

[ansibot]

cc @Akasurde @dagwieers @dav1x @jctanner @nerzhul @rhoop
click here for bot help

[sivel]

Can you explain in more detail how this is failing? We shouldn't be doing specific version checks like this, as vendors are known to backport changes like this into older versions. Instead we should rely on functionality tests.

[MikeKlebolt]

The ssl module doesn't have _create_unverified_context in python < 2.7.9, causing the error below.

<localhost> ESTABLISH LOCAL CONNECTION FOR USER: root
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /tmp/ansible-tmp-1516036846.26-125420619837893 `" && echo ansible-tmp-1516036846.26-125420619837893="` echo /tmp/ansible-tmp-1516036846.26-125420619837893 `" ) && sleep 0'
<localhost> PUT /tmp/tmpZGfkK5 TO /tmp/ansible-tmp-1516036846.26-125420619837893/vsphere_guest.py
<localhost> EXEC /bin/sh -c 'chmod u+x /tmp/ansible-tmp-1516036846.26-125420619837893/ /tmp/ansible-tmp-1516036846.26-125420619837893/vsphere_guest.py && sleep 0'
<redacted>
The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_Dw4Ecm/ansible_module_vsphere_guest.py", line 1933, in <module>
    main()
  File "/tmp/ansible_Dw4Ecm/ansible_module_vsphere_guest.py", line 1802, in main
    ssl._create_default_https_context = ssl._create_unverified_context
AttributeError: 'module' object has no attribute '_create_unverified_context'

fatal: [10.12.32.116 -> localhost]: FAILED! => {
    "attempts": 1,
    "changed": false,
    "module_stderr": "Traceback (most recent call last):\n  File \"/tmp/ansible_Dw4Ecm/ansible_module_vsphere_guest.py\", line 1933, in <module>\n    main()\n  File \"/tmp/ansible_Dw4Ecm/ansible_module_vsphere_guest.py\", line 1802, in main\n    ssl._create_default_https_context = ssl._create_unverified_context\nAttributeError: 'module' object has no attribute '_create_unverified_context'\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE",
    "rc": 0
}

- name:  Create the VM
  vsphere_guest:
   vcenter_hostname: "{{ vcenter_hostname }}"
   username: "{{ username }}"
   password: "{{ password }}"
   validate_certs: False
   cluster: "{{ cluster }}"
   guest: "{{ guest }}"
   state: present
   vm_hardware:
    memory_mb: "{{ memory_gb|int * 1024 }}"
    num_cpus: "{{ num_cpus|int }}"
    osid: "{{ osid }}"
    scsi: "{{ scsi }}"
    vm_cdrom:
     type: "client"
   vm_disk: "{{ disks }}"
   vm_nic: "{{ nics }}"
   esxi:
    datacenter: "{{ datacenter }}"
    hostname: "{{ hostname }}"

[sivel]

I believe we need to expand our logic at

ansible/lib/ansible/modules/cloud/vmware/vsphere_guest.py

Line 1797 in 2a45374

if validate_certs and not hasattr(ssl, 'SSLContext') and not vcenter_hostname.startswith('http://'):

If validate_certs is supplied in any form explicitly by the caller, we should fail if we don't have the correct python version.

As you can see in that logic, we are also checking for functionality, and not just doing a version comparison.

I don't think we should silently ignore a value that is passed in by the user.

[ansibot]

The test ansible-test sanity --test pep8 [?] failed with the following errors:

lib/ansible/modules/cloud/vmware/vsphere_guest.py:1802:52: E231 missing whitespace after ','
lib/ansible/modules/cloud/vmware/vsphere_guest.py:1802:54: E231 missing whitespace after ','

click here for bot help

[MikeKlebolt]

I'm not sure I completely follow what you're saying. Are you thinking something like this instead or am I misunderstanding?

    if validate_certs and not hasattr(ssl, 'SSLContext') and not vcenter_hostname.startswith('http://'):
        module.fail_json(msg='pysphere does not support verifying certificates with python < 2.7.9.  Either update python or set '
                             'validate_certs=False on the task')

    if not validate_certs and hasattr(ssl, 'SSLContext'):
        ssl._create_default_https_context = ssl._create_unverified_context

[sivel]

I'm not sure I completely follow what you're saying. Are you thinking something like this instead or am I misunderstanding?

That is partially correct. We should check for functionality, not for a specific version.

However, I was saying, that if a user explicitly tries to influence the results of certification verification, either with a True or False value, we should fail. But based on the current message, indicating you should use validate_certs=False, I think your hasattr may be sufficient.

@Akasurde will need to review.

[MikeKlebolt]

Cool. I've updated my commit to reflect what we've discussed for the the time being.

[Akasurde]

@MikeKlebolt Are you still working on this ? Also, Could you please add -vvvv output here so that we can take it forward?

In one of the IRC meeting of VMware SIG, we decided to merge all vsphere_guest PRs based on -vvv output. This decision was taken as vsphere_guest is deprecated in version 2.5. You can find the details here

Let me know if you need any other information.

[MikeKlebolt]

@Akasurde, I have recently switched to the vmware_guest module after finding out that this module was being deprecated.

Anyways, here is the vvvv output for you.

TASK [Preparation : Create the VM] ****************************************************************************************************
task path: /etc/ansible/roles/Preparation/tasks/prepareVM.yml:187
Using module file /data01/ansible/lib/ansible/modules/cloud/vmware/vsphere_guest.py
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: root
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /tmp/ansible-local-13403ONMcxi/ansible-tmp-1519051847.41-243491483079223 `" && echo ansible-tmp-1519051847.41-243491483079223="` echo /tmp/ansible-local-13403ONMcxi/ansible-tmp-1519051847.41-243491483079223 `" ) && sleep 0'
<localhost> PUT /tmp/ansible-local-13403ONMcxi/tmpYxxoxa TO /tmp/ansible-local-13403ONMcxi/ansible-tmp-1519051847.41-243491483079223/vsphere_guest.py
<localhost> EXEC /bin/sh -c 'chmod u+x /tmp/ansible-local-13403ONMcxi/ansible-tmp-1519051847.41-243491483079223/ /tmp/ansible-local-13403ONMcxi/ansible-tmp-1519051847.41-243491483079223/vsphere_guest.py && sleep 0'
<localhost> EXEC /bin/sh -c '/usr/bin/python /tmp/ansible-local-13403ONMcxi/ansible-tmp-1519051847.41-243491483079223/vsphere_guest.py && sleep 0'
The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_2lDCBn/ansible_module_vsphere_guest.py", line 1933, in <module>
    main()
  File "/tmp/ansible_2lDCBn/ansible_module_vsphere_guest.py", line 1802, in main
    ssl._create_default_https_context = ssl._create_unverified_context
AttributeError: 'module' object has no attribute '_create_unverified_context'

fatal: [10.12.32.120 -> localhost]: FAILED! => {
    "changed": false,
    "failed": true,
    "module_stderr": "Traceback (most recent call last):\n  File \"/tmp/ansible_2lDCBn/ansible_module_vsphere_guest.py\", line 1933, in <module>\n    main()\n  File \"/tmp/ansible_2lDCBn/ansible_module_vsphere_guest.py\", line 1802, in main\n    ssl._create_default_https_context = ssl._create_unverified_context\nAttributeError: 'module' object has no attribute '_create_unverified_context'\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE",
    "rc": 1
}

[Akasurde]

@MikeKlebolt Thanks for output. Sorry for not being specific about logs. I meant about success output of -vvvv after applying patch.

[MikeKlebolt]

My mistake...here you go.

Using module file /data01/ansible/lib/ansible/modules/cloud/vmware/_vsphere_guest.py
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: root
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /tmp/ansible-tmp-1519056036.42-15896784017891 `" && echo ansible-tmp-151905
6036.42-15896784017891="` echo /tmp/ansible-tmp-1519056036.42-15896784017891 `" ) && sleep 0'
<localhost> PUT /tmp/ansible-local-16062MJVCiN/tmpBv0bjh TO /tmp/ansible-tmp-1519056036.42-15896784017891/_vsphere_guest.py
<localhost> EXEC /bin/sh -c 'chmod u+x /tmp/ansible-tmp-1519056036.42-15896784017891/ /tmp/ansible-tmp-1519056036.42-15896784017891/_vs
phere_guest.py && sleep 0'
<localhost> EXEC /bin/sh -c '/usr/bin/python /tmp/ansible-tmp-1519056036.42-15896784017891/_vsphere_guest.py && sleep
0'
<localhost> EXEC /bin/sh -c 'rm -f -r /tmp/ansible-tmp-1519056036.42-15896784017891/ > /dev/null 2>&1 && sleep 0'
[DEPRECATION WARNING]: The 'vsphere_guest' module has been deprecated. Use 'vmware_guest' instead.. This feature will be removed in
version 2.9. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.
ok: [10.12.32.120 -> localhost] => {
    "changed": false,
    "failed": false,
    "invocation": {
        "module_args": {
            "cluster": "clustername",
            "esxi": {
                "datacenter": "datacenter",
                "hostname": "hostname"
            },
            "force": false,
            "from_template": null,
            "guest": "guestname",
            "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "power_on_after_clone": true,
            "resource_pool": null,
            "snapshot_to_clone": null,
            "state": "present",
            "template_src": null,
            "username": "user@domain.com",
            "validate_certs": false,
            "vcenter_hostname": "10.10.10.10",
            "vm_disk": {
                "disk1": {
                    "datastore": "local-datastore",
                    "size_gb": 20,
                    "type": "thin"
                }
            },
            "vm_hardware": {
                "memory_mb": "1024",
                "num_cpus": "1",
                "osid": "rhel6_64Guest",
                "scsi": "paravirtual",
                "vm_cdrom": {
                    "type": "client"
                }
            },
            "vm_hw_version": null,
            "vm_nic": {
                "nic1": {
                    "network": "jumpkick",
                    "network_type": "standard",
                    "type": "vmxnet3"
                }
            },
            "vmware_guest_facts": null
            }
         }
    }
}

[Akasurde]

@MikeKlebolt Thanks for output. One more thing - rebase this branch and we will merge it.

[ansibot]

@MikeKlebolt This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

[MikeKlebolt]

Fixing this now.

[MikeKlebolt]

Hi @Akasurde, is this still making it in to 2.5?

[Akasurde]

rebuild_merge

[Akasurde]

@MikeKlebolt I will backport this to 2.5

[Akasurde]

@MikeKlebolt Backport PR #37018