From a99c4b77103ea7d3a0dbecfe241f3b27550b2f8b Mon Sep 17 00:00:00 2001 From: Jonathan Rios Date: Tue, 1 Aug 2023 15:37:47 +0200 Subject: [PATCH] LITE-28075 Add abort action for deployment requests --- connect_ext_ppr/errors.py | 2 + connect_ext_ppr/models/deployment.py | 16 ++++ connect_ext_ppr/models/models_utils.py | 38 ++++++++ connect_ext_ppr/models/task.py | 10 ++ connect_ext_ppr/webapp.py | 42 +++++++- tests/api/test_deployment_requests.py | 128 ++++++++++++++++++++++++- tests/models.py/test_model_utils.py | 56 +++++++++++ 7 files changed, 290 insertions(+), 2 deletions(-) create mode 100644 connect_ext_ppr/models/models_utils.py create mode 100644 tests/models.py/test_model_utils.py diff --git a/connect_ext_ppr/errors.py b/connect_ext_ppr/errors.py index 31f1a3c..f38f099 100644 --- a/connect_ext_ppr/errors.py +++ b/connect_ext_ppr/errors.py @@ -103,4 +103,6 @@ class ExtensionValidationError(ExtensionErrorBase): 2: "{field}: This values {values} are invalid.", 3: "At least one choice needs to be specified.", 4: "Cannot applied PPR to {entity} {values}.", + 5: "Transition not allowed: can not set {field_name} from `{source}` to" + " '{target}', allowed {field_name} sources for '{target}' are '{allowed}'.", } diff --git a/connect_ext_ppr/models/deployment.py b/connect_ext_ppr/models/deployment.py index f494e59..a3f0a56 100644 --- a/connect_ext_ppr/models/deployment.py +++ b/connect_ext_ppr/models/deployment.py @@ -7,6 +7,7 @@ from connect_ext_ppr.models.enums import DeploymentRequestStatusChoices, DeploymentStatusChoices from connect_ext_ppr.models.ppr import PPRVersion from connect_ext_ppr.models.replicas import Product +from connect_ext_ppr.models.models_utils import transition class Deployment(Model): @@ -70,6 +71,21 @@ class DeploymentRequest(Model): innerjoin=True, ) + @property + def track_field(self): + return 'status' + + @transition(target=STATUSES.aborting, sources=[STATUSES.pending, STATUSES.processing]) + def aborting(self, tasks, by): + self.aborted_at = datetime.utcnow() + self.aborted_by = by + for task in tasks: + task.abort(self) + + @transition(target=STATUSES.aborted, sources=[STATUSES.aborting]) + def abort(self): + ... + class MarketplaceConfiguration(Model): __tablename__ = 'marketplace_configuration' diff --git a/connect_ext_ppr/models/models_utils.py b/connect_ext_ppr/models/models_utils.py new file mode 100644 index 0000000..4ae86b2 --- /dev/null +++ b/connect_ext_ppr/models/models_utils.py @@ -0,0 +1,38 @@ +import types + +from connect_ext_ppr.errors import ExtensionValidationError + + +class transition: + + def __init__(self, target, sources) -> None: + self.target = target + self.sources = sources if isinstance(sources, list) else [sources] + + def __call__(self, fn): + def inner(*args, **kwargs): + self._validate_transition(args[0]) + setattr(self.instance, self.field_name, self.target) + return fn(*args, **kwargs) + return inner + + def __get__(self, instance, owner=None): + return types.MethodType(self, instance) if instance is not None else self + + def _validate_transition(self, instance): + self.instance = instance + assert getattr(self.instance, 'track_field', False), ( + "Must implement a `track_field` property to target the transition field of the model." + ) + + self.field_name = self.instance.track_field + current_state = getattr(self.instance, self.field_name) + if current_state not in self.sources: + raise ExtensionValidationError.VAL_005( + format_kwargs={ + 'source': current_state, + 'field_name': self.field_name, + 'target': self.target, + 'allowed': ', '.join(self.sources), + }, + ) diff --git a/connect_ext_ppr/models/task.py b/connect_ext_ppr/models/task.py index a6d473d..e6d8159 100644 --- a/connect_ext_ppr/models/task.py +++ b/connect_ext_ppr/models/task.py @@ -5,6 +5,7 @@ from connect_ext_ppr.db import Model from connect_ext_ppr.models.enums import TasksStatusChoices, TaskTypesChoices from connect_ext_ppr.models.deployment import DeploymentRequest +from connect_ext_ppr.models.models_utils import transition class Task(Model): @@ -31,3 +32,12 @@ class Task(Model): finished_at = db.Column(db.DateTime(), nullable=True) aborted_at = db.Column(db.DateTime(), nullable=True) aborted_by = db.Column(db.String(20), nullable=True) + + @property + def track_field(self): + return 'status' + + @transition(target=STATUSES.aborted, sources=[STATUSES.pending]) + def abort(self, dr): + self.aborted_at = dr.aborted_at + self.aborted_by = dr.aborted_by diff --git a/connect_ext_ppr/webapp.py b/connect_ext_ppr/webapp.py index c45c761..17dd1a4 100644 --- a/connect_ext_ppr/webapp.py +++ b/connect_ext_ppr/webapp.py @@ -242,7 +242,6 @@ def list_deployment_request_tasks( self, depl_req_id: str, db: VerboseBaseSession = Depends(get_db), - client: ConnectClient = Depends(get_installation_client), installation: dict = Depends(get_installation), ): dr = ( @@ -264,6 +263,47 @@ def list_deployment_request_tasks( status_code=status.HTTP_404_NOT_FOUND, ) + @router.post( + '/deployments/requests/{depl_req_id}/abort', + summary='Abort a deployment request', + response_model=DeploymentRequestSchema, + ) + def abort( + self, + depl_req_id: str, + db: VerboseBaseSession = Depends(get_db), + client: ConnectClient = Depends(get_installation_client), + installation: dict = Depends(get_installation), + request: Request = None, + ): + dr = ( + db.query(DeploymentRequest) + .filter( + DeploymentRequest.deployment.has(account_id=installation['owner']['id']), + DeploymentRequest.id == depl_req_id, + ) + .one_or_none() + ) + if dr: + origin_state = dr.status + tasks = ( + db + .query(Task) + .filter_by(deployment_request=dr.id, status=Task.STATUSES.pending) + ) + user_data = get_user_data_from_auth_token(request.headers['connect-auth']) + dr.aborting(tasks, user_data['name']) + db.flush() + if origin_state == DeploymentRequest.STATUSES.pending: + dr.abort() + db.commit() + hub = get_hub(client, dr.deployment.hub_id) + return get_deployment_request_schema(dr, hub) + raise ExtensionHttpError.EXT_001( + format_kwargs={'obj_id': depl_req_id}, + status_code=status.HTTP_404_NOT_FOUND, + ) + @router.get( '/deployments/{deployment_id}', summary='Deployment details', diff --git a/tests/api/test_deployment_requests.py b/tests/api/test_deployment_requests.py index 12dfd01..215b6c7 100644 --- a/tests/api/test_deployment_requests.py +++ b/tests/api/test_deployment_requests.py @@ -207,7 +207,7 @@ def test_list_deployment_request_tasks_not_found( dep1 = deployment_factory(account_id=installation['owner']['id']) dep2 = deployment_factory(account_id='PA-123-456') - dr1 = deployment_request_factory(deployment=dep1) + dr1 = deployment_request_factory(deployment=dep1, status='done') bad_dr = deployment_request_factory(deployment=dep2) task_factory(deployment_request=dr1) @@ -703,6 +703,7 @@ def test_create_deployment_request_w_open_request( 'id': 'HB-0000-0001', 'name': 'Another Hub for the best', } + mocker.patch('connect_ext_ppr.webapp.get_client_object', side_effect=[hub_data]) dep = deployment_factory(account_id=installation['owner']['id'], hub_id=hub_data['id']) @@ -734,3 +735,128 @@ def test_create_deployment_request_w_open_request( assert response.json()['errors'] == [ 'Cannot create a new request, an open one already exists.', ] + + +@pytest.mark.parametrize( + 'dr_origin,dr_final', + (('pending', 'aborted'), ('processing', 'aborting')), +) +def test_abort_deployment_request( + dbsession, + mocker, + deployment_factory, + deployment_request_factory, + installation, + api_client, + task_factory, + dr_origin, + dr_final, +): + hub_data = { + 'id': 'HB-0000-0001', + 'name': 'Another Hub for the best', + } + + mocker.patch( + 'connect_ext_ppr.webapp.get_hub', + return_value=hub_data, + ) + + dep1 = deployment_factory(account_id=installation['owner']['id'], hub_id=hub_data['id']) + dep2 = deployment_factory(account_id='PA-123-456') + + dr1 = deployment_request_factory(deployment=dep1, status=dr_origin) + deployment_request_factory(deployment=dep1) + deployment_request_factory(deployment=dep2) + + t1 = task_factory(deployment_request=dr1, status='pending') + t2 = task_factory(deployment_request=dr1, task_index='002', status='pending') + + response = api_client.post( + f'/api/deployments/requests/{dr1.id}/abort', + installation=installation, + headers={ + "connect-auth": ( + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1Ijp7Im9pZCI6IlNVLTI5NS02ODktN" + "jI4IiwibmFtZSI6Ik5lcmkifX0.U_T6vuXnD293hcWNTJZ9QBViteNv8JXUL2gM0BezQ-k" + ), + }, + ) + response_item = response.json() + events = response_item.pop('events') + assert response.status_code == 200 + assert dr1.status == dr_final + assert response_item == { + 'id': dr1.id, + 'deployment': { + 'id': dep1.id, + 'product': { + 'id': dep1.product.id, + 'name': dep1.product.name, + 'icon': dep1.product.logo, + }, + 'hub': hub_data, + }, + 'ppr': { + 'id': dr1.ppr_id, + 'version': dr1.ppr.version, + }, + 'status': dr1.status.value, + 'manually': dr1.manually, + 'delegate_l2': dr1.delegate_l2, + + } + assert list(events.keys()) == ['created', 'aborted'] + assert list(events['created'].keys()) == ['at', 'by'] + assert list(events['aborted'].keys()) == ['at', 'by'] + for task in (t1, t2): + assert task.status == 'aborted' + assert task.aborted_at == dr1.aborted_at + assert task.aborted_by == dr1.aborted_by + + +def test_abort_deployment_request_not_allow( + dbsession, + mocker, + deployment_factory, + deployment_request_factory, + installation, + api_client, + task_factory, +): + hub_data = { + 'id': 'HB-0000-0001', + 'name': 'Another Hub for the best', + } + dep1 = deployment_factory(account_id=installation['owner']['id'], hub_id=hub_data['id']) + dep2 = deployment_factory(account_id='PA-123-456') + + origin_status = 'done' + dr1 = deployment_request_factory(deployment=dep1, status=origin_status) + deployment_request_factory(deployment=dep1) + deployment_request_factory(deployment=dep2) + + t1 = task_factory(deployment_request=dr1, status='pending') + t2 = task_factory(deployment_request=dr1, task_index='002', status='pending') + + response = api_client.post( + f'/api/deployments/requests/{dr1.id}/abort', + installation=installation, + headers={ + "connect-auth": ( + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1Ijp7Im9pZCI6IlNVLTI5NS02ODktN" + "jI4IiwibmFtZSI6Ik5lcmkifX0.U_T6vuXnD293hcWNTJZ9QBViteNv8JXUL2gM0BezQ-k" + ), + }, + ) + error = response.json() + + assert response.status_code == 400 + assert (t1.status, t2.status) == ('pending', 'pending') + assert dr1.status == origin_status + assert error == { + 'error_code': 'VAL_005', 'errors': [ + "Transition not allowed: can not set status from `done` to 'aborting'" + ", allowed status sources for 'aborting' are 'pending, processing'.", + ], + } diff --git a/tests/models.py/test_model_utils.py b/tests/models.py/test_model_utils.py new file mode 100644 index 0000000..52ec90b --- /dev/null +++ b/tests/models.py/test_model_utils.py @@ -0,0 +1,56 @@ +from connect.client import ClientError + +from connect_ext_ppr.models.models_utils import transition + +import pytest + + +class Animal: + + def __init__(self, foo) -> None: + self.foo = foo + + @property + def track_field(self): + return 'foo' + + @transition(target='parrot', sources=['bird']) + def convert(self): + ... + + +class ImproperlyConfigured: + def __init__(self, foo) -> None: + self.foo = foo + + @transition(target='parrot', sources=['bird']) + def convert(self): + ... + + +def test_transition_ok(): + obj = Animal(foo='bird') + assert obj.convert() is None + + +def test_transition_error(): + obj = Animal(foo='dog') + + with pytest.raises(ClientError) as ex: + obj.convert() + + assert ex.value.message == ( + "Transition not allowed: can not set foo from `dog` to 'parrot'" + ", allowed foo sources for 'parrot' are 'bird'." + ) + + +def test_improperly_configured_error(): + obj = ImproperlyConfigured(foo='bird') + + with pytest.raises(AssertionError) as ex: + obj.convert() + + assert ex.value.args[0] == ( + "Must implement a `track_field` property to target the transition field of the model." + )