From 446416e80a4de639fdcaaadc9e4cf26cd9d0299c Mon Sep 17 00:00:00 2001 From: Sebastian Krott Date: Mon, 3 Nov 2025 17:58:06 +0100 Subject: [PATCH 1/2] Add flavor permission rules Flavor access is currently controlled via private/public flavors and the flavor access list aka `FlavorProjects`. To restrict access to a flavor, it must be turned private and access must be granted individually to each project. To support external customers, we want to restrict access to certain flavors for arbitrary domains while keeping these flavors publicly accessible on other domain without additional configuration needs. Additionally, we want to enable project owners to configure project-specific flavor access. This adds flavor permission rules as a new mechanism for controlling flavor access. These rules are independent of flavor privacy and the flavor access list. So a flavor is only available to a project if allowed by both the flavor permission rules and the flavor access list. Each 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` Change-Id: I5e1332d111c07ee714f2965c558f393c8568ffaf --- ...177f53f978b_add_flavor_permission_rules.py | 61 ++++ nova/db/api/models.py | 31 ++ nova/exception.py | 15 + nova/objects/__init__.py | 1 + nova/objects/fields.py | 22 ++ nova/objects/flavor.py | 2 + nova/objects/flavor_permission_rule.py | 246 +++++++++++++ nova/tests/unit/db/api/test_migrations.py | 29 ++ .../tests/unit/fake_flavor_permission_rule.py | 50 +++ .../objects/test_flavor_permission_rule.py | 325 ++++++++++++++++++ nova/tests/unit/objects/test_objects.py | 2 + 11 files changed, 784 insertions(+) create mode 100644 nova/db/api/migrations/versions/f177f53f978b_add_flavor_permission_rules.py create mode 100644 nova/objects/flavor_permission_rule.py create mode 100644 nova/tests/unit/fake_flavor_permission_rule.py create mode 100644 nova/tests/unit/objects/test_flavor_permission_rule.py 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..d84d5038c35 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 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/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', From 90989d8a7811c40fd1a51dc6cc3fc9613c1d89e5 Mon Sep 17 00:00:00 2001 From: Sebastian Krott Date: Fri, 10 Jul 2026 09:29:18 +0200 Subject: [PATCH 2/2] Enforce permission rules for flavor objects Enforce flavor permission rules for all flavor object get-functions by extending the filtering of the default flavor query. All get-functions are extended with an `include_denied` parameter that allows to return flavors denied at project scope also for non-admin contexts. Domain scope rules still apply. A flavor is 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` Just like the existing flavor privacy mechanisms (private flavors/flavor access list), the flavor permissions rules do not take effect for admin contexts and do not restrict the flavor object create, save and destroy methods in any way. Change-Id: Ic17dcc21a07a383e26822a5fbf7cf5cfc5383ec6 --- nova/objects/flavor.py | 86 ++++++++++++++++------ nova/tests/functional/db/test_flavor.py | 96 +++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 20 deletions(-) diff --git a/nova/objects/flavor.py b/nova/objects/flavor.py index d84d5038c35..0e4d80206e4 100644 --- a/nova/objects/flavor.py +++ b/nova/objects/flavor.py @@ -310,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( @@ -322,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: @@ -339,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: @@ -351,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() @@ -420,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']) @@ -632,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( @@ -692,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/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())