From cdea73bd9cc811b2d267dd937cf3289dc1a39a15 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Thu, 22 Oct 2020 15:02:35 +0100 Subject: [PATCH] BlockDeviceMapping: Add encryption fields This change adds the `encrypted`, `encryption_secret_uuid`, `encryption_format` and `encryption_options` to the BlockDeviceMapping object and associated table in the database. Change-Id: I6178c9bc249ef4d448de375dc16d82b2d087fc90 --- nova/block_device.py | 4 +- .../ccb0fa1a2252_add_encryption_fields_to_.py | 59 +++++++++++++++++++ nova/db/main/models.py | 12 +++- nova/objects/block_device.py | 47 ++++++++++----- nova/tests/unit/compute/test_api.py | 8 ++- nova/tests/unit/db/main/test_migrations.py | 33 +++++++++++ nova/tests/unit/objects/test_block_device.py | 5 ++ nova/tests/unit/objects/test_objects.py | 2 +- 8 files changed, 149 insertions(+), 21 deletions(-) create mode 100644 nova/db/main/migrations/versions/ccb0fa1a2252_add_encryption_fields_to_.py diff --git a/nova/block_device.py b/nova/block_device.py index c0c69d4a71fd..31d163f811a6 100644 --- a/nova/block_device.py +++ b/nova/block_device.py @@ -52,7 +52,9 @@ bdm_new_fields = set(['source_type', 'destination_type', 'guest_format', 'device_type', 'disk_bus', 'boot_index', 'device_name', 'delete_on_termination', 'snapshot_id', 'volume_id', 'volume_size', 'image_id', 'no_device', - 'connection_info', 'tag', 'volume_type']) + 'connection_info', 'tag', 'volume_type', 'encrypted', + 'encryption_secret_uuid', 'encryption_format', + 'encryption_options']) bdm_db_only_fields = set(['id', 'instance_uuid', 'attachment_id', 'uuid']) diff --git a/nova/db/main/migrations/versions/ccb0fa1a2252_add_encryption_fields_to_.py b/nova/db/main/migrations/versions/ccb0fa1a2252_add_encryption_fields_to_.py new file mode 100644 index 000000000000..1fd3fb4780ae --- /dev/null +++ b/nova/db/main/migrations/versions/ccb0fa1a2252_add_encryption_fields_to_.py @@ -0,0 +1,59 @@ +# 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 encryption fields to BlockDeviceMapping + +Revision ID: ccb0fa1a2252 +Revises: 16f1fbcab42b +Create Date: 2022-01-12 15:22:47.524285 +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'ccb0fa1a2252' +down_revision = '16f1fbcab42b' +branch_labels = None +depends_on = None + + +def upgrade(): + for prefix in ('', 'shadow_'): + table_name = prefix + 'block_device_mapping' + with op.batch_alter_table(table_name, schema=None) as batch_op: + batch_op.add_column( + sa.Column( + 'encrypted', + sa.Boolean(), + nullable=True, + ) + ) + batch_op.add_column( + sa.Column( + 'encryption_secret_uuid', + sa.String(length=36), + nullable=True, + ) + ) + batch_op.add_column( + sa.Column('encryption_format', + sa.String(length=128), + nullable=True, + ) + ) + batch_op.add_column( + sa.Column('encryption_options', + sa.String(length=4096), + nullable=True, + ) + ) diff --git a/nova/db/main/models.py b/nova/db/main/models.py index f2f58b2db167..7551584c1c94 100644 --- a/nova/db/main/models.py +++ b/nova/db/main/models.py @@ -654,9 +654,15 @@ class BlockDeviceMapping(BASE, NovaBase, models.SoftDeleteMixin): attachment_id = sa.Column(sa.String(36)) + encrypted = sa.Column(sa.Boolean, default=False) + encryption_secret_uuid = sa.Column(sa.String(36)) + encryption_format = sa.Column(sa.String(128)) + encryption_options = sa.Column(sa.String(4096)) # TODO(stephenfin): Remove once we drop the security_groups field from the # Instance table. Until then, this is tied to the SecurityGroup table + + class SecurityGroupInstanceAssociation(BASE, NovaBase, models.SoftDeleteMixin): __tablename__ = 'security_group_instance_association' __table_args__ = ( @@ -679,7 +685,7 @@ class SecurityGroup(BASE, NovaBase, models.SoftDeleteMixin): name='uniq_security_groups0project_id0' 'name0deleted'), ) - id = sa.Column(sa.Integer, primary_key=True) + id = sa.Column(sa.Integer, primary_key = True) name = sa.Column(sa.String(255)) description = sa.Column(sa.String(255)) @@ -687,8 +693,8 @@ class SecurityGroup(BASE, NovaBase, models.SoftDeleteMixin): project_id = sa.Column(sa.String(255)) instances = orm.relationship(Instance, - secondary="security_group_instance_association", - primaryjoin='and_(' + secondary = "security_group_instance_association", + primaryjoin = 'and_(' 'SecurityGroup.id == ' 'SecurityGroupInstanceAssociation.security_group_id,' 'SecurityGroupInstanceAssociation.deleted == 0,' diff --git a/nova/objects/block_device.py b/nova/objects/block_device.py index 97199cf17a52..5d21db955785 100644 --- a/nova/objects/block_device.py +++ b/nova/objects/block_device.py @@ -67,7 +67,9 @@ class BlockDeviceMapping(base.NovaPersistentObject, base.NovaObject, # Version 1.18: Added attachment_id # Version 1.19: Added uuid # Version 1.20: Added volume_type - VERSION = '1.20' + # Version 1.21: Added encrypted, encryption_secret_uuid, encryption_format + # and encryption_options + VERSION = '1.21' fields = { 'id': fields.IntegerField(), @@ -93,10 +95,20 @@ class BlockDeviceMapping(base.NovaPersistentObject, base.NovaObject, 'attachment_id': fields.UUIDField(nullable=True), # volume_type field can be a volume type name or ID(UUID). 'volume_type': fields.StringField(nullable=True), + 'encrypted': fields.BooleanField(default=False), + 'encryption_secret_uuid': fields.UUIDField(nullable=True), + 'encryption_format': fields.BlockDeviceEncryptionFormatTypeField( + nullable=True), + 'encryption_options': fields.StringField(nullable=True), } def obj_make_compatible(self, primitive, target_version): target_version = versionutils.convert_version_to_tuple(target_version) + if target_version < (1, 21): + primitive.pop('encrypted', None) + primitive.pop('encryption_secret_uuid', None) + primitive.pop('encryption_format', None) + primitive.pop('encryption_options', None) if target_version < (1, 20) and 'volume_type' in primitive: del primitive['volume_type'] if target_version < (1, 19) and 'uuid' in primitive: @@ -312,22 +324,29 @@ class BlockDeviceMapping(base.NovaPersistentObject, base.NovaObject, return block_device.BlockDeviceDict(self).get_image_mapping() def obj_load_attr(self, attrname): - if attrname not in BLOCK_DEVICE_OPTIONAL_ATTRS: - raise exception.ObjectActionError( - action='obj_load_attr', - reason='attribute %s not lazy-loadable' % attrname) if not self._context: raise exception.OrphanedObjectError(method='obj_load_attr', objtype=self.obj_name()) - - LOG.debug("Lazy-loading '%(attr)s' on %(name)s using uuid %(uuid)s", - {'attr': attrname, - 'name': self.obj_name(), - 'uuid': self.instance_uuid, - }) - self.instance = objects.Instance.get_by_uuid(self._context, - self.instance_uuid) - self.obj_reset_changes(fields=['instance']) + if attrname == 'encrypted': + # We attempt to load this if we're creating a BDM object during an + # attach volume request, for example. Use the default in that case. + self.obj_set_defaults(attrname) + elif attrname not in BLOCK_DEVICE_OPTIONAL_ATTRS: + raise exception.ObjectActionError( + action='obj_load_attr', + reason='attribute %s not lazy-loadable' % attrname) + else: + LOG.debug( + "Lazy-loading '%(attr)s' on %(name)s using uuid %(uuid)s", + { + 'attr': attrname, + 'name': self.obj_name(), + 'uuid': self.instance_uuid, + } + ) + self.instance = objects.Instance.get_by_uuid(self._context, + self.instance_uuid) + self.obj_reset_changes(fields=['instance']) @base.NovaObjectRegistry.register diff --git a/nova/tests/unit/compute/test_api.py b/nova/tests/unit/compute/test_api.py index 38db08d95280..33752fb1b3d3 100644 --- a/nova/tests/unit/compute/test_api.py +++ b/nova/tests/unit/compute/test_api.py @@ -3479,7 +3479,9 @@ class _ComputeAPIUnitTestMixIn(object): 'device_type': None, 'snapshot_id': '1-snapshot', 'device_name': '/dev/vda', 'destination_type': 'volume', 'delete_on_termination': False, - 'tag': None, 'volume_type': None}) + 'tag': None, 'volume_type': None, + 'encrypted': None, 'encryption_format': None, + 'encryption_secret_uuid': None, 'encryption_options': None}) limits_patcher = mock.patch.object( self.compute_api.volume_api, 'get_absolute_limits', @@ -3542,7 +3544,9 @@ class _ComputeAPIUnitTestMixIn(object): 'device_type': None, 'snapshot_id': None, 'device_name': '/dev/vdh', 'destination_type': 'local', 'delete_on_termination': True, - 'tag': None, 'volume_type': None}) + 'tag': None, 'volume_type': None, + 'encrypted': False, 'encryption_format': None, + 'encryption_secret_uuid': None, 'encryption_options': None}) quiesced = [False, False] diff --git a/nova/tests/unit/db/main/test_migrations.py b/nova/tests/unit/db/main/test_migrations.py index 2b3f01b7047f..d2c4ef9762c1 100644 --- a/nova/tests/unit/db/main/test_migrations.py +++ b/nova/tests/unit/db/main/test_migrations.py @@ -241,6 +241,12 @@ class NovaMigrationsWalk( 'Index %s on table %s should not exist' % (index, table_name), ) + def assertColumnExists(self, connection, table_name, column): + self.assertTrue( + oslodbutils.column_exists(connection, table_name, column), + 'Column %s on table %s should exist' % (column, table_name), + ) + def _migrate_up(self, connection, revision): if revision == self.init_version: # no tests for the initial revision alembic_api.upgrade(self.config, revision) @@ -281,6 +287,33 @@ class NovaMigrationsWalk( # no check for the MySQL-specific change + def _check_ccb0fa1a2252(self, connection): + for prefix in ('', 'shadow_'): + table_name = prefix + 'block_device_mapping' + table = oslodbutils.get_table(connection, table_name) + + self.assertColumnExists(connection, table_name, 'encrypted') + self.assertColumnExists( + connection, table_name, 'encryption_secret_uuid') + self.assertColumnExists( + connection, table_name, 'encryption_format') + self.assertColumnExists( + connection, table_name, 'encryption_options') + + # Only check for the expected types if we're using sqlite because + # other databases' types may be different. For example, Boolean + # may be represented as an integer in MySQL + if connection.engine.name != 'sqlite': + return + + self.assertIsInstance(table.c.encrypted.type, sa.types.Boolean) + self.assertIsInstance( + table.c.encryption_secret_uuid.type, sa.types.String) + self.assertIsInstance( + table.c.encryption_format.type, sa.types.String) + self.assertIsInstance( + table.c.encryption_options.type, sa.types.String) + def test_single_base_revision(self): """Ensure we only have a single base revision. diff --git a/nova/tests/unit/objects/test_block_device.py b/nova/tests/unit/objects/test_block_device.py index ad43bed8bfa8..6b5c2ac78302 100644 --- a/nova/tests/unit/objects/test_block_device.py +++ b/nova/tests/unit/objects/test_block_device.py @@ -276,6 +276,11 @@ class _TestBlockDeviceMappingObject(object): mock_inst_get_by_uuid.assert_called_once_with( self.context, bdm.instance_uuid) + def test_obj_load_attr_encrypted(self): + bdm = objects.BlockDeviceMapping(self.context, **self.fake_bdm()) + del bdm.encrypted + self.assertEqual(bdm.fields['encrypted'].default, bdm.encrypted) + def test_obj_make_compatible_pre_1_17(self): values = {'source_type': 'volume', 'volume_id': 'fake-vol-id', 'destination_type': 'volume', diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 2930bf194010..a39b6c6a8f5b 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1046,7 +1046,7 @@ class TestRegistry(test.NoDBTestCase): object_data = { 'Aggregate': '1.3-f315cb68906307ca2d1cca84d4753585', 'AggregateList': '1.3-3ea55a050354e72ef3306adefa553957', - 'BlockDeviceMapping': '1.20-45a6ad666ddf14bbbedece2293af77e2', + 'BlockDeviceMapping': '1.21-220abb8aa1450e759b72fce8ec6ff955', 'BlockDeviceMappingList': '1.18-73bcbbae5ef5e8adcedbc821db869306', 'BuildRequest': '1.3-077dee42bed93f8a5b62be77657b7152', 'BuildRequestList': '1.0-cd95608eccb89fbc702c8b52f38ec738',