From ab17bd0902a8ab23ca20e77192284d2b2b171adb Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Thu, 7 Jul 2022 14:22:48 +0200 Subject: [PATCH 1/2] XML-RPC API: Fix complex objects are return value This bug is appearing in "get_item_resolved_value" and could be triggered for example when "arch" was asked with a distro. The fix is to catch all types in "remote.py" and convert them on a per-type basis. This commit also adds tests for the known cases that would cause trouble in the past. --- cobbler/items/distro.py | 2 ++ cobbler/items/system.py | 2 ++ cobbler/remote.py | 35 ++++++++++++++++++++- tests/xmlrpcapi/non_object_calls_test.py | 39 ++++++++++++++++++++---- 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/cobbler/items/distro.py b/cobbler/items/distro.py index 8fafba90be..92ae38854d 100644 --- a/cobbler/items/distro.py +++ b/cobbler/items/distro.py @@ -24,6 +24,8 @@ class Distro(item.Item): A Cobbler distribution object """ + # Constants + TYPE_NAME = "distro" COLLECTION_TYPE = "distro" def __init__(self, api, *args, **kwargs): diff --git a/cobbler/items/system.py b/cobbler/items/system.py index 1c11471d28..b47476a39e 100644 --- a/cobbler/items/system.py +++ b/cobbler/items/system.py @@ -764,6 +764,8 @@ class System(Item): A Cobbler system object. """ + # Constants + TYPE_NAME = "system" COLLECTION_TYPE = "system" def __init__(self, api, *args, **kwargs): diff --git a/cobbler/remote.py b/cobbler/remote.py index c2733e175e..b33e5c99cd 100644 --- a/cobbler/remote.py +++ b/cobbler/remote.py @@ -22,6 +22,7 @@ from typing import Dict, List, Optional, Union from xmlrpc.server import SimpleXMLRPCRequestHandler +from cobbler import enums from cobbler import autoinstall_manager from cobbler import configgen from cobbler.items import ( @@ -790,7 +791,39 @@ def get_item_resolved_value(self, item_uuid: str, attribute: str): .. seealso:: Logically identical to :func:`~cobbler.api.CobblerAPI.get_item_resolved_value` """ self._log("get_item_resolved_value(%s)" % item_uuid, attribute=attribute) - return self.api.get_item_resolved_value(item_uuid, attribute) + return_value = self.api.get_item_resolved_value(item_uuid, attribute) + if return_value is None: + self._log( + "get_item_resolved_value(%s): returned None" % item_uuid, + attribute=attribute, + ) + raise ValueError( + 'None is not a valid value for the resolved attribute "%s". Please fix the item(s) ' + 'starting at uuid "%s"' % (attribute, item_uuid) + ) + elif isinstance(return_value, item.Item): + return return_value.name + elif isinstance(return_value, enums.ConvertableEnum): + return return_value.value + elif isinstance( + return_value, (enums.DHCP, enums.NetworkInterfaceType, enums.BaudRates) + ): + return return_value.name + elif isinstance(return_value, dict): + return self.xmlrpc_hacks(return_value) + + if not isinstance( + return_value, (str, int, float, bool, tuple, bytes, bytearray, dict, list) + ): + self._log( + "get_item_resolved_value(%s): Cannot return XML-RPC compliant type. Please add a case to convert" + ' type "%s" to an XML-RPC compliant type!' + % (item_uuid, type(return_value)) + ) + raise ValueError( + "Cannot return XML-RPC compliant type. See logs for more information!" + ) + return return_value def set_item_resolved_value( self, item_uuid: str, attribute: str, value, token=None diff --git a/tests/xmlrpcapi/non_object_calls_test.py b/tests/xmlrpcapi/non_object_calls_test.py index a24024f08b..bbed38ea79 100644 --- a/tests/xmlrpcapi/non_object_calls_test.py +++ b/tests/xmlrpcapi/non_object_calls_test.py @@ -4,6 +4,8 @@ import time import re +from tests.conftest import does_not_raise + TEST_POWER_MANAGEMENT = True TEST_SYSTEM = "" @@ -190,8 +192,26 @@ def test_get_random_mac(remote, token): assert match_obj +@pytest.mark.parametrize( + "input_attribute,checked_object,expected_result,expected_exception", + [ + ("kernel_options", "system", {"a": "1", "b": "2", "d": "~"}, does_not_raise()), + ("arch", "distro", "x86_64", does_not_raise()), + ("distro", "profile", "testdistro_item_resolved_value", does_not_raise()), + ("profile", "system", "testprofile_item_resolved_value", does_not_raise()), + ], +) def test_get_item_resolved_value( - remote, token, create_distro, create_profile, create_system, create_kernel_initrd + remote, + token, + create_distro, + create_profile, + create_system, + create_kernel_initrd, + input_attribute, + checked_object, + expected_result, + expected_exception, ): # Arrange fk_kernel = "vmlinuz1" @@ -207,11 +227,18 @@ def test_get_item_resolved_value( create_profile(name_profile, name_distro, "a=1 b=2 c=3 c=4 c=5 d e") test_system_handle = create_system(name_system, name_profile) remote.modify_system(test_system_handle, "kernel_options", "!c !e", token=token) - test_system = remote.get_system(name_system, token=token) - expected_result = {"a": "1", "b": "2", "d": None} + if checked_object == "distro": + test_item = remote.get_distro(name_distro, token=token) + elif checked_object == "profile": + test_item = remote.get_profile(name_profile, token=token) + elif checked_object == "system": + test_item = remote.get_system(name_system, token=token) + else: + raise ValueError("checked_object has wrong value") # Act - result = remote.get_item_resolved_value(test_system.get("uid"), "kernel_options") + with expected_exception: + result = remote.get_item_resolved_value(test_item.get("uid"), input_attribute) - # Assert - assert expected_result == result + # Assert + assert expected_result == result From 43e301adaf66151ceecede00eb010075c5a6ff8d Mon Sep 17 00:00:00 2001 From: Enno Gotthold Date: Thu, 7 Jul 2022 14:46:34 +0200 Subject: [PATCH 2/2] XML-RPC API: Fix complex objects are return value This bug is appearing in "get_item_resolved_value" and could be triggered for example when "arch" was asked with a distro. The fix is to catch all types in "remote.py" and convert them on a per-type basis. This commit also adds tests for the known cases that would cause trouble in the past. --- cobbler/remote.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cobbler/remote.py b/cobbler/remote.py index b33e5c99cd..1d0a9a9e62 100644 --- a/cobbler/remote.py +++ b/cobbler/remote.py @@ -801,12 +801,11 @@ def get_item_resolved_value(self, item_uuid: str, attribute: str): 'None is not a valid value for the resolved attribute "%s". Please fix the item(s) ' 'starting at uuid "%s"' % (attribute, item_uuid) ) - elif isinstance(return_value, item.Item): - return return_value.name elif isinstance(return_value, enums.ConvertableEnum): return return_value.value elif isinstance( - return_value, (enums.DHCP, enums.NetworkInterfaceType, enums.BaudRates) + return_value, + (enums.DHCP, enums.NetworkInterfaceType, enums.BaudRates, item.Item), ): return return_value.name elif isinstance(return_value, dict):