From b80df59d111f31ca090b3d313050a23a4245825b Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Wed, 17 Oct 2018 18:36:57 -0400 Subject: [PATCH] fix: Address small issues with revision rollback controller 1. There is no exception called `InvalidRollback` in Deckhand (it was removed a while back). Instead, the only exception that db_api.revision_rollback raises is RevisionNotFound from the revision_get call internally. So catch that instead from the controller. 2. The default value of parameters is `str` so when revision_id of '0' is passed to the db module for processing, it skips over the check for `if revision_id == 0` as revision_id is a str, not int. So this leverages builtin int converter logic in falcon [0] but requires uplifting the version of falcon to at least 1.3.0 to make use of it [1]. [0] https://falcon.readthedocs.io/en/stable/api/routing.html#field-converters [1] https://falcon.readthedocs.io/en/1.3.0/api/routing.html#field-converters Change-Id: I068cd9e9b6818a5d51501f2718ee2d40d556c094 --- deckhand/control/rollback.py | 2 +- deckhand/service.py | 3 +- .../unit/control/test_api_initialization.py | 4 +-- .../test_revisions_rollback_controller.py | 31 +++++++++++++++++++ requirements.txt | 2 +- 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/deckhand/control/rollback.py b/deckhand/control/rollback.py index 97988b05..2474e1ab 100644 --- a/deckhand/control/rollback.py +++ b/deckhand/control/rollback.py @@ -47,7 +47,7 @@ class RollbackResource(api_base.BaseResource): try: rollback_revision = db_api.revision_rollback( revision_id, latest_revision) - except errors.InvalidRollback as e: + except errors.RevisionNotFound as e: with excutils.save_and_reraise_exception(): LOG.exception(e.format_message()) diff --git a/deckhand/service.py b/deckhand/service.py index 3d0daff3..405959d0 100644 --- a/deckhand/service.py +++ b/deckhand/service.py @@ -64,7 +64,8 @@ def configure_app(app, version=''): ('revisions/{revision_id}/validations/{validation_name}' '/entries/{entry_id}', validations.ValidationsResource()), - ('rollback/{revision_id}', rollback.RollbackResource()) + # min=0 is used as revision rollback supports 0. + ('rollback/{revision_id:int(min=0)}', rollback.RollbackResource()) ] for path, res in v1_0_routes: diff --git a/deckhand/tests/unit/control/test_api_initialization.py b/deckhand/tests/unit/control/test_api_initialization.py index 3eb82f65..cddfccda 100644 --- a/deckhand/tests/unit/control/test_api_initialization.py +++ b/deckhand/tests/unit/control/test_api_initialization.py @@ -99,8 +99,6 @@ class TestApi(test_base.DeckhandTestCase): self.revision_tags_resource()), mock.call('/api/v1.0/revisions/{revision_id}/tags/{tag}', self.revision_tags_resource()), - mock.call('/api/v1.0/rollback/{revision_id}', - self.rollback_resource()), mock.call('/api/v1.0/revisions/{revision_id}/validations', self.validations_resource()), mock.call('/api/v1.0/revisions/{revision_id}/validations/' @@ -109,6 +107,8 @@ class TestApi(test_base.DeckhandTestCase): mock.call('/api/v1.0/revisions/{revision_id}/validations/' '{validation_name}/entries/{entry_id}', self.validations_resource()), + mock.call('/api/v1.0/rollback/{revision_id:int(min=0)}', + self.rollback_resource()), mock.call('/versions', self.versions_resource()) ], any_order=True) diff --git a/deckhand/tests/unit/control/test_revisions_rollback_controller.py b/deckhand/tests/unit/control/test_revisions_rollback_controller.py index d317d4da..0ada8e11 100644 --- a/deckhand/tests/unit/control/test_revisions_rollback_controller.py +++ b/deckhand/tests/unit/control/test_revisions_rollback_controller.py @@ -21,6 +21,37 @@ from deckhand import factories from deckhand.tests.unit.control import base as test_base +class TestRevisionsRollbackController(test_base.BaseControllerTest): + """Test basic scenarios for the ``RollbackResource`` controller.""" + + def test_rollback_to_revision_0(self): + rules = {'deckhand:create_cleartext_documents': '@', + 'deckhand:list_revisions': '@'} + self.policy.set_rules(rules) + + # Create revision 1. + documents_factory = factories.DocumentFactory(1, [1]) + payload = documents_factory.gen_test({}) + resp = self.app.simulate_put( + '/api/v1.0/buckets/mop/documents', + headers={'Content-Type': 'application/x-yaml'}, + body=yaml.safe_dump_all(payload)) + self.assertEqual(200, resp.status_code) + + # Rollback to revision 0 (thereby creating revision 1 in effect). + resp = self.app.simulate_post( + '/api/v1.0/rollback/%s' % 0, + headers={'Content-Type': 'application/x-yaml'}) + self.assertEqual(201, resp.status_code) + + # Validate that 2 revisions now exist. + resp = self.app.simulate_get( + '/api/v1.0/revisions', + headers={'Content-Type': 'application/x-yaml'}) + self.assertEqual(200, resp.status_code) + self.assertEqual(2, yaml.safe_load(resp.text)['count']) + + class TestRevisionsRollbackControllerNegativeRBAC( test_base.BaseControllerTest): """Test suite for validating negative RBAC scenarios for revisions rollback diff --git a/requirements.txt b/requirements.txt index bb5ec42f..959225bf 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,7 +8,7 @@ alembic==1.0.1 beaker==1.9.1 cryptography==2.3.1 deepdiff==3.3.0 -falcon==1.2.0 +falcon==1.4.1 jsonpath-ng==1.4.3 jsonschema==2.6.0 keystoneauth1==3.4.0