From eee39db9e97cc69f2e237a656d864745e8c4de23 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Fri, 21 Nov 2014 21:57:14 +0200 Subject: [PATCH] Adds keypair type database migration X509 certificates are used by Windows for passwordless authentication (WinRM) in a way which can be considered consistent with the usage of SSH keys on Linux, as both are based on public / private keypairs. Adds 'type' field to the keypair object and database tables. Adds necessary database migration to add keypair type to the database tables. This commit is necessary in order to have different types of keypairs (e.g. ssh keypair, x509 keypair) Note: currently, the added column can only be populated with 'ssh' values, since the 'x509' keypair implementation is added in the next commits. Partially implements: blueprint keypair-x509-certificates Change-Id: Iae18de5e2cea01c58690d5abd872d495265ab198 --- .../api/openstack/compute/contrib/keypairs.py | 9 +++ .../openstack/compute/plugins/v3/keypairs.py | 9 +++ nova/compute/api.py | 3 + .../versions/275_add_keypair_type.py | 57 +++++++++++++++++++ nova/db/sqlalchemy/models.py | 4 +- nova/objects/keypair.py | 17 +++++- nova/tests/unit/db/test_db_api.py | 26 +++++---- nova/tests/unit/db/test_migrations.py | 29 ++++++++++ nova/tests/unit/objects/test_keypair.py | 8 +++ nova/tests/unit/objects/test_objects.py | 4 +- 10 files changed, 149 insertions(+), 17 deletions(-) create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/275_add_keypair_type.py diff --git a/nova/api/openstack/compute/contrib/keypairs.py b/nova/api/openstack/compute/contrib/keypairs.py index 3911c06e2a..cbecdf674d 100644 --- a/nova/api/openstack/compute/contrib/keypairs.py +++ b/nova/api/openstack/compute/contrib/keypairs.py @@ -107,7 +107,16 @@ class KeypairController(object): authorize(context, action='show') try: + # Since this method returns the whole object, functional test + # test_keypairs_get is failing, receiving an unexpected field + # 'type', which was added to the keypair object. + # TODO(claudiub): Revert the changes in the next commit, which will + # enable nova-api to return the keypair type. keypair = self.api.get_key_pair(context, context.user_id, id) + keypair = self._filter_keypair(keypair, created_at=True, + deleted=True, deleted_at=True, + id=True, user_id=True, + updated_at=True) except exception.KeypairNotFound as exc: raise webob.exc.HTTPNotFound(explanation=exc.format_message()) return {'keypair': keypair} diff --git a/nova/api/openstack/compute/plugins/v3/keypairs.py b/nova/api/openstack/compute/plugins/v3/keypairs.py index ce3eb3027d..9f96358310 100644 --- a/nova/api/openstack/compute/plugins/v3/keypairs.py +++ b/nova/api/openstack/compute/plugins/v3/keypairs.py @@ -115,7 +115,16 @@ class KeypairController(wsgi.Controller): authorize(context, action='show') try: + # Since this method returns the whole object, functional test + # test_keypairs_get is failing, receiving an unexpected field + # 'type', which was added to the keypair object. + # TODO(claudiub): Revert the changes in the next commit, which will + # enable nova-api to return the keypair type. keypair = self.api.get_key_pair(context, context.user_id, id) + keypair = self._filter_keypair(keypair, created_at=True, + deleted=True, deleted_at=True, + id=True, user_id=True, + updated_at=True) except exception.KeypairNotFound as exc: raise webob.exc.HTTPNotFound(explanation=exc.format_message()) # TODO(oomichi): It is necessary to filter a response of keypair with diff --git a/nova/compute/api.py b/nova/compute/api.py index 72cd42119b..91de525208 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -62,6 +62,7 @@ from nova.network.security_group import security_group_base from nova import notifications from nova import objects from nova.objects import base as obj_base +from nova.objects import keypair as keypair_obj from nova.objects import quotas as quotas_obj from nova.objects import security_group as security_group_obj from nova.openstack.common import log as logging @@ -3734,6 +3735,7 @@ class KeypairAPI(base.Base): keypair = objects.KeyPair(context) keypair.user_id = user_id keypair.name = key_name + keypair.type = keypair_obj.KEYPAIR_TYPE_SSH keypair.fingerprint = fingerprint keypair.public_key = public_key keypair.create() @@ -3754,6 +3756,7 @@ class KeypairAPI(base.Base): keypair = objects.KeyPair(context) keypair.user_id = user_id keypair.name = key_name + keypair.type = keypair_obj.KEYPAIR_TYPE_SSH keypair.fingerprint = fingerprint keypair.public_key = public_key keypair.create() diff --git a/nova/db/sqlalchemy/migrate_repo/versions/275_add_keypair_type.py b/nova/db/sqlalchemy/migrate_repo/versions/275_add_keypair_type.py new file mode 100644 index 0000000000..39889066ee --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/275_add_keypair_type.py @@ -0,0 +1,57 @@ +# Copyright (c) 2015 Cloudbase Solutions SRL +# 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 sqlalchemy import MetaData, Column, Table +from sqlalchemy import Enum + +from nova.objects import keypair + + +def upgrade(migrate_engine): + """Function adds key_pairs type field.""" + meta = MetaData(bind=migrate_engine) + key_pairs = Table('key_pairs', meta, autoload=True) + shadow_key_pairs = Table('shadow_key_pairs', meta, autoload=True) + + enum = Enum('ssh', 'x509', metadata=meta, name='keypair_types') + enum.create() + + keypair_type = Column('type', enum, nullable=False, + server_default=keypair.KEYPAIR_TYPE_SSH) + + if hasattr(key_pairs.c, 'type'): + key_pairs.c.type.drop() + + if hasattr(shadow_key_pairs.c, 'type'): + shadow_key_pairs.c.type.drop() + + key_pairs.create_column(keypair_type) + shadow_key_pairs.create_column(keypair_type.copy()) + + +def downgrade(migrate_engine): + """Function removes key_pairs type field.""" + meta = MetaData(bind=migrate_engine) + key_pairs = Table('key_pairs', meta, autoload=True) + shadow_key_pairs = Table('shadow_key_pairs', meta, autoload=True) + enum = Enum(metadata=meta, name='keypair_types') + + if hasattr(key_pairs.c, 'type'): + key_pairs.c.type.drop() + + if hasattr(shadow_key_pairs.c, 'type'): + shadow_key_pairs.c.type.drop() + + enum.drop() diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index edc048ce41..8fae58ef87 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -756,7 +756,7 @@ class ProviderFirewallRule(BASE, NovaBase): class KeyPair(BASE, NovaBase): - """Represents a public key pair for ssh.""" + """Represents a public key pair for ssh / WinRM.""" __tablename__ = 'key_pairs' __table_args__ = ( schema.UniqueConstraint("user_id", "name", "deleted", @@ -770,6 +770,8 @@ class KeyPair(BASE, NovaBase): fingerprint = Column(String(255)) public_key = Column(MediumText()) + type = Column(Enum('ssh', 'x509', name='keypair_types'), + nullable=False, server_default='ssh') class Migration(BASE, NovaBase): diff --git a/nova/objects/keypair.py b/nova/objects/keypair.py index 0dd3cd0d25..aca274cb77 100644 --- a/nova/objects/keypair.py +++ b/nova/objects/keypair.py @@ -17,6 +17,9 @@ from nova import exception from nova import objects from nova.objects import base from nova.objects import fields +from nova import utils + +KEYPAIR_TYPE_SSH = 'ssh' # TODO(berrange): Remove NovaObjectDictCompat @@ -24,7 +27,8 @@ class KeyPair(base.NovaPersistentObject, base.NovaObject, base.NovaObjectDictCompat): # Version 1.0: Initial version # Version 1.1: String attributes updated to support unicode - VERSION = '1.1' + # Version 1.2: Added keypair type + VERSION = '1.2' fields = { 'id': fields.IntegerField(), @@ -32,8 +36,15 @@ class KeyPair(base.NovaPersistentObject, base.NovaObject, 'user_id': fields.StringField(nullable=True), 'fingerprint': fields.StringField(nullable=True), 'public_key': fields.StringField(nullable=True), + 'type': fields.StringField(nullable=False), } + def obj_make_compatible(self, primitive, target_version): + super(KeyPair, self).obj_make_compatible(primitive, target_version) + target_version = utils.convert_version_to_tuple(target_version) + if target_version < (1, 2) and 'type' in primitive: + del primitive['type'] + @staticmethod def _from_db_object(context, keypair, db_keypair): for key in keypair.fields: @@ -68,7 +79,8 @@ class KeyPair(base.NovaPersistentObject, base.NovaObject, class KeyPairList(base.ObjectListBase, base.NovaObject): # Version 1.0: Initial version # KeyPair <= version 1.1 - VERSION = '1.0' + # Version 1.1: KeyPair <= version 1.2 + VERSION = '1.1' fields = { 'objects': fields.ListOfObjectsField('KeyPair'), @@ -76,6 +88,7 @@ class KeyPairList(base.ObjectListBase, base.NovaObject): child_versions = { '1.0': '1.1', # NOTE(danms): KeyPair was at 1.1 before we added this + '1.1': '1.2', } @base.remotable_classmethod diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index b002eb3998..88cf1dbc8f 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -5790,6 +5790,7 @@ class KeyPairTestCase(test.TestCase, ModelsObjectComparatorMixin): def test_key_pair_create(self): param = { 'name': 'test_1', + 'type': 'ssh', 'user_id': 'test_user_id_1', 'public_key': 'test_public_key_1', 'fingerprint': 'test_fingerprint_1' @@ -5802,16 +5803,17 @@ class KeyPairTestCase(test.TestCase, ModelsObjectComparatorMixin): self._assertEqualObjects(key_pair, param, ignored_keys) def test_key_pair_create_with_duplicate_name(self): - params = {'name': 'test_name', 'user_id': 'test_user_id'} + params = {'name': 'test_name', 'user_id': 'test_user_id', + 'type': 'ssh'} self._create_key_pair(params) self.assertRaises(exception.KeyPairExists, self._create_key_pair, params) def test_key_pair_get(self): params = [ - {'name': 'test_1', 'user_id': 'test_user_id_1'}, - {'name': 'test_2', 'user_id': 'test_user_id_2'}, - {'name': 'test_3', 'user_id': 'test_user_id_3'} + {'name': 'test_1', 'user_id': 'test_user_id_1', 'type': 'ssh'}, + {'name': 'test_2', 'user_id': 'test_user_id_2', 'type': 'ssh'}, + {'name': 'test_3', 'user_id': 'test_user_id_3', 'type': 'ssh'} ] key_pairs = [self._create_key_pair(p) for p in params] @@ -5825,7 +5827,7 @@ class KeyPairTestCase(test.TestCase, ModelsObjectComparatorMixin): self.ctxt, param['user_id'], param['name']) def test_key_pair_get_deleted(self): - param = {'name': 'test_1', 'user_id': 'test_user_id_1'} + param = {'name': 'test_1', 'user_id': 'test_user_id_1', 'type': 'ssh'} key_pair_created = self._create_key_pair(param) db.key_pair_destroy(self.ctxt, param['user_id'], param['name']) @@ -5842,9 +5844,9 @@ class KeyPairTestCase(test.TestCase, ModelsObjectComparatorMixin): def test_key_pair_get_all_by_user(self): params = [ - {'name': 'test_1', 'user_id': 'test_user_id_1'}, - {'name': 'test_2', 'user_id': 'test_user_id_1'}, - {'name': 'test_3', 'user_id': 'test_user_id_2'} + {'name': 'test_1', 'user_id': 'test_user_id_1', 'type': 'ssh'}, + {'name': 'test_2', 'user_id': 'test_user_id_1', 'type': 'ssh'}, + {'name': 'test_3', 'user_id': 'test_user_id_2', 'type': 'ssh'} ] key_pairs_user_1 = [self._create_key_pair(p) for p in params if p['user_id'] == 'test_user_id_1'] @@ -5859,9 +5861,9 @@ class KeyPairTestCase(test.TestCase, ModelsObjectComparatorMixin): def test_key_pair_count_by_user(self): params = [ - {'name': 'test_1', 'user_id': 'test_user_id_1'}, - {'name': 'test_2', 'user_id': 'test_user_id_1'}, - {'name': 'test_3', 'user_id': 'test_user_id_2'} + {'name': 'test_1', 'user_id': 'test_user_id_1', 'type': 'ssh'}, + {'name': 'test_2', 'user_id': 'test_user_id_1', 'type': 'ssh'}, + {'name': 'test_3', 'user_id': 'test_user_id_2', 'type': 'ssh'} ] for p in params: self._create_key_pair(p) @@ -5873,7 +5875,7 @@ class KeyPairTestCase(test.TestCase, ModelsObjectComparatorMixin): self.assertEqual(count_2, 1) def test_key_pair_destroy(self): - param = {'name': 'test_1', 'user_id': 'test_user_id_1'} + param = {'name': 'test_1', 'user_id': 'test_user_id_1', 'type': 'ssh'} self._create_key_pair(param) db.key_pair_destroy(self.ctxt, param['user_id'], param['name']) diff --git a/nova/tests/unit/db/test_migrations.py b/nova/tests/unit/db/test_migrations.py index f97f17add8..2c9c0862d1 100644 --- a/nova/tests/unit/db/test_migrations.py +++ b/nova/tests/unit/db/test_migrations.py @@ -758,6 +758,35 @@ class NovaMigrationsCheckers(test_migrations.ModelsMigrationsSync, self.assertIndexNotExists(engine, 'instances', 'instances_project_id_deleted_idx') + def _pre_upgrade_275(self, engine): + # Create a keypair record so we can test that the upgrade will set + # 'ssh' as default value in the new column for the previous keypair + # entries. + key_pairs = oslodbutils.get_table(engine, 'key_pairs') + fake_keypair = {'name': 'test-migr'} + key_pairs.insert().execute(fake_keypair) + + def _check_275(self, engine, data): + self.assertColumnExists(engine, 'key_pairs', 'type') + self.assertColumnExists(engine, 'shadow_key_pairs', 'type') + + key_pairs = oslodbutils.get_table(engine, 'key_pairs') + shadow_key_pairs = oslodbutils.get_table(engine, 'shadow_key_pairs') + self.assertIsInstance(key_pairs.c.type.type, + sqlalchemy.types.String) + self.assertIsInstance(shadow_key_pairs.c.type.type, + sqlalchemy.types.String) + + # Make sure the keypair entry will have the type 'ssh' + key_pairs = oslodbutils.get_table(engine, 'key_pairs') + keypair = key_pairs.select( + key_pairs.c.name == 'test-migr').execute().first() + self.assertEqual('ssh', keypair.type) + + def _post_downgrade_275(self, engine): + self.assertColumnNotExists(engine, 'key_pairs', 'type') + self.assertColumnNotExists(engine, 'shadow_key_pairs', 'type') + class TestNovaMigrationsSQLite(NovaMigrationsCheckers, test.TestCase, diff --git a/nova/tests/unit/objects/test_keypair.py b/nova/tests/unit/objects/test_keypair.py index cd1630cf7e..c8f2d807db 100644 --- a/nova/tests/unit/objects/test_keypair.py +++ b/nova/tests/unit/objects/test_keypair.py @@ -27,6 +27,7 @@ fake_keypair = { 'deleted': False, 'id': 123, 'name': 'foo-keypair', + 'type': 'ssh', 'user_id': 'fake-user', 'fingerprint': 'fake-fingerprint', 'public_key': 'fake\npublic\nkey', @@ -98,6 +99,13 @@ class _TestKeyPairObject(object): self.assertEqual(1, keypair.KeyPairList.get_count_by_user(self.context, 'fake-user')) + def test_obj_make_compatible(self): + keypair_obj = keypair.KeyPair(context=self.context) + fake_keypair_copy = dict(fake_keypair) + + keypair_obj.obj_make_compatible(fake_keypair_copy, '1.1') + self.assertNotIn('type', fake_keypair_copy) + class TestMigrationObject(test_objects._LocalTest, _TestKeyPairObject): diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 12d34fe234..b55d724f77 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1151,8 +1151,8 @@ object_data = { 'InstanceNUMATopology': '1.1-86b95d263c4c68411d44c6741b8d2bb0', 'InstancePCIRequest': '1.1-e082d174f4643e5756ba098c47c1510f', 'InstancePCIRequests': '1.1-bc7c6684d8579ee49d6a3b8aef756918', - 'KeyPair': '1.1-3410f51950d052d861c11946a6ae621a', - 'KeyPairList': '1.0-71132a568cc5d078ba1748a9c02c87b8', + 'KeyPair': '1.2-adf0be7b68e0b9f1ec011e23a9761354', + 'KeyPairList': '1.1-152dc1efcc46014cc10656a0d0ac5bb0', 'Migration': '1.1-67c47726c2c71422058cd9d149d6d3ed', 'MigrationList': '1.1-8c5f678edc72a592d591a13b35e54353', 'MyObj': '1.6-02b1e712b7ee334fa3fefe024c340977',