openstack
/
murano
Code Issues Proposed changes

Streamline and simplify package filtering api 

Introduce catalogue parameter to allow distinguishing between catalogue
of applications (packages that one can deploy) and managable-apps
(packages that one can edit).
Streamline include_disabled filtering.
Allow filtering out your own packages for admin requests.

Change-Id: Ica031058d963246ff4d76e52cfc8b8b44d207c9a
Partial-Bug: #1448226
Relates-Bug: #1448135
This commit is contained in:
Kirill Zaitsev 2015-04-27 17:32:13 +03:00
parent d91f4b9d30
commit e76c0657e4
5 changed files with 368 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
doc/source/specification/murano-repository.rst
View File

@ -37,12 +37,12 @@ Methods for application package management
- ``class_definition``: list of class names used by a package
- ``is_public``: determines whether the package is shared for other tenants
- ``enabled``: determines whether the package is browsed in the Application Catalog
- ``owner_id``: id of a tenant which user not an owned the package
- ``owner_id``: id of a tenant that owns the package
List packages
-------------
`/v1/catalog/packages?{marker}{limit}{order_by}{type}{category}{fqn}{owned}{class_name} [GET]`
`/v1/catalog/packages?{marker}{limit}{order_by}{type}{category}{fqn}{owned}{catalog}{class_name} [GET]`
This is the compound request to list and search through application catalog.
If there are no search parameters all packages that is_public, enabled and belong to the user's tenant will be listed.
@ -54,6 +54,9 @@ For an admin role all packages are available.
+----------------------+-------------+------------------------------------------------------------------------------------------------------------------------------+
| Attribute | Type | Description |
+======================+=============+==============================================================================================================================+
| ``catalog`` | bool | If false (default) - search packages, that current user can edit (own for non-admin, all for admin) |
| | | If true - search packages, that current user can deploy (i.e. his own + public) |
+----------------------+-------------+------------------------------------------------------------------------------------------------------------------------------+
| ``marker`` | string | A package identifier marker may be specified. When present only packages which occur after the identifier ID will be listed |
+----------------------+-------------+------------------------------------------------------------------------------------------------------------------------------+
| ``limit`` | string | When present the maximum number of results returned will not exceed the specified value. |
@ -68,13 +71,13 @@ For an admin role all packages are available.
+----------------------+-------------+------------------------------------------------------------------------------------------------------------------------------+
| ``fqn`` | string | Allows to point a fully qualified package name for a search |
+----------------------+-------------+------------------------------------------------------------------------------------------------------------------------------+
| ``owned`` | bool | Search only from packages owned by user tenant |
| ``owned`` | bool | Search only from packages owned by current tenant |
+----------------------+-------------+------------------------------------------------------------------------------------------------------------------------------+
| ``include_disabled`` | bool | Include disabled packages in a the result |
+----------------------+-------------+------------------------------------------------------------------------------------------------------------------------------+
| ``search`` | string | Gives opportunity to search specified data by all the package parameters |
+----------------------+-------------+------------------------------------------------------------------------------------------------------------------------------+
| ``class_name`` | string |Search only for packages, that use specified class |
| ``class_name`` | string | Search only for packages, that use specified class |
+----------------------+-------------+------------------------------------------------------------------------------------------------------------------------------+
**Response 200 (application/json)**

6
murano/api/v1/catalog.py
View File

@ -178,13 +178,17 @@ class Controller(object):
policy.check("get_package", req.context)
filters = _get_filters(req.GET.items())
limit = _validate_limit(filters.get('limit'))
if limit is None:
limit = CONF.packages_opts.limit_param_default
limit = min(CONF.packages_opts.api_limit_max, limit)
result = {}
packages = db_api.package_search(filters, req.context, limit)
catalog = req.GET.pop('catalog', '').lower() == 'true'
packages = db_api.package_search(
filters, req.context, limit, catalog=catalog)
if len(packages) == limit:
result['next_marker'] = packages[-1].id
result['packages'] = [package.to_dict() for package in packages]

62
murano/db/catalog/api.py
View File

@ -255,8 +255,11 @@ def package_update(pkg_id_or_name, changes, context):
return pkg
def package_search(filters, context, limit=None):
def package_search(filters, context, limit=None, catalog=False):
"""Search packages with different filters
Catalog param controls the base query creation. Catalog queries
only search packages a user can deploy. Non-catalog queries searches
packages a user can edit.
* Admin is allowed to browse all the packages
* Regular user is allowed to browse all packages belongs to user tenant
and all other packages marked is_public.
@ -266,45 +269,28 @@ def package_search(filters, context, limit=None):
request and then to use the ID of the last package from the response
as the marker parameter in a subsequent limited request.
"""
session = db_session.get_session()
pkg = models.Package
# If the packages search specifies the inclusion of disabled packages,
# we handle this differently for admins vs non-admins:
# For admins: *don't* require pkg.enabled == True (no changes to query)
# For non-admins: add an OR-condition to filter for packages that are owned
# by the tenant in the current context
# Otherwise: in all other cases, we return only enabled packages
if filters.get('include_disabled', '').lower() == 'true':
include_disabled = True
else:
include_disabled = False
query = session.query(pkg)
if context.is_admin:
if not include_disabled:
# NOTE(efedorova): is needed for SA 0.7.9, but could be done
# simpler in SA 0.8. See http://goo.gl/9stlKu for a details
query = session.query(pkg).filter(pkg.__table__.c.enabled)
else:
query = session.query(pkg)
elif filters.get('owned', '').lower() == 'true':
if not include_disabled:
query = session.query(pkg).filter(
(pkg.owner_id == context.tenant) & pkg.enabled
)
else:
query = session.query(pkg).filter(pkg.owner_id == context.tenant)
if catalog:
# Only show packages one can deploy, i.e. own + public
query = query.filter(or_(
pkg.owner_id == context.tenant, pkg.is_public)
)
else:
if not include_disabled:
query = session.query(pkg).filter(
or_((pkg.is_public & pkg.enabled),
((pkg.owner_id == context.tenant) & pkg.enabled))
)
else:
query = session.query(pkg).filter(
or_((pkg.is_public & pkg.enabled),
pkg.owner_id == context.tenant)
)
# Show packages one can edit.
if not context.is_admin:
query = query.filter(pkg.owner_id == context.tenant)
# No else here admin can edit everything.
if not filters.get('include_disabled', '').lower() == 'true':
query = query.filter(pkg.enabled)
if filters.get('owned', '').lower() == 'true':
query = query.filter(pkg.owner_id == context.tenant)
if 'type' in filters.keys():
query = query.filter(pkg.type == filters['type'].title())
@ -347,12 +333,14 @@ def package_search(filters, context, limit=None):
query = query.filter(or_(*conditions))
sort_keys = [SEARCH_MAPPING[sort_key] for sort_key in
filters.get('order_by', []) or ['name']]
filters.get('order_by', ['name'])]
marker = filters.get('marker')
# TODO(btully): sort_dir is always None - not getting passed as a filter?
sort_dir = filters.get('sort_dir')
if marker is not None: # set marker to real object instead of its id
marker = _package_get(marker, session)
query = utils.paginate_query(
query, pkg, limit, sort_keys, marker, sort_dir)

171
murano/tests/unit/api/v1/test_catalog.py
View File

@ -18,6 +18,7 @@ import cStringIO
import imghdr
import json
import os
import uuid
import mock
from oslo.utils import timeutils
@ -35,6 +36,176 @@ class TestCatalogApi(test_base.ControllerTest, test_base.MuranoApiTestCase):
def setUp(self):
super(TestCatalogApi, self).setUp()
self.controller = catalog.Controller()
_, self.test_package = self._test_package()
def _add_pkg(self, tenant_name, public=False, classes=None, **kwargs):
package_to_upload = self.test_package.copy()
package_to_upload['is_public'] = public
package_to_upload['fully_qualified_name'] = str(uuid.uuid4())
if classes:
package_to_upload['class_definitions'] = classes
else:
package_to_upload['class_definitions'] = []
package_to_upload.update(kwargs)
return db_catalog_api.package_upload(
package_to_upload, tenant_name)
def test_packages_filtering_admin(self):
self.is_admin = True
self._set_policy_rules(
{'get_package': ''}
)
for i in range(7):
self.expect_policy_check('get_package')
pkg = self._add_pkg('test_tenant', type='Library')
self._add_pkg('test_tenant')
self._add_pkg('test_tenant2', public=True, type='Library')
self._add_pkg('test_tenant3')
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'False',
'owned': 'False'}))
self.assertEqual(len(result['packages']), 4)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'False',
'owned': 'True'}))
self.assertEqual(len(result['packages']), 2)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'True',
'owned': 'False'}))
self.assertEqual(len(result['packages']), 3)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'True',
'owned': 'True'}))
self.assertEqual(len(result['packages']), 2)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={
'owned': 'True',
'fqn': pkg.fully_qualified_name}))
self.assertEqual(len(result['packages']), 1)
self.assertEqual(result['packages'][0]['fully_qualified_name'],
pkg.fully_qualified_name)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={
'owned': 'True',
'type': 'Library'}))
self.assertEqual(len(result['packages']), 1)
self.assertEqual(result['packages'][0]['fully_qualified_name'],
pkg.fully_qualified_name)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={
'type': 'Library'}))
self.assertEqual(len(result['packages']), 2)
def test_packages_filtering_non_admin(self):
self.is_admin = False
self._set_policy_rules(
{'get_package': ''}
)
for i in range(7):
self.expect_policy_check('get_package')
pkg = self._add_pkg('test_tenant', type='Library')
self._add_pkg('test_tenant')
self._add_pkg('test_tenant2', public=True, type='Library')
self._add_pkg('test_tenant3')
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'False',
'owned': 'False'}))
self.assertEqual(len(result['packages']), 2)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'False',
'owned': 'True'}))
self.assertEqual(len(result['packages']), 2)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'True',
'owned': 'False'}))
self.assertEqual(len(result['packages']), 3)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'True',
'owned': 'True'}))
self.assertEqual(len(result['packages']), 2)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={
'owned': 'True',
'fqn': pkg.fully_qualified_name}))
self.assertEqual(len(result['packages']), 1)
self.assertEqual(result['packages'][0]['fully_qualified_name'],
pkg.fully_qualified_name)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={
'owned': 'True',
'type': 'Library'}))
self.assertEqual(len(result['packages']), 1)
self.assertEqual(result['packages'][0]['fully_qualified_name'],
pkg.fully_qualified_name)
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={
'type': 'Library'}))
self.assertEqual(len(result['packages']), 1)
def test_packages(self):
self._set_policy_rules(
{'get_package': ''}
)
for i in range(9):
self.expect_policy_check('get_package')
result = self.controller.search(self._get('/v1/catalog/packages/'))
self.assertEqual(len(result['packages']), 0)
self._add_pkg('test_tenant')
self._add_pkg('test_tenant')
self._add_pkg('other_tenant')
self._add_pkg('other_tenant')
# non-admin should only see 2 pkgs he can edit.
self.is_admin = False
result = self.controller.search(self._get('/v1/catalog/packages/'))
self.assertEqual(len(result['packages']), 2)
# can only deploy his + public
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'True'}))
self.assertEqual(len(result['packages']), 2)
# admin can edit anything
self.is_admin = True
result = self.controller.search(self._get('/v1/catalog/packages/'))
self.assertEqual(len(result['packages']), 4)
# admin can only deploy his + public
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'True'}))
self.assertEqual(len(result['packages']), 2)
self._add_pkg('test_tenant', public=True)
self._add_pkg('other_tenant', public=True)
# non-admin can't edit other_tenants public pkgs
self.is_admin = False
result = self.controller.search(self._get('/v1/catalog/packages/'))
self.assertEqual(len(result['packages']), 3)
# can deploy mine + other public
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'True'}))
self.assertEqual(len(result['packages']), 4)
# admin can edit anything
self.is_admin = True
result = self.controller.search(self._get('/v1/catalog/packages/'))
self.assertEqual(len(result['packages']), 6)
# can deploy mine + public
result = self.controller.search(self._get(
'/v1/catalog/packages/', params={'catalog': 'True'}))
self.assertEqual(len(result['packages']), 4)
def _test_package(self):
package_dir = os.path.abspath(

160
murano/tests/unit/db/test_catalog.py
View File

@ -30,6 +30,9 @@ class CatalogDBTestCase(base.MuranoWithDBTestCase):
self.context = utils.dummy_context(tenant_id=self.tenant_id)
self.context_2 = utils.dummy_context(tenant_id=self.tenant_id_2)
self.context_admin = utils.dummy_context(tenant_id=self.tenant_id)
self.context_admin.is_admin = True
def _create_categories(self):
api.category_add('cat1')
api.category_add('cat2')
@ -58,6 +61,163 @@ class CatalogDBTestCase(base.MuranoWithDBTestCase):
'value': value
}
def test_package_search_search(self):
pkg1 = api.package_upload(
self._stub_package(
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
pkg2 = api.package_upload(
self._stub_package(
tags=[],
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
res = api.package_search(
{'search': 'tag1'}, self.context)
self.assertEqual(len(res), 1)
res = api.package_search(
{'search': pkg1.fully_qualified_name}, self.context)
self.assertEqual(len(res), 1)
res = api.package_search(
{'search': pkg2.fully_qualified_name}, self.context)
self.assertEqual(len(res), 1)
res = api.package_search(
{'search': 'not_a_valid_uuid'}, self.context)
self.assertEqual(len(res), 0)
res = api.package_search(
{'search': 'some text'}, self.context)
self.assertEqual(len(res), 2)
def test_package_search_tags(self):
api.package_upload(
self._stub_package(
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
api.package_upload(
self._stub_package(
tags=[],
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
res = api.package_search(
{'tag': ['tag1']}, self.context)
self.assertEqual(len(res), 1)
res = api.package_search(
{'tag': ['tag2']}, self.context)
self.assertEqual(len(res), 1)
res = api.package_search(
{'tag': ['tag3']}, self.context)
self.assertEqual(len(res), 0)
def test_package_search_type(self):
api.package_upload(
self._stub_package(
type="Application",
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
api.package_upload(
self._stub_package(
type="Library",
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
res = api.package_search(
{'type': 'Library'}, self.context)
self.assertEqual(len(res), 1)
res = api.package_search(
{'type': 'Application'}, self.context)
self.assertEqual(len(res), 1)
def test_package_search_disabled(self):
api.package_upload(
self._stub_package(
is_public=True,
enabled=True,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
api.package_upload(
self._stub_package(
is_public=True,
enabled=False,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
res = api.package_search(
{'include_disabled': 'false'}, self.context)
self.assertEqual(len(res), 1)
res = api.package_search(
{'include_disabled': 'true'}, self.context)
self.assertEqual(len(res), 2)
def test_package_search_owned(self):
api.package_upload(
self._stub_package(
is_public=True,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
api.package_upload(
self._stub_package(
is_public=True,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id_2)
res = api.package_search({'owned': 'true'}, self.context_admin)
self.assertEqual(len(res), 1)
res = api.package_search({'owned': 'false'}, self.context_admin)
self.assertEqual(len(res), 2)
def test_package_search_no_filters_catalog(self):
res = api.package_search({}, self.context, catalog=True)
self.assertEqual(len(res), 0)
api.package_upload(
self._stub_package(
is_public=True,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
api.package_upload(
self._stub_package(
is_public=False,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
api.package_upload(
self._stub_package(
is_public=True,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id_2)
api.package_upload(
self._stub_package(
is_public=False,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id_2)
# catalog=True should show public + mine
res = api.package_search({}, self.context, catalog=True)
self.assertEqual(len(res), 3)
res = api.package_search({}, self.context_admin, catalog=True)
self.assertEqual(len(res), 3)
def test_package_search_no_filters(self):
res = api.package_search({}, self.context)
self.assertEqual(len(res), 0)
api.package_upload(
self._stub_package(
is_public=True,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
api.package_upload(
self._stub_package(
is_public=False,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id)
api.package_upload(
self._stub_package(
is_public=True,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id_2)
api.package_upload(
self._stub_package(
is_public=False,
fully_qualified_name=str(uuid.uuid4())), self.tenant_id_2)
# I can only edit mine pkgs
res = api.package_search({}, self.context)
self.assertEqual(len(res), 2)
for pkg in res:
self.assertEqual(pkg.owner_id, self.tenant_id)
# Admin can see everything
res = api.package_search({}, self.context_admin)
self.assertEqual(len(res), 4)
def test_list_empty_categories(self):
res = api.category_get_names()
self.assertEqual(0, len(res))