Set inventory plugin insecure if no cafile defined

[seansackowitz]

In Tower 3.7, I was running into an issue with a self signed certificate on my RHV cluster. Disabling secure connections when the cafile is not defined fixes this issue, in the same way that it worked in previous versions of Tower with the ovirt4 inventory plugin.

[ovirt-infra]

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

1. Type ci test please on this PR to trigger automated tests for it.
2. Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically. ( keep in mind this list might be overwritten if the job XML is refreshed, for permanent whitelisting, please follow ovirt dynamic plugin improvements #3 option )
3. If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

[mnecas]

Hi @seansackowitz,

this is a bit gray area I have talked about it with the creator of this plugin in [1].
Please check it out. I'm opened up to further discussion/any notes.

[1] https://github.com/ansible/ansible/pull/49085/files#r377061353
cc @bverschueren

[seansackowitz]

While I do agree with the points brought up in that discussion, I still find that the overall experience as a user is failing to meet expectations. And by that I mean, when using the provided RHV credential type in Tower, which doesn't require the ca to be specified, but doesn't allow you to set OVIRT_INSECURE, you are unable to utilize the provided inventory source for ovirt, as it is requiring the ca file by default, and there is no option to override that requirement.

So, I see two solutions:

1. Edit https://github.com/ansible/awx/blob/devel/awx/main/models/credential/__init__.py#L1042 to enable setting the insecure flag in the credential itself
2. Make the change here to follow convention even within this repository: https://github.com/oVirt/ovirt-ansible-collection/blob/master/plugins/module_utils/ovirt.py#L377

[mnecas]

@seansackowitz good point that it breaks backwards compatibility with the older plugin and user cannot edit it.
To be honest I would probably continue with your suggestion.

[mnecas]

How about to use insecure when specified and if not default to cafile?

insecure=self.get_option('ovirt_insecure') if self.get_option('ovirt_insecure') is not None else not self.get_option('ovirt_cafile')

[seansackowitz]

I just tested your suggestion, but it appears that ovirt_insecure is being defaulted to false somewhere else.

[mnecas]

I did a little bit of investigation and the default value is added in the DOCUMENTATION.
I removed the default value and worked as it should.

ovirt-ansible-collection/plugins/inventory/ovirt.py

Lines 47 to 50 in 44e9c32

	ovirt_insecure:
	description: A boolean flag that indicates if the server TLS certificate and host name should be checked.
	required: False
	default: False

[bverschueren]

If I recall correctly, default variables are set by specifying it in doc_fragments.

I initially followed the philosophy of the SDK where insecure and ca_file are decoupled as I thought toggling insecure mode on the presence of a ca_file as a bit awkward. In this scenario however it makes sense.

Please think about updating the documentation too as this change has an impact on the plugins behaviour.

[mwperina]

+1