From a97865de0c28c466e075a78c876f2af2e24e4964 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Mon, 8 Apr 2019 16:44:21 -0400 Subject: [PATCH] Refactor HostInsights for better reuse of the error handling of the Insights API calls. --- awx/api/views/__init__.py | 132 ++++++++++++++--------- awx/main/tests/unit/api/test_views.py | 97 ----------------- awx/playbooks/action_plugins/insights.py | 2 +- 3 files changed, 83 insertions(+), 148 deletions(-) diff --git a/awx/api/views/__init__.py b/awx/api/views/__init__.py index 9068629a5e..f88995b53e 100644 --- a/awx/api/views/__init__.py +++ b/awx/api/views/__init__.py @@ -31,7 +31,7 @@ from django.utils.translation import ugettext_lazy as _ # Django REST Framework -from rest_framework.exceptions import PermissionDenied, ParseError +from rest_framework.exceptions import APIException, PermissionDenied, ParseError from rest_framework.parsers import FormParser from rest_framework.permissions import AllowAny, IsAuthenticated from rest_framework.renderers import JSONRenderer, StaticHTMLRenderer @@ -1613,17 +1613,57 @@ class HostActivityStreamList(SubListAPIView): return qs.filter(Q(host=parent) | Q(inventory=parent.inventory)) +class BadGateway(APIException): + status_code = status.HTTP_502_BAD_GATEWAY + default_detail = '' + default_code = 'bad_gateway' + + +class GatewayTimeout(APIException): + status_code = status.HTTP_504_GATEWAY_TIMEOUT + default_detail = '' + default_code = 'gateway_timeout' + + class HostInsights(GenericAPIView): model = models.Host serializer_class = serializers.EmptySerializer - def _extract_insights_creds(self, credential): - return (credential.get_input('username', default=''), credential.get_input('password', default='')) + def _call_insights_api(self, url, session, headers): + try: + res = session.get(url, headers=headers, timeout=120) + except requests.exceptions.SSLError: + raise BadGateway(_('SSLError while trying to connect to {}').format(url)) + except requests.exceptions.Timeout: + raise GatewayTimeout(_('Request to {} timed out.').format(url)) + except requests.exceptions.RequestException as e: + raise BadGateway(_('Unknown exception {} while trying to GET {}').format(e, url)) - def _get_insights(self, url, username, password): + if res.status_code == 401: + raise BadGateway( + _('Unauthorized access. Please check your Insights Credential username and password.')) + elif res.status_code != 200: + raise BadGateway( + _( + 'Failed to access the Insights API at URL {}.' + ' Server responded with {} status code and message {}' + ).format(url, res.status_code, res.content) + ) + + try: + return res.json() + except ValueError: + raise BadGateway( + _('Expected JSON response from Insights but instead got {}').format(res.content)) + + def _get_session(self, username, password): session = requests.Session() session.auth = requests.auth.HTTPBasicAuth(username, password) + + return session + + def _get_headers(self): license = get_license(show_key=False).get('license_type', 'UNLICENSED') headers = { 'Content-Type': 'application/json', @@ -1633,46 +1673,43 @@ class HostInsights(GenericAPIView): license ) } - return session.get(url, headers=headers, timeout=120) - def _call_insights_api(self, url, username, password): - try: - res = self._get_insights(url, username, password) - except requests.exceptions.SSLError: - return (dict(error=_('SSLError while trying to connect to {}').format(url)), - status.HTTP_502_BAD_GATEWAY) - except requests.exceptions.Timeout: - return (dict(error=_('Request to {} timed out.').format(url)), - status.HTTP_504_GATEWAY_TIMEOUT) - except requests.exceptions.RequestException as e: - return (dict(error=_('Unknown exception {} while trying to GET {}').format(e, url)), - status.HTTP_502_BAD_GATEWAY) + return headers - if res.status_code == 401: - msg = _('Unauthorized access. Please check your Insights Credential username and password.') - return (dict(error=msg), status.HTTP_502_BAD_GATEWAY) - elif res.status_code != 200: - msg = _( - 'Failed to access the Insights API at URL {}.' - ' Server responded with {} status code and message {}' - ).format(url, res.status_code, res.content) - return (dict(error=msg), status.HTTP_502_BAD_GATEWAY) + def _get_platform_id(self, host, session, headers): + url = '{}/api/inventory/v1/hosts?insights_id={}'.format( + settings.INSIGHTS_URL_BASE, host.insights_system_id) + res = self._call_insights_api(url, session, headers) + platform_id = res['results'][0]['id'] - try: - res.json() - except ValueError: - return (dict(error=_('Expected JSON response from Insights but instead got {}').format(res.content)), - status.HTTP_502_BAD_GATEWAY) + return platform_id - return res + def _get_reports(self, platform_id, session, headers): + url = '{}/api/insights/v1/system/{}/reports/'.format( + settings.INSIGHTS_URL_BASE, platform_id) - def get_insights(self, url, username, password): - res = self._call_insights_api(url, username, password) - if isinstance(res, tuple): # This value was constructed based on a bad response from the API. - return res + return self._call_insights_api(url, session, headers) - filtered_insights_content = filter_insights_api_response(res.json()) - return (dict(insights_content=filtered_insights_content), status.HTTP_200_OK) + def _get_remediations(self, platform_id, session, headers): + url = '{}/api/remediations/v1/?system={}'.format( + settings.INSIGHTS_URL_BASE, platform_id) + + remediations = [] + + # Iterate over all of the pages of content. + while url: + data = self._call_insights_api(url, session, headers) + remediations.extend(data['data']) + + url = data['links']['next'] # Will be `None` if this is the last page. + + return remediations + + def _get_insights(self, platform_id, session, headers): + reports = self._get_reports(platform_id, session, headers) + remediations = self._get_remediations(platform_id, session, headers) + + return {'insights_content': filter_insights_api_response(reports, remediations)} def get(self, request, *args, **kwargs): host = self.get_object() @@ -1692,19 +1729,14 @@ class HostInsights(GenericAPIView): status=status.HTTP_404_NOT_FOUND ) - (username, password) = self._extract_insights_creds(cred) + username = cred.get_input('username', default='') + password = cred.get_input('password', default='') + session = self._get_session(username, password) + headers = self._get_headers() + platform_id = self._get_platform_id(host, session, headers) - host_url = '{}/api/inventory/v1/hosts?insights_id={}'.format( - settings.INSIGHTS_URL_BASE, host.insights_system_id) - res = self._call_insights_api(host_url, username, password) - if isinstance(res, tuple): # This value was constructed based on a bad response from the API. - return Response(res[0], status=res[1]) - platform_id = res.json()['results'][0]['id'] - - reports_url = '{}/api/insights/v1/system/{}/reports/'.format( - settings.INSIGHTS_URL_BASE, platform_id) - (msg, err_code) = self.get_insights(reports_url, username, password) - return Response(msg, status=err_code) + data = self._get_insights(platform_id, session, headers) + return Response(data, status=status.HTTP_200_OK) class GroupList(ListCreateAPIView): diff --git a/awx/main/tests/unit/api/test_views.py b/awx/main/tests/unit/api/test_views.py index f27f4f1a15..2734a18a5c 100644 --- a/awx/main/tests/unit/api/test_views.py +++ b/awx/main/tests/unit/api/test_views.py @@ -122,103 +122,6 @@ class TestInventoryInventorySourcesUpdate: assert response.data == expected -class TestHostInsights(): - - @pytest.fixture - def patch_parent(self, mocker): - mocker.patch('awx.api.generics.GenericAPIView') - - @pytest.mark.parametrize("status_code, exception, error, message", [ - (502, requests.exceptions.SSLError, 'SSLError while trying to connect to https://myexample.com/whocares/me/', None,), - (504, requests.exceptions.Timeout, 'Request to https://myexample.com/whocares/me/ timed out.', None,), - (502, requests.exceptions.RequestException, 'booo!', 'Unknown exception booo! while trying to GET https://myexample.com/whocares/me/'), - ]) - def test_get_insights_request_exception(self, patch_parent, mocker, status_code, exception, error, message): - view = HostInsights() - mocker.patch.object(view, '_get_insights', side_effect=exception(error)) - - (msg, code) = view.get_insights('https://myexample.com/whocares/me/', 'ignore', 'ignore') - assert code == status_code - assert msg['error'] == message or error - - def test_get_insights_non_200(self, patch_parent, mocker): - view = HostInsights() - Response = namedtuple('Response', 'status_code content') - mocker.patch.object(view, '_get_insights', return_value=Response(500, 'mock 500 err msg')) - - (msg, code) = view.get_insights('https://myexample.com/whocares/me/', 'ignore', 'ignore') - assert msg['error'] == ( - 'Failed to access the Insights API at URL' - ' https://myexample.com/whocares/me/. Server responded with 500 status code ' - 'and message mock 500 err msg') - - def test_get_insights_401(self, patch_parent, mocker): - view = HostInsights() - Response = namedtuple('Response', 'status_code content') - mocker.patch.object(view, '_get_insights', return_value=Response(401, '')) - - (msg, code) = view.get_insights('https://myexample.com/whocares/me/', 'ignore', 'ignore') - assert msg['error'] == 'Unauthorized access. Please check your Insights Credential username and password.' - - def test_get_insights_malformed_json_content(self, patch_parent, mocker): - view = HostInsights() - - class Response(): - status_code = 200 - content = 'booo!' - - def json(self): - raise ValueError('we do not care what this is') - - mocker.patch.object(view, '_get_insights', return_value=Response()) - - (msg, code) = view.get_insights('https://myexample.com/whocares/me/', 'ignore', 'ignore') - assert msg['error'] == 'Expected JSON response from Insights but instead got booo!' - assert code == 502 - - #def test_get_not_insights_host(self, patch_parent, mocker, mock_response_new): - #def test_get_not_insights_host(self, patch_parent, mocker): - def test_get_not_insights_host(self, mocker): - - view = HostInsights() - - host = Host() - host.insights_system_id = None - - mocker.patch.object(view, 'get_object', return_value=host) - - resp = view.get(None) - - assert resp.data['error'] == 'This host is not recognized as an Insights host.' - assert resp.status_code == 404 - - def test_get_no_credential(self, patch_parent, mocker): - view = HostInsights() - - class MockInventory(): - insights_credential = None - name = 'inventory_name_here' - - class MockHost(): - insights_system_id = 'insights_system_id_value' - inventory = MockInventory() - - mocker.patch.object(view, 'get_object', return_value=MockHost()) - - resp = view.get(None) - - assert resp.data['error'] == 'The Insights Credential for "inventory_name_here" was not found.' - assert resp.status_code == 404 - - def test_get_insights_user_agent(self, patch_parent, mocker): - with mock.patch.object(requests.Session, 'get') as get: - HostInsights()._get_insights('https://example.org', 'joe', 'example') - assert get.call_count == 1 - args, kwargs = get.call_args_list[0] - assert args == ('https://example.org',) - assert re.match(r'AWX [^\s]+ \(open\)', kwargs['headers']['User-Agent']) - - class TestSurveySpecValidation: def test_create_text_encrypted(self): diff --git a/awx/playbooks/action_plugins/insights.py b/awx/playbooks/action_plugins/insights.py index 3a465f3c5d..838265cbdf 100644 --- a/awx/playbooks/action_plugins/insights.py +++ b/awx/playbooks/action_plugins/insights.py @@ -81,7 +81,7 @@ class ActionModule(ActionBase): url = res.json()['links']['next'] # will be None if we're on the last page - for item in res.json()['remediations']: + for item in res.json()['data']: playbook_url = '{}/api/remediations/v1/remediations/{}/playbook'.format( insights_url, item['id']) res = session.get(playbook_url, timeout=120)