diff --git a/README.rst b/README.rst index b1ceca3e..4d79e9a7 100644 --- a/README.rst +++ b/README.rst @@ -108,6 +108,10 @@ Deckhand has the following integration points: * `PostgreSQL `_ is used to persist information to correlate workflows with users and history of workflow commands. + .. note:: + + Currently, other database backends are not supported. + Though, being a low-level service, has many other UCP services that integrate with it, including: diff --git a/deckhand/control/buckets.py b/deckhand/control/buckets.py index 25e04a43..c8a72af9 100644 --- a/deckhand/control/buckets.py +++ b/deckhand/control/buckets.py @@ -25,7 +25,6 @@ from deckhand.engine import document_validation from deckhand.engine import secrets_manager from deckhand import errors as deckhand_errors from deckhand import policy -from deckhand import types LOG = logging.getLogger(__name__) @@ -80,9 +79,6 @@ class BucketsResource(api_base.BaseResource): if document['metadata'].get('storagePolicy') == 'encrypted': secret_data = self.secrets_mgr.create(document) document['data'] = secret_data - elif any([document['schema'].startswith(t) - for t in types.DOCUMENT_SECRET_TYPES]): - document['data'] = {'secret': document['data']} def _create_revision_documents(self, bucket_name, documents, validations): diff --git a/deckhand/control/views/document.py b/deckhand/control/views/document.py index cc27ca90..ed9d47d7 100644 --- a/deckhand/control/views/document.py +++ b/deckhand/control/views/document.py @@ -39,8 +39,6 @@ class ViewBuilder(common.ViewBuilder): continue if document['schema'].startswith(types.VALIDATION_POLICY_SCHEMA): continue - if document['is_secret']: - document['data'] = document['data']['secret'] resp_obj = {x: document[x] for x in attrs} resp_obj.setdefault('status', {}) resp_obj['status']['bucket'] = document['bucket_name'] diff --git a/deckhand/db/sqlalchemy/api.py b/deckhand/db/sqlalchemy/api.py index 83099418..f8cd31c6 100644 --- a/deckhand/db/sqlalchemy/api.py +++ b/deckhand/db/sqlalchemy/api.py @@ -246,7 +246,8 @@ def _documents_create(bucket_name, values_list, session=None): for values in values_list: values.setdefault('data', {}) values = _fill_in_metadata_defaults(values) - values['is_secret'] = 'secret' in values['data'] + values['is_secret'] = values['schema'].startswith( + types.DOCUMENT_SECRET_TYPES) # Hash the document's metadata and data to later efficiently check # whether those data have changed. diff --git a/deckhand/db/sqlalchemy/models.py b/deckhand/db/sqlalchemy/models.py index ece6c53f..16f05752 100644 --- a/deckhand/db/sqlalchemy/models.py +++ b/deckhand/db/sqlalchemy/models.py @@ -18,6 +18,7 @@ from oslo_utils import timeutils from sqlalchemy import Boolean from sqlalchemy import Column from sqlalchemy import DateTime +from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.ext import declarative from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy import ForeignKey @@ -131,7 +132,7 @@ class Document(BASE, DeckhandBase): # NOTE(fmontei): ``metadata`` is reserved by the DB, so ``_metadata`` # must be used to store document metadata information in the DB. _metadata = Column(oslo_types.JsonEncodedDict(), nullable=False) - data = Column(oslo_types.JsonEncodedDict(), nullable=True, default={}) + data = Column(JSONB, nullable=True) data_hash = Column(String, nullable=False) metadata_hash = Column(String, nullable=False) is_secret = Column(Boolean, nullable=False, default=False) diff --git a/deckhand/engine/schema/base_schema.py b/deckhand/engine/schema/base_schema.py index 787e724b..f9ed563c 100644 --- a/deckhand/engine/schema/base_schema.py +++ b/deckhand/engine/schema/base_schema.py @@ -29,7 +29,7 @@ schema = { 'additionalProperties': True, 'required': ['schema', 'name'] }, - 'data': {'type': ['string', 'object']} + 'data': {'type': ['string', 'integer', 'array', 'object']} }, 'additionalProperties': False, 'required': ['schema', 'metadata'] diff --git a/deckhand/engine/schema/v1_0/data_schema_schema.py b/deckhand/engine/schema/v1_0/data_schema_schema.py index 66a6dfa1..d4dbaf11 100644 --- a/deckhand/engine/schema/v1_0/data_schema_schema.py +++ b/deckhand/engine/schema/v1_0/data_schema_schema.py @@ -50,7 +50,7 @@ schema = { 'type': 'object', 'properties': { '$schema': { - 'type': 'string' + 'type': ['string', 'integer', 'array', 'object'] } }, 'additionalProperties': True, diff --git a/deckhand/engine/schema/v1_0/document_schema.py b/deckhand/engine/schema/v1_0/document_schema.py index a287ffe6..5f5a2651 100644 --- a/deckhand/engine/schema/v1_0/document_schema.py +++ b/deckhand/engine/schema/v1_0/document_schema.py @@ -94,7 +94,7 @@ schema = { 'required': ['schema', 'name', 'layeringDefinition'] }, 'data': { - 'type': 'object' + 'type': ['string', 'integer', 'array', 'object'] } }, 'additionalProperties': False, diff --git a/deckhand/engine/secrets_manager.py b/deckhand/engine/secrets_manager.py index f4b0e8a4..e412c8c9 100644 --- a/deckhand/engine/secrets_manager.py +++ b/deckhand/engine/secrets_manager.py @@ -71,9 +71,9 @@ class SecretsManager(object): resp = self.barbican_driver.create_secret(**kwargs) secret_ref = resp['secret_href'] - created_secret = {'secret': secret_ref} + created_secret = secret_ref elif encryption_type == CLEARTEXT: - created_secret = {'secret': secret_doc['data']} + created_secret = secret_doc['data'] return created_secret @@ -137,15 +137,21 @@ class SecretsSubstitution(object): src_schema = sub['src']['schema'] src_name = sub['src']['name'] src_path = sub['src']['path'] - if src_path == '.': - src_path = '.secret' # 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, **{'metadata.layeringDefinition.abstract': False}) - src_secret = utils.jsonpath_parse(src_doc['data'], src_path) + + # If the data is a dictionary, retrieve the nested secret + # via jsonpath_parse, else the secret is the primitive/string + # stored in the data section itself. + if isinstance(src_doc.get('data'), dict): + src_secret = utils.jsonpath_parse(src_doc.get('data', {}), + src_path) + else: + src_secret = src_doc['data'] dest_path = sub['dest']['path'] dest_pattern = sub['dest'].get('pattern', None) diff --git a/deckhand/tests/functional/gabbits/document-crud-success-unusual-documents.yaml b/deckhand/tests/functional/gabbits/document-crud-success-unusual-documents.yaml new file mode 100644 index 00000000..6a37ef8a --- /dev/null +++ b/deckhand/tests/functional/gabbits/document-crud-success-unusual-documents.yaml @@ -0,0 +1,90 @@ +# Test compatability with some unusual documents. +# +# 1. Purges existing data to ensure test isolation. +# 2. Creates some unusual documents that push on corner cases. +# 3. Verifies each individual document's content is returned as expected. +# 4. Verifies that all documents pass schema validation. + +defaults: + request_headers: + content-type: application/x-yaml + response_headers: + content-type: application/x-yaml + +tests: + - name: purge + desc: Begin testing from known state. + DELETE: /api/v1.0/revisions + status: 204 + response_headers: null + + - name: initialize + desc: Create initial documents + PUT: /api/v1.0/buckets/mop/documents + status: 200 + data: <@resources/unusual-documents.yaml + + - name: verity_list_content + desc: Verify list content + GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/documents?schema=unusual/List/v1 + status: 200 + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].data: + - a + - b + - c + + - name: verity_dict_with_secret_content + desc: Verify DictWithSecret content + GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/documents?schema=unusual/DictWithSecret/v1 + status: 200 + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].data: + secret: a + public: b + + - name: verity_string_content + desc: Verify String content + GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/documents?schema=unusual/String/v1 + status: 200 + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].data: strings are useful + + - name: verity_integer_content + desc: Verify Integer content + GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/documents?schema=unusual/Integer/v1 + status: 200 + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].data: 9000 + + - name: verify_document_validation_success_in_list_view + desc: Check document validation success shows in list view + GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/validations + status: 200 + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].count: 1 + $.[0].results[0].status: success + + - name: verify_document_validation_success_in_details_view + desc: Check document validation success shows in detailed view + GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/validations/deckhand-schema-validation + status: 200 + response_multidoc_jsonpaths: + $.`len`: 1 + $.[0].count: 9 + $.[0].results[*].status: + # 9 documents are created in total, including DataSchema documents. + - success + - success + - success + - success + - success + - success + - success + - success + - success diff --git a/deckhand/tests/functional/gabbits/resources/unusual-documents.yaml b/deckhand/tests/functional/gabbits/resources/unusual-documents.yaml new file mode 100644 index 00000000..5cdf6f9d --- /dev/null +++ b/deckhand/tests/functional/gabbits/resources/unusual-documents.yaml @@ -0,0 +1,98 @@ +--- +schema: deckhand/DataSchema/v1 +metadata: + schema: metadata/Control/v1 + name: unusual/List/v1 +data: + $schema: http://json-schema.org/schema# + type: array + items: + type: string +--- +schema: unusual/List/v1 +metadata: + schema: metadata/Document/v1 + name: unusual-list + storagePolicy: cleartext + layeringDefinition: + abstract: false + layer: site +data: + - a + - b + - c +--- +schema: deckhand/DataSchema/v1 +metadata: + schema: metadata/Control/v1 + name: unusual/DictWithSecret/v1 +data: + $schema: http://json-schema.org/schema# + type: object + properties: + secret: + type: string + public: + type: string + additionalProperties: false + required: + - secret + - public +--- +schema: unusual/DictWithSecret/v1 +metadata: + schema: metadata/Document/v1 + name: dict-with-secret + storagePolicy: cleartext + layeringDefinition: + abstract: false + layer: site +data: + secret: a + public: b +--- +schema: deckhand/DataSchema/v1 +metadata: + schema: metadata/Control/v1 + name: unusual/String/v1 +data: + $schema: http://json-schema.org/schema# + type: string +--- +schema: unusual/String/v1 +metadata: + schema: metadata/Document/v1 + name: some-label + storagePolicy: cleartext + layeringDefinition: + abstract: false + layer: site +data: strings are useful +--- +schema: deckhand/DataSchema/v1 +metadata: + schema: metadata/Control/v1 + name: unusual/Integer/v1 +data: + $schema: http://json-schema.org/schema# + type: integer +--- +schema: unusual/Integer/v1 +metadata: + schema: metadata/Document/v1 + name: MTU + storagePolicy: cleartext + layeringDefinition: + abstract: false + layer: site +data: 9000 +--- +schema: deckhand/LayeringPolicy/v1 +metadata: + schema: metadata/Control/v1 + name: layering-policy +data: + layerOrder: + - global + - region + - site diff --git a/deckhand/tests/unit/control/test_buckets_controller.py b/deckhand/tests/unit/control/test_buckets_controller.py index e46cd342..5a0935b8 100644 --- a/deckhand/tests/unit/control/test_buckets_controller.py +++ b/deckhand/tests/unit/control/test_buckets_controller.py @@ -81,8 +81,7 @@ class TestBucketsController(test_base.BaseControllerTest): actual = sorted([(d['schema'], d['metadata']['name']) for d in created_documents]) self.assertEqual(expected, actual) - self.assertEqual(payload[0]['data'], - created_documents[0]['data']) + self.assertEqual(payload[0]['data'], created_documents[0]['data']) # Verify whether creating a cleartext secret works. rules = {'deckhand:create_cleartext_documents': '@'} @@ -102,8 +101,7 @@ class TestBucketsController(test_base.BaseControllerTest): with mock.patch.object(buckets.BucketsResource, 'secrets_mgr', autospec=True) as mock_secrets_mgr: - mock_secrets_mgr.create.return_value = { - 'secret': payload[0]['data']} + mock_secrets_mgr.create.return_value = payload[0]['data'] _do_test(payload) # Verify whether any document can be encrypted if its @@ -118,8 +116,7 @@ class TestBucketsController(test_base.BaseControllerTest): payload[-1]['metadata']['storagePolicy'] = 'encrypted' with mock.patch.object(buckets.BucketsResource, 'secrets_mgr', autospec=True) as mock_secrets_mgr: - mock_secrets_mgr.create.return_value = { - 'secret': payload[-1]['data']} + mock_secrets_mgr.create.return_value = payload[-1]['data'] _do_test([payload[-1]]) def test_create_delete_then_recreate_document_in_different_bucket(self): diff --git a/deckhand/tests/unit/control/test_rendered_documents_controller.py b/deckhand/tests/unit/control/test_rendered_documents_controller.py index c092feba..ac1b086f 100644 --- a/deckhand/tests/unit/control/test_rendered_documents_controller.py +++ b/deckhand/tests/unit/control/test_rendered_documents_controller.py @@ -219,8 +219,7 @@ class TestRenderedDocumentsControllerNegativeRBAC( with mock.patch.object(buckets.BucketsResource, 'secrets_mgr', autospec=True) as mock_secrets_mgr: - mock_secrets_mgr.create.return_value = { - 'secret': payload[0]['data']} + mock_secrets_mgr.create.return_value = payload[0]['data'] resp = self.app.simulate_put( '/api/v1.0/buckets/mop/documents', headers={'Content-Type': 'application/x-yaml'}, diff --git a/deckhand/tests/unit/control/test_revision_documents_controller.py b/deckhand/tests/unit/control/test_revision_documents_controller.py index d17e2dbb..c89c780c 100644 --- a/deckhand/tests/unit/control/test_revision_documents_controller.py +++ b/deckhand/tests/unit/control/test_revision_documents_controller.py @@ -64,8 +64,7 @@ class TestRevisionDocumentsControllerNegativeRBAC( payload = [secrets_factory.gen_test('Certificate', 'encrypted')] with mock.patch.object(buckets.BucketsResource, 'secrets_mgr', autospec=True) as mock_secrets_mgr: - mock_secrets_mgr.create.return_value = { - 'secret': payload[0]['data']} + mock_secrets_mgr.create.return_value = payload[0]['data'] resp = self.app.simulate_put( '/api/v1.0/buckets/mop/documents', headers={'Content-Type': 'application/x-yaml'}, diff --git a/deckhand/tests/unit/control/test_revisions_rollback_controller.py b/deckhand/tests/unit/control/test_revisions_rollback_controller.py index ff9bef93..257f700e 100644 --- a/deckhand/tests/unit/control/test_revisions_rollback_controller.py +++ b/deckhand/tests/unit/control/test_revisions_rollback_controller.py @@ -61,8 +61,7 @@ class TestRevisionsRollbackControllerNegativeRBAC( with mock.patch.object(buckets.BucketsResource, 'secrets_mgr', autospec=True) as mock_secrets_mgr: - mock_secrets_mgr.create.return_value = { - 'secret': payload[0]['data']} + mock_secrets_mgr.create.return_value = payload[0]['data'] resp = self.app.simulate_put( '/api/v1.0/buckets/mop/documents', headers={'Content-Type': 'application/x-yaml'}, diff --git a/deckhand/tests/unit/db/test_documents.py b/deckhand/tests/unit/db/test_documents.py index 740e6ac0..bc582b41 100644 --- a/deckhand/tests/unit/db/test_documents.py +++ b/deckhand/tests/unit/db/test_documents.py @@ -144,7 +144,7 @@ class TestDocuments(base.TestDbBase): self.assertEqual(document, filtered_documents[0]) def test_create_certificate(self): - rand_secret = {'secret': test_utils.rand_password()} + rand_secret = test_utils.rand_password() bucket_name = test_utils.rand_name('bucket') for expected_len, storage_policy in enumerate( diff --git a/deckhand/tests/unit/engine/test_document_layering_and_substitution.py b/deckhand/tests/unit/engine/test_document_layering_and_substitution.py index 02a28b2f..799e3817 100644 --- a/deckhand/tests/unit/engine/test_document_layering_and_substitution.py +++ b/deckhand/tests/unit/engine/test_document_layering_and_substitution.py @@ -45,7 +45,7 @@ class TestDocumentLayeringWithSubstitution( secrets_factory = factories.DocumentSecretFactory() certificate = secrets_factory.gen_test( - 'Certificate', 'cleartext', data={'secret': 'global-secret'}, + 'Certificate', 'cleartext', data='global-secret', name='global-cert') global_expected = {'a': {'x': 1, 'y': 2}, 'c': 'global-secret'} @@ -84,7 +84,7 @@ class TestDocumentLayeringWithSubstitution( documents[1]['metadata']['labels'] = {} secrets_factory = factories.DocumentSecretFactory() certificate = secrets_factory.gen_test( - 'Certificate', 'cleartext', data={'secret': 'global-secret'}, + 'Certificate', 'cleartext', data='global-secret', name='global-cert') global_expected = {'a': {'x': 1, 'y': 2}, 'c': 'global-secret'} @@ -140,8 +140,7 @@ class TestDocumentLayeringWithSubstitution( name = kwargs['name'] prefix = name.split('-')[0] return secrets_factory.gen_test( - 'Certificate', 'cleartext', - data={'secret': '%s-secret' % prefix}, + 'Certificate', 'cleartext', data='%s-secret' % prefix, name='%s' % name) with mock.patch.object( diff --git a/deckhand/tests/unit/engine/test_secrets_manager.py b/deckhand/tests/unit/engine/test_secrets_manager.py index e7c4eb0a..a493df16 100644 --- a/deckhand/tests/unit/engine/test_secrets_manager.py +++ b/deckhand/tests/unit/engine/test_secrets_manager.py @@ -41,8 +41,7 @@ class TestSecretsManager(test_base.TestDbBase): created_secret = self.secrets_manager.create(secret_doc) if encryption_type == 'cleartext': - self.assertIn('secret', created_secret) - self.assertEqual(secret_data, created_secret['secret']) + self.assertEqual(secret_data, created_secret) elif encryption_type == 'encrypted': expected_kwargs = { 'name': secret_doc['metadata']['name'], @@ -52,9 +51,7 @@ class TestSecretsManager(test_base.TestDbBase): } self.mock_barbican_driver.create_secret.assert_called_once_with( **expected_kwargs) - - self.assertIn('secret', created_secret) - self.assertEqual(self.secret_ref, created_secret['secret']) + self.assertEqual(self.secret_ref, created_secret) def test_create_cleartext_certificate(self): self._test_create_secret('cleartext', 'Certificate') @@ -100,7 +97,7 @@ class TestSecretsSubstitution(test_base.TestDbBase): def test_secret_substitution_single_cleartext(self): certificate = self.secrets_factory.gen_test( - 'Certificate', 'cleartext', data={'secret': 'CERTIFICATE DATA'}) + 'Certificate', 'cleartext', data='CERTIFICATE DATA') certificate['metadata']['name'] = 'example-cert' document_mapping = { @@ -130,7 +127,7 @@ class TestSecretsSubstitution(test_base.TestDbBase): def test_secret_substitution_single_cleartext_with_pattern(self): passphrase = self.secrets_factory.gen_test( - 'Passphrase', 'cleartext', data={'secret': 'my-secret-password'}) + 'Passphrase', 'cleartext', data='my-secret-password') passphrase['metadata']['name'] = 'example-password' document_mapping = { @@ -170,11 +167,11 @@ class TestSecretsSubstitution(test_base.TestDbBase): def test_secret_substitution_double_cleartext(self): certificate = self.secrets_factory.gen_test( - 'Certificate', 'cleartext', data={'secret': 'CERTIFICATE DATA'}) + 'Certificate', 'cleartext', data='CERTIFICATE DATA') certificate['metadata']['name'] = 'example-cert' certificate_key = self.secrets_factory.gen_test( - 'CertificateKey', 'cleartext', data={'secret': 'KEY DATA'}) + 'CertificateKey', 'cleartext', data='KEY DATA') certificate_key['metadata']['name'] = 'example-key' document_mapping = { @@ -215,15 +212,15 @@ class TestSecretsSubstitution(test_base.TestDbBase): def test_secret_substitution_multiple_cleartext(self): certificate = self.secrets_factory.gen_test( - 'Certificate', 'cleartext', data={'secret': 'CERTIFICATE DATA'}) + 'Certificate', 'cleartext', data='CERTIFICATE DATA') certificate['metadata']['name'] = 'example-cert' certificate_key = self.secrets_factory.gen_test( - 'CertificateKey', 'cleartext', data={'secret': 'KEY DATA'}) + 'CertificateKey', 'cleartext', data='KEY DATA') certificate_key['metadata']['name'] = 'example-key' passphrase = self.secrets_factory.gen_test( - 'Passphrase', 'cleartext', data={'secret': 'my-secret-password'}) + 'Passphrase', 'cleartext', data='my-secret-password') passphrase['metadata']['name'] = 'example-password' document_mapping = { diff --git a/doc/source/testing.rst b/doc/source/testing.rst index 51ba40b6..d2740038 100644 --- a/doc/source/testing.rst +++ b/doc/source/testing.rst @@ -20,6 +20,16 @@ Testing Unit testing ============ +Prerequisites +------------- + +`pifpaf `_ is used to spin up a temporary +postgresql database for unit tests. The DB URL is set up as an environment +variable via ``PIFPAF_URL`` which is referenced by Deckhand's unit test suite. + +Guide +----- + Unit testing currently uses an in-memory sqlite database. Since Deckhand's primary function is to serve as the back-end storage for UCP, the majority of unit tests perform actual database operations. Mocking is used sparingly @@ -28,7 +38,7 @@ of a very deep stack; Deckhand only communicates with Keystone and Barbican. As such, validating database operations is paramount to correctly testing Deckhand. -To run unit tests using sqlite, execute:: +To run unit tests using PostgreSQL, execute:: $ tox -epy27 $ tox -epy35 @@ -40,24 +50,11 @@ unit tests, run:: for example. -To run unit tests using postgresql, postgresql must be installed in your -environment. Then execute:: - - $ tox -epy27-postgresql - $ tox -epy35-postgresql - -against a py27- or py35-backed environment, respectively. Individual unit tests -can be executed the same way as above. - -`pifpaf `_ is used to spin up a temporary -postgresql database. The URL is set up as an environment variable via -``PIFPAF_URL``. - .. warning:: It is **not** recommended to run postgresql-backed unit tests concurrently. Only run them serially. This is because, to guarantee true test isolation, - the DB tables are re-created each test run. Only one instance of postgresql + the DB tables are re-created each test run. Only one instance of PostgreSQL is created across all threads, thus causing major conflicts if concurrency > 1. diff --git a/tox.ini b/tox.ini index ae1c4371..702e9a2d 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{35,27},pep8,py{27,35}-postgresql,bandit,docs +envlist = py{35,27},pep8,bandit,docs [testenv] usedevelop = True @@ -19,21 +19,11 @@ commands = rm -Rf .testrepository/times.dbm [testenv:py27] -commands = - {[testenv]commands} - ostestr '{posargs}' - -[testenv:py27-postgresql] commands = {[testenv]commands} {toxinidir}/tools/unit-tests.sh '{posargs}' [testenv:py35] -commands = - {[testenv]commands} - ostestr '{posargs}' - -[testenv:py35-postgresql] commands = {[testenv]commands} {toxinidir}/tools/unit-tests.sh '{posargs}'