Rework Proxmox Cloud Provider

[I3urny]

What does this PR do?

Rework Proxmox Cloud Provider

What issues does this PR fix or reference?

Fixes: #9 #12 #21 #29

Previous Behavior

Removed undocumented features:

• use_dns
  flag that set the IP address by using a DNS resolver -> not quite sure why this was a thing. I guess they wanted to configure the IP only on their DNS server. But that indicates a bad network/server architecture.
• verify_ssl
  flag to disable ssl verification for the proxmox url -> big security nono, especially since the user account most likely is full-admin (MitM with admin creds)
• _reconfigure_clone()
  function that reconfigured a VM -> generally not a bad idea but badly implemented. it reconfigured right after cloning a VM inside the create() function instead of being a separate function.
• username & password / CSRF & Ticket
  using username and password as credentials needs a lot of code to use the correct CSRF token and ticket -> the correct way (with the currently supported API) is to use an API token that is scoped and has an expiration date
• _get_properties()
  a function to retrieve valid options for the API call which drops invalid options -> the cloud module shouldn't try to reimplement the proxmox API business logic but rather let the call fail and report back

New Behavior

The Proxmox cloud module is now basically a wrapper without reimplementing any business logic of Proxmox API. Instead, the proxmox API is used to check for correctness of inputs.

I updated the documentation and test cases, however, I'm not quite sure all of those tests make sense. I also don't know if chasing 100% code coverage is feasible.

Regarding the new functionality please check the documentation.
If anything is unclear we should probably add it to the documentation anyway.

While we're at it we should probably also remove the version coupling to salt. With the extensions now being in a separate repository they can (and should) be versioned independently. The documentation currently does not indicate that.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

No

[codecov-commenter]

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files	Coverage Δ
src/saltext/proxmox/version.py	100.00% <100.00%> (ø)
tests/unit/clouds/test_proxmox.py	100.00% <100.00%> (+3.20%)	⬆️
src/saltext/proxmox/clouds/proxmox.py	82.29% <85.00%> (+38.38%)	⬆️

📢 Thoughts on this report? Let us know!.

[nicholasmhughes]

chore (blocking): please add a changelog

[I3urny]

Are you talking about a changelog as explained in https://docs.saltproject.io/en/latest/topics/development/changelog.html ?

[nicholasmhughes]

Yup.

[I3urny]

Added changelogs for the fixed issues. Is that enough or do we need a separate changelog for the rework like an extended version of this PRs description?

I believe the remaining changes that are not listed in the other changelogs are these:

removed option to configure a VM's ip address using dns (use_dns)
removed option to ignore ssl errors (verify_ssl)
removed authentication via username & password
removed automatic setting of next available vmid during creation (_get_next_vmid())

changed parameter name "location" to "storage" in avail_images() as to not confuse it with location (datacenter node)
changed create via clone so that the VM cannot be reconfigured during the clone operation

added function to reconfigure an existing VM
added function to clone an existing VM

[nicholasmhughes]

Looks good 👍