From 755efb1f3c3677cb53d37ad3a46e17cb5e9e5ce0 Mon Sep 17 00:00:00 2001 From: Erik Olof Gunnar Andersson Date: Thu, 14 Dec 2023 15:54:54 -0800 Subject: [PATCH] Improve Dyntext coverage and removed unusable code - Removed poll_response code that wasn't working. - Removed unused code. - Fixed non-lazy loaded log lines. Change-Id: Ib8b0f4d61c1ee94f48b020c70ef260397d358172 --- designate/backend/impl_dynect.py | 136 ++++++------------ designate/tests/unit/backend/test_dynect.py | 152 ++++++++++++++++++++ 2 files changed, 199 insertions(+), 89 deletions(-) diff --git a/designate/backend/impl_dynect.py b/designate/backend/impl_dynect.py index e30dbfd5a..05ba2093a 100644 --- a/designate/backend/impl_dynect.py +++ b/designate/backend/impl_dynect.py @@ -15,7 +15,6 @@ # under the License. import time -from eventlet import Timeout from oslo_log import log as logging from oslo_serialization import jsonutils import requests @@ -26,7 +25,6 @@ import designate.conf from designate import exceptions from designate import utils - CONF = designate.conf.CONF LOG = logging.getLogger(__name__) CFG_GROUP_NAME = 'backend:dynect' @@ -46,12 +44,9 @@ class DynClientError(exceptions.Backend): self.url = url self.method = method self.details = details - formatted_string = '{} (HTTP {} to {} - {}) - {}'.format( - self.msgs, - self.method, - self.url, - self.http_status, - self.details + formatted_string = ( + f'{self.msgs} (HTTP {self.method} to {self.url} - ' + f'{self.http_status}) - {self.details}' ) if job_id: formatted_string += f' (Job-ID: {job_id})' @@ -101,16 +96,14 @@ class DynClient: https://help.dynect.net/rest/ """ - def __init__(self, customer_name, user_name, password, - endpoint="https://api.dynect.net:443", - api_version='3.5.6', headers=None, verify=True, retries=1, - timeout=10, timings=False, pool_maxsize=10, - pool_connections=10): + def __init__(self, customer_name, user_name, password, timeout, timings, + verify=True, retries=1, pool_maxsize=10, pool_connections=10): + self.endpoint = 'https://api.dynect.net:443' + self.api_version = '3.5.6' + self.customer_name = customer_name self.user_name = user_name self.password = password - self.endpoint = endpoint - self.api_version = api_version self.times = [] # [("item", starttime, endtime), ...] self.timings = timings @@ -125,17 +118,17 @@ class DynClient: session.headers = { 'Content-Type': 'application/json', 'Accept': 'application/json', - 'API-Version': api_version, - 'User-Agent': 'DynECTClient'} + 'API-Version': self.api_version, + 'User-Agent': 'DynECTClient' + } - if headers is not None: - session.headers.update(headers) - - adapter = HTTPAdapter(max_retries=int(retries), - pool_maxsize=int(pool_maxsize), - pool_connections=int(pool_connections), - pool_block=True) - session.mount(endpoint, adapter) + adapter = HTTPAdapter( + max_retries=int(retries), + pool_maxsize=int(pool_maxsize), + pool_connections=int(pool_connections), + pool_block=True + ) + session.mount(self.endpoint, adapter) self.http = session def _http_log_req(self, method, url, kwargs): @@ -149,25 +142,12 @@ class DynClient: header = "-H '{}: {}'".format(element, kwargs['headers'][element]) string_parts.append(header) - LOG.debug("REQ: %s", " ".join(string_parts)) + LOG.debug('REQ: %s', ' '.join(string_parts)) if 'data' in kwargs: - LOG.debug("REQ BODY: %s\n" % (kwargs['data'])) + LOG.debug('REQ BODY: %s\n', kwargs['data']) def _http_log_resp(self, resp): - LOG.debug( - "RESP: [%s] %s\n" % - (resp.status_code, - resp.headers)) - if resp._content_consumed: - LOG.debug( - "RESP BODY: %s\n" % - resp.text) - - def get_timings(self): - return self.times - - def reset_timings(self): - self.times = [] + LOG.debug('RESP: [%s] %s\n', resp.status_code, resp.headers) def _request(self, method, url, **kwargs): """ @@ -206,8 +186,7 @@ class DynClient: else: self._http_log_req(method, url, kwargs) - if self.timings: - start_time = time.monotonic() + start_time = time.monotonic() resp = self.http.request(method, url, **kwargs) if self.timings: self.times.append((f"{method} {url}", @@ -215,35 +194,10 @@ class DynClient: self._http_log_resp(resp) if resp.status_code >= 400: - LOG.debug( - "Request returned failure status: %s", - resp.status_code) + LOG.debug('Request returned failure status: %s', resp.status_code) raise DynClientError.from_response(resp) return resp - def poll_response(self, response): - """ - The API might return a job nr in the response in case of a async - response: https://github.com/fog/fog/issues/575 - """ - status = response.status - - timeout = Timeout(CONF[CFG_GROUP_NAME].job_timeout) - try: - while status == 307: - time.sleep(1) - url = response.headers.get('Location') - LOG.debug("Polling %s", url) - - polled_response = self.get(url) - status = response.status - except Timeout as t: - if t == timeout: - raise DynTimeoutError('Timeout reached when pulling job.') - finally: - timeout.cancel() - return polled_response - def request(self, method, url, retries=2, **kwargs): if self.token is None and not self.authing: self.login() @@ -258,9 +212,6 @@ class DynClient: else: raise - if response.status_code == 307: - response = self.poll_response(response) - return response.json() def login(self): @@ -282,18 +233,10 @@ class DynClient: response = self.request('POST', *args, **kwargs) return response - def get(self, *args, **kwargs): - response = self.request('GET', *args, **kwargs) - return response - def put(self, *args, **kwargs): response = self.request('PUT', *args, **kwargs) return response - def patch(self, *args, **kwargs): - response = self.request('PATCH', *args, **kwargs) - return response - def delete(self, *args, **kwargs): response = self.request('DELETE', *args, **kwargs) return response @@ -330,8 +273,13 @@ class DynECTBackend(base.Backend): timings=CONF[CFG_GROUP_NAME].timings) def create_zone(self, context, zone): - LOG.info('Creating zone %(d_id)s / %(d_name)s', - {'d_id': zone['id'], 'd_name': zone['name']}) + LOG.info( + 'Creating zone %(d_id)s / %(d_name)s', + { + 'd_id': zone['id'], + 'd_name': zone['name'] + } + ) url = '/Secondary/%s' % zone['name'].rstrip('.') data = { @@ -351,8 +299,10 @@ class DynECTBackend(base.Backend): except DynClientError as e: for emsg in e.msgs: if emsg['ERR_CD'] == 'TARGET_EXISTS': - LOG.info("Zone already exists, updating existing " - "zone instead %s", zone['name']) + LOG.info( + 'Zone already exists, updating existing zone ' + 'instead %s', zone['name'] + ) client.put(url, data=data) break else: @@ -362,17 +312,25 @@ class DynECTBackend(base.Backend): client.logout() def delete_zone(self, context, zone, zone_params=None): - LOG.info('Deleting zone %(d_id)s / %(d_name)s', - {'d_id': zone['id'], 'd_name': zone['name']}) + LOG.info( + 'Deleting zone %(d_id)s / %(d_name)s', { + 'd_id': zone['id'], + 'd_name': zone['name'] + } + ) url = '/Zone/%s' % zone['name'].rstrip('.') client = self.get_client() try: client.delete(url) except DynClientError as e: if e.http_status == 404: - LOG.warning("Attempt to delete %(d_id)s / %(d_name)s " - "caused 404, ignoring.", - {'d_id': zone['id'], 'd_name': zone['name']}) + LOG.warning( + 'Attempt to delete %(d_id)s / %(d_name)s caused 404, ' + 'ignoring.', { + 'd_id': zone['id'], + 'd_name': zone['name'] + } + ) pass else: raise diff --git a/designate/tests/unit/backend/test_dynect.py b/designate/tests/unit/backend/test_dynect.py index 31af7f2ec..febeb75ff 100644 --- a/designate/tests/unit/backend/test_dynect.py +++ b/designate/tests/unit/backend/test_dynect.py @@ -14,14 +14,21 @@ # License for the specific language governing permissions and limitations # under the License. + from unittest import mock +from oslo_config import fixture as cfg_fixture import oslotest.base import requests_mock from designate.backend import impl_dynect +import designate.conf from designate import context +from designate import exceptions from designate import objects +from designate.tests import base_fixtures + +CONF = designate.conf.CONF MASTERS = [ '192.0.2.1' @@ -146,10 +153,33 @@ ACTIVATE_SUCCESS = { } +class DynClientTestsCase(oslotest.base.BaseTestCase): + def setUp(self): + super().setUp() + + def test_dyn_client_auth_error(self): + client = impl_dynect.DynClient( + 'customer_name', 'user_name', 'password', 'timeout', 'timings' + ) + client.token = 'fake' + client._request = mock.Mock() + client._request.side_effect = impl_dynect.DynClientAuthError() + + self.assertRaisesRegex( + impl_dynect.DynClientAuthError, + r'None \(HTTP None to None - None\) - None', + client.request, 'POST', '/Test/' + ) + + class DynECTTestsCase(oslotest.base.BaseTestCase): def setUp(self): super().setUp() + self.useFixture(cfg_fixture.Config(CONF)) + self.stdlog = base_fixtures.StandardLogging() + self.useFixture(self.stdlog) + self.context = mock.Mock() self.admin_context = mock.Mock() mock.patch.object( @@ -181,6 +211,17 @@ class DynECTTestsCase(oslotest.base.BaseTestCase): objects.PoolTarget.from_dict(self.target) ) + def test_masters_only_allow_port_53(self): + self.target['masters'] = [ + {'host': '192.0.2.1', 'port': 5354} + ] + self.assertRaisesRegex( + exceptions.ConfigurationError, + 'DynECT only supports mDNS instances on port 53', + impl_dynect.DynECTBackend, + objects.PoolTarget.from_dict(self.target) + ) + @requests_mock.mock() def test_create_zone(self, req_mock): req_mock.post( @@ -205,6 +246,65 @@ class DynECTTestsCase(oslotest.base.BaseTestCase): self.backend.create_zone(self.context, self.zone) + @requests_mock.mock() + def test_create_zone_with_timings(self, req_mock): + CONF.set_override('timings', True, 'backend:dynect') + + req_mock.post( + '%s/Session' % self.base_address, + json=LOGIN_SUCCESS, + ) + req_mock.delete( + '%s/Session' % self.base_address, + json=LOGOUT_SUCCESS, + ) + + req_mock.post( + '%s/Secondary/example.com' % self.base_address, + json=ZONE_CREATE, + status_code=200, + ) + req_mock.put( + '%s/Secondary/example.com' % self.base_address, + json=ACTIVATE_SUCCESS, + status_code=200, + ) + + self.backend.create_zone(self.context, self.zone) + + @requests_mock.mock() + def test_create_zone_missing_contact_and_tsig(self, req_mock): + self.target['options'] = [ + {'key': 'username', 'value': 'example'}, + {'key': 'password', 'value': 'secret'}, + ] + + backend = impl_dynect.DynECTBackend( + objects.PoolTarget.from_dict(self.target) + ) + + req_mock.post( + '%s/Session' % self.base_address, + json=LOGIN_SUCCESS, + ) + req_mock.delete( + '%s/Session' % self.base_address, + json=LOGOUT_SUCCESS, + ) + + req_mock.post( + '%s/Secondary/example.com' % self.base_address, + json=ZONE_CREATE, + status_code=200, + ) + req_mock.put( + '%s/Secondary/example.com' % self.base_address, + json=ACTIVATE_SUCCESS, + status_code=200, + ) + + backend.create_zone(self.context, self.zone) + @requests_mock.mock() def test_create_zone_raise_dynclienterror(self, req_mock): # https://api.dynect.net:443/REST/Session @@ -273,6 +373,36 @@ class DynECTTestsCase(oslotest.base.BaseTestCase): self.backend.delete_zone(self.context, self.zone) + @requests_mock.mock() + def test_delete_zone_not_found(self, req_mock): + req_mock.post( + '%s/Session' % self.base_address, + json=LOGIN_SUCCESS, + ) + req_mock.delete( + '%s/Session' % self.base_address, + json=LOGOUT_SUCCESS, + ) + + req_mock.delete( + '%s/Zone/example.com' % self.base_address, + json=INVALID_ZONE_DELETE_DATA, + status_code=404 + ) + req_mock.put( + '%s/Secondary/example.com' % self.base_address, + json=ACTIVATE_SUCCESS, + status_code=200, + ) + + self.backend.delete_zone(self.context, self.zone) + + self.assertIn( + 'Attempt to delete e2bed4dc-9d01-11e4-89d3-123b93f75cba / ' + 'example.com. caused 404, ignoring.', + self.stdlog.logger.output + ) + @requests_mock.mock() def test_delete_zone_raise_dynclienterror(self, req_mock): req_mock.post( @@ -300,6 +430,28 @@ class DynECTTestsCase(oslotest.base.BaseTestCase): self.backend.delete_zone, self.context, self.zone, ) + @requests_mock.mock() + def test_request(self, req_mock): + req_mock.post( + '%s/Session' % self.base_address, + json=LOGIN_SUCCESS, + ) + req_mock.post( + 'https://api.dynect.net:443/' + ) + req_mock.post( + 'https://api.dynect.net:443/REST' + ) + + client = self.backend.get_client() + + self.assertEqual(200, + client._request( + 'POST', + 'https://api.dynect.net:443/REST').status_code + ) + self.assertEqual(200, client._request('POST', '').status_code) + def test_error_from_response(self): error_data = dict(INVALID_ZONE_DELETE_DATA) mock_response = mock.Mock()