ansible-collections / vmware.vmware

Ansible VMWare Collection
GNU General Public License v3.0
7 stars 10 forks source link

migrating community vm info modules into guest_info #51

Closed mikemorency closed 3 months ago

mikemorency commented 3 months ago
SUMMARY

This is an attempt to migrate vm info modules from community.vmware (vmware_guest_info, vmware_vm_info) into vmware.vmware. Since there is already a vm info module in vmware.vmware (guest_info), the functionality from the two community modules has been added to guest_info

This module also requires the rest and soap APIs. So for the simulator tests i added an API gateway so both APIs can be accessed on the same port.

All of the changes should be backwards compatible. We do plan on deprecating the module's old output and removing it at some point in the future. More on that in the additional info section.

ISSUE TYPE
COMPONENT NAME

guest_info

ADDITIONAL INFORMATION

While the inputs for guest_info has not changed, the output should probably be changed. Theres some duplicated information, and some of the values previously returned by guest_info have similar names.

Here is how the guest_info module currently returns info:

guests: [
  {
    old values here
  }
]

For now, we are returning the old values in two locations to the user:

guests: [
  {
    old and new values mixed together
    .....
    identity: {
        old values here
    }
  }
]

At some point in the future, the old values not nested under identity will be removed. Resulting in: It makes more sense to me if the old values are nested in an attribute like:

guests: [
  {
    new values here
    ......
    identity: {
        old values here
   }
  }
]
mariolenz commented 3 months ago

@mikemorency Sorry, didn't find the time to review yet. Will try to do it tomorrow!

mariolenz commented 3 months ago

The return value looks OK to replace comunity.vmware.vmware_guest_info, didn't have a closer look at comunity.vmware.vmware_vm_info yet.

But it's weird that I run into this SSL error. The certificate of the vCenter is signed by a private CA, but the OS trusts this CA. Both curl and comunity.vmware.vmware_guest_info work fine, but vmware.vmware.guest_info doesn't.

@mikemorency Any idea how to make the module use the OS-wide trusted CAs? (DISCLAIMER: I don't think this is a new problem, I dimly remember similar issues in comunity.vmware.)

PS Possibly related:

mariolenz commented 3 months ago

But it's weird that I run into this SSL error. The certificate of the vCenter is signed by a private CA, but the OS trusts this CA. Both curl and comunity.vmware.vmware_guest_info work fine, but vmware.vmware.guest_info doesn't.

@mikemorency Looks like something connected to requests / certifi. As far as I can see, it looks like they think "it's not a bug but a feature" to use their own set of trusted CAs. ATM I tend to disagree...

machacekondra commented 3 months ago

But it's weird that I run into this SSL error. The certificate of the vCenter is signed by a private CA, but the OS trusts this CA. Both curl and comunity.vmware.vmware_guest_info work fine, but vmware.vmware.guest_info doesn't.

@mikemorency Looks like something connected to requests / certifi. As far as I can see, it looks like they think "it's not a bug but a feature" to use their own set of trusted CAs. ATM I tend to disagree...

I think it's worth to at least mention in documentation how to use system wide certificates, if don't support it by some parameter, but most of the users would be expect it to be used IMHO, they are not aware that we use requests library, so we should mention it in doc.

mikemorency commented 3 months ago

The REST util is the only thing that relies on requests right now. I think that is why mario experienced the issue with this module and not the community one, it uses pyvmomi.

I added a note to the rest api docs. The user can set an environment variable to specify the ca bundle to use (instead of the certifi one provided). REQUESTS_CA_BUNDLE, https://requests.readthedocs.io/en/latest/user/advanced/

mariolenz commented 3 months ago

I added a note to the rest api docs. The user can set an environment variable to specify the ca bundle to use (instead of the certifi one provided). REQUESTS_CA_BUNDLE, https://requests.readthedocs.io/en/latest/user/advanced/

Excellent! Thanks!