Merge "Fix for cross-tenant package and class isolation"

This commit is contained in:
Jenkins 2015-04-15 12:15:31 +00:00 committed by Gerrit Code Review
commit 7740580e8b
7 changed files with 425 additions and 21 deletions

View File

@ -149,7 +149,6 @@ class Controller(object):
policy.check('publicize_package', req.context)
break
package = db_api.package_update(package_id, body, req.context)
return package.to_dict()
def get(self, req, package_id):

View File

@ -139,7 +139,8 @@ class TaskExecutor(object):
murano_client_factory = lambda: \
self._environment.clients.get_murano_client(self._environment)
with package_loader.ApiPackageLoader(
murano_client_factory) as pkg_loader:
murano_client_factory,
self._environment.tenant_id) as pkg_loader:
return self._execute(pkg_loader)
finally:
if self._model['Objects'] is None:

View File

@ -235,11 +235,22 @@ def package_update(pkg_id_or_name, changes, context):
session = db_session.get_session()
with session.begin():
pkg = _package_get(pkg_id_or_name, session)
was_private = not pkg.is_public
if not context.is_admin:
_authorize_package(pkg, context)
for change in changes:
pkg = operation_methods[change['op']](pkg, change)
became_public = pkg.is_public
class_names = [clazz.name for clazz in pkg.class_definitions]
if was_private and became_public:
with db_session.get_lock("public_packages", session):
_check_for_existing_classes(session, class_names, None,
check_public=True,
ignore_package_with_id=pkg.id)
_check_for_public_packages_with_fqn(session,
pkg.fully_qualified_name,
pkg.id)
session.add(pkg)
return pkg
@ -359,16 +370,37 @@ def package_upload(values, tenant_id):
composite_attr_to_func = {'categories': _get_categories,
'tags': _get_tags,
'class_definitions': _get_class_definitions}
with session.begin():
is_public = values.get('is_public', False)
if is_public:
public_lock = db_session.get_lock("public_packages", session)
else:
public_lock = None
tenant_lock = db_session.get_lock("classes_of_" + tenant_id, session)
try:
_check_for_existing_classes(session, values.get('class_definitions'),
tenant_id, check_public=is_public)
if is_public:
_check_for_public_packages_with_fqn(
session,
values.get('fully_qualified_name'))
for attr, func in composite_attr_to_func.iteritems():
if values.get(attr):
result = func(values[attr], session)
setattr(package, attr, result)
del values[attr]
package.update(values)
package.owner_id = tenant_id
package.save(session)
tenant_lock.commit()
if public_lock is not None:
public_lock.commit()
except Exception:
tenant_lock.rollback()
if public_lock is not None:
public_lock.rollback()
raise
return package
@ -442,3 +474,49 @@ def category_delete(category_id):
LOG.error(msg)
raise exc.HTTPNotFound(msg)
session.delete(category)
def _check_for_existing_classes(session, class_names, tenant_id,
check_public=False,
ignore_package_with_id=None):
if not class_names:
return
q = session.query(models.Class.name).filter(
models.Class.name.in_(class_names))
private_filter = None
public_filter = None
predicate = None
if tenant_id is not None:
private_filter = models.Class.package.has(
models.Package.owner_id == tenant_id)
if check_public:
public_filter = models.Class.package.has(
models.Package.is_public)
if private_filter is not None and public_filter is not None:
predicate = sa.or_(private_filter, public_filter)
elif private_filter is not None:
predicate = private_filter
elif public_filter is not None:
predicate = public_filter
if predicate is not None:
q = q.filter(predicate)
if ignore_package_with_id is not None:
q = q.filter(models.Class.package_id != ignore_package_with_id)
if q.first() is not None:
msg = _('Class with the same full name is already '
'registered in the visibility scope')
LOG.error(msg)
raise exc.HTTPConflict(msg)
def _check_for_public_packages_with_fqn(session, fqn,
ignore_package_with_id=None):
q = session.query(models.Package.id).\
filter(models.Package.is_public).\
filter(models.Package.fully_qualified_name == fqn)
if ignore_package_with_id is not None:
q = q.filter(models.Package.id != ignore_package_with_id)
if q.first() is not None:
msg = _('Package with the same Name is already made public')
LOG.error(msg)
raise exc.HTTPConflict(msg)

View File

@ -0,0 +1,199 @@
# Copyright 2015 OpenStack Foundation.
#
# 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 alembic import op
import sqlalchemy as sa
import murano.db.migration.helpers as helpers
from murano.db.sqla import types as st
"""fix_unique_constraints
Revision ID: 008
Revises: 007
Create Date: 2015-04-08 14:01:06.458512
"""
# revision identifiers, used by Alembic.
revision = '008'
down_revision = '007'
MYSQL_ENGINE = 'InnoDB'
MYSQL_CHARSET = 'utf8'
def upgrade():
engine = op.get_bind()
if engine.dialect.dialect_description == 'mysql+mysqldb':
engine.execute('SET FOREIGN_KEY_CHECKS=0')
if engine.dialect.dialect_description == 'postgresql+psycopg2':
op.drop_constraint('package_to_tag_package_id_fkey',
'package_to_tag', 'foreignkey')
op.drop_constraint('package_to_tag_tag_id_fkey',
'package_to_tag', 'foreignkey')
op.drop_constraint('package_to_category_package_id_fkey',
'package_to_category', 'foreignkey')
op.drop_constraint('class_definition_package_id_fkey',
'class_definition', 'foreignkey')
helpers.transform_table(
'package', {}, {},
sa.Column('created', sa.DateTime(), nullable=False),
sa.Column('updated', sa.DateTime(), nullable=False),
sa.Column('id', sa.String(length=36), nullable=False),
sa.Column('archive', st.LargeBinary(), nullable=True),
sa.Column('fully_qualified_name',
sa.String(length=128),
nullable=False),
sa.Column('type', sa.String(length=20), nullable=False),
sa.Column('author', sa.String(length=80), nullable=True),
sa.Column('name', sa.String(length=80), nullable=False),
sa.Column('enabled', sa.Boolean(), nullable=True),
sa.Column('description', sa.Text(), nullable=False),
sa.Column('is_public', sa.Boolean(), nullable=True),
sa.Column('logo', st.LargeBinary(), nullable=True),
sa.Column('owner_id', sa.String(length=36), nullable=False),
sa.Column('ui_definition', sa.Text(), nullable=True),
sa.Column('supplier_logo', sa.types.LargeBinary),
sa.Column('supplier', sa.types.Text()),
sa.PrimaryKeyConstraint('id'),
mysql_engine=MYSQL_ENGINE,
mysql_charset=MYSQL_CHARSET
)
helpers.transform_table(
'class_definition', {}, {},
sa.Column('created', sa.DateTime(), nullable=False),
sa.Column('updated', sa.DateTime(), nullable=False),
sa.Column('id', sa.String(length=36), nullable=False),
sa.Column('name',
sa.String(length=128),
nullable=False),
sa.Column('package_id', sa.String(length=36), nullable=True),
sa.ForeignKeyConstraint(['package_id'], ['package.id'], ),
sa.PrimaryKeyConstraint('id'),
mysql_engine=MYSQL_ENGINE,
mysql_charset=MYSQL_CHARSET
)
if engine.dialect.dialect_description == 'mysql+mysqldb':
engine.execute('SET FOREIGN_KEY_CHECKS=1')
if engine.dialect.dialect_description == 'postgresql+psycopg2':
op.create_foreign_key('package_to_tag_package_id_fkey',
'package_to_tag', 'package',
['package_id'], ['id'])
op.create_foreign_key('package_to_tag_tag_id_fkey',
'package_to_tag', 'tag',
['tag_id'], ['id'])
op.create_foreign_key('package_to_category_package_id_fkey',
'package_to_category', 'package',
['package_id'], ['id'])
op.create_foreign_key('class_definition_package_id_fkey',
'class_definition', 'package',
['package_id'], ['id'])
op.create_index('ix_package_fqn_and_owner', table_name='package',
columns=['fully_qualified_name', 'owner_id'], unique=True)
op.create_index('ix_class_definition_name',
'class_definition',
['name'])
def downgrade():
op.drop_index('ix_package_fqn_and_owner', table_name='package')
op.drop_index('ix_class_definition_name', table_name='class_definition')
engine = op.get_bind()
if engine.dialect.dialect_description == 'mysql+mysqldb':
engine.execute('SET FOREIGN_KEY_CHECKS=0')
if engine.dialect.dialect_description == 'postgresql+psycopg2':
op.drop_constraint('package_to_tag_package_id_fkey',
'package_to_tag', 'foreignkey')
op.drop_constraint('package_to_tag_tag_id_fkey',
'package_to_tag', 'foreignkey')
op.drop_constraint('package_to_category_package_id_fkey',
'package_to_category', 'foreignkey')
op.drop_constraint('class_definition_package_id_fkey',
'class_definition', 'foreignkey')
helpers.transform_table(
'class_definition', {}, {},
sa.Column('created', sa.DateTime(), nullable=False),
sa.Column('updated', sa.DateTime(), nullable=False),
sa.Column('id', sa.String(length=36), nullable=False),
sa.Column('name',
sa.String(length=128),
nullable=False,
unique=True),
sa.Column('package_id', sa.String(length=36), nullable=True),
sa.PrimaryKeyConstraint('id'),
mysql_engine=MYSQL_ENGINE,
mysql_charset=MYSQL_CHARSET
)
helpers.transform_table(
'package', {}, {},
sa.Column('created', sa.DateTime(), nullable=False),
sa.Column('updated', sa.DateTime(), nullable=False),
sa.Column('id', sa.String(length=36), nullable=False),
sa.Column('archive', st.LargeBinary(), nullable=True),
sa.Column('fully_qualified_name',
sa.String(length=128),
nullable=False,
unique=True),
sa.Column('type', sa.String(length=20), nullable=False),
sa.Column('author', sa.String(length=80), nullable=True),
sa.Column('name', sa.String(length=80), nullable=False),
sa.Column('enabled', sa.Boolean(), nullable=True),
sa.Column('description', sa.Text(), nullable=False),
sa.Column('is_public', sa.Boolean(), nullable=True),
sa.Column('logo', st.LargeBinary(), nullable=True),
sa.Column('owner_id', sa.String(length=36), nullable=False),
sa.Column('ui_definition', sa.Text(), nullable=True),
sa.Column('supplier_logo', sa.types.LargeBinary),
sa.Column('supplier', sa.types.Text()),
sa.PrimaryKeyConstraint('id'),
mysql_engine=MYSQL_ENGINE,
mysql_charset=MYSQL_CHARSET
)
if engine.dialect.dialect_description == 'mysql+mysqldb':
engine.execute('SET FOREIGN_KEY_CHECKS=1')
if engine.dialect.dialect_description == 'postgresql+psycopg2':
op.create_foreign_key('package_to_tag_package_id_fkey',
'package_to_tag', 'package',
['package_id'], ['id'])
op.create_foreign_key('package_to_tag_tag_id_fkey',
'package_to_tag', 'tag',
['tag_id'], ['id'])
op.create_foreign_key('package_to_category_package_id_fkey',
'package_to_category', 'package',
['package_id'], ['id'])
op.create_foreign_key('class_definition_package_id_fkey',
'class_definition', 'package',
['package_id'], ['id'])
op.create_index('ix_class_definition_name',
'class_definition',
['name'])

View File

@ -220,15 +220,15 @@ class Instance(Base):
class Package(Base, TimestampMixin):
"""Represents a meta information about application package."""
__tablename__ = 'package'
__table_args__ = (sa.Index('ix_package_fqn_and_owner',
'fully_qualified_name',
'owner_id', unique=True),)
id = sa.Column(sa.String(36),
primary_key=True,
default=uuidutils.generate_uuid)
archive = sa.Column(st.LargeBinary())
fully_qualified_name = sa.Column(sa.String(128),
nullable=False,
index=True,
unique=True)
nullable=False)
type = sa.Column(sa.String(20), nullable=False, default='class')
author = sa.Column(sa.String(80), default='Openstack')
supplier = sa.Column(st.JsonBlob(), nullable=True, default={})
@ -251,7 +251,8 @@ class Package(Base, TimestampMixin):
cascade='save-update, merge',
lazy='joined')
class_definitions = sa_orm.relationship(
"Class", cascade='save-update, merge, delete', lazy='joined')
"Class", cascade='save-update, merge, delete', lazy='joined',
backref='package')
def to_dict(self):
d = self.__dict__.copy()

View File

@ -45,12 +45,13 @@ class PackageLoader(six.with_metaclass(abc.ABCMeta)):
class ApiPackageLoader(PackageLoader):
def __init__(self, murano_client_factory):
def __init__(self, murano_client_factory, tenant_id):
self._cache_directory = self._get_cache_directory()
self._murano_client_factory = murano_client_factory
self.tenant_id = tenant_id
def get_package_by_class(self, name):
filter_opts = {'class_name': name, 'limit': 1}
filter_opts = {'class_name': name}
try:
package_definition = self._get_definition(filter_opts)
except LookupError:
@ -59,7 +60,7 @@ class ApiPackageLoader(PackageLoader):
return self._get_package_by_definition(package_definition)
def get_package(self, name):
filter_opts = {'fqn': name, 'limit': 1}
filter_opts = {'fqn': name}
try:
package_definition = self._get_definition(filter_opts)
except LookupError:
@ -82,15 +83,19 @@ class ApiPackageLoader(PackageLoader):
def _get_definition(self, filter_opts):
try:
packages = self._murano_client_factory().packages.filter(
**filter_opts)
try:
return packages.next()
except StopIteration:
packages = list(self._murano_client_factory().packages.filter(
**filter_opts))
if len(packages) > 1:
LOG.debug('Ambiguous package resolution: '
'more then 1 package found for query "{0}", '
'will resolve based on the ownership'.
format(filter_opts))
return get_best_package_match(packages, self.tenant_id)
elif len(packages) == 1:
return packages[0]
else:
LOG.debug('There are no packages matching filter '
'{0}'.format(filter_opts))
# TODO(smelikyan): This exception should be replaced with one
# defined in python-muranoclient
raise LookupError()
except muranoclient_exc.HTTPException:
LOG.debug('Failed to get package definition from repository')
@ -187,3 +192,19 @@ class DirectoryPackageLoader(PackageLoader):
self._packages_by_name[package.full_name] = package
self._processed_entries.add(entry)
def get_best_package_match(packages, tenant_id):
public = None
other = []
for package in packages:
if package.owner_id == tenant_id:
return package
elif package.is_public:
public = package
else:
other.append(package)
if public is not None:
return public
elif other:
return other[0]

View File

@ -26,14 +26,16 @@ class CatalogDBTestCase(base.MuranoWithDBTestCase):
def setUp(self):
super(CatalogDBTestCase, self).setUp()
self.tenant_id = str(uuid.uuid4())
self.tenant_id_2 = str(uuid.uuid4())
self.context = utils.dummy_context(tenant_id=self.tenant_id)
self.context_2 = utils.dummy_context(tenant_id=self.tenant_id_2)
def _create_categories(self):
api.category_add('cat1')
api.category_add('cat2')
def _stub_package(self):
return {
def _stub_package(self, **kwargs):
base = {
'archive': "archive blob here",
'fully_qualified_name': 'com.example.package',
'type': 'class',
@ -46,6 +48,15 @@ class CatalogDBTestCase(base.MuranoWithDBTestCase):
'logo': "logo blob here",
'ui_definition': '{}',
}
base.update(**kwargs)
return base
def get_change(self, op, path, value):
return {
'op': op,
'path': path,
'value': value
}
def test_list_empty_categories(self):
res = api.category_get_names()
@ -87,3 +98,97 @@ class CatalogDBTestCase(base.MuranoWithDBTestCase):
self.assertRaises(exc.HTTPNotFound,
api.package_get, package.id, self.context)
def test_package_upload_to_different_tenants_with_same_fqn(self):
values = self._stub_package()
api.package_upload(values, self.tenant_id)
api.package_upload(values, self.tenant_id_2)
def test_package_upload_public_public_fqn_violation(self):
values = self._stub_package(is_public=True)
api.package_upload(values, self.tenant_id)
values = self._stub_package(is_public=True)
self.assertRaises(exc.HTTPConflict, api.package_upload,
values, self.tenant_id_2)
def test_package_upload_public_private_no_fqn_violation(self):
values = self._stub_package(is_public=True)
api.package_upload(values, self.tenant_id)
values = self._stub_package(is_public=False)
api.package_upload(values, self.tenant_id_2)
def test_package_upload_private_public_no_fqn_violation(self):
values = self._stub_package()
api.package_upload(values, self.tenant_id)
values = self._stub_package(is_public=True)
api.package_upload(values, self.tenant_id_2)
def test_class_name_is_unique(self):
value = self._stub_package(class_definitions=('foo', 'bar'))
api.package_upload(value, self.tenant_id)
value = self._stub_package(class_definitions=('bar', 'baz'),
fully_qualified_name='com.example.package2')
self.assertRaises(exc.HTTPConflict, api.package_upload, value,
self.tenant_id)
def test_class_name_uniqueness_not_enforced_in_different_tenants(self):
value = self._stub_package(class_definitions=('foo', 'bar'))
api.package_upload(value, self.tenant_id)
value = self._stub_package(class_definitions=('foo', 'bar'),
fully_qualified_name='com.example.package2')
api.package_upload(value, self.tenant_id_2)
def test_class_name_public_public_violation(self):
value = self._stub_package(class_definitions=('foo', 'bar'),
is_public=True)
api.package_upload(value, self.tenant_id)
value = self._stub_package(class_definitions=('foo', 'bar'),
is_public=True,
fully_qualified_name='com.example.package2')
self.assertRaises(exc.HTTPConflict, api.package_upload,
value, self.tenant_id_2)
def test_class_name_public_private_no_violation(self):
value = self._stub_package(class_definitions=('foo', 'bar'),
is_public=True)
api.package_upload(value, self.tenant_id)
value = self._stub_package(class_definitions=('foo', 'bar'),
is_public=False,
fully_qualified_name='com.example.package2')
api.package_upload(value, self.tenant_id_2)
def test_class_name_private_public_no_violation(self):
value = self._stub_package(class_definitions=('foo', 'bar'),
is_public=False)
api.package_upload(value, self.tenant_id)
value = self._stub_package(class_definitions=('foo', 'bar'),
is_public=True,
fully_qualified_name='com.example.package2')
api.package_upload(value, self.tenant_id_2)
def test_package_make_public(self):
id = api.package_upload(self._stub_package(), self.tenant_id).id
patch = self.get_change('replace', ['is_public'], True)
api.package_update(id, [patch], self.context)
package = api.package_get(id, self.context)
self.assertEqual(True, package.is_public)
def test_package_update_public_public_fqn_violation(self):
id1 = api.package_upload(self._stub_package(), self.tenant_id).id
id2 = api.package_upload(self._stub_package(), self.tenant_id_2).id
patch = self.get_change('replace', ['is_public'], True)
api.package_update(id1, [patch], self.context)
self.assertRaises(exc.HTTPConflict, api.package_update,
id2, [patch], self.context_2)
def test_package_update_public_public_class_name_violation(self):
id1 = api.package_upload(self._stub_package(
class_definitions=('foo', 'bar')), self.tenant_id).id
id2 = api.package_upload(self._stub_package(
class_definitions=('foo', 'bar'),
fully_qualified_name='com.example.package2'), self.tenant_id_2).id
patch = self.get_change('replace', ['is_public'], True)
api.package_update(id1, [patch], self.context)
self.assertRaises(exc.HTTPConflict, api.package_update,
id2, [patch], self.context_2)