From ceb677d489640fdaf70b4eb171d937ea77c28662 Mon Sep 17 00:00:00 2001 From: Tobias Urdin Date: Thu, 29 Aug 2024 11:33:35 +0200 Subject: [PATCH] Add SWIFT_PANEL_FULL_LISTING config option The swiftclient supports setting full_listing to True which will ignore the limit/marker parameters and internally do a while loop to retrieve all the containers or objects. We pass in full_listing=True to both get_container() and get_account() and in both cases that is bad, one reason it's bad is because it ignores any limit sent. Now that in itself is not bad since we dont use those parameters at all, in fact we rely on client side pagination in Angular using st-pagination for the hz-dynamic-table that lists all containers and objects. The bad part here is that with full_listing if we have a customer with 100k containers or 100k objects the Horizon REST API will try to gather all those resources and return it in the API response to the Angular client side code. This makes it easy for a end-user to starve Horizon of resources, create a container, upload 1M objects, go to Horizon and try to list the container and Horizon will after some refreshes hang because it's processing the requests for a long time or because it runs out of memory and crashes. This adds the configuration option SWIFT_PANEL_FULL_LISTING that defaults to True keeping the current behaviour but can be set to False by operators to prevent this issue until the Swift panel has been migrated to use correct pagination. Change-Id: Id41200aaeec3df4aff1ace887a42352728fc4419 Signed-off-by: Tobias Urdin --- doc/source/configuration/settings.rst | 13 ++++++++++ openstack_dashboard/api/swift.py | 10 +++++--- .../templates/containers/ngindex.html | 4 +++ .../dashboards/project/containers/views.py | 7 ++++++ openstack_dashboard/defaults.py | 8 ++++++ .../test/unit/api/test_swift.py | 25 ++++++++++++++++--- ...-full-listing-config-3c1e03ba0b741916.yaml | 8 ++++++ 7 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/swift-full-listing-config-3c1e03ba0b741916.yaml diff --git a/doc/source/configuration/settings.rst b/doc/source/configuration/settings.rst index 6a2dac6e9f..b59a92d1bb 100644 --- a/doc/source/configuration/settings.rst +++ b/doc/source/configuration/settings.rst @@ -2575,6 +2575,19 @@ user friendly display name which will be rendered on the dashboard. If no display is specified for a storage policy, the storage policy name will be used verbatim. +SWIFT_PANEL_FULL_LISTING +~~~~~~~~~~~~~~~~~~~~~~~~ + +.. versionadded:: 2026.1(Gazpacho) + +Perform full listing of containers and objects in the Swift panel. + +Default: ``True`` + +Note that the Swift panel does client side pagination and retrieves +all containers and objects from the API that can have an negative +effect on Horizon's resource consumption if this is True. + Django Settings =============== diff --git a/openstack_dashboard/api/swift.py b/openstack_dashboard/api/swift.py index a6a843449a..da36112741 100644 --- a/openstack_dashboard/api/swift.py +++ b/openstack_dashboard/api/swift.py @@ -164,12 +164,13 @@ def swift_object_exists(request, container_name, object_name): @safe_swift_exception def swift_get_containers(request, marker=None, prefix=None): limit = settings.API_RESULT_LIMIT + full_list = settings.SWIFT_PANEL_FULL_LISTING headers, containers = swift_api(request).get_account(limit=limit + 1, marker=marker, prefix=prefix, - full_listing=True) + full_listing=full_list) container_objs = [Container(c) for c in containers] - if len(container_objs) > limit: + if (len(container_objs) > limit): return (container_objs[0:-1], True) return (container_objs, False) @@ -261,16 +262,17 @@ def swift_delete_container(request, name): def swift_get_objects(request, container_name, prefix=None, marker=None, limit=None): limit = limit or settings.API_RESULT_LIMIT + full_listing = settings.SWIFT_PANEL_FULL_LISTING kwargs = dict(prefix=prefix, marker=marker, limit=limit + 1, delimiter=FOLDER_DELIMITER, - full_listing=True) + full_listing=full_listing) headers, objects = swift_api(request).get_container(container_name, **kwargs) object_objs = _objectify(objects, container_name) - if len(object_objs) > limit: + if (len(object_objs) > limit): return (object_objs[0:-1], True) return (object_objs, False) diff --git a/openstack_dashboard/dashboards/project/containers/templates/containers/ngindex.html b/openstack_dashboard/dashboards/project/containers/templates/containers/ngindex.html index de1eac7a26..43bdcb87b9 100644 --- a/openstack_dashboard/dashboards/project/containers/templates/containers/ngindex.html +++ b/openstack_dashboard/dashboards/project/containers/templates/containers/ngindex.html @@ -20,4 +20,8 @@ {% block main %} + + {% if SWIFT_PANEL_FULL_LISTING is False %} +
{% blocktrans %}Listing only {{ API_RESULT_LIMIT }} containers/objects because full listing is disabled in settings.{% endblocktrans %}
+ {% endif %} {% endblock %} diff --git a/openstack_dashboard/dashboards/project/containers/views.py b/openstack_dashboard/dashboards/project/containers/views.py index 2f0dbbfc0f..af6e8245a0 100644 --- a/openstack_dashboard/dashboards/project/containers/views.py +++ b/openstack_dashboard/dashboards/project/containers/views.py @@ -17,9 +17,16 @@ # under the License. +from django.conf import settings from django.views import generic class NgIndexView(generic.TemplateView): """View for managing Swift containers.""" template_name = 'project/containers/ngindex.html' + + def get_context_data(self, *args, **kwargs): + context = super().get_context_data(*args, **kwargs) + context['API_RESULT_LIMIT'] = settings.API_RESULT_LIMIT + context['SWIFT_PANEL_FULL_LISTING'] = settings.SWIFT_PANEL_FULL_LISTING + return context diff --git a/openstack_dashboard/defaults.py b/openstack_dashboard/defaults.py index a44f0a36b4..ce98abd7a5 100644 --- a/openstack_dashboard/defaults.py +++ b/openstack_dashboard/defaults.py @@ -325,6 +325,14 @@ SWIFT_FILE_TRANSFER_CHUNK_SIZE = 512 * 1024 # name to be rendered. SWIFT_STORAGE_POLICY_DISPLAY_NAMES = {} +# Perform full listing of containers and objects in the Swift +# panel. Defaults to True. +# +# Note that the Swift panel does client side pagination and retrieves +# all containers and objects from the API that can have an negative +# effect on Horizon's resource consumption if this is True. +SWIFT_PANEL_FULL_LISTING = True + # NOTE: The default value of USER_MENU_LINKS will be set after loading # local_settings if it is not configured. USER_MENU_LINKS = None diff --git a/openstack_dashboard/test/unit/api/test_swift.py b/openstack_dashboard/test/unit/api/test_swift.py index 64d38f88d0..9e641190d4 100644 --- a/openstack_dashboard/test/unit/api/test_swift.py +++ b/openstack_dashboard/test/unit/api/test_swift.py @@ -17,6 +17,8 @@ # under the License. from unittest import mock +from django.test.utils import override_settings + from horizon import exceptions from openstack_dashboard import api @@ -25,7 +27,7 @@ from openstack_dashboard.test import helpers as test @mock.patch('swiftclient.client.Connection') class SwiftApiTests(test.APIMockTestCase): - def test_swift_get_containers(self, mock_swiftclient): + def _test_swift_get_containers(self, mock_swiftclient, full_listing): containers = self.containers.list() cont_data = [c._apidict for c in containers] swift_api = mock_swiftclient.return_value @@ -36,7 +38,15 @@ class SwiftApiTests(test.APIMockTestCase): self.assertEqual(len(containers), len(conts)) self.assertFalse(more) swift_api.get_account.assert_called_once_with( - limit=1001, marker=None, prefix=None, full_listing=True) + limit=1001, marker=None, prefix=None, + full_listing=full_listing) + + def test_swift_get_containers_default(self, mock_swiftclient): + self._test_swift_get_containers(mock_swiftclient, full_listing=True) + + @override_settings(SWIFT_PANEL_FULL_LISTING=False) + def test_swift_get_containers_full_list_false(self, mock_swiftclient): + self._test_swift_get_containers(mock_swiftclient, full_listing=False) def test_swift_get_container_with_data(self, mock_swiftclient): container = self.containers.first() @@ -136,7 +146,7 @@ class SwiftApiTests(test.APIMockTestCase): swift_api.post_container.assert_called_once_with(container.name, headers=headers) - def test_swift_get_objects(self, mock_swiftclient): + def _test_swift_get_objects(self, mock_swiftclient, full_listing): container = self.containers.first() objects = self.objects.list() @@ -154,7 +164,14 @@ class SwiftApiTests(test.APIMockTestCase): marker=None, prefix=None, delimiter='/', - full_listing=True) + full_listing=full_listing) + + def test_swift_get_objects_default(self, mock_swiftclient): + self._test_swift_get_objects(mock_swiftclient, full_listing=True) + + @override_settings(SWIFT_PANEL_FULL_LISTING=False) + def test_swift_get_objects_full_list_false(self, mock_swiftclient): + self._test_swift_get_objects(mock_swiftclient, full_listing=False) def test_swift_get_object_with_data_non_chunked(self, mock_swiftclient): container = self.containers.first() diff --git a/releasenotes/notes/swift-full-listing-config-3c1e03ba0b741916.yaml b/releasenotes/notes/swift-full-listing-config-3c1e03ba0b741916.yaml new file mode 100644 index 0000000000..0cde555f10 --- /dev/null +++ b/releasenotes/notes/swift-full-listing-config-3c1e03ba0b741916.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Added new configuration option ``SWIFT_PANEL_FULL_LISTING`` that + defaults to ``True``. This configuration option can be set to + ``False`` to prevent Horizon from doing full listing of containers + and objects in the Swift panel that can cause high resource + consumption in Horizon.