From 55b13dc4eb1834206696472262e97ef49fe9647d Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Thu, 19 Oct 2017 21:26:07 +0100 Subject: [PATCH] Only allow one LayeringPolicy to exist in the system. This PS enforces the design requirement that says that only 1 layering policy can exist in the system at once. Attempting to create another layering policy with a different name is a 409 error. The existing layering policy can be updated by passing in a document with the same `metadata.name` and `schema` as the existing one. Closes-Bug: https://github.com/att-comdev/deckhand/issues/12 Change-Id: I7cad2d600c931c8701c3faaf2967be782984528b --- deckhand/control/buckets.py | 3 +- deckhand/control/validations.py | 24 ++++---- deckhand/db/sqlalchemy/api.py | 55 ++++++++++++++++- deckhand/errors.py | 8 +++ deckhand/factories.py | 9 ++- .../unit/control/test_buckets_controller.py | 27 ++++++++ .../tests/unit/db/test_layering_policies.py | 61 +++++++++++++++++++ deckhand/types.py | 1 - ...ique-layering-policy-bac2940c3deeba51.yaml | 7 +++ 9 files changed, 174 insertions(+), 21 deletions(-) create mode 100644 deckhand/tests/unit/db/test_layering_policies.py create mode 100644 releasenotes/notes/fix-unique-layering-policy-bac2940c3deeba51.yaml diff --git a/deckhand/control/buckets.py b/deckhand/control/buckets.py index 8653b79d..a2c33b05 100644 --- a/deckhand/control/buckets.py +++ b/deckhand/control/buckets.py @@ -90,7 +90,8 @@ class BucketsResource(api_base.BaseResource): try: created_documents = db_api.documents_create( bucket_name, documents, validations=validations) - except deckhand_errors.DocumentExists as e: + except (deckhand_errors.DocumentExists, + deckhand_errors.SingletonDocumentConflict) as e: raise falcon.HTTPConflict(description=e.format_message()) except Exception as e: raise falcon.HTTPInternalServerError(description=six.text_type(e)) diff --git a/deckhand/control/validations.py b/deckhand/control/validations.py index eec10a4f..77000783 100644 --- a/deckhand/control/validations.py +++ b/deckhand/control/validations.py @@ -56,13 +56,17 @@ class ValidationsResource(api_base.BaseResource): def on_get(self, req, resp, revision_id, validation_name=None, entry_id=None): if all([validation_name, entry_id]): - self._show_validation_entry( + resp_body = self._show_validation_entry( req, resp, revision_id, validation_name, entry_id) elif validation_name: - self._list_validation_entries(req, resp, revision_id, + resp_body = self._list_validation_entries(req, resp, revision_id, validation_name) else: - self._list_all_validations(req, resp, revision_id) + resp_body = self._list_all_validations(req, resp, revision_id) + + resp.status = falcon.HTTP_200 + resp.append_header('Content-Type', 'application/x-yaml') + resp.body = resp_body @policy.authorize('deckhand:show_validation') def _show_validation_entry(self, req, resp, revision_id, validation_name, @@ -80,9 +84,7 @@ class ValidationsResource(api_base.BaseResource): raise falcon.HTTPNotFound(description=e.format_message()) resp_body = self.view_builder.show_entry(entry) - resp.status = falcon.HTTP_200 - resp.append_header('Content-Type', 'application/x-yaml') - resp.body = resp_body + return resp_body @policy.authorize('deckhand:list_validations') def _list_validation_entries(self, req, resp, revision_id, @@ -94,9 +96,7 @@ class ValidationsResource(api_base.BaseResource): raise falcon.HTTPNotFound(description=e.format_message()) resp_body = self.view_builder.list_entries(entries) - resp.status = falcon.HTTP_200 - resp.append_header('Content-Type', 'application/x-yaml') - resp.body = resp_body + return resp_body @policy.authorize('deckhand:list_validations') def _list_all_validations(self, req, resp, revision_id): @@ -104,8 +104,6 @@ class ValidationsResource(api_base.BaseResource): validations = db_api.validation_get_all(revision_id) except errors.RevisionNotFound as e: raise falcon.HTTPNotFound(description=e.format_message()) - resp_body = self.view_builder.list(validations) - resp.status = falcon.HTTP_200 - resp.append_header('Content-Type', 'application/x-yaml') - resp.body = resp_body + resp_body = self.view_builder.list(validations) + return resp_body diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 92050894..adf7a1a7 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -98,6 +98,53 @@ def raw_query(query, **kwargs): return get_engine().execute(stmt) +def require_unique_document_schema(schema=None): + """Decorator to enforce only one singleton document exists in the system. + + An example of a singleton document is a ``LayeringPolicy`` document. + + Only one singleton document can exist within the system at any time. It is + an error to attempt to insert a new document with the same ``schema`` if it + has a different ``metadata.name`` than the existing document. + + A singleton document that already exists can be updated, if the document + that is passed in has the same name/schema as the existing one. + + The existing singleton document can be replaced by first deleting it + and only then creating a new one. + + :raises SingletonDocumentConflict: if a singleton document in the system + already exists and any of the documents to be created has the same + ``schema`` but has a ``metadata.name`` that differs from the one + already registered. + """ + def decorator(f): + if schema not in types.DOCUMENT_SCHEMA_TYPES: + raise errors.DeckhandException( + 'Unrecognized document schema %s.' % schema) + + @functools.wraps(f) + def wrapper(bucket_name, documents, *args, **kwargs): + existing_documents = revision_get_documents( + schema=schema, deleted=False, include_history=False) + existing_document_names = [x['name'] for x in existing_documents] + # `conflict_names` is calculated by checking whether any documents + # in `documents` is a layering policy with a name not found in + # `existing_documents`. + conflicting_names = [ + x['metadata']['name'] for x in documents + if x['metadata']['name'] not in existing_document_names and + x['schema'].startswith(schema)] + if existing_document_names and conflicting_names: + raise errors.SingletonDocumentConflict( + document=existing_document_names[0], + conflict=conflicting_names) + return f(bucket_name, documents, *args, **kwargs) + return wrapper + return decorator + + +@require_unique_document_schema(types.LAYERING_POLICY_SCHEMA) def documents_create(bucket_name, documents, validations=None, session=None): """Create a set of documents and associated bucket. @@ -203,7 +250,8 @@ def _documents_create(bucket_name, values_list, session=None): try: existing_document = document_get( - raw_dict=True, **{x: values[x] for x in filters}) + raw_dict=True, deleted=False, + **{x: values[x] for x in filters}) except errors.DocumentNotFound: # Ignore bad data at this point. Allow creation to bubble up the # error related to bad data. @@ -214,7 +262,8 @@ def _documents_create(bucket_name, values_list, session=None): # Ignore redundant validation policies as they are allowed to exist # in multiple buckets. if (existing_document['bucket_name'] != bucket_name and - existing_document['schema'] != types.VALIDATION_POLICY_SCHEMA): + not existing_document['schema'].startswith( + types.VALIDATION_POLICY_SCHEMA)): raise errors.DocumentExists( schema=existing_document['schema'], name=existing_document['name'], @@ -644,7 +693,7 @@ def revision_get_documents(revision_id=None, include_history=True, .filter_by(id=revision_id)\ .one() else: - # If no revision_id is specified, grab the newest one. + # If no revision_id is specified, grab the latest one. revision = session.query(models.Revision)\ .order_by(models.Revision.created_at.desc())\ .first() diff --git a/deckhand/errors.py b/deckhand/errors.py index 5de31341..3d49fc09 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -188,6 +188,14 @@ class DocumentExists(DeckhandException): code = 409 +class SingletonDocumentConflict(DeckhandException): + msg_fmt = ("A singleton document by the name %(document)s already " + "exists in the system. The new document %(conflict)s cannot be " + "created. To create a document with a new name, delete the " + "current one first.") + code = 409 + + class LayeringPolicyNotFound(DeckhandException): msg_fmt = ("LayeringPolicy with schema %(schema)s not found in the " "system.") diff --git a/deckhand/factories.py b/deckhand/factories.py index 368addb5..a9bb30c6 100644 --- a/deckhand/factories.py +++ b/deckhand/factories.py @@ -93,7 +93,7 @@ class DocumentFactory(DeckhandFactory): "layerOrder": [] }, "metadata": { - "name": "layering-policy", + "name": "placeholder", "schema": "metadata/Control/v%s" % DeckhandFactory.API_VERSION }, "schema": "deckhand/LayeringPolicy/v%s" % DeckhandFactory.API_VERSION @@ -157,7 +157,10 @@ class DocumentFactory(DeckhandFactory): layer_order = ["global", "region", "site"] else: raise ValueError("'num_layers' must be a value between 1 - 3.") - self.LAYERING_DEFINITION['data']['layerOrder'] = layer_order + self.layering_definition = copy.deepcopy(self.LAYERING_DEFINITION) + self.layering_definition['metadata']['name'] = test_utils.rand_name( + 'layering-policy') + self.layering_definition['data']['layerOrder'] = layer_order if not isinstance(docs_per_layer, (list, tuple)): raise TypeError("'docs_per_layer' must be a list or tuple " @@ -223,7 +226,7 @@ class DocumentFactory(DeckhandFactory): :type site_parent_selectors: list :returns: Rendered template of the form specified above. """ - rendered_template = [self.LAYERING_DEFINITION] + rendered_template = [self.layering_definition] layer_order = rendered_template[0]['data']['layerOrder'] for layer_idx in range(self.num_layers): diff --git a/deckhand/tests/unit/control/test_buckets_controller.py b/deckhand/tests/unit/control/test_buckets_controller.py index dd6f297d..3309e809 100644 --- a/deckhand/tests/unit/control/test_buckets_controller.py +++ b/deckhand/tests/unit/control/test_buckets_controller.py @@ -19,6 +19,7 @@ from oslo_config import cfg from deckhand.control import buckets from deckhand import factories +from deckhand.tests import test_utils from deckhand.tests.unit.control import base as test_base CONF = cfg.CONF @@ -134,6 +135,32 @@ schema: self.assertEqual(400, resp.status_code) self.assertRegexpMatches(resp.text, error_re[idx]) + def test_put_conflicting_layering_policy(self): + rules = {'deckhand:create_cleartext_documents': '@'} + self.policy.set_rules(rules) + + payload = factories.DocumentFactory(1, [1]).gen_test({})[0] + + # Create the first layering policy. + resp = self.app.simulate_put( + '/api/v1.0/bucket/mop/documents', + headers={'Content-Type': 'application/x-yaml'}, + body=yaml.safe_dump_all([payload])) + self.assertEqual(200, resp.status_code) + + # Validate that a layering policy with a different, conflicting name + # raises the expected exception. + error_re = ('.*A singleton document by the name %s already exists in ' + 'the system.' % payload['metadata']['name']) + payload['metadata']['name'] = test_utils.rand_name('layering-policy') + resp = self.app.simulate_put( + '/api/v1.0/bucket/mop/documents', + headers={'Content-Type': 'application/x-yaml'}, + body=yaml.safe_dump_all([payload])) + self.assertEqual(409, resp.status_code) + resp_error = ' '.join(resp.text.split()) + self.assertRegexpMatches(resp_error, error_re) + class TestBucketsControllerNegativeRBAC(test_base.BaseControllerTest): """Test suite for validating negative RBAC scenarios for bucket diff --git a/deckhand/tests/unit/db/test_layering_policies.py b/deckhand/tests/unit/db/test_layering_policies.py new file mode 100644 index 00000000..fba08542 --- /dev/null +++ b/deckhand/tests/unit/db/test_layering_policies.py @@ -0,0 +1,61 @@ +# Copyright 2017 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from deckhand import errors +from deckhand import factories +from deckhand.tests import test_utils +from deckhand.tests.unit.db import base + + +class LayeringPoliciesBaseTest(base.TestDbBase): + + def setUp(self): + super(LayeringPoliciesBaseTest, self).setUp() + # Will create 3 documents: layering policy, plus a global and site + # document. + self.documents_factory = factories.DocumentFactory(2, [1, 1]) + self.document_mapping = { + "_GLOBAL_DATA_1_": {"data": {"a": {"x": 1, "y": 2}}}, + "_SITE_DATA_1_": {"data": {"a": {"x": 7, "z": 3}, "b": 4}}, + "_SITE_ACTIONS_1_": { + "actions": [{"method": "merge", "path": "."}]} + } + self.bucket_name = test_utils.rand_name('bucket') + + def _create_layering_policy(self): + payload = self.documents_factory.gen_test(self.document_mapping) + self.create_documents(self.bucket_name, payload) + return payload[0] + + +class TestLayeringPolicies(LayeringPoliciesBaseTest): + + def test_update_layering_policy(self): + layering_policy = self._create_layering_policy() + layering_policy['data'] = {'data': {'layerOrder': ['region', 'site']}} + self.create_documents(self.bucket_name, [layering_policy]) + + def test_create_new_layering_policy_after_first_deleted(self): + self._create_layering_policy() + self.create_documents(self.bucket_name, []) + self._create_layering_policy() + + +class TestLayeringPoliciesNegative(LayeringPoliciesBaseTest): + + def test_create_conflicting_layering_policy_fails(self): + layering_policy = self._create_layering_policy() + layering_policy['metadata']['name'] = 'another-layering-policy' + self.assertRaises(errors.SingletonDocumentConflict, + self._create_layering_policy) diff --git a/deckhand/types.py b/deckhand/types.py index 3a4d5e4f..23f04ad2 100644 --- a/deckhand/types.py +++ b/deckhand/types.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -# TODO(fmontei): Make all these version-less. DOCUMENT_SCHEMA_TYPES = ( CERTIFICATE_SCHEMA, CERTIFICATE_KEY_SCHEMA, diff --git a/releasenotes/notes/fix-unique-layering-policy-bac2940c3deeba51.yaml b/releasenotes/notes/fix-unique-layering-policy-bac2940c3deeba51.yaml new file mode 100644 index 00000000..fa25ce24 --- /dev/null +++ b/releasenotes/notes/fix-unique-layering-policy-bac2940c3deeba51.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Deckhand will allow only one document with the schema ``LayeringPolicy`` + to exist in the system at a time. To update the existing layering policy, + the layerign policy with the same name as the existing one should be + passed in. To create a new layering policy, delete the existing one first.