Update the code so we don't end up in an infinite loop in case API

returns invalid response or similar. Also add a test case for it.
apache · Jul 31, 2023 · c49ecd0 · c49ecd0
1 parent c906009
commit c49ecd0
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 5 deletions.
diff --git a/libcloud/compute/drivers/azure_arm.py b/libcloud/compute/drivers/azure_arm.py
@@ -25,7 +25,7 @@
 import binascii
 
 from libcloud.utils import iso8601
-from libcloud.utils.py3 import basestring, urlparse, parse_qs
+from libcloud.utils.py3 import parse_qs, urlparse, basestring
 from libcloud.common.types import LibcloudError
 from libcloud.compute.base import (
     Node,
@@ -64,6 +64,10 @@
 VM_EXTENSION_API_VERSION = "2015-06-15"
 VM_SIZE_API_VERSION = "2015-06-15"  # this API is deprecated
 
+# If pagination code in the list_nodes() method has still not completed after this mount of
+# seconds, we will break early from while True loop to avoid infinite loop under edge conditions.
+LIST_NODES_PAGINATION_TIMEOUT = 60
+
 
 class AzureImage(NodeImage):
     """Represents a Marketplace node image that an Azure VM can boot from."""
@@ -469,11 +473,19 @@ def list_nodes(self, ex_resource_group=None, ex_fetch_nic=True, ex_fetch_power_s
                 self.subscription_id
             )
         params = {"api-version": VM_API_VERSION}
+
+        now_ts = int(time.time())
+        deadline_ts = now_ts + LIST_NODES_PAGINATION_TIMEOUT
+
         nodes = []
-        while True:
+        while time.time() < deadline_ts:
             r = self.connection.request(action, params=params)
-            nodes.extend(self._to_node(n, fetch_nic=ex_fetch_nic, fetch_power_state=ex_fetch_power_state) for n in r.object["value"])
+            nodes.extend(
+                self._to_node(n, fetch_nic=ex_fetch_nic, fetch_power_state=ex_fetch_power_state)
+                for n in r.object["value"]
+            )
             if not r.object.get("nextLink"):
+                # No next page
                 break
             parsed_next_link = urlparse.urlparse(r.object["nextLink"])
             params.update({k: v[0] for k, v in parse_qs(parsed_next_link.query).items()})

diff --git a/...zure_arm/_77777777_7777_7777_7777_777777777777_oauth2_token_PAGINATION_INFINITE_LOOP.json b/...zure_arm/_77777777_7777_7777_7777_777777777777_oauth2_token_PAGINATION_INFINITE_LOOP.json
@@ -0,0 +1 @@
+{"expires_in":"3600","token_type":"Bearer","expires_on":"1111111111","not_before":"1111111111","resource":"https://management.core.windows.net/","access_token":"3333333333333333333333333333333333333333333333333333333"}
diff --git a/...ptions_99999999_providers_Microsoft_Compute_virtualMachines_PAGINATION_INFINITE_LOOP.json b/...ptions_99999999_providers_Microsoft_Compute_virtualMachines_PAGINATION_INFINITE_LOOP.json
@@ -0,0 +1,53 @@
+{
+  "value": [
+    {
+      "properties": {
+        "vmId": "CCEEBF63-E92B-4A50-9949-6E44BFC61D3F",
+        "additionalCapabilities": {
+          "ultraSSDEnabled": "False",
+          "hibernationEnabled": "False"
+        },
+        "hardwareProfile": {
+          "vmSize": "Standard_A1"
+        },
+        "storageProfile": {
+          "imageReference": {
+            "publisher": "OpenLogic",
+            "offer": "CentOS",
+            "sku": "7.3",
+            "version": "latest"
+          },
+          "osDisk": {
+            "osType": "Linux",
+            "name": "test-node-disk-1",
+            "createOption": "FromImage",
+            "caching": "ReadWrite",
+            "managedDisk": {
+              "storageAccountType": "Standard_LRS",
+              "id": "/subscriptions/99999999-9999-9999-9999-999999999999/resourceGroups/000000/providers/Microsoft.Compute/disks/test-node-disk-1"
+            }
+          },
+          "dataDisks": []
+        },
+        "osProfile": {
+          "computerName": "test-node-1",
+          "adminUsername": "user",
+          "linuxConfiguration": {
+            "disablePasswordAuthentication": false
+          },
+          "secrets": []
+        },
+        "networkProfile": {
+          "networkInterfaces": []
+        },
+        "provisioningState": "Running"
+      },
+      "type": "Microsoft.Compute/virtualMachines",
+      "location": "eastus",
+      "tags": {},
+      "id": "/subscriptions/99999999-9999-9999-9999-999999999999/resourceGroups/000000/providers/Microsoft.Compute/virtualMachines/test-node-1",
+      "name": "test-node-1"
+    }
+  ],
+  "nextLink": "https://management.azure.com:443/subscriptions/99999999-9999-9999-9999-999999999999/providers/Microsoft.Compute/virtualMachines?api-version=2021-11-01&$skiptoken=1!/Subscriptions/99999999-9999-9999-9999-999999999999/ResourceGroups/000000/VMs/DDFEBF64-E92B-4A50-9949-6E44BFC61D4G"
+}
diff --git a/libcloud/test/compute/test_azure_arm.py b/libcloud/test/compute/test_azure_arm.py
@@ -20,7 +20,7 @@
 from unittest import mock
 
 from libcloud.test import MockHttp, LibcloudTestCase, unittest
-from libcloud.utils.py3 import httplib, urlparse, parse_qs, urlunquote
+from libcloud.utils.py3 import httplib, parse_qs, urlparse, urlunquote
 from libcloud.common.types import LibcloudError
 from libcloud.compute.base import NodeSize, NodeLocation, StorageVolume, VolumeSnapshot
 from libcloud.compute.types import Provider, NodeState, StorageVolumeState, VolumeSnapshotState
@@ -42,6 +42,7 @@ class AzureNodeDriverTests(LibcloudTestCase):
     APPLICATION_PASS = "p4ssw0rd"
 
     def setUp(self):
+        AzureMockHttp.type = None
         Azure = get_driver(Provider.AZURE_ARM)
         Azure.connectionCls.conn_class = AzureMockHttp
         self.driver = Azure(
@@ -458,6 +459,18 @@ def test_list_nodes(self, fps_mock):
 
         fps_mock.assert_called()
 
+    @mock.patch(
+        "libcloud.compute.drivers.azure_arm.AzureNodeDriver._fetch_power_state",
+        return_value=NodeState.UPDATING,
+    )
+    @mock.patch("libcloud.compute.drivers.azure_arm.LIST_NODES_PAGINATION_TIMEOUT", 1)
+    def test_list_nodes_pagination_timeout_reached(self, fps_mock):
+        # Verify we don't end up in an infinite loop in case server returns a bad response or
+        # similar
+        AzureMockHttp.type = "PAGINATION_INFINITE_LOOP"
+        nodes = self.driver.list_nodes()
+        self.assertTrue(len(nodes) >= 1)
+
     @mock.patch(
         "libcloud.compute.drivers.azure_arm.AzureNodeDriver" "._fetch_power_state",
         return_value=NodeState.UPDATING,
@@ -824,7 +837,7 @@ def fn(method, url, body, headers):
                 AzureNodeDriverTests.SUBSCRIPTION_ID,
             )
             unquoted_url = urlunquote(url)
-            if "$skiptoken=" in unquoted_url:
+            if "$skiptoken=" in unquoted_url and self.type != "PAGINATION_INFINITE_LOOP":
                 parsed_url = urlparse.urlparse(unquoted_url)
                 params = parse_qs(parsed_url.query)
                 file_name += "_" + params["$skiptoken"][0].split("!")[0]