From 78b9fd180b92975521a12b6b1279d435a8642fbc Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Fri, 24 Apr 2020 08:57:44 +0200 Subject: [PATCH 1/9] Resolve SudsLogFilter zeep doesn't log the full request in anything other than DEBUG logging. Change-Id: I32b55bb3f68d6668dd92906742b0427d2838a5c6 --- oslo_vmware/service.py | 29 ------------ oslo_vmware/tests/test_service.py | 74 ------------------------------- 2 files changed, 103 deletions(-) diff --git a/oslo_vmware/service.py b/oslo_vmware/service.py index 737d19dc..df67a524 100644 --- a/oslo_vmware/service.py +++ b/oslo_vmware/service.py @@ -436,32 +436,3 @@ def __repr__(self): def __str__(self): return "vSphere object" - - -class SudsLogFilter(logging.Filter): - """Filter to mask/truncate vCenter credentials in suds logs.""" - - def filter(self, record): - if not hasattr(record.msg, 'childAtPath'): - return True - - # Suds will log vCenter credentials if SessionManager.Login or - # SessionManager.SessionIsActive fails. - login = (record.msg.childAtPath('/Envelope/Body/Login') or - record.msg.childAtPath('/Envelope/Body/SessionIsActive')) - if login is None: - return True - - if login.childAtPath('userName') is not None: - login.childAtPath('userName').setText('***') - if login.childAtPath('password') is not None: # nosec - login.childAtPath('password').setText('***') # nosec - - session_id = login.childAtPath('sessionID') - if session_id is not None: - session_id.setText(session_id.getText()[-5:]) - - return True - -# Set log filter to mask/truncate vCenter credentials in suds logs. -suds.client.log.addFilter(SudsLogFilter()) diff --git a/oslo_vmware/tests/test_service.py b/oslo_vmware/tests/test_service.py index 336ef0d0..8aedc79d 100644 --- a/oslo_vmware/tests/test_service.py +++ b/oslo_vmware/tests/test_service.py @@ -535,77 +535,3 @@ def test_send_with_connection_timeout(self): headers=mock.sentinel.req_headers, timeout=120, verify=transport.verify) - - -class SudsLogFilterTest(base.TestCase): - """Tests for SudsLogFilter.""" - - def setUp(self): - super(SudsLogFilterTest, self).setUp() - self.log_filter = service.SudsLogFilter() - - self.login = mock.Mock(spec=suds.sax.element.Element) - self.username = suds.sax.element.Element('username').setText('admin') - self.password = suds.sax.element.Element('password').setText( - 'password') - self.session_id = suds.sax.element.Element('session_id').setText( - 'abcdef') - - def login_child_at_path_mock(path): - if path == 'userName': - return self.username - if path == 'password': - return self.password - if path == 'sessionID': - return self.session_id - - self.login.childAtPath.side_effect = login_child_at_path_mock - - def test_filter_with_no_child_at_path(self): - message = mock.Mock(spec=object) - record = mock.Mock(msg=message) - self.assertTrue(self.log_filter.filter(record)) - - def test_filter_with_login_failure(self): - message = mock.Mock(spec=suds.sax.element.Element) - - def child_at_path_mock(path): - if path == '/Envelope/Body/Login': - return self.login - - message.childAtPath.side_effect = child_at_path_mock - record = mock.Mock(msg=message) - - self.assertTrue(self.log_filter.filter(record)) - self.assertEqual('***', self.username.getText()) - self.assertEqual('***', self.password.getText()) - self.assertEqual('bcdef', self.session_id.getText()) - - def test_filter_with_session_is_active_failure(self): - message = mock.Mock(spec=suds.sax.element.Element) - - def child_at_path_mock(path): - if path == '/Envelope/Body/SessionIsActive': - return self.login - - message.childAtPath.side_effect = child_at_path_mock - record = mock.Mock(msg=message) - - self.assertTrue(self.log_filter.filter(record)) - self.assertEqual('***', self.username.getText()) - self.assertEqual('***', self.password.getText()) - self.assertEqual('bcdef', self.session_id.getText()) - - def test_filter_with_unknown_failure(self): - message = mock.Mock(spec=suds.sax.element.Element) - - def child_at_path_mock(path): - return None - - message.childAtPath.side_effect = child_at_path_mock - record = mock.Mock(msg=message) - - self.assertTrue(self.log_filter.filter(record)) - self.assertEqual('admin', self.username.getText()) - self.assertEqual('password', self.password.getText()) - self.assertEqual('abcdef', self.session_id.getText()) From 3b3f8618e1681fbe9645b17fc0819a081128b1da Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Wed, 6 May 2020 10:57:49 +0200 Subject: [PATCH 2/9] Add/Update moref helper functions We need to update `get_moref()` to support the way `zeep` is creating and passing a ManagedObjectReference to the vCenter. Since the attribute access changed from suds to zeep (before: .value and ._type; after: ._value_1 and .type), we introduce two helper functions to be used everywhere a moref value/type needs to be extracted from a ManagedObjectReference object or string. These functions are intended to stop the actual SOAP library implementation to leak out of oslo_vmware and in depending code. Having leaked abstractions in other code make changes harder. In that spirit, we also change all references inside of oslo_vmware to using `get_moref_value()` and `get_moref_type()` instead of directly accessing the attributes. Change-Id: I208460c9293478f261176c25e8f0e2552b1dde69 --- oslo_vmware/api.py | 2 +- oslo_vmware/pbm.py | 6 +-- oslo_vmware/service.py | 5 ++- oslo_vmware/tests/test_pbm.py | 9 +++-- oslo_vmware/tests/test_service.py | 32 ++++++++-------- oslo_vmware/tests/test_vim_util.py | 9 ++--- oslo_vmware/vim_util.py | 61 +++++++++++++++++++++++++----- 7 files changed, 83 insertions(+), 41 deletions(-) diff --git a/oslo_vmware/api.py b/oslo_vmware/api.py index b08fb279..8bd3b037 100644 --- a/oslo_vmware/api.py +++ b/oslo_vmware/api.py @@ -425,7 +425,7 @@ def _poll_task(self, task): "task: %s.", task) else: - task_detail = {'id': task.value} + task_detail = {'id': vim_util.get_moref_value(task)} # some internal tasks do not have 'name' set if getattr(task_info, 'name', None): task_detail['name'] = task_info.name diff --git a/oslo_vmware/pbm.py b/oslo_vmware/pbm.py index cd562456..46e971ae 100644 --- a/oslo_vmware/pbm.py +++ b/oslo_vmware/pbm.py @@ -162,7 +162,7 @@ def convert_datastores_to_hubs(pbm_client_factory, datastores): hubs = [] for ds in datastores: hub = pbm_client_factory.create('ns0:PbmPlacementHub') - hub.hubId = ds.value + hub.hubId = vim_util.get_moref_value(ds) hub.hubType = 'Datastore' hubs.append(hub) return hubs @@ -178,7 +178,7 @@ def filter_datastores_by_hubs(hubs, datastores): filtered_dss = [] hub_ids = [hub.hubId for hub in hubs] for ds in datastores: - if ds.value in hub_ids: + if vim_util.get_moref_value(ds) in hub_ids: filtered_dss.append(ds) return filtered_dss @@ -217,7 +217,7 @@ def get_profiles(session, vm): profile_manager = pbm.service_content.profileManager object_ref = pbm.client.factory.create('ns0:PbmServerObjectRef') - object_ref.key = vm.value + object_ref.key = vim_util.get_moref_value(vm) object_ref.objectType = 'virtualMachine' return session.invoke_api(pbm, 'PbmQueryAssociatedProfile', diff --git a/oslo_vmware/service.py b/oslo_vmware/service.py index df67a524..ce4199a1 100644 --- a/oslo_vmware/service.py +++ b/oslo_vmware/service.py @@ -283,7 +283,8 @@ def _retrieve_properties_ex_fault_checker(response): f_name = f_type.__class__.__name__ fault_list.append(f_name) if f_name == exceptions.NO_PERMISSION: - details['object'] = f_type.object.value + details['object'] = \ + vim_util.get_moref_value(f_type.object) details['privilegeId'] = f_type.privilegeId if fault_list: @@ -358,7 +359,7 @@ def request_handler(managed_object, **kwargs): op_id = '%s-%s' % (self.op_id_prefix, uuidutils.generate_uuid()) LOG.debug('Invoking %s.%s with opID=%s', - managed_object._type, + vim_util.get_moref_type(managed_object), attr_name, op_id) self._set_soap_headers(op_id) diff --git a/oslo_vmware/tests/test_pbm.py b/oslo_vmware/tests/test_pbm.py index bc05f7e7..ddb7e712 100644 --- a/oslo_vmware/tests/test_pbm.py +++ b/oslo_vmware/tests/test_pbm.py @@ -24,6 +24,7 @@ import six.moves.urllib.request as urllib from oslo_vmware import pbm +from oslo_vmware import vim_util from oslo_vmware.tests import base @@ -110,8 +111,7 @@ def test_filter_hubs_by_profile(self): profile=profile_id) def _create_datastore(self, value): - ds = mock.Mock() - ds.value = value + ds = vim_util.get_moref(value, 'Datastore') return ds def test_convert_datastores_to_hubs(self): @@ -146,7 +146,8 @@ def test_filter_datastores_by_hubs(self): filtered_ds = pbm.filter_datastores_by_hubs(hubs, datastores) self.assertEqual(len(hubs), len(filtered_ds)) - filtered_ds_values = [ds.value for ds in filtered_ds] + filtered_ds_values = [vim_util.get_moref_value(ds) + for ds in filtered_ds] self.assertEqual(set(hub_ids), set(filtered_ds_values)) def test_get_pbm_wsdl_location(self): @@ -182,7 +183,7 @@ def test_get_profiles(self): session.invoke_api.return_value = profile_id value = 'vm-1' - vm = mock.Mock(value=value) + vm = vim_util.get_moref(value, 'VirtualMachine') ret = pbm.get_profiles(session, vm) self.assertEqual(profile_id, ret) diff --git a/oslo_vmware/tests/test_service.py b/oslo_vmware/tests/test_service.py index 8aedc79d..2489f6cd 100644 --- a/oslo_vmware/tests/test_service.py +++ b/oslo_vmware/tests/test_service.py @@ -93,8 +93,8 @@ def test_request_handler(self): resp = mock.Mock() def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) return resp svc_obj = service.Service() @@ -108,8 +108,8 @@ def test_request_handler_with_retrieve_properties_ex_fault(self): managed_object = 'Datacenter' def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) return None svc_obj = service.Service() @@ -253,8 +253,8 @@ def test_request_handler_with_http_cannot_send_error(self): managed_object = 'VirtualMachine' def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) raise httplib.CannotSendRequest() svc_obj = service.Service() @@ -269,8 +269,8 @@ def test_request_handler_with_http_response_not_ready_error(self): managed_object = 'VirtualMachine' def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) raise httplib.ResponseNotReady() svc_obj = service.Service() @@ -285,8 +285,8 @@ def test_request_handler_with_http_cannot_send_header_error(self): managed_object = 'VirtualMachine' def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) raise httplib.CannotSendHeader() svc_obj = service.Service() @@ -301,8 +301,8 @@ def test_request_handler_with_connection_error(self): managed_object = 'VirtualMachine' def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) raise requests.ConnectionError() svc_obj = service.Service() @@ -317,8 +317,8 @@ def test_request_handler_with_http_error(self): managed_object = 'VirtualMachine' def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) raise requests.HTTPError() svc_obj = service.Service() @@ -340,8 +340,8 @@ def _test_request_handler_with_exception(self, message, exception): managed_object = 'VirtualMachine' def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) raise Exception(message) svc_obj = service.Service() diff --git a/oslo_vmware/tests/test_vim_util.py b/oslo_vmware/tests/test_vim_util.py index c5c20fea..ae9cffc2 100644 --- a/oslo_vmware/tests/test_vim_util.py +++ b/oslo_vmware/tests/test_vim_util.py @@ -30,8 +30,8 @@ class VimUtilTest(base.TestCase): def test_get_moref(self): moref = vim_util.get_moref("vm-0", "VirtualMachine") - self.assertEqual("vm-0", moref.value) - self.assertEqual("VirtualMachine", moref._type) + self.assertEqual("vm-0", vim_util.get_moref_value(moref)) + self.assertEqual("VirtualMachine", vim_util.get_moref_type(moref)) def test_build_selection_spec(self): client_factory = mock.Mock() @@ -218,8 +218,7 @@ def test_get_object_properties_with_empty_moref(self): @mock.patch('oslo_vmware.vim_util.cancel_retrieval') def test_get_object_properties(self, cancel_retrieval): vim = mock.Mock() - moref = mock.Mock() - moref._type = "VirtualMachine" + moref = vim_util.get_moref('fake-ref', 'VirtualMachine') retrieve_result = mock.Mock() def vim_RetrievePropertiesEx_side_effect(pc, specSet, options, @@ -235,7 +234,7 @@ def vim_RetrievePropertiesEx_side_effect(pc, specSet, options, prop_spec = propSet[0] self.assertTrue(prop_spec.all) self.assertEqual(['name'], prop_spec.pathSet) - self.assertEqual(moref._type, prop_spec.type) + self.assertEqual(vim_util.get_moref_type(moref), prop_spec.type) objSet = property_filter_spec.objectSet self.assertEqual(1, len(objSet)) diff --git a/oslo_vmware/vim_util.py b/oslo_vmware/vim_util.py index 3015038e..a84f1994 100644 --- a/oslo_vmware/vim_util.py +++ b/oslo_vmware/vim_util.py @@ -20,29 +20,70 @@ import logging from oslo_utils import timeutils -from suds import sudsobject - -try: - import suds.eventlet_patch -except ImportError: - pass +import zeep.helpers +from zeep import xsd LOG = logging.getLogger(__name__) -def get_moref(value, type_): +def get_moref(value, type_, client_factory=None): """Get managed object reference. :param value: value of the managed object :param type_: type of the managed object + :param client_factory: factory object to create a ManagedObjectReference :returns: managed object reference with given value and type """ - moref = sudsobject.Property(value) - moref._type = type_ + if client_factory: + moref = client_factory.create('ns0:ManagedObjectReference', + value, type=type_) + else: + moref_type = xsd.ComplexType(attributes=[xsd.Attribute('type', + xsd.String())]) + moref_type = moref_type.extend(xsd.String()) + moref = moref_type(value, type=type_) return moref +def get_moref_value(moref): + """Get the value/id of a managed object reference + + This function accepts a string representation of a ManagedObjectReference + like `VirtualMachine:vm-123` or only `vm-123`, but is also able to extract + it from the actual object as returned by the API. + """ + if isinstance(moref, str): + # handle strings like VirtualMachine:vm-12312, but also vm-123123 + if ':' in moref: + splits = moref.split(':') + return splits[1] + return moref + + # assume it's a ManagedObjectReference object as created by `get_moref()` + # or returned by a request + return moref._value_1 + + +def get_moref_type(moref): + """Get the type of a managed object reference + + This function accepts a string representation of a ManagedObjectReference + like `VirtualMachine:vm-123`, but is also able to extract it from the + actual object as returned by the API. + """ + if isinstance(moref, str): + # handle strings like VirtualMachine:vm-12312 + if ':' in moref: + splits = moref.split(':') + return splits[0] + return None + + # assume it's a ManagedObjectReference object as created by `get_moref()` + # or returned by a request + return moref.type + + def build_selection_spec(client_factory, name): """Builds the selection spec. @@ -308,7 +349,7 @@ def get_object_properties(vim, moref, properties_to_collect, skip_op_id=False): len(properties_to_collect) == 0) property_spec = build_property_spec( client_factory, - type_=moref._type, + type_=get_moref_type(moref), properties_to_collect=properties_to_collect, all_properties=all_properties) object_spec = build_object_spec(client_factory, moref, []) From 710bdf0d2a005904da2e6c961aec0b110645c088 Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Wed, 6 May 2020 11:05:30 +0200 Subject: [PATCH 3/9] Replace suds-jurko with zeep in requirements Change-Id: I91d454df4b4e70063f9dc36bc870a3d35179c86f --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 01c66724..d412eb2a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,7 +15,7 @@ oslo.utils>=3.33.0 # Apache-2.0 PyYAML>=3.10 # MIT lxml!=3.7.0,>=3.4.1 # BSD -suds-jurko>=0.6 # LGPLv3+ +zeep>=3.4.0 # MIT eventlet!=0.18.3,!=0.20.1,<0.21.0,>=0.18.2 # MIT requests>=2.14.2 # Apache-2.0 urllib3>=1.21.1 # MIT From cae5f326b82ec72371717a87fcbb4a380aa510b8 Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Wed, 6 May 2020 11:50:09 +0200 Subject: [PATCH 4/9] Add is_vim_instance() helper function This function can be used to check if an object as returned by the API is of a certain type or descends from it. It's necessary to have this function in oslo.vmware, because implementing it in depending projects would leak zeep specificas through the oslo.vmware library abstraction. Change-Id: Ifa0f38ee98cc7f78be0af96c34ad76a2a33ecf89 --- oslo_vmware/vim_util.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/oslo_vmware/vim_util.py b/oslo_vmware/vim_util.py index a84f1994..ed59af8d 100644 --- a/oslo_vmware/vim_util.py +++ b/oslo_vmware/vim_util.py @@ -668,3 +668,30 @@ def propset_dict(propset): return {} return {prop.name: prop.val for prop in propset} + + +def is_vim_instance(obj, vim_type_name): + """Implement isinstance for zeep types + + This walks through the XSD types of all the types `obj` extends and checks + if their name corresponds to the parameter `vim_type_name`. + """ + + def _obj_type_name(o): + return o._xsd_type.__class__.__name__ + + extension_type_names = set() + extension_types = [obj] + for t in extension_types: + name = _obj_type_name(t) + + if name in extension_type_names: + continue + extension_type_names.add(name) + + if name == vim_type_name: + return True + + extension_types.extend(t._xsd_type._extension_types) + + return False From 0f2fd86c202e535573ed096a7ecf6e4d1f5818b3 Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Wed, 6 May 2020 13:31:39 +0200 Subject: [PATCH 5/9] Add serialize_object() helper function This function converts a zeep object to a dictionary - an OrderedDict to be precise. Some depending code had this implemented itself, but since it's specific to the SOAP library, we pull it into oslo.vmware so we don't leak specificas of the underlying SOAP library through the oslo.vmware abstraction. Change-Id: I6534e9e7994b110817315192e5ef7343e9b3743e --- oslo_vmware/vim_util.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/oslo_vmware/vim_util.py b/oslo_vmware/vim_util.py index ed59af8d..57d2472f 100644 --- a/oslo_vmware/vim_util.py +++ b/oslo_vmware/vim_util.py @@ -695,3 +695,8 @@ def _obj_type_name(o): extension_types.extend(t._xsd_type._extension_types) return False + + +def serialize_object(obj): + """Convert an object and all attributes to OrderedDict""" + return zeep.helpers.serialize_object(obj) From 52d8383174e5bd78afbf948c9379d04b8815adfa Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Wed, 6 May 2020 14:42:59 +0200 Subject: [PATCH 6/9] Make Service use zeep instead of suds This is the biggest change in the migration from suds to zeep as it touches the main object containing the SOAP client. We define an interface for accessing the cookiejar, as it showed, that it might be placed on different objects for different SOAP libraries. We can now access it via '.client.cookiejar' on the 'Service' object. The MemoryCache implementation used for suds gets removed, because zeep comes with its own implementation. There are minimal differences in behavior: one cannot set the timeout for a specific key anymore. But as far as I have seen, this wasn't used anyways. There is also no need for RequestsTransport anymore, as zeep uses requests by default. We can also pass the parameters and the additional LocalFileAdapter into the session used by requests as was done before. While I can't say if the vSphere API still has the problems worked around with 'ServiceMessagePlugin', we use two plugins for zeep doing the same thing. For keeping compatibility with code written for suds' 'Client' object, we introduce a 'CompatibilityZeepClient' object. This is a zeep 'Client' extended to contain the cookiejar attributes and a 'factory' attribute, that behaves like the former 'factory' provided by suds. We also overwrite the 'soap_url' in this client's '__init__()', because zeep doesn't give us another way of changing it - it always takes the one from the WSDL binding. Change-Id: Icc781d025ef3987584dbe15dbb68554c22b9a2a1 --- oslo_vmware/rw_handles.py | 4 +- oslo_vmware/service.py | 259 ++++++++++++++------------- oslo_vmware/tests/test_api.py | 2 +- oslo_vmware/tests/test_rw_handles.py | 4 +- oslo_vmware/tests/test_service.py | 250 ++++++++++++-------------- oslo_vmware/tests/test_vim.py | 14 +- 6 files changed, 261 insertions(+), 272 deletions(-) diff --git a/oslo_vmware/rw_handles.py b/oslo_vmware/rw_handles.py index fcf26244..34f0af54 100644 --- a/oslo_vmware/rw_handles.py +++ b/oslo_vmware/rw_handles.py @@ -500,7 +500,7 @@ def __init__(self, session, host, port, rp_ref, vm_folder_ref, import_spec, url, thumbprint = self._find_vmdk_url(lease_info, host, port) self._vm_ref = lease_info.entity - cookies = session.vim.client.options.transport.cookiejar + cookies = session.vim.client.cookiejar # Create HTTP connection to write to VMDK URL if http_method == 'PUT': overwrite = 't' @@ -604,7 +604,7 @@ def __init__(self, session, host, port, vm_ref, vmdk_path, # find URL of the VMDK file to be read and open connection url, thumbprint = self._find_vmdk_url(lease_info, host, port) - cookies = session.vim.client.options.transport.cookiejar + cookies = session.vim.client.cookiejar self._conn = self._create_read_connection(url, cookies=cookies, ssl_thumbprint=thumbprint) diff --git a/oslo_vmware/service.py b/oslo_vmware/service.py index ce4199a1..39f4fa4e 100644 --- a/oslo_vmware/service.py +++ b/oslo_vmware/service.py @@ -21,23 +21,14 @@ import os import netaddr -from oslo_utils import timeutils from oslo_utils import uuidutils import requests import six import six.moves.http_client as httplib -import suds -try: - import suds.eventlet_patch -except ImportError: - pass - -from suds import cache -from suds import client -from suds import plugin -import suds.sax.element as element -from suds import transport +from lxml import etree +import zeep +from zeep.cache import InMemoryCache from oslo_vmware._i18n import _ from oslo_vmware import exceptions @@ -53,53 +44,68 @@ LOG = logging.getLogger(__name__) -class ServiceMessagePlugin(plugin.MessagePlugin): - """Suds plug-in handling some special cases while calling VI SDK.""" +class PruneEmptyNodesPlugin(zeep.Plugin): # list of XML elements which are allowed to be empty EMPTY_ELEMENTS = ["VirtualMachineEmptyProfileSpec"] - def add_attribute_for_value(self, node): - """Helper to handle AnyType. + def _isempty(self, el): + """Same implementation suds used to have""" + return el.text is None and not el.keys() and not len(el.getchildren()) + + def _prune(self, el): + pruned = [] + for child in el: + self._prune(child) + if self._isempty(child): + tag_name = etree.QName(child.tag).localname + if tag_name not in self.EMPTY_ELEMENTS: + pruned.append(child) + for p in pruned: + el.remove(p) - Suds does not handle AnyType properly. But VI SDK requires type - attribute to be set when AnyType is used. + def egress(self, envelope, http_headers, operation, binding_options): + """Prune empty nodes before sending the XML - :param node: XML value node + According to previous documentation here, the VI SDK throws server + errors if optional SOAP nodes are sent without values; e.g. as + opposed to test. Seeing that `zeep` can create such nodes + if optional elements in a sequence are not specified, we try to find + them and remove them. """ - if node.name == 'value' or node.name == 'val': - node.set('xsi:type', 'xsd:string') - # removeKey may be a 'int' or a 'string' - if node.name == 'removeKey': + self._prune(envelope) + return envelope, http_headers + + +class AddAnyTypeTypeAttributePlugin(zeep.Plugin): + + def _add_attribute_for_value(self, el): + tag_name = etree.QName(el.tag).localname + xsi_ns = zeep.xsd.const.xsi_ns + if tag_name in ('value', 'val'): + el.set(xsi_ns('type'), 'xsd:string') + elif tag_name == 'removeKey': try: - int(node.text) - node.set('xsi:type', 'xsd:int') + int(el.text) + el.set(xsi_ns('type'), 'xsd:int') except (ValueError, TypeError): - node.set('xsi:type', 'xsd:string') + el.set(xsi_ns('type'), 'xsd:string') - def prune(self, el): - pruned = [] - for c in el.children: - self.prune(c) - if c.isempty(False) and c.name not in self.EMPTY_ELEMENTS: - pruned.append(c) - for p in pruned: - el.children.remove(p) - - def marshalled(self, context): - """Modifies the envelope document before it is sent. + for child in el: + self._add_attribute_for_value(child) - This method provides the plug-in with the opportunity to prune empty - nodes and fix nodes before sending it to the server. + def egress(self, envelope, http_headers, operation, binding_options): + """Add an `xsi:type` attribute to value nodes - :param context: send context + The VI SDK requires a type attribute to be set when AnyType is used, + but `zeep` only does it when explicitly provided with a special object, + we don't want to leak through the abstraction. """ - # Suds builds the entire request object based on the WSDL schema. - # VI SDK throws server errors if optional SOAP nodes are sent - # without values; e.g., as opposed to test. - - self.prune(context.envelope) - context.envelope.walk(self.add_attribute_for_value) + # TODO(jkulik): Once we're on a zeep version containing + # https://github.com/mvantellingen/python-zeep/pull/1079 we should be + # able to remove this plugin. + self._add_attribute_for_value(envelope) + return envelope, http_headers class Response(six.BytesIO): @@ -161,59 +167,54 @@ def send(self, request, stream=False, timeout=None, return self._build_response_from_file(request) -class RequestsTransport(transport.Transport): - def __init__(self, cacert=None, insecure=True, pool_maxsize=10, - connection_timeout=None, pool_block=False): - transport.Transport.__init__(self) - # insecure flag is used only if cacert is not - # specified. - self.verify = cacert if cacert else not insecure - self.session = requests.Session() - self.session.mount('file:///', - LocalFileAdapter(pool_maxsize=pool_maxsize)) - self.session.mount('https://', requests.adapters.HTTPAdapter( - pool_connections=pool_maxsize, pool_maxsize=pool_maxsize, - pool_block=pool_block)) - self.cookiejar = self.session.cookies - self._connection_timeout = connection_timeout +class FactoryCompatibilityProxy(object): + + def __init__(self, _client): + self._client = _client + self._factory_cache = {} - def open(self, request): - resp = self.session.get(request.url, verify=self.verify) - return six.BytesIO(resp.content) + def create(self, obj, *args, **kwargs): + ns, obj = obj.split(':', 1) + if ns not in self._factory_cache: + self._factory_cache[ns] = self._client.type_factory(ns) + factory = self._factory_cache[ns] + return getattr(factory, obj)(*args, **kwargs) - def send(self, request): - resp = self.session.post(request.url, - data=request.message, - headers=request.headers, - verify=self.verify, - timeout=self._connection_timeout) - return transport.Reply(resp.status_code, resp.headers, resp.content) +class CompatibilityZeepClient(zeep.client.Client): + """zeep Client with added `factory` attribute -class MemoryCache(cache.ObjectCache): - def __init__(self): - self._cache = {} + The `factory` attribute is necessary for compatibility with the older suds + version of oslo.vmware. There's a lot of code using it, which would have to + be changed to use `Client.type_factory` instead. - def get(self, key): - """Retrieves the value for a key or None.""" - now = timeutils.utcnow_ts() - for k in list(self._cache): - (timeout, _value) = self._cache[k] - if timeout and now >= timeout: - del self._cache[k] + We also support setting the soap_url to something else than what the WSDL + ports return, which zeep otherwise doesn't support. - return self._cache.get(key, (0, None))[1] + This class also creates a backend-independent interface for accessing the + cookiejar. + """ - def put(self, key, value, time=CACHE_TIMEOUT): - """Sets the value for a key.""" - timeout = 0 - if time != 0: - timeout = timeutils.utcnow_ts() + time - self._cache[key] = (timeout, value) - return True + def __init__(self, *args, **kwargs): + soap_url = kwargs.pop('soap_url', None) + super(CompatibilityZeepClient, self).__init__(*args, **kwargs) -_CACHE = MemoryCache() + self.factory = FactoryCompatibilityProxy(self) + + # we cannot set this otherwise. it's parsed from the WSDL. we might + # need to reconfigure our vcenters to send proper WSDL service and + # ports + if soap_url is not None: + self.service._binding_options['address'] = soap_url + + @property + def cookiejar(self): + return self.transport.session.cookies + + @cookiejar.setter + def cookiejar(self, cookies): + self.transport.session.cookies = cookies class Service(object): @@ -228,18 +229,30 @@ def __init__(self, wsdl_url=None, soap_url=None, self.wsdl_url = wsdl_url self.soap_url = soap_url self.op_id_prefix = op_id_prefix - LOG.debug("Creating suds client with soap_url='%s' and wsdl_url='%s'", + + LOG.debug("Creating zeep client with soap_url='%s' and wsdl_url='%s'", self.soap_url, self.wsdl_url) - transport = RequestsTransport(cacert=cacert, - insecure=insecure, - pool_maxsize=pool_maxsize, - connection_timeout=connection_timeout, - pool_block=pool_block) - self.client = client.Client(self.wsdl_url, - transport=transport, - location=self.soap_url, - plugins=[ServiceMessagePlugin()], - cache=_CACHE) + session = requests.Session() + session.mount('https://', requests.adapters.HTTPAdapter( + pool_connections=pool_maxsize, pool_maxsize=pool_maxsize, + pool_block=pool_block)) + session.mount('file:///', LocalFileAdapter(pool_maxsize=pool_maxsize)) + session.verify = cacert if cacert else not insecure + + cache = InMemoryCache(CACHE_TIMEOUT) + + transport = \ + zeep.transports.Transport(session=session, + operation_timeout=connection_timeout, + cache=cache) + + plugins = [PruneEmptyNodesPlugin(), AddAnyTypeTypeAttributePlugin()] + + self.client = CompatibilityZeepClient(self.wsdl_url, + transport=transport, + soap_url=self.soap_url, + plugins=plugins) + self._service_content = None self._vc_session_cookie = None @@ -304,14 +317,14 @@ def _set_soap_headers(self, op_id): """ headers = [] if self._vc_session_cookie: - elem = element.Element('vcSessionCookie').setText( - self._vc_session_cookie) + elem = etree.Element('vcSessionCookie') + elem.text = self._vc_session_cookie headers.append(elem) if op_id: - elem = element.Element('operationID').setText(op_id) + elem = etree.Element('operationID') + elem.text = op_id headers.append(elem) - if headers: - self.client.set_options(soapheaders=headers) + return headers @property def service_content(self): @@ -321,7 +334,7 @@ def service_content(self): def get_http_cookie(self): """Return the vCenter session cookie.""" - cookies = self.client.options.transport.cookiejar + cookies = self.client.cookiejar for cookie in cookies: if cookie.name.lower() == 'vmware_soap_session': return cookie.value @@ -362,7 +375,9 @@ def request_handler(managed_object, **kwargs): vim_util.get_moref_type(managed_object), attr_name, op_id) - self._set_soap_headers(op_id) + headers = self._set_soap_headers(op_id) + if headers: + kwargs['_soapheaders'] = headers request = getattr(self.client.service, attr_name) response = request(managed_object, **kwargs) if (attr_name.lower() == 'retrievepropertiesex'): @@ -373,28 +388,24 @@ def request_handler(managed_object, **kwargs): # check of the SOAP response. raise - except suds.WebFault as excep: + except zeep.exceptions.Fault as excep: fault_string = None - if excep.fault: - fault_string = excep.fault.faultstring - - doc = excep.document - detail = None - if doc is not None: - detail = doc.childAtPath('/detail') - if not detail: - # NOTE(arnaud): this is needed with VC 5.1 - detail = doc.childAtPath('/Envelope/Body/Fault/detail') + if excep.message: + fault_string = excep.message + fault_list = [] details = {} - if detail: - for fault in detail.getChildren(): - fault_type = fault.get('type') + if excep.detail: + for fault in excep.detail.getchildren(): + type_ns = '{' + fault.nsmap['xsi'] + '}' + fault_type = fault.get('{}type'.format(type_ns)) if fault_type.endswith(exceptions.SECURITY_ERROR): fault_type = exceptions.NOT_AUTHENTICATED fault_list.append(fault_type) - for child in fault.getChildren(): - details[child.name] = child.getText() + for child in fault.getchildren(): + name = etree.QName(child.tag).localname + details[name] = child.text + raise exceptions.VimFaultException(fault_list, fault_string, excep, details) diff --git a/oslo_vmware/tests/test_api.py b/oslo_vmware/tests/test_api.py index b3e11db4..efc5dd4d 100644 --- a/oslo_vmware/tests/test_api.py +++ b/oslo_vmware/tests/test_api.py @@ -319,7 +319,7 @@ def test_invoke_api_with_vim_fault_exception_details(self): api_session = self._create_api_session(True) fault_string = 'Invalid property.' fault_list = [exceptions.INVALID_PROPERTY] - details = {u'name': suds.sax.text.Text(u'фира')} + details = {u'name': u'фира'} module = mock.Mock() module.api.side_effect = exceptions.VimFaultException(fault_list, diff --git a/oslo_vmware/tests/test_rw_handles.py b/oslo_vmware/tests/test_rw_handles.py index ba33ce54..7406f6b8 100644 --- a/oslo_vmware/tests/test_rw_handles.py +++ b/oslo_vmware/tests/test_rw_handles.py @@ -198,7 +198,7 @@ def session_invoke_api_side_effect(module, method, *args, **kwargs): vim_cookie = mock.Mock() vim_cookie.name = 'name' vim_cookie.value = 'value' - session.vim.client.options.transport.cookiejar = [vim_cookie] + session.vim.client.cookiejar = [vim_cookie] return session def test_init_failure(self): @@ -299,7 +299,7 @@ def session_invoke_api_side_effect(module, method, *args, **kwargs): vim_cookie = mock.Mock() vim_cookie.name = 'name' vim_cookie.value = 'value' - session.vim.client.options.transport.cookiejar = [vim_cookie] + session.vim.client.cookiejar = [vim_cookie] return session def test_init_failure(self): diff --git a/oslo_vmware/tests/test_service.py b/oslo_vmware/tests/test_service.py index 2489f6cd..89d1859e 100644 --- a/oslo_vmware/tests/test_service.py +++ b/oslo_vmware/tests/test_service.py @@ -17,48 +17,95 @@ import requests import six import six.moves.http_client as httplib -import suds +import zeep import ddt +from lxml import etree from oslo_vmware import exceptions from oslo_vmware import service from oslo_vmware.tests import base from oslo_vmware import vim_util +def load_xml(xml): + parser = etree.XMLParser( + remove_blank_text=True, remove_comments=True, resolve_entities=False + ) + return etree.fromstring(xml.strip(), parser=parser) + + @ddt.ddt -class ServiceMessagePluginTest(base.TestCase): - """Test class for ServiceMessagePlugin.""" +class AddAnyTypeTypeAttributePluginTest(base.TestCase): + """Test class for AddAnyTypeTypeAttributePlugin.""" def setUp(self): - super(ServiceMessagePluginTest, self).setUp() - self.plugin = service.ServiceMessagePlugin() + super(AddAnyTypeTypeAttributePluginTest, self).setUp() + self.plugin = service.AddAnyTypeTypeAttributePlugin() + + @ddt.data(('value', 'foo', 'string'), + ('removeKey', 1, 'int'), + ('removeKey', 'foo', 'string'), + ('flavor', 'foo', None)) + @ddt.unpack + def test_add_attribute_for_value(self, name, value, xsd_type): + xml_str = ''' + + + {} + + + ''' + xml = load_xml(xml_str.format(name, '', value, name)) + attr = 'xsi:type="xsd:{}"'.format(xsd_type) \ + if xsd_type is not None else '' + xml_expected = load_xml(xml_str.format(name, attr, value, name)) + + self.plugin._add_attribute_for_value(xml) + self.assertEqual(etree.tostring(xml), etree.tostring(xml_expected)) - @ddt.data(('value', 'foo', 'string'), ('removeKey', '1', 'int'), - ('removeKey', 'foo', 'string')) + +@ddt.ddt +class PruneEmptyNodesPluginTest(base.TestCase): + """Test class for PruneEmptyNodesPlugin.""" + + def setUp(self): + super(PruneEmptyNodesPluginTest, self).setUp() + self.plugin = service.PruneEmptyNodesPlugin() + + @ddt.data(('', True), + ('', False), + ('foo', False), + ('', False), + ('', True), + ('', False)) @ddt.unpack - def test_add_attribute_for_value(self, name, text, expected_xsd_type): - node = mock.Mock() - node.name = name - node.text = text - self.plugin.add_attribute_for_value(node) - node.set.assert_called_once_with('xsi:type', - 'xsd:%s' % expected_xsd_type) - - def test_marshalled(self): - context = mock.Mock() - self.plugin.prune = mock.Mock() - self.plugin.marshalled(context) - self.plugin.prune.assert_called_once_with(context.envelope) - context.envelope.walk.assert_called_once_with( - self.plugin.add_attribute_for_value) + def test_isempty(self, xml_str, should_be_empty): + xml = load_xml(xml_str) + self.assertEqual(self.plugin._isempty(xml), should_be_empty, xml_str) + + @ddt.data(('foo' + '', + 'foo'), + ('text', + 'text'), + ('', + ''), + ('' + '', + '' + '')) + @ddt.unpack + def test_prune(self, xml_str, xml_str_expected): + xml = load_xml(xml_str) + self.plugin._prune(xml) + self.assertEqual(etree.tostring(xml), xml_str_expected) class ServiceTest(base.TestCase): def setUp(self): super(ServiceTest, self).setUp() - patcher = mock.patch('suds.client.Client') + patcher = mock.patch('oslo_vmware.service.CompatibilityZeepClient') self.addCleanup(patcher.stop) self.SudsClientMock = patcher.start() @@ -124,23 +171,20 @@ def test_request_handler_with_web_fault(self): managed_object = 'VirtualMachine' fault_list = ['Fault'] - doc = mock.Mock() - def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) - fault = mock.Mock(faultstring="MyFault") + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) fault_children = mock.Mock() - fault_children.name = "name" - fault_children.getText.return_value = "value" + fault_children.tag = "name" + fault_children.text = "value" child = mock.Mock() child.get.return_value = fault_list[0] - child.getChildren.return_value = [fault_children] + child.nsmap = {'xsi': ''} + child.getchildren.return_value = [fault_children] detail = mock.Mock() - detail.getChildren.return_value = [child] - doc.childAtPath.return_value = detail - raise suds.WebFault(fault, doc) + detail.getchildren.return_value = [child] + raise zeep.exceptions.Fault(message="MyFault", detail=detail) svc_obj = service.Service() service_mock = svc_obj.client.service @@ -152,13 +196,11 @@ def side_effect(mo, **kwargs): self.assertEqual(fault_list, ex.fault_list) self.assertEqual({'name': 'value'}, ex.details) self.assertEqual("MyFault", ex.msg) - doc.childAtPath.assert_called_once_with('/detail') def test_request_handler_with_empty_web_fault_doc(self): def side_effect(mo, **kwargs): - fault = mock.Mock(faultstring="MyFault") - raise suds.WebFault(fault, None) + raise zeep.exceptions.Fault(message="MyFault") svc_obj = service.Service() service_mock = svc_obj.client.service @@ -175,23 +217,20 @@ def test_request_handler_with_vc51_web_fault(self): managed_object = 'VirtualMachine' fault_list = ['Fault'] - doc = mock.Mock() - def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) - fault = mock.Mock(faultstring="MyFault") + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) fault_children = mock.Mock() - fault_children.name = "name" - fault_children.getText.return_value = "value" + fault_children.tag = "name" + fault_children.text = "value" child = mock.Mock() child.get.return_value = fault_list[0] - child.getChildren.return_value = [fault_children] + child.nsmap = {'xsi': ''} + child.getchildren.return_value = [fault_children] detail = mock.Mock() - detail.getChildren.return_value = [child] - doc.childAtPath.side_effect = [None, detail] - raise suds.WebFault(fault, doc) + detail.getchildren.return_value = [child] + raise zeep.exceptions.Fault(message="MyFault", detail=detail) svc_obj = service.Service() service_mock = svc_obj.client.service @@ -203,29 +242,24 @@ def side_effect(mo, **kwargs): self.assertEqual(fault_list, ex.fault_list) self.assertEqual({'name': 'value'}, ex.details) self.assertEqual("MyFault", ex.msg) - exp_calls = [mock.call('/detail'), - mock.call('/Envelope/Body/Fault/detail')] - self.assertEqual(exp_calls, doc.childAtPath.call_args_list) def test_request_handler_with_security_error(self): managed_object = 'VirtualMachine' - doc = mock.Mock() def side_effect(mo, **kwargs): - self.assertEqual(managed_object, mo._type) - self.assertEqual(managed_object, mo.value) - fault = mock.Mock(faultstring="MyFault") + self.assertEqual(managed_object, vim_util.get_moref_type(mo)) + self.assertEqual(managed_object, vim_util.get_moref_value(mo)) fault_children = mock.Mock() - fault_children.name = "name" - fault_children.getText.return_value = "value" + fault_children.tag = "name" + fault_children.text = "value" child = mock.Mock() child.get.return_value = 'vim25:SecurityError' - child.getChildren.return_value = [fault_children] + child.nsmap = {'xsi': ''} + child.getchildren.return_value = [fault_children] detail = mock.Mock() - detail.getChildren.return_value = [child] - doc.childAtPath.return_value = detail - raise suds.WebFault(fault, doc) + detail.getchildren.return_value = [child] + raise zeep.exceptions.Fault(message="MyFault", detail=detail) svc_obj = service.Service() service_mock = svc_obj.client.service @@ -237,7 +271,6 @@ def side_effect(mo, **kwargs): self.assertEqual([exceptions.NOT_AUTHENTICATED], ex.fault_list) self.assertEqual({'name': 'value'}, ex.details) self.assertEqual("MyFault", ex.msg) - doc.childAtPath.assert_called_once_with('/detail') def test_request_handler_with_attribute_error(self): managed_object = 'VirtualMachine' @@ -373,7 +406,7 @@ def test_get_session_cookie(self): cookie = mock.Mock() cookie.name = 'vmware_soap_session' cookie.value = cookie_value - svc_obj.client.options.transport.cookiejar = [cookie] + svc_obj.client.cookiejar = [cookie] self.assertEqual(cookie_value, svc_obj.get_http_cookie()) def test_get_session_cookie_with_no_cookie(self): @@ -381,7 +414,7 @@ def test_get_session_cookie_with_no_cookie(self): cookie = mock.Mock() cookie.name = 'cookie' cookie.value = 'xyz' - svc_obj.client.options.transport.cookiejar = [cookie] + svc_obj.client.cookiejar = [cookie] self.assertIsNone(svc_obj.get_http_cookie()) def test_set_soap_headers(self): @@ -409,74 +442,22 @@ def fake_set_options(*args, **kwargs): svc_obj._set_soap_headers('fira-12345') -class MemoryCacheTest(base.TestCase): - """Test class for MemoryCache.""" - - def test_get_set(self): - cache = service.MemoryCache() - cache.put('key1', 'value1') - cache.put('key2', 'value2') - self.assertEqual('value1', cache.get('key1')) - self.assertEqual('value2', cache.get('key2')) - self.assertIsNone(cache.get('key3')) - - @mock.patch('suds.reader.DefinitionsReader.open') - @mock.patch('suds.reader.DocumentReader.download', create=True) - def test_shared_cache(self, mock_reader, mock_open): - cache1 = service.Service().client.options.cache - cache2 = service.Service().client.options.cache - self.assertIs(cache1, cache2) - - @mock.patch('oslo_utils.timeutils.utcnow_ts') - def test_cache_timeout(self, mock_utcnow_ts): - mock_utcnow_ts.side_effect = [100, 125, 150, 175, 195, 200, 225] - - cache = service.MemoryCache() - cache.put('key1', 'value1', 10) - cache.put('key2', 'value2', 75) - cache.put('key3', 'value3', 100) - - self.assertIsNone(cache.get('key1')) - self.assertEqual('value2', cache.get('key2')) - self.assertIsNone(cache.get('key2')) - self.assertEqual('value3', cache.get('key3')) - - -class RequestsTransportTest(base.TestCase): - """Tests for RequestsTransport.""" - - def test_open(self): - transport = service.RequestsTransport() - - data = b"Hello World" - resp = mock.Mock(content=data) - transport.session.get = mock.Mock(return_value=resp) - - request = mock.Mock(url=mock.sentinel.url) - self.assertEqual(data, - transport.open(request).getvalue()) - transport.session.get.assert_called_once_with(mock.sentinel.url, - verify=transport.verify) - - def test_send(self): - transport = service.RequestsTransport() - - resp = mock.Mock(status_code=mock.sentinel.status_code, - headers=mock.sentinel.headers, - content=mock.sentinel.content) - transport.session.post = mock.Mock(return_value=resp) +class TransportTest(base.TestCase): + """Tests for LocalFileAdapter and Transport parameters.""" + def setUp(self): + super(TransportTest, self).setUp() - request = mock.Mock(url=mock.sentinel.url, - message=mock.sentinel.message, - headers=mock.sentinel.req_headers) - reply = transport.send(request) + def new_client_init(self, url, **kwargs): + self.transport = kwargs['transport'] + return - self.assertEqual(mock.sentinel.status_code, reply.code) - self.assertEqual(mock.sentinel.headers, reply.headers) - self.assertEqual(mock.sentinel.content, reply.message) + mock.patch.object(service.CompatibilityZeepClient, + '__init__', new=new_client_init).start() + self.addCleanup(mock.patch.stopall) def test_set_conn_pool_size(self): - transport = service.RequestsTransport(pool_maxsize=100) + transport = service.Service(pool_maxsize=100).client.transport + local_file_adapter = transport.session.adapters['file:///'] self.assertEqual(100, local_file_adapter._pool_connections) self.assertEqual(100, local_file_adapter._pool_maxsize) @@ -486,7 +467,7 @@ def test_set_conn_pool_size(self): @mock.patch('os.path.getsize') def test_send_with_local_file_url(self, get_size_mock): - transport = service.RequestsTransport() + transport = service.Service(pool_maxsize=100).client.transport url = 'file:///foo' request = requests.PreparedRequest() @@ -522,16 +503,13 @@ def readinto_mock(buf): self.assertEqual(data, resp.content) def test_send_with_connection_timeout(self): - transport = service.RequestsTransport(connection_timeout=120) + transport = service.Service(connection_timeout=120).client.transport - request = mock.Mock(url=mock.sentinel.url, - message=mock.sentinel.message, - headers=mock.sentinel.req_headers) with mock.patch.object(transport.session, "post") as mock_post: - transport.send(request) + transport.post(mock.sentinel.url, mock.sentinel.message, + mock.sentinel.req_headers) mock_post.assert_called_once_with( mock.sentinel.url, data=mock.sentinel.message, headers=mock.sentinel.req_headers, - timeout=120, - verify=transport.verify) + timeout=120) diff --git a/oslo_vmware/tests/test_vim.py b/oslo_vmware/tests/test_vim.py index ddfa82eb..34ac7ae4 100644 --- a/oslo_vmware/tests/test_vim.py +++ b/oslo_vmware/tests/test_vim.py @@ -21,10 +21,10 @@ import mock from oslo_i18n import fixture as i18n_fixture -import suds from oslo_vmware import exceptions from oslo_vmware.tests import base +from oslo_vmware import service from oslo_vmware import vim @@ -33,9 +33,9 @@ class VimTest(base.TestCase): def setUp(self): super(VimTest, self).setUp() - patcher = mock.patch('suds.client.Client') + patcher = mock.patch('oslo_vmware.service.CompatibilityZeepClient') self.addCleanup(patcher.stop) - self.SudsClientMock = patcher.start() + self.ClientMock = patcher.start() self.useFixture(i18n_fixture.ToggleLazy(True)) @mock.patch.object(vim.Vim, '__getattr__', autospec=True) @@ -46,7 +46,7 @@ def test_service_content(self, getattr_mock): vim_obj.service_content getattr_mock.assert_called_once_with(vim_obj, 'RetrieveServiceContent') getattr_ret.assert_called_once_with('ServiceInstance') - self.assertEqual(self.SudsClientMock.return_value, vim_obj.client) + self.assertEqual(self.ClientMock.return_value, vim_obj.client) self.assertEqual(getattr_ret.return_value, vim_obj.service_content) def test_configure_non_default_host_port(self): @@ -78,14 +78,14 @@ def test_configure_with_wsdl_url_override(self): self.assertEqual('https://www.example.com/sdk', vim_obj.soap_url) -class VMwareSudsTest(base.TestCase): +class VMwareZeepTest(base.TestCase): def setUp(self): - super(VMwareSudsTest, self).setUp() + super(VMwareZeepTest, self).setUp() def new_client_init(self, url, **kwargs): return - mock.patch.object(suds.client.Client, + mock.patch.object(service.CompatibilityZeepClient, '__init__', new=new_client_init).start() self.addCleanup(mock.patch.stopall) self.vim = self._vim_create() From 76e4013f0c8ec3cefc8972ffdcd556f554f9c0ad Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Wed, 6 May 2020 16:24:30 +0200 Subject: [PATCH 7/9] fix tests for "Optionally block on new connections (making pool_maxsize a hard limit)" Change-Id: I8a88bfc5c95c6e5bbfa34d8c4a15f293160a72e8 --- oslo_vmware/tests/test_api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/oslo_vmware/tests/test_api.py b/oslo_vmware/tests/test_api.py index efc5dd4d..a1967a90 100644 --- a/oslo_vmware/tests/test_api.py +++ b/oslo_vmware/tests/test_api.py @@ -137,6 +137,7 @@ def test_vim(self): wsdl_url=api_session._vim_wsdl_loc, cacert=self.cert_mock, insecure=False, + pool_block=False, pool_maxsize=VMwareAPISessionTest.POOL_SIZE, connection_timeout=None, op_id_prefix='oslo.vmware') From 95695caed30035a0b7885835aca130c96078cc3b Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Wed, 6 May 2020 16:25:34 +0200 Subject: [PATCH 8/9] fix tests for "VmdkReadHandle to read the chunk size passed as parameter" Change-Id: Ib7f5be0a32f5023fb8681af250ffe658476f6859 --- oslo_vmware/tests/test_rw_handles.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oslo_vmware/tests/test_rw_handles.py b/oslo_vmware/tests/test_rw_handles.py index 7406f6b8..2c6c81cb 100644 --- a/oslo_vmware/tests/test_rw_handles.py +++ b/oslo_vmware/tests/test_rw_handles.py @@ -314,7 +314,7 @@ def test_init_failure(self): 100) def test_read(self): - chunk_size = rw_handles.READ_CHUNKSIZE + chunk_size = 65536 session = self._create_mock_session() handle = rw_handles.VmdkReadHandle(session, '10.1.2.3', 443, 'vm-1', '[ds] disk1.vmdk', From 3f783230ec7ebe0c2275601a7a2d98e9b70d2437 Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Tue, 9 Feb 2021 13:39:24 +0100 Subject: [PATCH 9/9] Handle existing type on removeKey The AddAnyTypeTypeAttributePlugin needs to keep the type of a removeKey, if it's already set and only default to setting string or int otherwise. Thre reason for this is, that the vSphere API expects the type of the key of the array to be set as the type of the removeKey e.g. a ManagedObjectReference. Change-Id: I7aaf53b69e7f80716d316234213925be985179ce --- oslo_vmware/service.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/oslo_vmware/service.py b/oslo_vmware/service.py index 39f4fa4e..aa6bd12d 100644 --- a/oslo_vmware/service.py +++ b/oslo_vmware/service.py @@ -85,11 +85,15 @@ def _add_attribute_for_value(self, el): if tag_name in ('value', 'val'): el.set(xsi_ns('type'), 'xsd:string') elif tag_name == 'removeKey': - try: - int(el.text) - el.set(xsi_ns('type'), 'xsd:int') - except (ValueError, TypeError): - el.set(xsi_ns('type'), 'xsd:string') + # we only add a type, if there wasn't any type set, because there + # are operations that need a ManagedObjectReference type on the + # removeKey + if not el.get(xsi_ns('type'), None): + try: + int(el.text) + el.set(xsi_ns('type'), 'xsd:int') + except (ValueError, TypeError): + el.set(xsi_ns('type'), 'xsd:string') for child in el: self._add_attribute_for_value(child)