Add support for all parameters when cloning a VM with the proxmox sal…

…t-cloud driver

Fixes #62580

changelog/62580.fixed

@@ -0,0 +1 @@
+Fixed missing parameters when cloning a VM with the proxmox salt-cloud driver

salt/cloud/clouds/proxmox.py

@@ -31,6 +31,7 @@
 import re
 import socket
 import time
+import urllib
 
 import salt.config as config
 import salt.utils.cloud
@@ -192,7 +193,12 @@ def query(conn_type, option, post_data=None):
     elif conn_type == "get":
         response = requests.get(full_url, verify=verify_ssl, cookies=ticket)
 
-    response.raise_for_status()
+    try:
+        response.raise_for_status()
+    except requests.exceptions.RequestException:
+        # Log the details of the response.
+        log.error("Error in %s query to %s:\n%s", conn_type, full_url, response.text)
+        raise
 
     try:
         returned_data = response.json()
@@ -560,20 +566,18 @@ def _reconfigure_clone(vm_, vmid):
         log.warning("Reconfiguring clones is only available under `qemu`")
         return
 
-    # TODO: Support other settings here too as these are not the only ones that can be modified after a clone operation
+    # Determine which settings can be reconfigured.
+    query_path = "nodes/{}/qemu/{}/config"
+    valid_settings = set(_get_properties(query_path.format("{node}", "{vmid}"), "POST"))
+
     log.info("Configuring cloned VM")
 
     # Modify the settings for the VM one at a time so we can see any problems with the values
     # as quickly as possible
     for setting in vm_:
-        if re.match(r"^(ide|sata|scsi)(\d+)$", setting):
-            postParams = {setting: vm_[setting]}
-            query(
-                "post",
-                "nodes/{}/qemu/{}/config".format(vm_["host"], vmid),
-                postParams,
-            )
-
+        postParams = None
+        if setting == "vmid":
+            pass  # vmid gets passed in the URL and can't be reconfigured
         elif re.match(r"^net(\d+)$", setting):
             # net strings are a list of comma seperated settings. We need to merge the settings so that
             # the setting in the profile only changes the settings it touches and the other settings
@@ -593,6 +597,13 @@ def _reconfigure_clone(vm_, vmid):
 
             # Convert the dictionary back into a string list
             postParams = {setting: _dictionary_to_stringlist(new_setting)}
+
+        elif setting == "sshkeys":
+            postParams = {setting: urllib.parse.quote(vm_[setting], safe="")}
+        elif setting in valid_settings:
+            postParams = {setting: vm_[setting]}
+
+        if postParams:
             query(
                 "post",
                 "nodes/{}/qemu/{}/config".format(vm_["host"], vmid),

tests/unit/cloud/clouds/test_proxmox.py

@@ -4,14 +4,15 @@
 
 import io
 import textwrap
+import urllib
 
 import requests
 
 from salt import config
 from salt.cloud.clouds import proxmox
 from tests.support.helpers import TstSuiteLoggingHandler
 from tests.support.mixins import LoaderModuleMockMixin
-from tests.support.mock import ANY, MagicMock, patch
+from tests.support.mock import ANY, MagicMock, call, patch
 from tests.support.unit import TestCase
 
 PROFILE = {
@@ -98,9 +99,12 @@ def test__dictionary_to_stringlist(self):
         result = proxmox._dictionary_to_stringlist({"a": "a", "b": "b"})
         self.assertEqual(result, "a=a,b=b")
 
-    def test__reconfigure_clone(self):
+    def test__reconfigure_clone_net_hdd(self):
         # The return_value is for the net reconfigure assertions, it is irrelevant for the rest
-        with patch.object(
+        with patch(
+            "salt.cloud.clouds.proxmox._get_properties",
+            MagicMock(return_value=["net0", "ide0", "sata0", "scsi0"]),
+        ), patch.object(
             proxmox, "query", return_value={"net0": "c=overwritten,g=h"}
         ) as query:
             # Test a vm that lacks the required attributes
@@ -127,6 +131,59 @@ def test__reconfigure_clone(self):
                 "post", "nodes/127.0.0.1/qemu/0/config", {"scsi0": "data"}
             )
 
+    def test__reconfigure_clone_params(self):
+        """
+        Test cloning a VM with parameters to be reconfigured.
+        """
+        vmid = 201
+        properties = {
+            "ide2": "cdrom",
+            "sata1": "satatest",
+            "scsi0": "bootvol",
+            "net0": "model=virtio",
+            "agent": "1",
+            "args": "argsvalue",
+            "balloon": "128",
+            "ciuser": "root",
+            "cores": "2",
+            "description": "desc",
+            "memory": "256",
+            "name": "new2",
+            "onboot": "0",
+            "sshkeys": "ssh-rsa ABCDEF user@host\n",
+        }
+        query_calls = [call("get", "nodes/myhost/qemu/{}/config".format(vmid))]
+        for key, value in properties.items():
+            if key == "sshkeys":
+                value = urllib.parse.quote(value, safe="")
+            query_calls.append(
+                call(
+                    "post",
+                    "nodes/myhost/qemu/{}/config".format(vmid),
+                    {key: value},
+                )
+            )
+
+        mock_query = MagicMock(return_value="")
+        with patch(
+            "salt.cloud.clouds.proxmox._get_properties",
+            MagicMock(return_value=list(properties.keys())),
+        ), patch("salt.cloud.clouds.proxmox.query", mock_query):
+            vm_ = {
+                "profile": "my_proxmox",
+                "driver": "proxmox",
+                "technology": "qemu",
+                "name": "new2",
+                "host": "myhost",
+                "clone": True,
+                "clone_from": 123,
+                "ip_address": "10.10.10.10",
+            }
+            vm_.update(properties)
+
+            proxmox._reconfigure_clone(vm_, vmid)
+            mock_query.assert_has_calls(query_calls, any_order=True)
+
     def test_clone(self):
         """
         Test that an integer value for clone_from