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',