diff --git a/shipyard_airflow/control/configdocs/configdocs_helper.py b/shipyard_airflow/control/configdocs/configdocs_helper.py index 49ddca2d..5fe5580e 100644 --- a/shipyard_airflow/control/configdocs/configdocs_helper.py +++ b/shipyard_airflow/control/configdocs/configdocs_helper.py @@ -26,7 +26,7 @@ from oslo_config import cfg import requests from shipyard_airflow.control.configdocs.deckhand_client import ( - DeckhandClient, DeckhandPaths, DeckhandRejectedInputError, + DeckhandClient, DeckhandError, DeckhandPaths, DeckhandRejectedInputError, DeckhandResponseError, DocumentExistsElsewhereError, NoRevisionsExistError) from shipyard_airflow.control.service_endpoints import ( Endpoints, get_endpoint, get_token) @@ -342,8 +342,15 @@ class ConfigdocsHelper(object): if version in (BUFFER, COMMITTED): if revision_dict.get(version): revision_id = revision_dict.get(version).get('id') - return self.deckhand.get_rendered_docs_from_revision( - revision_id=revision_id) + try: + return self.deckhand.get_rendered_docs_from_revision( + revision_id=revision_id) + except DeckhandError as de: + raise ApiError( + title='Deckhand indicated an error while rendering', + description=de.response_message, + status=falcon.HTTP_500, + retry=False) else: raise ApiError( title='This revision does not exist', @@ -469,14 +476,10 @@ class ConfigdocsHelper(object): } exception['exception'] = ex - def get_validations_for_revision(self, revision_id): - """ - Use the endpoints for each of the UCP components to validate the - version indicated. Uses: - https://github.com/att-comdev/ucp-integration/blob/master/docs/api-conventions.md#post-v10validatedesign - format. - """ + def _get_validations_from_ucp_components(self, revision_id): + """Invoke other UCP components to retrieve their validations""" resp_msgs = [] + error_count = 0 validation_threads = ConfigdocsHelper._get_validation_threads( ConfigdocsHelper._get_validation_endpoints(), revision_id, @@ -490,7 +493,6 @@ class ConfigdocsHelper(object): validation_thread.get('thread').join() # check on the response, extract the validations - error_count = 0 for validation_thread in validation_threads: th_name = validation_thread.get('name') val_response = validation_thread.get('response', @@ -517,12 +519,43 @@ class ConfigdocsHelper(object): source=th_name ) resp_msgs.append(val_msg) - # Deckhand does it differently. Incorporate those validation - # failures, this may have to change if we store validations from other - # sources in Deckhand. - dh_validations = self._get_deckhand_validations(revision_id) + return (error_count, resp_msgs) + + def get_validations_for_revision(self, revision_id): + """Retrieves validations for a revision + + Invokes Deckhand to render the revision, which will either succeed, or + fail and return validaiton failures. If there are any failures, the + process will not proceed to validate against the other UCP components. + Upon success from Deckhand rendering, uses the endpoints for each of + the UCP components to validate the version indicated. + Responds in the format defined here: + https://github.com/att-comdev/ucp-integration/blob/master/docs/api-conventions.md#post-v10validatedesign + """ + resp_msgs = [] + error_count = 0 + + # Capture the messages from trying to render the revision. + render_errors = self.deckhand.get_render_errors(revision_id) + resp_msgs.extend(render_errors) + error_count += len(render_errors) + LOG.debug("Deckhand errors from rendering: %s", error_count) + # Incorporate stored validation errors from Deckhand (prevalidations) + # Note: This may have to change to be later in the code if we store + # validations from other sources in Deckhand. + dh_validations = self._get_deckhand_validation_errors(revision_id) error_count += len(dh_validations) resp_msgs.extend(dh_validations) + LOG.debug("Deckhand validations: %s", len(dh_validations)) + + # Only invoke the other validations if Deckhand has not returned any. + if (error_count == 0): + (cpnt_ec, cpnt_msgs) = self._get_validations_from_ucp_components( + revision_id) + resp_msgs.extend(cpnt_msgs) + error_count += cpnt_ec + LOG.debug("UCP component validations: %s", cpnt_ec) + # return the formatted status response return ConfigdocsHelper._format_validations_to_status( resp_msgs, error_count) @@ -537,9 +570,8 @@ class ConfigdocsHelper(object): return ConfigdocsHelper._format_validations_to_status( dh_validations, error_count) - def _get_deckhand_validations(self, revision_id): - # Returns any validations that deckhand has on hand for this - # revision. + def _get_deckhand_validation_errors(self, revision_id): + # Returns stored validation errors that deckhand has for this revision. resp_msgs = [] deckhand_val = self.deckhand.get_all_revision_validations(revision_id) if deckhand_val.get('results'): diff --git a/shipyard_airflow/control/configdocs/deckhand_client.py b/shipyard_airflow/control/configdocs/deckhand_client.py index bb56f006..f2f1bd5e 100644 --- a/shipyard_airflow/control/configdocs/deckhand_client.py +++ b/shipyard_airflow/control/configdocs/deckhand_client.py @@ -207,6 +207,31 @@ class DeckhandClient(object): self._handle_bad_response(response) return response.text + def get_render_errors(self, revision_id): + """Retrieve any error messages from rendering configdocs or [] + + Entries in the list returned are {error: ..., message: ...} format. + """ + url = DeckhandClient.get_path( + DeckhandPaths.RENDERED_REVISION_DOCS + ).format(revision_id) + + errors = [] + + LOG.debug("Retrieving rendered docs checking for validation messages") + response = self._get_request(url) + if response.status_code >= 400: + err_resp = yaml.safe_load(response.text) + errors = err_resp.get('details', {}).get('messageList', []) + if not errors: + # default message if none were specified. + errors.append({ + "error": True, + "message": ("Deckhand has reported an error but did not " + "specify messages. Response: {}".format( + response.text))}) + return errors + def get_rendered_docs_from_revision(self, revision_id, bucket_id=None): """ Returns the full set of rendered documents for a revision @@ -459,19 +484,25 @@ class DeckhandClient(object): ) ) +# +# Exceptions +# + + +class DeckhandError(Exception): + """Base exception for for all exceptions raised by this client""" + def __init__(self, response_message=None): + super().__init__() + self.response_message = response_message + # # Deckhand stateful messages wrapped as exceptions # -class DeckhandStatefulError(Exception): - """ - Base exception for errors that indicate some stateful-based - condition in deckhand. Not intended for use directly - """ - def __init__(self, response_message=None): - super().__init__() - self.response_message = response_message +class DeckhandStatefulError(DeckhandError): + """Base exception for errors for stateful-based conflicts in Deckhand.""" + pass class NoRevisionsExistError(DeckhandStatefulError): @@ -495,15 +526,14 @@ class DocumentExistsElsewhereError(DeckhandStatefulError): # -class DeckhandResponseError(Exception): +class DeckhandResponseError(DeckhandError): """ Indicates that a response was returned from Deckhand that was not expected """ def __init__(self, status_code, response_message=None): - super().__init__() + super().__init__(response_message) self.status_code = status_code - self.response_message = response_message class DeckhandRejectedInputError(DeckhandResponseError): @@ -518,12 +548,10 @@ class DeckhandRejectedInputError(DeckhandResponseError): # -class DeckhandAccessError(Exception): +class DeckhandAccessError(DeckhandError): """ Used to indicate that accessing Deckhand has failed. This is not the same as a bad response from Deckhand: See DeckhandResponseError """ - def __init__(self, response_message=None): - super().__init__() - self.response_message = response_message + pass diff --git a/tests/unit/control/test_configdocs_helper.py b/tests/unit/control/test_configdocs_helper.py index a1782426..fd2b2ff5 100644 --- a/tests/unit/control/test_configdocs_helper.py +++ b/tests/unit/control/test_configdocs_helper.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import json +import mock from unittest.mock import patch import yaml @@ -23,7 +24,7 @@ from shipyard_airflow.control.configdocs import configdocs_helper from shipyard_airflow.control.configdocs.configdocs_helper import ( BufferMode, ConfigdocsHelper) from shipyard_airflow.control.configdocs.deckhand_client import ( - DeckhandClient, DeckhandPaths, DeckhandResponseError, + DeckhandClient, DeckhandResponseError, NoRevisionsExistError) from shipyard_airflow.errors import ApiError, AppError @@ -504,18 +505,19 @@ def test_get_collection_docs(): assert len(yaml_str) == 16 -def _fake_get_validation_endpoints(): - val_ep = '{}/validatedesign' - return [ - { - 'name': 'Drydock', - 'url': val_ep.format('drydock') - }, - { - 'name': 'Armada', - 'url': val_ep.format('armada') - }, - ] +val_ep = '{}/validatedesign' + + +val_endpoints = [ + { + 'name': 'Drydock', + 'url': val_ep.format('drydock') + }, + { + 'name': 'Armada', + 'url': val_ep.format('armada') + }, +] def _fake_get_validations_for_component(url, design_reference, response, @@ -535,40 +537,54 @@ def _fake_get_validations_for_component(url, design_reference, response, "errorCount": 2, "messageList": [ { "message" : "broke it 1", "error": true}, - { "message" : "speeling error", "error": true} + { "message" : "speeling error", "error": true}, + { "message" : "good things", "error": false} ] }, "code": 400 } - """) % url) -def test_get_validations_for_revision(): +dh_render_val_list = [{"error": True, "message": "broken!"}] + + +@mock.patch.object(DeckhandClient, 'get_render_errors', + return_value=dh_render_val_list) +def test_get_validations_for_revision_dh_render(get_endpoint): """ Tests the functionality of the get_validations_for_revision method """ - with patch('shipyard_airflow.control.configdocs.deckhand_client.' - 'DeckhandClient.get_path') as mock_get_path: - mock_get_path.return_value = 'path{}' - helper = ConfigdocsHelper(CTX) - hold_ve = helper.__class__._get_validation_endpoints - hold_vfc = helper.__class__._get_validations_for_component - helper.__class__._get_validation_endpoints = ( - _fake_get_validation_endpoints) - helper.__class__._get_validations_for_component = ( - _fake_get_validations_for_component) - helper._get_deckhand_validations = lambda revision_id: [] - try: - val_status = helper.get_validations_for_revision(3) - err_count = val_status['details']['errorCount'] - err_list_count = len(val_status['details']['messageList']) - assert err_count == err_list_count - assert val_status['details']['errorCount'] == 4 - finally: - helper.__class__._get_validation_endpoints = hold_ve - helper.__class__._get_validations_for_component = hold_vfc - mock_get_path.assert_called_with(DeckhandPaths.RENDERED_REVISION_DOCS) + helper = ConfigdocsHelper(CTX) + hold_ve = helper.__class__._get_validation_endpoints + helper._get_deckhand_validation_errors = lambda revision_id: [] + val_status = helper.get_validations_for_revision(3) + err_count = val_status['details']['errorCount'] + err_list_count = len(val_status['details']['messageList']) + assert err_count == err_list_count + assert val_status['details']['errorCount'] == 1 + assert val_status['details']['messageList'][0]['message'] == 'broken!' + + +@mock.patch.object(DeckhandClient, 'get_render_errors', + return_value=[]) +@mock.patch.object(DeckhandClient, 'get_path', + return_value='path{}') +@mock.patch.object(ConfigdocsHelper, '_get_validation_endpoints', + return_value=val_endpoints) +@mock.patch.object(ConfigdocsHelper, '_get_validations_for_component', + new=_fake_get_validations_for_component) +def test_get_validations_for_revision(p1, p2, p3): + """ + Tests the functionality of the get_validations_for_revision method + """ + helper = ConfigdocsHelper(CTX) + helper._get_deckhand_validation_errors = lambda revision_id: [] + val_status = helper.get_validations_for_revision(3) + err_count = val_status['details']['errorCount'] + err_list_count = len(val_status['details']['messageList']) + assert err_list_count == 6 + assert val_status['details']['errorCount'] == 4 def test_generate_validation_message(): @@ -746,7 +762,7 @@ errors: """) -def test__get_deckhand_validations(): +def test__get_deckhand_validation_errors(): """ Tets the functionality of processing a response from deckhand """ @@ -757,7 +773,7 @@ def test__get_deckhand_validations(): lambda reivsion_id, subset_name: FK_VAL_SUBSET_RESP) helper.deckhand._get_entry_validation_response = ( lambda reivsion_id, subset_name, entry_id: FK_VAL_ENTRY_RESP) - assert len(helper._get_deckhand_validations(5)) == 2 + assert len(helper._get_deckhand_validation_errors(5)) == 2 FK_VAL_ENTRY_RESP_EMPTY = FakeResponse( @@ -786,7 +802,7 @@ def test__get_deckhand_validations_empty_errors(): lambda reivsion_id, subset_name: FK_VAL_SUBSET_RESP) helper.deckhand._get_entry_validation_response = ( lambda reivsion_id, subset_name, entry_id: FK_VAL_ENTRY_RESP_EMPTY) - assert len(helper._get_deckhand_validations(5)) == 0 + assert len(helper._get_deckhand_validation_errors(5)) == 0 FK_VAL_BASE_RESP_EMPTY = FakeResponse( @@ -801,14 +817,14 @@ results: [] """) -def test__get_deckhand_validations_empty_results(): +def test__get_deckhand_validation_errors_empty_results(): """ Tets the functionality of processing a response from deckhand """ helper = ConfigdocsHelper(CTX) helper.deckhand._get_base_validation_resp = ( lambda revision_id: FK_VAL_BASE_RESP_EMPTY) - assert len(helper._get_deckhand_validations(5)) == 0 + assert len(helper._get_deckhand_validation_errors(5)) == 0 def test_tag_buffer():