Merge pull request #346 from CloudVE/aws-image-find-scope

Fix the two effects behind the AWS suite's runtime: unscoped tag search and transport page size

CHANGELOG.rst

@@ -1,3 +1,44 @@
+4.4.0 - unreleased
+------------------
+
+## Fixes
+* **Paginated AWS calls no longer use the caller's result limit as the
+  transport page size.** ``BotoEC2Service._get_paginated_results`` set
+  ``PaginationConfig={'MaxItems': limit, 'PageSize': limit}``, conflating how
+  many results the caller wants with how many the service returns per
+  request. Against a scan that matches sparsely that walks the collection in
+  tiny increments: the same filtered ``describe_images`` took 977.6s at a
+  page size of 5 and 10.0s at 1000, for one result either way. ``MaxItems``
+  still bounds what the caller receives; ``PageSize`` is now a full page,
+  clamped to whatever bounds the service model declares for the operation
+  (``DescribeRouteTables`` permits 100 where most permit more, several require
+  at least 5, and falling outside them is a hard ``InvalidParameterValue``).
+  Since ``DEFAULT_RESULT_LIMIT`` is 50, every paginated AWS call was affected,
+  not just filtered searches.
+
+## Enhancements
+* **New ``aws_page_size`` configuration value.** How many records to request
+  from AWS per call while satisfying a list method, defaulting to 500. It is
+  a transport setting, distinct from ``default_result_limit``, which bounds
+  how many results the caller receives; the two used to be the same number.
+  It is clamped to what the service permits for the call in hand, so a value
+  outside those bounds is adjusted rather than rejected. AWS-specific
+  because it only means anything where the provider walks pages itself:
+  GCP, Azure and OpenStack each return a single page plus a continuation
+  token and let the caller drive.
+* **``AWSImageService.find`` no longer scans every public image to run its
+  tag search.** ``find(label=...)`` issues two ``describe_images`` calls, one
+  filtered on ``name`` and one on ``tag:Name``, and neither was scoped by
+  ``Owners``. The ``tag:Name`` half can only ever match images in the calling
+  account - AMI tags are not visible across accounts, so an image owned by
+  anyone else cannot satisfy the filter however it is tagged - so omitting
+  ``Owners`` never widened what it could find. It only made EC2 evaluate the
+  filter against the whole regional catalogue: measured in ap-southeast-1,
+  10.0s unscoped against 0.1s scoped, for identical single-image results.
+  The ``name`` half is unchanged and still searches public images, which is
+  what most callers want; an explicit ``owners`` argument still overrides
+  both.
+
 4.3.1 - August 2, 2026 (sha 8fabc1e2d3916e2c100bdb18075f2caa3bd38b38)
 ---------------------------------------------------------------------
 

cloudbridge/providers/aws/helpers.py

@@ -25,6 +25,18 @@ log = logging.getLogger(__name__)
 
 T = TypeVar("T")
 
+# How many records to request per call while satisfying a list method,
+# overridable per provider with the ``aws_page_size`` config value. This is a
+# transport setting and is deliberately not ``default_result_limit``, which
+# bounds how many results the caller receives - see
+# BotoGenericService._page_size for why conflating them is expensive.
+#
+# AWS-specific because it only means anything where the provider walks pages
+# itself. GCP, Azure and OpenStack each return a single page plus a
+# continuation token and let the caller drive, so there is nothing for a page
+# size to amplify; boto3's paginator is the odd one out.
+DEFAULT_PAGE_SIZE = 500
+
 
 def trim_empty_params(params_dict: dict[str, Any]) -> dict[str, Any]:
     """
@@ -153,6 +165,46 @@ class BotoGenericService(object):
         else:
             return None
 
+    def _page_size(self, client: Any, list_op: str) -> int:
+        """
+        Transport page size for a paginated call.
+
+        ``PageSize`` is how many items the service returns per request;
+        ``MaxItems`` is how many the caller asked for. Using the caller's
+        limit for both makes a small limit walk a large scan in tiny
+        increments: a filtered ``describe_images`` that takes 10.0s at a page
+        size of 1000 takes 977.6s at 5, for the same single result. Ask for a
+        full page regardless of the limit and let ``MaxItems`` do the
+        bounding - at worst one page more data is fetched than was wanted,
+        and a limit larger than a page is served over several requests.
+
+        The size comes from the ``aws_page_size`` config value so it can be
+        tuned per provider, then is clamped to whatever the service model
+        declares for this operation. EC2's bounds differ per call -
+        ``DescribeRouteTables`` permits 100 where most permit 1000, and
+        several require at least 5 - and falling outside them is a hard
+        ``InvalidParameterValue`` rather than a clamp, so neither the default
+        nor a configured value is passed through unchecked.
+        """
+        page_size = int(self.provider._get_config_value(
+            'aws_page_size', DEFAULT_PAGE_SIZE))
+        api_name = client.meta.method_to_api_mapping.get(list_op)
+        if not api_name:
+            return page_size
+        input_shape = client.meta.service_model.operation_model(
+            api_name).input_shape
+        max_results = input_shape.members.get('MaxResults') if input_shape \
+            else None
+        # Not every operation declares bounds, even where the documentation
+        # gives them; the default is within all of the undeclared ones.
+        bounds = max_results.metadata if max_results else {}
+        ceiling, floor = bounds.get('max'), bounds.get('min')
+        if ceiling:
+            page_size = min(page_size, ceiling)
+        if floor:
+            page_size = max(page_size, floor)
+        return page_size
+
     def _get_list_operation(self) -> str:
         """
         This function discovers the list operation for a particular resource
@@ -195,7 +247,9 @@ class BotoGenericService(object):
         paginator = client.get_paginator(list_op)
         PaginationConfig: dict[str, Any] = {}
         if limit:
-            PaginationConfig = {'MaxItems': limit, 'PageSize': limit}
+            PaginationConfig = {
+                'MaxItems': limit,
+                'PageSize': self._page_size(client, list_op)}
 
         if marker:
             PaginationConfig.update({'StartingToken': marker})

cloudbridge/providers/aws/services.py

@@ -806,8 +806,22 @@ class AWSImageService(BaseImageService):
             log.debug("Searching for AWS Image Service %s", label)
             obj_list.extend(
                 self.svc.find(filters={'name': label}, **extra_args))
+            # A tag filter can only ever match images in the calling account.
+            # AMI tags are not visible across accounts, so an image owned by
+            # anyone else can never satisfy tag:Name however it is tagged -
+            # asking for every image carrying any visible tag, unscoped across
+            # a whole region, returns only this account's. Leaving Owners off
+            # therefore does not widen what the search can find; it only makes
+            # EC2 evaluate the filter against every public image in the
+            # region. Measured in ap-southeast-1, identical single-image
+            # results in 10.0s unscoped against 0.1s scoped.
+            #
+            # An explicit owners argument still wins, so a caller can ask for
+            # someone else's images and get the same (empty) answer as before.
+            tag_args = dict(extra_args)
+            tag_args.setdefault('Owners', ['self'])
             obj_list.extend(
-                self.svc.find(filters={'tag:Name': label}, **extra_args))
+                self.svc.find(filters={'tag:Name': label}, **tag_args))
         return ClientPagedResultList(self.provider, obj_list)
 
     # Intentionally extends the base list() with a leading filter_by_owner

tests/test_aws_pagination.py

@@ -0,0 +1,119 @@
+"""
+Unit tests for ``BotoEC2Service`` server-side pagination.
+
+``_get_paginated_results`` hands boto3 a ``PaginationConfig`` built from two
+separate numbers: ``MaxItems``, how many results the caller asked for, and
+``PageSize``, how many the service returns per request. They used to be the
+same value, which made a small result limit walk a large scan in tiny
+increments - a filtered ``describe_images`` measured 977.6s at a page size of
+5 against 10.0s at 1000, for the same single result.
+
+Separating them moves where page boundaries fall, and the resume token
+handed back to callers is defined in terms of those boundaries. These tests
+pin the round trip: paging a collection with a small limit must still visit
+every object exactly once and terminate, whatever the transport page size.
+
+Exercised against moto so they run in CI without cloud credentials.
+"""
+import unittest
+
+from moto import mock_aws
+
+from cloudbridge.factory import CloudProviderFactory
+from cloudbridge.factory import ProviderList
+from cloudbridge.providers.aws.helpers import DEFAULT_PAGE_SIZE
+
+# More objects than one page at the result limit below, so paging is forced.
+OBJECT_COUNT = 7
+RESULT_LIMIT = 2
+
+
+class AWSPaginationTestCase(unittest.TestCase):
+
+    def setUp(self):
+        self.mock = mock_aws()
+        self.mock.start()
+        self.provider = CloudProviderFactory().create_provider(
+            ProviderList.AWS,
+            {'aws_access_key': 'a', 'aws_secret_key': 'b',
+             'aws_region_name': 'us-east-1',
+             'default_result_limit': RESULT_LIMIT})
+
+    def tearDown(self):
+        self.mock.stop()
+
+    def _create_volumes(self):
+        created = []
+        for i in range(OBJECT_COUNT):
+            vol = self.provider.storage.volumes.create(
+                'cb-page-%d' % i, 1)
+            created.append(vol.id)
+        return created
+
+    def test_paging_visits_every_object_exactly_once(self):
+        expected = self._create_volumes()
+
+        seen = []
+        page = self.provider.storage.volumes.list()
+        seen.extend(o.id for o in page)
+        pages = 1
+        while page.is_truncated:
+            page = self.provider.storage.volumes.list(marker=page.marker)
+            seen.extend(o.id for o in page)
+            pages += 1
+            self.assertLess(pages, OBJECT_COUNT + 5,
+                            "Paging failed to terminate; the resume token is "
+                            "probably not advancing.")
+
+        self.assertEqual(sorted(seen), sorted(expected),
+                         "Paging must visit every object exactly once.")
+        self.assertEqual(len(seen), len(set(seen)),
+                         "Paging returned duplicates across pages.")
+
+    def test_a_page_holds_no_more_than_the_result_limit(self):
+        # The caller's limit bounds what comes back, independently of how
+        # much was fetched per request to produce it.
+        self._create_volumes()
+        page = self.provider.storage.volumes.list()
+        self.assertLessEqual(len(page), RESULT_LIMIT)
+
+    def _page_size(self, list_op):
+        # pylint:disable=protected-access
+        return self.provider.storage.volumes.svc._page_size(
+            self.provider.ec2_conn.meta.client, list_op)
+
+    def test_transport_page_size_is_independent_of_the_result_limit(self):
+        # The regression this guards: PageSize tracking the result limit is
+        # what made a sparse scan pathological.
+        page_size = self._page_size('describe_volumes')
+        self.assertEqual(page_size, DEFAULT_PAGE_SIZE)
+        self.assertNotEqual(page_size, RESULT_LIMIT)
+
+    def test_page_size_is_clamped_to_the_operations_ceiling(self):
+        # DescribeRouteTables allows 100 where most EC2 describes allow more,
+        # and exceeding a ceiling is a hard InvalidParameterValue.
+        self.assertEqual(self._page_size('describe_route_tables'), 100)
+
+    def test_page_size_survives_an_unknown_operation(self):
+        self.assertEqual(self._page_size('not_an_operation'),
+                         DEFAULT_PAGE_SIZE)
+
+    def test_configured_page_size_is_used(self):
+        self.provider.config['aws_page_size'] = 250
+        self.assertEqual(self._page_size('describe_volumes'), 250)
+
+    def test_configured_page_size_is_clamped_to_the_ceiling(self):
+        # A configured value is no safer than the default: DescribeRouteTables
+        # would reject anything above 100 outright.
+        self.provider.config['aws_page_size'] = 900
+        self.assertEqual(self._page_size('describe_route_tables'), 100)
+
+    def test_configured_page_size_is_raised_to_the_floor(self):
+        # Several EC2 describes require at least 5, so a smaller configured
+        # value would be rejected rather than honoured.
+        self.provider.config['aws_page_size'] = 1
+        self.assertEqual(self._page_size('describe_vpcs'), 5)
+
+
+if __name__ == "__main__":
+    unittest.main()