diff --git a/deckhand/control/common.py b/deckhand/control/common.py index 372a0e47..743d8b62 100644 --- a/deckhand/control/common.py +++ b/deckhand/control/common.py @@ -14,6 +14,8 @@ import functools +import falcon + class ViewBuilder(object): """Model API responses as dictionaries.""" @@ -51,6 +53,24 @@ def sanitize_params(allowed_params): def wrapper(self, req, *func_args, **func_kwargs): req_params = req.params or {} sanitized_params = {} + # This maps which type should be enforced per query parameter. + # Everything not included in type dict below is assumed to be a + # string or a list of strings. + type_dict = {'limit': int} + + def _enforce_query_filter_type(key, val): + if key in type_dict: + cast_type = type_dict[key] + try: + cast_val = cast_type(val) + except Exception: + raise falcon.HTTPInvalidParam( + 'Query parameter %s must be of type %s.' % ( + key, cast_type), + key) + return cast_val + else: + return val def _convert_to_dict(sanitized_params, filter_key, filter_val): # Key-value pairs like metadata.label=foo=bar need to be @@ -68,21 +88,25 @@ def sanitize_params(allowed_params): pass for key, val in req_params.items(): + param_val = _enforce_query_filter_type(key, val) + if not isinstance(val, list): val = [val] - is_key_val_pair = '=' in val[0] + + is_key_val_pair = isinstance(val, list) and '=' in val[0] + if key in allowed_params: if key in _mapping: if is_key_val_pair: _convert_to_dict( sanitized_params, _mapping[key], val) else: - sanitized_params[_mapping[key]] = req_params[key] + sanitized_params[_mapping[key]] = param_val else: if is_key_val_pair: _convert_to_dict(sanitized_params, key, val) else: - sanitized_params[key] = req_params[key] + sanitized_params[key] = param_val func_args = func_args + (sanitized_params,) return func(self, req, *func_args, **func_kwargs) diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index df4107e0..ced619ae 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -41,7 +41,7 @@ class RevisionDocumentsResource(api_base.BaseResource): @common.sanitize_params([ 'schema', 'metadata.name', 'metadata.layeringDefinition.abstract', 'metadata.layeringDefinition.layer', 'metadata.label', - 'status.bucket', 'order', 'sort']) + 'status.bucket', 'order', 'sort', 'limit']) def on_get(self, req, resp, sanitized_params, revision_id): """Returns all documents for a `revision_id`. @@ -55,6 +55,7 @@ class RevisionDocumentsResource(api_base.BaseResource): order_by = sanitized_params.pop('order', None) sort_by = sanitized_params.pop('sort', None) + limit = sanitized_params.pop('limit', None) filters = sanitized_params.copy() filters['metadata.storagePolicy'] = ['cleartext'] @@ -71,6 +72,8 @@ class RevisionDocumentsResource(api_base.BaseResource): # Sorts by creation date by default. documents = utils.multisort(documents, sort_by, order_by) + if limit is not None: + documents = documents[:limit] resp.status = falcon.HTTP_200 resp.body = self.view_builder.list(documents) @@ -95,7 +98,7 @@ class RenderedDocumentsResource(api_base.BaseResource): @policy.authorize('deckhand:list_cleartext_documents') @common.sanitize_params([ 'schema', 'metadata.name', 'metadata.layeringDefinition.layer', - 'metadata.label', 'status.bucket', 'order', 'sort']) + 'metadata.label', 'status.bucket', 'order', 'sort', 'limit']) def on_get(self, req, resp, sanitized_params, revision_id): include_encrypted = policy.conditional_authorize( 'deckhand:list_encrypted_documents', req.context, do_raise=False) @@ -135,6 +138,7 @@ class RenderedDocumentsResource(api_base.BaseResource): # returns concrete documents, so no filtering for that is needed here. order_by = sanitized_params.pop('order', None) sort_by = sanitized_params.pop('sort', None) + limit = sanitized_params.pop('limit', None) user_filters = sanitized_params.copy() rendered_documents = [ @@ -145,6 +149,9 @@ class RenderedDocumentsResource(api_base.BaseResource): rendered_documents = utils.multisort( rendered_documents, sort_by, order_by) + if limit is not None: + rendered_documents = rendered_documents[:limit] + resp.status = falcon.HTTP_200 resp.body = self.view_builder.list(rendered_documents) self._post_validate(rendered_documents) diff --git a/deckhand/tests/functional/gabbits/revision-documents/revision-documents-filters-negative.yaml b/deckhand/tests/functional/gabbits/revision-documents/revision-documents-filters-negative.yaml index 4bc703bf..0d87bc31 100644 --- a/deckhand/tests/functional/gabbits/revision-documents/revision-documents-filters-negative.yaml +++ b/deckhand/tests/functional/gabbits/revision-documents/revision-documents-filters-negative.yaml @@ -46,3 +46,10 @@ tests: schema: example/Kind/v2 status: 200 response_multidoc_jsonpaths: null + + - name: filter_by_limit_illegal_value + desc: Verify that illegal limit value returns 400 + GET: /api/v1.0/revisions/$HISTORY['initialize'].$RESPONSE['$.[0].status.revision']/documents + query_parameters: + limit: 'illegal' + status: 400 diff --git a/deckhand/tests/functional/gabbits/revision-documents/revision-documents-filters.yaml b/deckhand/tests/functional/gabbits/revision-documents/revision-documents-filters.yaml index 3ad4c051..097dbc66 100644 --- a/deckhand/tests/functional/gabbits/revision-documents/revision-documents-filters.yaml +++ b/deckhand/tests/functional/gabbits/revision-documents/revision-documents-filters.yaml @@ -208,3 +208,21 @@ tests: - deckhand/LayeringPolicy/v1 - example/Kind/v1 - example/Kind/v1 + + - name: limit_by_permitted_int_value + desc: Verify revision documents limited by int value + GET: /api/v1.0/revisions/$RESPONSE['$.[0].status.revision']/documents + query_parameters: + sort: + - metadata.name + - schema + limit: 2 + status: 200 + response_multidoc_jsonpaths: + $.`len`: 2 + $.[*].metadata.name: + - global-1234 + - layering-policy + $.[*].schema: + - example/Kind/v1 + - deckhand/LayeringPolicy/v1 diff --git a/deckhand/tests/unit/control/test_revision_documents_controller.py b/deckhand/tests/unit/control/test_revision_documents_controller.py index b1abe9bf..e525d2d4 100644 --- a/deckhand/tests/unit/control/test_revision_documents_controller.py +++ b/deckhand/tests/unit/control/test_revision_documents_controller.py @@ -276,3 +276,43 @@ class TestRevisionDocumentsControllerSorting(test_base.BaseControllerTest): self.assertEqual(3, len(retrieved_documents)) self.assertEqual(expected_schemas, [d['schema'] for d in retrieved_documents]) + + def test_list_revision_documents_sorting_by_schema_then_limit(self): + rules = {'deckhand:list_cleartext_documents': '@', + 'deckhand:list_encrypted_documents': '@', + 'deckhand:create_cleartext_documents': '@'} + self.policy.set_rules(rules) + + documents_factory = factories.DocumentFactory(2, [1, 1]) + documents = documents_factory.gen_test({ + '_SITE_ACTIONS_1_': { + 'actions': [{'method': 'merge', 'path': '.'}] + } + }) + schemas = ['deckhand/Certificate/v1', + 'deckhand/CertificateKey/v1', + 'deckhand/LayeringPolicy/v1'] + for idx in range(len(documents)): + documents[idx]['schema'] = schemas[idx] + + for limit in (0, 1, 2, 3): + expected_schemas = schemas[:limit] + + resp = self.app.simulate_put( + '/api/v1.0/buckets/mop/documents', + headers={'Content-Type': 'application/x-yaml'}, + body=yaml.safe_dump_all(documents)) + self.assertEqual(200, resp.status_code) + revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][ + 'revision'] + + resp = self.app.simulate_get( + '/api/v1.0/revisions/%s/documents' % revision_id, + params={'sort': 'schema', 'limit': limit}, params_csv=False, + headers={'Content-Type': 'application/x-yaml'}) + self.assertEqual(200, resp.status_code) + retrieved_documents = list(yaml.safe_load_all(resp.text)) + + self.assertEqual(limit, len(retrieved_documents)) + self.assertEqual(expected_schemas, + [d['schema'] for d in retrieved_documents]) diff --git a/docs/source/api_ref.rst b/docs/source/api_ref.rst index d8654281..6b7bcf82 100644 --- a/docs/source/api_ref.rst +++ b/docs/source/api_ref.rst @@ -86,6 +86,7 @@ Supported query string parameters: "asc". Controls the order in which the ``sort`` result is returned: "asc" returns sorted results in ascending order, while "desc" returns results in descending order. +* ``limit`` - int - Controls number of documents returned by this endpoint. GET ``/revisions/{revision_id}/rendered-documents`` ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -96,7 +97,7 @@ consumers will interact with for their configuration. Valid query parameters are the same as for ``/revisions/{revision_id}/documents``, minus the parameters in -``metadata.layeringDetinition``, which are not supported. +``metadata.layeringDefinition``, which are not supported. Raises a ``409 Conflict`` if a ``layeringPolicy`` document could not be found.