diff --git a/nova/db/api/migrations/versions/f177f53f978b_add_flavor_permission_rules.py b/nova/db/api/migrations/versions/f177f53f978b_add_flavor_permission_rules.py new file mode 100644 index 00000000000..3fa2c60d860 --- /dev/null +++ b/nova/db/api/migrations/versions/f177f53f978b_add_flavor_permission_rules.py @@ -0,0 +1,61 @@ +# 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. + +"""add_flavor_permission_rules + +Revision ID: f177f53f978b +Revises: cdeec0c85668 +Create Date: 2025-10-24 15:43:26.554412 +""" + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'f177f53f978b' +down_revision = 'cdeec0c85668' +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_table( + 'flavor_permission_rules', + sa.Column('created_at', sa.DateTime), + sa.Column('updated_at', sa.DateTime), + sa.Column('id', sa.Integer, primary_key=True), + sa.Column('uuid', sa.String(36), nullable=False), + sa.Column('project_id', sa.String(255), nullable=False), + sa.Column('flavor_id', sa.Integer, sa.ForeignKey('flavors.id'), + nullable=True), + sa.Column( + 'type', + sa.Enum('allow', 'deny', name='flavor_permission_rules0type'), + nullable=False + ), + sa.Column( + 'scope', + sa.Enum('domain', 'project', name='flavor_permission_rules0scope'), + nullable=False + ), + sa.UniqueConstraint('uuid', name='uniq_flavor_permission_rules0uuid'), + sa.UniqueConstraint( + 'flavor_id', 'project_id', + name='uniq_flavor_permission_rules0flavor_id0project_id' + ), + sa.Index('flavor_permission_rules_uuid_idx', 'uuid'), + sa.Index('flavor_permission_rules_project_id_flavor_id_idx', + 'project_id', 'flavor_id'), + mysql_engine='InnoDB', + mysql_charset='utf8', + ) diff --git a/nova/db/api/models.py b/nova/db/api/models.py index f3175a0b688..0efc06e8f24 100644 --- a/nova/db/api/models.py +++ b/nova/db/api/models.py @@ -273,6 +273,37 @@ class FlavorProjects(BASE): primaryjoin='FlavorProjects.flavor_id == Flavors.id') +class FlavorPermissionRule(BASE): + """Represents a flavor permission rule for a project""" + + __tablename__ = 'flavor_permission_rules' + __table_args__ = ( + schema.UniqueConstraint( + 'uuid', name='uniq_flavor_permission_rules0uuid'), + schema.UniqueConstraint( + 'flavor_id', + 'project_id', + name='uniq_flavor_permission_rules0flavor_id0project_id' + ), + sa.Index('flavor_permission_rules_uuid_idx', 'uuid'), + sa.Index('flavor_permission_rules_project_id_flavor_id_idx', + 'project_id', 'flavor_id'), + ) + + id = sa.Column(sa.Integer, primary_key=True) + uuid = sa.Column(sa.String(36), nullable=False) + project_id = sa.Column(sa.String(255), nullable=False) + # Use NULL to represent all flavors + flavor_id = sa.Column( + sa.Integer, sa.ForeignKey('flavors.id'), nullable=True) + type = sa.Column( + sa.Enum('allow', 'deny', name='flavor_permission_rules0type'), + nullable=False) + scope = sa.Column( + sa.Enum('domain', 'project', name='flavor_permission_rules0scope'), + nullable=False) + + class BuildRequest(BASE): """Represents the information passed to the scheduler.""" diff --git a/nova/exception.py b/nova/exception.py index 58cdb68d7b7..a91d5c5bc40 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -1164,6 +1164,21 @@ class FlavorAccessExists(NovaException): "and project %(project_id)s combination.") +class FlavorPermissionRuleExists(NovaException): + msg_fmt = _("Flavor permission rule already exists for uuid %(uuid)s or " + "combination of project %(project_id)s and flavor " + "%(flavor_id)s.") + + +class FlavorPermissionRuleNotFound(NotFound): + msg_fmt = _("Flavor permission rule not found for id %(id)s") + + +class FlavorPermissionRuleNotFoundForProjectFlavor(NotFound): + msg_fmt = _("Flavor permission rule not found for project %(project_id)s " + "and flavor %(flavor_id)s combination.") + + class InvalidSharedStorage(NovaException): msg_fmt = _("%(path)s is not on shared storage: %(reason)s") diff --git a/nova/objects/__init__.py b/nova/objects/__init__.py index 5f7e4382518..87e4c1f5807 100644 --- a/nova/objects/__init__.py +++ b/nova/objects/__init__.py @@ -34,6 +34,7 @@ def register_all(): __import__('nova.objects.ec2') __import__('nova.objects.external_event') __import__('nova.objects.flavor') + __import__('nova.objects.flavor_permission_rule') __import__('nova.objects.host_mapping') __import__('nova.objects.hv_spec') __import__('nova.objects.image_meta') diff --git a/nova/objects/fields.py b/nova/objects/fields.py index d268de8e631..98bf329a0c2 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -1052,6 +1052,20 @@ class InstanceTaskState(BaseNovaEnum): SHELVING_OFFLOADING, UNSHELVING, IN_CLUSTER_VMOTION) +class FlavorPermissionRuleType(BaseNovaEnum): + ALLOW = 'allow' + DENY = 'deny' + + ALL = (ALLOW, DENY) + + +class FlavorPermissionRuleScope(BaseNovaEnum): + DOMAIN = 'domain' + PROJECT = 'project' + + ALL = (DOMAIN, PROJECT) + + class InstancePowerState(Enum): _UNUSED = '_unused' NOSTATE = 'pending' @@ -1401,6 +1415,14 @@ class InstanceTaskStateField(BaseEnumField): AUTO_TYPE = InstanceTaskState() +class FlavorPermissionRuleTypeField(BaseEnumField): + AUTO_TYPE = FlavorPermissionRuleType() + + +class FlavorPermissionRuleScopeField(BaseEnumField): + AUTO_TYPE = FlavorPermissionRuleScope() + + class InstancePowerStateField(BaseEnumField): AUTO_TYPE = InstancePowerState() diff --git a/nova/objects/flavor.py b/nova/objects/flavor.py index 2e9e41706ff..0e4d80206e4 100644 --- a/nova/objects/flavor.py +++ b/nova/objects/flavor.py @@ -191,6 +191,8 @@ def _flavor_destroy(context, flavor_id=None, flavorid=None): filter_by(flavor_id=result.id).delete() context.session.query(api_models.FlavorExtraSpecs).\ filter_by(flavor_id=result.id).delete() + context.session.query(api_models.FlavorPermissionRule).\ + filter_by(flavor_id=result.id).delete() context.session.delete(result) return result @@ -308,7 +310,34 @@ def _from_db_object(context, flavor, db_flavor, expected_attrs=None): return flavor @staticmethod - def _flavor_get_query_from_db(context): + def _flavor_not_denied_by_permission_rules(project_id): + """Return a WHERE clause that is true when the flavor is not denied. + + A flavor is denied for project_id if: + - There is a 'deny' rule matching the flavor_id, OR + - There is a 'deny' rule with flavor_id=NULL (default deny) AND there + is no 'allow' rule matching the flavor_id. + """ + FPR = api_models.FlavorPermissionRule + specific_deny = sa.exists().where(sa.and_( + FPR.project_id == project_id, + FPR.type == 'deny', + FPR.flavor_id == api_models.Flavors.id, + )) + default_deny = sa.exists().where(sa.and_( + FPR.project_id == project_id, + FPR.type == 'deny', + FPR.flavor_id.is_(None), + )) + specific_allow = sa.exists().where(sa.and_( + FPR.project_id == project_id, + FPR.type == 'allow', + FPR.flavor_id == api_models.Flavors.id, + )) + return ~sa.or_(specific_deny, sa.and_(default_deny, ~specific_allow)) + + @staticmethod + def _flavor_get_query_from_db(context, include_denied): # We don't use a database context decorator on this method because this # method is not executing a query, it's only building one. query = context.session.query(api_models.Flavors).options( @@ -320,14 +349,24 @@ def _flavor_get_query_from_db(context): api_models.Flavors.projects.any(project_id=context.project_id) ]) query = query.filter(sa.or_(*the_filter)) + if context.project_domain_id: + query = query.filter( + Flavor._flavor_not_denied_by_permission_rules( + context.project_domain_id)) + if not include_denied and context.project_id: + query = query.filter( + Flavor._flavor_not_denied_by_permission_rules( + context.project_id)) + return query @staticmethod @db_utils.require_context @api_db_api.context_manager.reader - def _flavor_get_from_db(context, id): + def _flavor_get_from_db(context, id, include_denied): """Returns a dict describing specific flavor.""" - result = Flavor._flavor_get_query_from_db(context).\ + result = Flavor._flavor_get_query_from_db( + context, include_denied=include_denied).\ filter_by(id=id).\ first() if not result: @@ -337,9 +376,10 @@ def _flavor_get_from_db(context, id): @staticmethod @db_utils.require_context @api_db_api.context_manager.reader - def _flavor_get_by_name_from_db(context, name): + def _flavor_get_by_name_from_db(context, name, include_denied): """Returns a dict describing specific flavor.""" - result = Flavor._flavor_get_query_from_db(context).\ + result = Flavor._flavor_get_query_from_db( + context, include_denied=include_denied).\ filter_by(name=name).\ first() if not result: @@ -349,10 +389,11 @@ def _flavor_get_by_name_from_db(context, name): @staticmethod @db_utils.require_context @api_db_api.context_manager.reader - def _flavor_get_by_flavor_id_from_db(context, flavor_id): + def _flavor_get_by_flavor_id_from_db(context, flavor_id, include_denied): """Returns a dict describing specific flavor_id.""" flavor_id = _unaliased_flavor_id(flavor_id) - result = Flavor._flavor_get_query_from_db(context).\ + result = Flavor._flavor_get_query_from_db( + context, include_denied=include_denied).\ filter_by(flavorid=flavor_id).\ order_by(expression.asc(api_models.Flavors.id)).\ first() @@ -418,21 +459,24 @@ def _obj_from_primitive(cls, context, objver, primitive): return self @base.remotable_classmethod - def get_by_id(cls, context, id): - db_flavor = cls._flavor_get_from_db(context, id) + def get_by_id(cls, context, id, include_denied=False): + db_flavor = cls._flavor_get_from_db(context, id, + include_denied=include_denied) return cls._from_db_object(context, cls(context), db_flavor, expected_attrs=['extra_specs']) @base.remotable_classmethod - def get_by_name(cls, context, name): - db_flavor = cls._flavor_get_by_name_from_db(context, name) + def get_by_name(cls, context, name, include_denied=False): + db_flavor = cls._flavor_get_by_name_from_db( + context, name, include_denied=include_denied) return cls._from_db_object(context, cls(context), db_flavor, expected_attrs=['extra_specs']) @base.remotable_classmethod - def get_by_flavor_id(cls, context, flavor_id, read_deleted=None): - db_flavor = cls._flavor_get_by_flavor_id_from_db(context, - flavor_id) + def get_by_flavor_id(cls, context, flavor_id, read_deleted=None, + include_denied=False): + db_flavor = cls._flavor_get_by_flavor_id_from_db( + context, flavor_id, include_denied=include_denied) return cls._from_db_object(context, cls(context), db_flavor, expected_attrs=['extra_specs']) @@ -630,12 +674,13 @@ def _send_notification(self, action): @api_db_api.context_manager.reader def _flavor_get_all_from_db(context, inactive, filters, sort_key, sort_dir, - limit, marker): + limit, marker, include_denied): """Returns all flavors. """ filters = filters or {} - query = Flavor._flavor_get_query_from_db(context) + query = Flavor._flavor_get_query_from_db(context, + include_denied=include_denied) if 'min_memory_mb' in filters: query = query.filter( @@ -690,21 +735,24 @@ class FlavorList(base.ObjectListBase, base.NovaObject): @base.remotable_classmethod def get_all(cls, context, inactive=False, filters=None, - sort_key='flavorid', sort_dir='asc', limit=None, marker=None): + sort_key='flavorid', sort_dir='asc', limit=None, marker=None, + include_denied=False): api_db_flavors = _flavor_get_all_from_db(context, inactive=inactive, filters=filters, sort_key=sort_key, sort_dir=sort_dir, limit=limit, - marker=marker) + marker=marker, + include_denied=include_denied) return base.obj_make_list(context, cls(context), objects.Flavor, api_db_flavors, expected_attrs=['extra_specs']) @base.remotable_classmethod - def get_by_id(cls, context, ids): - query = Flavor._flavor_get_query_from_db(context).\ + def get_by_id(cls, context, ids, include_denied=False): + query = Flavor._flavor_get_query_from_db( + context, include_denied=include_denied).\ filter_by(disabled=False).\ filter(api_models.Flavors.id.in_(ids)) diff --git a/nova/objects/flavor_permission_rule.py b/nova/objects/flavor_permission_rule.py new file mode 100644 index 00000000000..c27ce3886b8 --- /dev/null +++ b/nova/objects/flavor_permission_rule.py @@ -0,0 +1,246 @@ +# Copyright (c) 2025 SAP SE +# All Rights Reserved. +# +# 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 oslo_db import exception as db_exc +from oslo_db.sqlalchemy.utils import paginate_query +import sqlalchemy as sa + +from nova.db.api import api as api_db_api +from nova.db.api import models as api_models +from nova.db import utils as db_utils +from nova import exception +from nova.objects import base +from nova.objects import fields + + +@base.NovaObjectRegistry.register +class FlavorPermissionRule(base.NovaPersistentObject, base.NovaObject): + """Restricts which flavors are permitted for which domains and projects. + + A flavor permission rule has a 'project_id', a scope ('domain' or + 'project'), an optional 'flavor_id' and a 'type' ('allow' or 'deny'). + + The two scopes of flavor permission rules are derived from the project + hierarchy: + - 'domain' scope rules have a domain 'project_id' and apply to all projects + within that domain + - 'project' scope rules have a 'project_id' of a non-domain project, apply + only to that project itself and are not inherited by sub-projects + + A flavor is permitted for a project if it is permitted at both the domain + scope and the project scope. Rules without a 'flavor_id' define the + domain's or project's default behavior for flavors without a + flavor-specific rule. If a domain or project does not have a default + behavior rule, then all flavors are permitted at that scope by default. + + There is at most one flavor permission rule for each combination of + 'project_id' and 'flavor_id'. Consequently, a flavor is only denied at a + scope if for the corresponding 'project_id': + - there is a 'deny' rule matching the 'flavor_id' + - OR there is a 'deny' rule without a 'flavor_id' AND there is no 'allow' + rule matching the 'flavor_id' + + Note: Flavor permission rules are independent of flavor privacy and the + corresponding flavor access list. A flavor is only available to a project + if allowed by both the flavor permission rules and the flavor access list. + """ + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'id': fields.IntegerField(), + 'uuid': fields.UUIDField(), + 'project_id': fields.StringField(), + 'flavor_id': fields.IntegerField(nullable=True), + 'type': fields.FlavorPermissionRuleTypeField(), + 'scope': fields.FlavorPermissionRuleScopeField(), + } + + @staticmethod + def _from_db_object(context, rule, db_rule): + # NOTE(sebkro) Delete fields are not implemented in the API DB models, + # but are inherited from NovaPersistentObject + ignore = {'deleted': False, 'deleted_at': None} + for field in rule.fields: + if field in ignore and not hasattr(db_rule, field): + setattr(rule, field, ignore[field]) + if field in db_rule: + setattr(rule, field, db_rule[field]) + rule._context = context + rule.obj_reset_changes() + return rule + + @staticmethod + @db_utils.require_context + @api_db_api.context_manager.reader + def _get_by_id_from_db(context, id): + query = context.session.query( + api_models.FlavorPermissionRule).filter_by(id=id) + db_rule = query.first() + if not db_rule: + raise exception.FlavorPermissionRuleNotFound(id=id) + return db_rule + + @staticmethod + @db_utils.require_context + @api_db_api.context_manager.reader + def _get_by_uuid_from_db(context, uuid): + query = context.session.query( + api_models.FlavorPermissionRule).filter_by(uuid=uuid) + db_rule = query.first() + if not db_rule: + raise exception.FlavorPermissionRuleNotFound(id=uuid) + return db_rule + + @staticmethod + @db_utils.require_context + @api_db_api.context_manager.reader + def _get_by_project_and_flavor_from_db(context, project_id, flavor_id): + query = context.session.query( + api_models.FlavorPermissionRule).filter_by( + project_id=project_id).filter_by(flavor_id=flavor_id) + db_rule = query.first() + if not db_rule: + raise exception.FlavorPermissionRuleNotFoundForProjectFlavor( + project_id=project_id, flavor_id=flavor_id) + return db_rule + + @staticmethod + @db_utils.require_context + @api_db_api.context_manager.writer + def _create_in_db(context, values): + db_rule = api_models.FlavorPermissionRule() + db_rule.update(values) + try: + db_rule.save(context.session) + except db_exc.DBDuplicateEntry: + raise exception.FlavorPermissionRuleExists( + uuid=values.get('uuid'), + project_id=values.get('project_id'), + flavor_id=values.get('flavor_id')) + return db_rule + + @staticmethod + @db_utils.require_context + @api_db_api.context_manager.writer + def _destroy_in_db(context, id): + result = context.session.query( + api_models.FlavorPermissionRule).filter_by(id=id).delete() + if not result: + raise exception.FlavorPermissionRuleNotFound(id=id) + + @staticmethod + @db_utils.require_context + @api_db_api.context_manager.writer + def _save(context, id, values): + db_rule = FlavorPermissionRule._get_by_id_from_db(context, id) + db_rule.update(values) + try: + db_rule.save(context.session) + except db_exc.DBDuplicateEntry: + raise exception.FlavorPermissionRuleExists( + uuid=values.get('uuid'), + project_id=values.get('project_id'), + flavor_id=values.get('flavor_id')) + return db_rule + + @base.remotable_classmethod + def get_by_id(cls, context, id): + db_rule = cls._get_by_id_from_db(context, id) + return cls._from_db_object(context, cls(context), db_rule) + + @base.remotable_classmethod + def get_by_uuid(cls, context, uuid): + db_rule = cls._get_by_uuid_from_db(context, uuid) + return cls._from_db_object(context, cls(context), db_rule) + + @base.remotable_classmethod + def get_by_project_and_flavor(cls, context, project_id, flavor_id): + db_rule = cls._get_by_project_and_flavor_from_db( + context, project_id, flavor_id) + return cls._from_db_object(context, cls(context), db_rule) + + @base.remotable + def create(self): + updates = self.obj_get_changes() + db_rule = self._create_in_db(self._context, updates) + self._from_db_object(self._context, self, db_rule) + + @base.remotable + def destroy(self): + self._destroy_in_db(self._context, self.id) + + @base.remotable + def save(self): + updates = self.obj_get_changes() + if updates: + db_rule = self._save(self._context, self.id, updates) + # Refresh updated_at. + self._from_db_object(self._context, self, db_rule) + + +@base.NovaObjectRegistry.register +class FlavorPermissionRuleList(base.ObjectListBase, base.NovaObject): + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'objects': fields.ListOfObjectsField('FlavorPermissionRule'), + } + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.objects = [] + self.obj_reset_changes() + + @staticmethod + @db_utils.require_context + @api_db_api.context_manager.reader + def _get_from_db(context, project_id=None, domain_id=None, + limit=None, marker=None): + query = context.session.query(api_models.FlavorPermissionRule) + + project_ids = [] + if project_id is not None: + project_ids.append(project_id) + if domain_id is not None: + project_ids.append(domain_id) + if project_ids: + query = query.filter(api_models.FlavorPermissionRule.project_id.in_(project_ids)) + + marker_rule = None + if marker is not None: + marker_query = context.session.query( + api_models.FlavorPermissionRule).filter_by(uuid=marker) + marker_rule = marker_query.first() + if not marker_rule: + raise exception.MarkerNotFound(marker=marker) + + query = paginate_query( + query, api_models.FlavorPermissionRule, limit, ['id'], + marker=marker_rule) + return query.all() + + @base.remotable_classmethod + def get_all(cls, context, filter_by_project=True, filter_by_domain=True, + limit=None, marker=None): + project_id = context.project_id if filter_by_project else None + domain_id = context.project_domain_id if filter_by_domain else None + db_rules = cls._get_from_db(context, project_id=project_id, + domain_id=domain_id, + limit=limit, marker=marker) + return base.obj_make_list( + context, cls(context), FlavorPermissionRule, db_rules + ) diff --git a/nova/tests/functional/db/test_flavor.py b/nova/tests/functional/db/test_flavor.py index c0435b5cbc6..bd3f8c36feb 100644 --- a/nova/tests/functional/db/test_flavor.py +++ b/nova/tests/functional/db/test_flavor.py @@ -175,3 +175,99 @@ def test_get_all_with_marker_not_found(self): flavor.create() self.assertRaises(exception.MarkerNotFound, self._test_get_all, 2, marker='noflavoratall') + + +class FlavorPermissionRuleFilterTestCase(test.NoDBTestCase): + """Tests for the flavor permission rule filtering in + _flavor_get_query_from_db. + + Uses a non-admin context with both project_id and project_domain_id set. + All flavors are public so the is_public filter never blocks visibility -- + only permission rules are tested here. + """ + USES_DB_SELF = True + + PROJECT_ID = 'fake-project' + DOMAIN_ID = 'fake-domain' + + def _get_context(self, include_domain=True): + return context.RequestContext( + 'fake-user', self.PROJECT_ID, + project_domain_id=self.DOMAIN_ID if include_domain else None) + + def setUp(self): + super().setUp() + self.useFixture(fixtures.Database()) + self.useFixture(fixtures.Database(database='api')) + self.context = self._get_context() + flavor = objects.Flavor(context=self.context, + **dict(fake_api_flavor, is_public=True)) + flavor.create() + self.flavor = flavor + + def _create_rule(self, project_id, rule_type, flavor_id=None): + scope = 'domain' if project_id == self.DOMAIN_ID else 'project' + rule = objects.FlavorPermissionRule( + context=self.context, + project_id=project_id, + type=rule_type, + flavor_id=flavor_id, + scope=scope, + ) + rule.create() + return rule + + def _flavor_visible(self, include_denied=False, context=None): + flavors = objects.FlavorList.get_all( + context or self.context, include_denied=include_denied) + return any(f.id == self.flavor.id for f in flavors) + + def test_no_rules_flavor_accessible(self): + self.assertTrue(self._flavor_visible()) + + def test_domain_specific_deny_hides_flavor(self): + self._create_rule(self.DOMAIN_ID, 'deny', flavor_id=self.flavor.id) + self.assertFalse(self._flavor_visible()) + # include_denied only bypasses project-level denial + self.assertFalse(self._flavor_visible(include_denied=True)) + + def test_domain_default_deny_hides_flavor(self): + self._create_rule(self.DOMAIN_ID, 'deny') + self.assertFalse(self._flavor_visible()) + # include_denied only bypasses project-level denial + self.assertFalse(self._flavor_visible(include_denied=True)) + + def test_domain_default_deny_overridden_by_specific_allow(self): + self._create_rule(self.DOMAIN_ID, 'deny') + self._create_rule(self.DOMAIN_ID, 'allow', flavor_id=self.flavor.id) + self.assertTrue(self._flavor_visible()) + + def test_project_specific_deny_hides_flavor(self): + self._create_rule(self.PROJECT_ID, 'deny', flavor_id=self.flavor.id) + self.assertFalse(self._flavor_visible()) + self.assertTrue(self._flavor_visible(include_denied=True)) + + def test_project_default_deny_hides_flavor(self): + self._create_rule(self.PROJECT_ID, 'deny') + self.assertFalse(self._flavor_visible()) + self.assertTrue(self._flavor_visible(include_denied=True)) + + def test_project_default_deny_overridden_by_specific_allow(self): + self._create_rule(self.PROJECT_ID, 'deny') + self._create_rule(self.PROJECT_ID, 'allow', flavor_id=self.flavor.id) + self.assertTrue(self._flavor_visible()) + + def test_no_domain_id_in_context_skips_domain_filter(self): + self._create_rule(self.DOMAIN_ID, 'deny', flavor_id=self.flavor.id) + self.assertTrue(self._flavor_visible( + context=self._get_context(include_domain=False))) + + def test_deny_for_other_project_does_not_affect_visibility(self): + self._create_rule('other-project', 'deny', flavor_id=self.flavor.id) + self._create_rule('other-project', 'deny') + self.assertTrue(self._flavor_visible()) + + def test_deny_for_other_domain_does_not_affect_visibility(self): + self._create_rule('other-domain', 'deny', flavor_id=self.flavor.id) + self._create_rule('other-domain', 'deny') + self.assertTrue(self._flavor_visible()) diff --git a/nova/tests/unit/db/api/test_migrations.py b/nova/tests/unit/db/api/test_migrations.py index 37565208071..bd3d60fa37d 100644 --- a/nova/tests/unit/db/api/test_migrations.py +++ b/nova/tests/unit/db/api/test_migrations.py @@ -196,6 +196,35 @@ def _check_cdeec0c85668(self, connection): # removal without creating it first, which is dumb pass + def _pre_upgrade_f177f53f978b(self, connection): + # we use the inspector here rather than oslo_db.utils.column_exists, + # since the latter will create a new connection + inspector = sqlalchemy.inspect(connection) + self.assertFalse(inspector.has_table('flavor_permission_rules')) + + def _check_f177f53f978b(self, connection): + # we use the inspector here rather than oslo_db.utils.column_exists, + # since the latter will create a new connection + inspector = sqlalchemy.inspect(connection) + self.assertTrue(inspector.has_table('flavor_permission_rules')) + columns = {x['name'] for x in + inspector.get_columns('flavor_permission_rules')} + expected_columns = {'id', 'uuid', 'project_id', 'flavor_id', 'type', + 'scope', 'created_at', 'updated_at'} + self.assertEqual(expected_columns, columns) + unique_constraints = [ + set(c['column_names']) for c in inspector.get_unique_constraints( + 'flavor_permission_rules')] + expected_unique_constraints = [{'uuid'}, {'flavor_id', 'project_id'}] + for c in expected_unique_constraints: + self.assertIn(c, unique_constraints) + indexes = [ + set(idx['column_names']) for idx in inspector.get_indexes( + 'flavor_permission_rules')] + expected_indexes = [{'uuid'}, {'project_id', 'flavor_id'}] + for idx in expected_indexes: + self.assertIn(idx, indexes) + def test_single_base_revision(self): """Ensure we only have a single base revision. diff --git a/nova/tests/unit/fake_flavor_permission_rule.py b/nova/tests/unit/fake_flavor_permission_rule.py new file mode 100644 index 00000000000..99ccf61c62b --- /dev/null +++ b/nova/tests/unit/fake_flavor_permission_rule.py @@ -0,0 +1,50 @@ +# Copyright (c) 2025 SAP SE +# All Rights Reserved. +# +# 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 oslo_utils import uuidutils + +from nova import objects +from nova.objects import fields + + +def fake_db_flavor_permission_rule(**updates): + db_rule = { + 'id': 1, + 'uuid': uuidutils.generate_uuid(), + 'project_id': 'fake-project', + 'flavor_id': 123, + 'type': fields.FlavorPermissionRuleType.ALLOW, + 'scope': fields.FlavorPermissionRuleScope.PROJECT, + } | updates + + for name, field in objects.FlavorPermissionRule.fields.items(): + if name in db_rule: + continue + if field.nullable: + db_rule[name] = None + elif field.default != fields.UnspecifiedDefault: + db_rule[name] = field.default + else: + raise Exception( + f'fake_db_flavor_permission_rule needs help with {name}') + + return db_rule + + +def fake_flavor_permission_rule_obj(context, db_rule=None, **updates): + if db_rule is None: + db_rule = fake_db_flavor_permission_rule() + return objects.FlavorPermissionRule._from_db_object( + context, objects.FlavorPermissionRule(), db_rule | updates) diff --git a/nova/tests/unit/objects/test_flavor_permission_rule.py b/nova/tests/unit/objects/test_flavor_permission_rule.py new file mode 100644 index 00000000000..55dc7732ed3 --- /dev/null +++ b/nova/tests/unit/objects/test_flavor_permission_rule.py @@ -0,0 +1,325 @@ +# Copyright (c) 2025 SAP SE +# All Rights Reserved. +# +# 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 datetime import datetime +from unittest import mock + +from nova import context +from nova.db.api import api as api_db_api +from nova.db.api import models as api_models +from nova import exception +from nova import objects +from nova.objects import fields +from nova import test +from nova.tests.unit import fake_flavor_permission_rule as fake_rule +from nova.tests.unit.objects import test_objects + + +class _TestFlavorPermissionRuleObject: + """Mixin for FlavorPermissionRule object test cases.""" + + _fake_db_rule = fake_rule.fake_db_flavor_permission_rule() + + def _get_fake_db_rule(self, **updates): + return fake_rule.fake_db_flavor_permission_rule(**updates) + + def _get_fake_rule_obj(self, db_rule=None): + if db_rule is None: + db_rule = self._fake_db_rule + return fake_rule.fake_flavor_permission_rule_obj( + self.context, db_rule) + + def _compare_obj(self, rule_obj, db_rule, db_allow_missing=None, + db_allow_none=None): + if db_allow_none is None: + db_allow_none = [] + if db_allow_missing is None: + db_allow_missing = [] + for field in rule_obj.fields: + if field not in db_rule and field in db_allow_missing: + self.assertTrue(rule_obj.obj_attr_is_set(field)) + continue + + db_val = db_rule[field] + if db_val is None and field in db_allow_none: + self.assertTrue(rule_obj.obj_attr_is_set(field)) + continue + + obj_val = getattr(rule_obj, field) + if isinstance(obj_val, datetime): + obj_val = obj_val.replace(tzinfo=None) + # Indirection API loses microsecond precision + if rule_obj.indirection_api: + db_val = db_val.replace(microsecond=0) + + self.assertEqual(db_val, obj_val, f'field: {field}') + + @staticmethod + @api_db_api.context_manager.writer + def _create_context_db_rule(context, fake_db_rule=None): + if fake_db_rule is None: + fake_db_rule = _TestFlavorPermissionRuleObject._fake_db_rule + fake_db_rule = fake_db_rule.copy() + fake_db_rule.pop('id') + db_rule = api_models.FlavorPermissionRule() + db_rule.update(fake_db_rule) + db_rule.save(context.session) + return db_rule + + def _create_db_rule(self, fake_db_rule=None): + return self._create_context_db_rule(self.context, fake_db_rule) + + +class TestFlavorPermissionRuleObjectNoDB(test.NoDBTestCase, + _TestFlavorPermissionRuleObject): + + def setUp(self): + super().setUp() + # Set up context like _BaseTestCase does + self.user_id = 'fake-user' + self.project_id = 'fake-project' + self.context = context.RequestContext(self.user_id, self.project_id) + + @mock.patch('nova.objects.FlavorPermissionRule._get_by_id_from_db') + def test_get_by_id(self, mock_get): + mock_get.return_value = self._fake_db_rule + rule = objects.FlavorPermissionRule.get_by_id( + self.context, self._fake_db_rule['id']) + self._compare_obj(rule, self._fake_db_rule) + mock_get.assert_called_once_with( + self.context, self._fake_db_rule['id']) + + @mock.patch('nova.objects.FlavorPermissionRule._get_by_uuid_from_db') + def test_get_by_uuid(self, mock_get): + mock_get.return_value = self._fake_db_rule + rule = objects.FlavorPermissionRule.get_by_uuid( + self.context, self._fake_db_rule['uuid']) + self._compare_obj(rule, self._fake_db_rule) + mock_get.assert_called_once_with( + self.context, self._fake_db_rule['uuid']) + + @mock.patch('nova.objects.FlavorPermissionRule.' + '_get_by_project_and_flavor_from_db') + def test_get_by_project_and_flavor(self, mock_get): + mock_get.return_value = self._fake_db_rule + rule = objects.FlavorPermissionRule.get_by_project_and_flavor( + self.context, self._fake_db_rule['project_id'], + self._fake_db_rule['flavor_id']) + self._compare_obj(rule, self._fake_db_rule) + mock_get.assert_called_once_with( + self.context, self._fake_db_rule['project_id'], + self._fake_db_rule['flavor_id']) + + @mock.patch('nova.objects.FlavorPermissionRule._create_in_db') + def test_create(self, mock_create): + mock_create.return_value = self._fake_db_rule + fake_db_rule = self._fake_db_rule.copy() + fake_db_rule.pop('id') + # Create rule with tracked changes + rule = objects.FlavorPermissionRule(context=self.context, + **fake_db_rule) + rule.create() + mock_create.assert_called_once_with(self.context, fake_db_rule) + self._compare_obj(rule, self._fake_db_rule) + + @mock.patch('nova.objects.FlavorPermissionRule._destroy_in_db') + def test_destroy(self, mock_destroy): + mock_destroy.return_value = self._fake_db_rule + rule = self._get_fake_rule_obj() + rule.destroy() + mock_destroy.assert_called_once_with(self.context, 1) + + @mock.patch('nova.objects.FlavorPermissionRule._save') + def test_save(self, mock_save): + new_type = fields.FlavorPermissionRuleType.DENY + mock_save.return_value = self._fake_db_rule | {'type': new_type} + rule = self._get_fake_rule_obj() + rule.type = new_type + rule.save() + mock_save.assert_called_once_with( + self.context, rule.id, {'type': new_type}) + + @mock.patch('nova.objects.FlavorPermissionRule._save') + def test_save_no_changes(self, mock_save): + rule = self._get_fake_rule_obj() + rule.obj_reset_changes() + rule.save() + mock_save.assert_not_called() + + +class _TestFlavorPermissionRuleObjectDB(_TestFlavorPermissionRuleObject): + """Mixin for FlavorPermissionRule object test cases with DB.""" + + def test_get_by_id(self): + db_rule = self._create_db_rule() + rule = objects.FlavorPermissionRule.get_by_id(self.context, db_rule.id) + self._compare_obj(rule, db_rule) + + def test_get_by_uuid(self): + db_rule = self._create_db_rule() + rule = objects.FlavorPermissionRule.get_by_uuid( + self.context, db_rule.uuid) + self._compare_obj(rule, db_rule) + + def test_get_by_project_and_flavor(self): + db_rule = self._create_db_rule() + rule = objects.FlavorPermissionRule.get_by_project_and_flavor( + self.context, db_rule.project_id, db_rule.flavor_id) + self._compare_obj(rule, db_rule) + + def test_create(self): + fake_db_rule = self._fake_db_rule.copy() + fake_db_rule.pop('id') + # Create rule with tracked changes + rule = objects.FlavorPermissionRule(context=self.context, + **fake_db_rule) + rule.create() + self.assertIsNotNone(rule.id) + self.assertIsNotNone(rule.created_at) + self._compare_obj(rule, fake_db_rule, db_allow_missing=['id'], + db_allow_none=['created_at']) + updated_rule = objects.FlavorPermissionRule.get_by_id( + self.context, rule.id) + self._compare_obj(updated_rule, fake_db_rule, db_allow_missing=['id'], + db_allow_none=['created_at']) + + def test_destroy(self): + db_rule = self._create_db_rule() + rule = objects.FlavorPermissionRule.get_by_id(self.context, db_rule.id) + rule.destroy() + self.assertRaises( + exception.FlavorPermissionRuleNotFound, + objects.FlavorPermissionRule.get_by_id, + self.context, db_rule.id) + + def test_save(self): + db_rule = self._create_db_rule() + rule = objects.FlavorPermissionRule.get_by_id(self.context, db_rule.id) + new_type = fields.FlavorPermissionRuleType.DENY + rule.type = new_type + rule.save() + updated_rule = objects.FlavorPermissionRule.get_by_id( + self.context, db_rule.id) + self.assertEqual(new_type, updated_rule.type) + + +class TestFlavorPermissionRuleObject( + test_objects._LocalTest, _TestFlavorPermissionRuleObjectDB): + pass + + +class TestFlavorPermissionRuleObjectRemote( + test_objects._RemoteTest, _TestFlavorPermissionRuleObjectDB): + pass + + +class TestFlavorPermissionRuleListObjectNoDB(test.NoDBTestCase, + _TestFlavorPermissionRuleObject): + + def setUp(self): + super().setUp() + # Set up context like _BaseTestCase does + self.user_id = 'fake-user' + self.project_id = 'fake-project' + self.context = context.RequestContext( + self.user_id, self.project_id, project_domain_id='fake-domain') + + @mock.patch('nova.objects.FlavorPermissionRuleList._get_from_db') + def test_get_all(self, mock_get): + db_rule1 = self._fake_db_rule + db_rule2 = self._fake_db_rule | { + 'project_id': 'fake-domain', + 'scope': fields.FlavorPermissionRuleScope.DOMAIN, + } + mock_get.return_value = [db_rule1, db_rule2] + rules = objects.FlavorPermissionRuleList.get_all(self.context) + self._compare_obj(rules[0], db_rule1) + self._compare_obj(rules[1], db_rule2) + mock_get.assert_called_once_with( + self.context, project_id=self.context.project_id, + domain_id=self.context.project_domain_id, + limit=None, marker=None) + + @mock.patch('nova.objects.FlavorPermissionRuleList._get_from_db') + def test_get_all_project_only(self, mock_get): + mock_get.return_value = [self._fake_db_rule] + objects.FlavorPermissionRuleList.get_all( + self.context, filter_by_domain=False) + mock_get.assert_called_once_with( + self.context, project_id=self.context.project_id, + domain_id=None, limit=None, marker=None) + + @mock.patch('nova.objects.FlavorPermissionRuleList._get_from_db') + def test_get_all_domain_only(self, mock_get): + mock_get.return_value = [] + objects.FlavorPermissionRuleList.get_all( + self.context, filter_by_project=False) + mock_get.assert_called_once_with( + self.context, project_id=None, + domain_id=self.context.project_domain_id, + limit=None, marker=None) + + +class _TestFlavorPermissionRuleListObject(_TestFlavorPermissionRuleObject): + + def test_get_by_project_and_domain(self): + db_rule_project = self._create_db_rule() + db_rule_domain = self._create_db_rule( + self._get_fake_db_rule( + project_id='fake-domain', + scope=fields.FlavorPermissionRuleScope.DOMAIN)) + ctx = context.RequestContext( + self.context.user_id, db_rule_project.project_id, + project_domain_id=db_rule_domain.project_id) + # Only the project-scoped rule is returned when filter_by_domain=False + rules = objects.FlavorPermissionRuleList.get_all( + ctx, filter_by_domain=False) + self.assertEqual(1, len(rules)) + self.assertEqual(db_rule_project.id, rules[0].id) + # Only the domain-scoped rule is returned when filter_by_project=False + rules = objects.FlavorPermissionRuleList.get_all( + ctx, filter_by_project=False) + self.assertEqual(1, len(rules)) + self.assertEqual(db_rule_domain.id, rules[0].id) + # Both rules are returned with defaults + rules = objects.FlavorPermissionRuleList.get_all(ctx) + self.assertEqual(2, len(rules)) + + def test_get_all(self): + db_rule1 = self._create_db_rule() + db_rule2 = self._create_db_rule( + self._get_fake_db_rule(project_id='project-2')) + rules = objects.FlavorPermissionRuleList.get_all( + self.context, filter_by_project=False, filter_by_domain=False) + self.assertEqual(2, len(rules)) + rules = objects.FlavorPermissionRuleList.get_all( + self.context, filter_by_project=False, filter_by_domain=False, limit=1) + self.assertEqual(1, len(rules)) + self.assertEqual(db_rule1.id, rules[0].id) + rules = objects.FlavorPermissionRuleList.get_all( + self.context, filter_by_project=False, filter_by_domain=False, limit=1, + marker=db_rule1.uuid) + self.assertEqual(1, len(rules)) + self.assertEqual(db_rule2.id, rules[0].id) + + +class TestFlavorPermissionRuleListObject( + test_objects._LocalTest, _TestFlavorPermissionRuleListObject): + pass + + +class TestRemoteFlavorPermissionRuleListObject( + test_objects._RemoteTest, _TestFlavorPermissionRuleListObject): + pass diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index bc6f2f6e8d6..865b40ab011 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1097,6 +1097,8 @@ def obj_name(cls): 'EC2InstanceMapping': '1.0-a4556eb5c5e94c045fe84f49cf71644f', 'Flavor': '1.2-4ce99b41327bb230262e5a8f45ff0ce3', 'FlavorList': '1.1-912b5ce24d48bc60cc9db96104f95581', + 'FlavorPermissionRule': '1.0-72bf113bf392f6c3269752a35f086326', + 'FlavorPermissionRuleList': '1.0-b0cf1820c6c279addd62f4e36d0099a8', 'HostMapping': '1.0-1a3390a696792a552ab7bd31a77ba9ac', 'HostMappingList': '1.1-18ac2bfb8c1eb5545bed856da58a79bc', 'HVSpec': '1.2-de06bcec472a2f04966b855a49c46b41',