airship
/
deckhand
Code Issues Proposed changes

Fix corner case for document re-creation in different bucket 

A document with a distinct metadata.name/schema can be re-created
in a different bucket after it has been deleted in its original
bucket. This is also true for layering policies.

This PS fixes the above problem. It also updates validation
documentation to be clearer.

Change-Id: If232f6ca613611995674f9d0149d5f4e5d155040
This commit is contained in:
Felipe Monteiro 2017-11-04 01:47:07 +00:00
parent 249475f153
commit ce799bd758
5 changed files with 145 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
deckhand/db/sqlalchemy/api.py
View File

@ -128,9 +128,6 @@ def require_unique_document_schema(schema=None):
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
@ -250,7 +247,7 @@ def _documents_create(bucket_name, values_list, session=None):
try:
existing_document = document_get(
raw_dict=True, deleted=False,
raw_dict=True, deleted=False, revision_id='latest',
**{x: values[x] for x in filters})
except errors.DocumentNotFound:
# Ignore bad data at this point. Allow creation to bubble up the
@ -311,12 +308,14 @@ def _make_hash(data):
json.dumps(data, sort_keys=True).encode('utf-8')).hexdigest()
def document_get(session=None, raw_dict=False, **filters):
"""Retrieve a document from the DB.
def document_get(session=None, raw_dict=False, revision_id=None, **filters):
"""Retrieve the first document for ``revision_id`` that match ``filters``.
:param session: Database session object.
:param raw_dict: Whether to retrieve the exact way the data is stored in
DB if ``True``, else the way users expect the data.
:param revision_id: The ID corresponding to the ``Revision`` object. If the
it is "latest", then retrieve the latest revision, if one exists.
:param filters: Dictionary attributes (including nested) used to filter
out revision documents.
:returns: Dictionary representation of retrieved document.
@ -324,6 +323,15 @@ def document_get(session=None, raw_dict=False, **filters):
"""
session = session or get_session()
if revision_id == 'latest':
revision = session.query(models.Revision)\
.order_by(models.Revision.created_at.desc())\
.first()
if revision:
filters['revision_id'] = revision.id
elif revision_id:
filters['revision_id'] = revision_id
# TODO(fmontei): Currently Deckhand doesn't support filtering by nested
# JSON fields via sqlalchemy. For now, filter the documents using all
# "regular" filters via sqlalchemy and all nested filters via Python.
@ -357,21 +365,20 @@ def document_get_all(session=None, raw_dict=False, revision_id=None,
:param raw_dict: Whether to retrieve the exact way the data is stored in
DB if ``True``, else the way users expect the data.
:param revision_id: The ID corresponding to the ``Revision`` object. If the
ID is ``None``, then retrieve the latest revision, if one exists.
it is "latest", then retrieve the latest revision, if one exists.
:param filters: Dictionary attributes (including nested) used to filter
out revision documents.
:returns: Dictionary representation of each retrieved document.
"""
session = session or get_session()
if revision_id is None:
# If no revision_id is specified, grab the newest one.
if revision_id == 'latest':
revision = session.query(models.Revision)\
.order_by(models.Revision.created_at.desc())\
.first()
if revision:
filters['revision_id'] = revision.id
else:
elif revision_id:
filters['revision_id'] = revision_id
# TODO(fmontei): Currently Deckhand doesn't support filtering by nested
@ -452,7 +459,7 @@ def revision_get(revision_id=None, session=None):
:param revision_id: The ID corresponding to the ``Revision`` object.
:param session: Database session object.
:returns: Dictionary representation of retrieved revision.
:raises: RevisionNotFound if the revision was not found.
:raises RevisionNotFound: if the revision was not found.
"""
session = session or get_session()
@ -474,7 +481,7 @@ def revision_get_latest(session=None):
:param session: Database session object.
:returns: Dictionary representation of latest revision.
:raises: RevisionNotFound if the latest revision was not found.
:raises RevisionNotFound: if the latest revision was not found.
"""
session = session or get_session()
@ -647,7 +654,7 @@ def _filter_revision_documents(documents, unique_only, **filters):
for document in documents:
# NOTE(fmontei): Only want to include non-validation policy documents
# for this endpoint.
if document['schema'] == types.VALIDATION_POLICY_SCHEMA:
if document['schema'].startswith(types.VALIDATION_POLICY_SCHEMA):
continue
if _apply_filters(document, **filters):
@ -682,7 +689,7 @@ def revision_get_documents(revision_id=None, include_history=True,
:param filters: Key-value pairs used for filtering out revision documents.
:returns: All revision documents for ``revision_id`` that match the
``filters``, including document revision history if applicable.
:raises: RevisionNotFound if the revision was not found.
:raises RevisionNotFound: if the revision was not found.
"""
session = session or get_session()
revision_documents = []

2
deckhand/engine/document_validation.py
View File

@ -85,7 +85,7 @@ class DocumentValidation(object):
been registered by external services via ``DataSchema`` documents.
"""
data_schemas = db_api.document_get_all(
schema=types.DATA_SCHEMA_SCHEMA)
schema=types.DATA_SCHEMA_SCHEMA, revision_id='latest')
for data_schema in data_schemas:
if cls.schema_re.match(data_schema['metadata']['name']):

2
deckhand/engine/secrets_manager.py
View File

@ -142,7 +142,7 @@ class SecretsSubstitution(object):
if src_path == '.':
src_path = '.secret'
# TODO(fmontei): Use secrets_manager for this logic. Need to
# TODO(fmontei): Use SecretsManager for this logic. Need to
# check Barbican for the secret if it has been encrypted.
src_doc = db_api.document_get(
schema=src_schema, name=src_name, is_secret=True,

60
deckhand/tests/unit/control/test_buckets_controller.py
View File

@ -110,6 +110,47 @@ class TestBucketsController(test_base.BaseControllerTest):
'secret': payload[-1]['data']}
_do_test([payload[-1]])
def test_create_delete_then_recreate_document_in_different_bucket(self):
"""Ordiniarly creating a document with the same metadata.name/schema
in a separate bucket raises an exception, but if we delete the document
and re-create it in a different bucket this should be a success
scenario.
"""
rules = {'deckhand:create_cleartext_documents': '@'}
self.policy.set_rules(rules)
payload = factories.DocumentFactory(2, [1, 1]).gen_test({})
bucket_name = test_utils.rand_name('bucket')
alt_bucket_name = test_utils.rand_name('bucket')
# Create the documents in the first bucket.
resp = self.app.simulate_put(
'/api/v1.0/buckets/%s/documents' % bucket_name,
headers={'Content-Type': 'application/x-yaml'},
body=yaml.safe_dump_all(payload))
self.assertEqual(200, resp.status_code)
documents = list(yaml.safe_load_all(resp.text))
self.assertEqual(3, len(documents))
self.assertEqual([bucket_name] * 3,
[d['status']['bucket'] for d in documents])
# Delete the documents from the first bucket.
resp = self.app.simulate_put(
'/api/v1.0/buckets/%s/documents' % bucket_name,
headers={'Content-Type': 'application/x-yaml'}, body=None)
self.assertEqual(200, resp.status_code)
# Re-create the documents in the second bucket.
resp = self.app.simulate_put(
'/api/v1.0/buckets/%s/documents' % alt_bucket_name,
headers={'Content-Type': 'application/x-yaml'},
body=yaml.safe_dump_all(payload))
self.assertEqual(200, resp.status_code)
documents = list(yaml.safe_load_all(resp.text))
self.assertEqual(3, len(documents))
self.assertEqual([alt_bucket_name] * 3,
[d['status']['bucket'] for d in documents])
class TestBucketsControllerNegative(test_base.BaseControllerTest):
"""Test suite for validating negative scenarios for bucket controller."""
@ -162,6 +203,25 @@ schema:
resp_error = ' '.join(resp.text.split())
self.assertRegexpMatches(resp_error, error_re)
def test_put_conflicting_document(self):
rules = {'deckhand:create_cleartext_documents': '@'}
self.policy.set_rules(rules)
payload = factories.DocumentFactory(1, [1]).gen_test({})[0]
bucket_name = test_utils.rand_name('bucket')
alt_bucket_name = test_utils.rand_name('bucket')
# Create document in `bucket_name`.
resp = self.app.simulate_put(
'/api/v1.0/buckets/%s/documents' % bucket_name,
headers={'Content-Type': 'application/x-yaml'},
body=yaml.safe_dump_all([payload]))
# Create same document in `alt_bucket_name` and validate conflict.
resp = self.app.simulate_put(
'/api/v1.0/buckets/%s/documents' % alt_bucket_name,
headers={'Content-Type': 'application/x-yaml'},
body=yaml.safe_dump_all([payload]))
self.assertEqual(409, resp.status_code)
class TestBucketsControllerNegativeRBAC(test_base.BaseControllerTest):
"""Test suite for validating negative RBAC scenarios for bucket

71
doc/source/validation.rst
View File

@ -26,19 +26,14 @@ The validation system provides a unified approach to complex validations that
require coordination of multiple documents and business logic that resides in
consumer services.
Services can report success or failure of named validations for a given
revision. Those validations can then be referenced by many ``ValidationPolicy``
control documents. The intended purpose use is to allow a simple mapping that
enables consuming services to be able to quickly check whether the
configuration in Deckhand is in a valid state for performing a specific
action.
Deckhand focuses on two types of validations: schema validations and policy
validations.
Deckhand-Provided Validations
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
In addition to allowing 3rd party services to report configurable validation
statuses, Deckhand provides a few internal validations which are made
available immediately upon document ingestion.
Deckhand provides a few internal validations which are made available
immediately upon document ingestion.
Here is a list of internal validations:
@ -59,3 +54,61 @@ documents are used to register a new validation mapping that other services
can reference to verify whether a Deckhand bucket is in a valid configuration.
For more information, refer to the ``DataSchema`` section in
:ref:`document-types`.
Schema Validations
^^^^^^^^^^^^^^^^^^
Schema validations are controlled by two mechanisms:
1) Deckhand's internal schema validation for sanity-checking the formatting
of the default documents that it understands. For example, Deckhand
will check that a ``LayeringPolicy``, ``ValidationPolicy`` or ``DataSchema``
adhere to the "skeleton" or schemas registered under
``deckhand.engine.schema``.
.. note::
Each document is always subjected to 2 stages of document validation:
the first stage checks whether the document adheres to the fundamental
building blocks: Does it have a ``schema``, ``metadata``, and ``data``
section? The second stage then checks whether the document's ``schema``
passes a more nuanced schema validation specific to that ``schema``.
2) Externally provided validations via ``DataSchema`` documents. These
documents can be registered by external services and subject the target
document's data section to *additional* validations, validations specified
by the ``data`` section of the ``DataSchema`` document.
For more information about ``DataSchema`` documents, please refer to
:ref:`document-types`.
Policy Validations
^^^^^^^^^^^^^^^^^^
*Not yet implemented*.
Validation Policies
^^^^^^^^^^^^^^^^^^^
Validation policies allow services to report success or failure of named
validations for a given revision. Those validations can then be referenced by
many ``ValidationPolicy`` control documents. The intended purpose use is to
allow a simple mapping that enables consuming services to be able to quickly
check whether the configuration in Deckhand is in a valid state for performing
a specific action.
.. note::
``ValidationPolicy`` documents are not the same as ``DataSchema`` documents.
A ``ValidationPolicy`` document can reference a list of internal Deckhand
validations in addition to externally registered ``DataSchema`` documents.
Once all the validations specified in the ``ValidationPolicy`` are executed
and succeed, then services can check whether the documents in a bucket are
stable, in accordance with the ``ValidationPolicy``.
Validation Module
-----------------
.. autoclass:: deckhand.engine.document_validation.DocumentValidation
:members:
:private-members: