api: Drop generating a keypair and add special chars to naming

sbauza · sbauza · commit a755e5d9f25c · 2022-07-28T11:05:50.000+02:00
As agreed in the spec, we will both drop the generation support for a keypair
but we'll also accept @ (at) and . (dot) chars in the keyname, all of them in
the same API microversion.

Rebased the work from I5de15935e83823afa545a250cf84f6a7a37036b4

APIImpact

Implements: blueprint keypair-generation-removal
Co-Authored-By: Nicolas Parquet &lt;nicolas.parquet@gandi.net&gt;

Change-Id: I6a7c71fb4385348c87067543d0454f302907395e
diff --git a/api-ref/source/os-keypairs.inc b/api-ref/source/os-keypairs.inc
@@ -44,12 +44,16 @@ Response
 .. literalinclude:: ../../doc/api_samples/os-keypairs/v2.35/keypairs-list-resp.json
    :language: javascript
 
-Create Or Import Keypair
-========================
+Import (or create) Keypair
+==========================
 
 .. rest_method:: POST /os-keypairs
 
-Generates or imports a keypair.
+Imports (or generates) a keypair.
+
+.. warning::
+
+   Generating a keypair is no longer possible starting from version 2.92.
 
 Normal response codes: 200, 201
 
@@ -65,7 +69,7 @@ Request
 .. rest_parameters:: parameters.yaml
 
   - keypair: keypair
-  - name: keypair_name
+  - name: keypair_name_in
   - public_key: keypair_public_key_in
   - type: keypair_type_in
   - user_id: keypair_userid_in
@@ -75,6 +79,11 @@ Request
 .. literalinclude:: ../../doc/api_samples/os-keypairs/v2.10/keypairs-import-post-req.json
    :language: javascript
 
+**Example Import Keypair (v2.92): JSON request**
+
+.. literalinclude:: ../../doc/api_samples/os-keypairs/v2.92/keypairs-import-post-req.json
+   :language: javascript
+
 Response
 --------
 
@@ -93,6 +102,11 @@ Response
 .. literalinclude:: ../../doc/api_samples/os-keypairs/v2.10/keypairs-import-post-resp.json
    :language: javascript
 
+**Example Import Keypair (v2.92): JSON response**
+
+.. literalinclude:: ../../doc/api_samples/os-keypairs/v2.92/keypairs-import-post-resp.json
+   :language: javascript
+
 Show Keypair Details
 ====================
 
diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml
@@ -4410,11 +4410,23 @@ keypair_links:
   required: false
   min_version: 2.35
 keypair_name:
+  in: body
+  required: true
+  type: string
+  description: |
+    The name for the keypair.
+keypair_name_in:
   in: body
   required: true
   type: string
   description: |
     A name for the keypair which will be used to reference it later.
+
+    .. note::
+
+      Since microversion 2.92, allowed characters are ASCII letters
+      ``[a-zA-Z]``, digits ``[0-9]`` and the following special
+      characters: ``[@._- ]``.
 keypair_private_key:
   description: |
     If you do not provide a public key on create, a new keypair will
@@ -4424,6 +4436,7 @@ keypair_private_key:
   in: body
   required: false
   type: string
+  max_version: 2.91
 keypair_public_key:
   description: |
     The keypair public key.
@@ -4432,10 +4445,11 @@ keypair_public_key:
   type: string
 keypair_public_key_in:
   description: |
-    The public ssh key to import. If you omit this value, a keypair is
-    generated for you.
+    The public ssh key to import.
+    Was optional before microversion 2.92 : if you were omitting this value, a
+    keypair was generated for you.
   in: body
-  required: false
+  required: true
   type: string
 keypair_type:
   in: body
diff --git a/doc/api_samples/os-keypairs/v2.92/keypairs-import-post-req.json b/doc/api_samples/os-keypairs/v2.92/keypairs-import-post-req.json
@@ -0,0 +1,8 @@
+{
+    "keypair": {
+        "name": "me.and.myself@this.nice.domain.com mooh.",
+        "type": "ssh",
+        "public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDx8nkQv/zgGgB4rMYmIf+6A4l6Rr+o/6lHBQdW5aYd44bd8JttDCE/F/pNRr0lRE+PiqSPO8nDPHw0010JeMH9gYgnnFlyY3/OcJ02RhIPyyxYpv9FhY+2YiUkpwFOcLImyrxEsYXpD/0d3ac30bNH6Sw9JD9UZHYcpSxsIbECHw== Generated-by-Nova",
+        "user_id": "fake"
+    }
+}
diff --git a/doc/api_samples/os-keypairs/v2.92/keypairs-import-post-resp.json b/doc/api_samples/os-keypairs/v2.92/keypairs-import-post-resp.json
@@ -0,0 +1,9 @@
+{
+    "keypair": {
+        "fingerprint": "1e:2c:9b:56:79:4b:45:77:f9:ca:7a:98:2c:b0:d5:3c",
+        "name": "me.and.myself@this.nice.domain.com mooh.",
+        "type": "ssh",
+        "public_key": "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDx8nkQv/zgGgB4rMYmIf+6A4l6Rr+o/6lHBQdW5aYd44bd8JttDCE/F/pNRr0lRE+PiqSPO8nDPHw0010JeMH9gYgnnFlyY3/OcJ02RhIPyyxYpv9FhY+2YiUkpwFOcLImyrxEsYXpD/0d3ac30bNH6Sw9JD9UZHYcpSxsIbECHw== Generated-by-Nova",
+        "user_id": "fake"
+    }
+}
diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json
@@ -19,7 +19,7 @@
             }
         ],
         "status": "CURRENT",
-        "version": "2.91",
+        "version": "2.92",
         "min_version": "2.1",
         "updated": "2013-07-23T11:33:21Z"
     }
diff --git a/doc/api_samples/versions/versions-get-resp.json b/doc/api_samples/versions/versions-get-resp.json
@@ -22,7 +22,7 @@
                 }
             ],
             "status": "CURRENT",
-            "version": "2.91",
+            "version": "2.92",
             "min_version": "2.1",
             "updated": "2013-07-23T11:33:21Z"
         }
diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py
@@ -249,6 +249,9 @@
              server responses regardless of policy configuration.
     * 2.91 - Add support to unshelve instance to a specific host and
              to pin/unpin AZ.
+    * 2.92 - Drop generation of keypair, add keypair name validation on
+             ``POST /os-keypairs`` and allow including @ and dot (.) characters
+             in keypair name.
 """
 
 # The minimum and maximum versions of the API supported
@@ -257,7 +260,7 @@
 # Note(cyeoh): This only applies for the v2.1 API once microversions
 # support is fully merged. It does not affect the V2 API.
 _MIN_API_VERSION = '2.1'
-_MAX_API_VERSION = '2.91'
+_MAX_API_VERSION = '2.92'
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 # Almost all proxy APIs which are related to network, images and baremetal
diff --git a/nova/api/openstack/compute/keypairs.py b/nova/api/openstack/compute/keypairs.py
@@ -43,15 +43,19 @@ def __init__(self):
     @wsgi.Controller.api_version("2.10")
     @wsgi.response(201)
     @wsgi.expected_errors((400, 403, 409))
-    @validation.schema(keypairs.create_v210)
+    @validation.schema(keypairs.create_v210, "2.10", "2.91")
+    @validation.schema(keypairs.create_v292, "2.92")
     def create(self, req, body):
         """Create or import keypair.
 
+        Keypair generations are allowed until version 2.91.
+        Afterwards, only imports are allowed.
+
         A policy check restricts users from creating keys for other users
 
         params: keypair object with:
             name (required) - string
-            public_key (optional) - string
+            public_key (optional or required if >=2.92) - string
             type (optional) - string
             user_id (optional) - string
         """
@@ -114,6 +118,8 @@ def _create(self, req, body, user_id=None, key_type=False):
                     context, user_id, name, params['public_key'],
                     key_type_value)
             else:
+                # public_key is a required field starting with 2.92 so this
+                # generation should only happen with older versions.
                 keypair, private_key = self.api.create_key_pair(
                     context, user_id, name, key_type_value)
                 keypair['private_key'] = private_key
diff --git a/nova/api/openstack/compute/rest_api_version_history.rst b/nova/api/openstack/compute/rest_api_version_history.rst
@@ -1211,3 +1211,11 @@ responses is now visible to all users. Previously this was an admin-only field.
 Add support to unshelve instance to a specific host.
 
 Add support to pin a server to an availability zone or unpin a server from any availability zone.
+
+.. _microversion 2.92:
+
+2.92
+----
+
+The ``POST /os-keypairs`` API now forbids to generate a keypair and allows new
+safe characters, specifically '@' and '.' (dot character).
diff --git a/nova/api/openstack/compute/schemas/keypairs.py b/nova/api/openstack/compute/schemas/keypairs.py
@@ -23,7 +23,7 @@
         'keypair': {
             'type': 'object',
             'properties': {
-                'name': parameter_types.name,
+                'name': parameter_types.keypair_name_special_chars,
                 'public_key': {'type': 'string'},
             },
             'required': ['name'],
@@ -46,7 +46,7 @@
         'keypair': {
             'type': 'object',
             'properties': {
-                'name': parameter_types.name,
+                'name': parameter_types.keypair_name_special_chars,
                 'type': {
                     'type': 'string',
                     'enum': ['ssh', 'x509']
@@ -67,7 +67,7 @@
         'keypair': {
             'type': 'object',
             'properties': {
-                'name': parameter_types.name,
+                'name': parameter_types.keypair_name_special_chars,
                 'type': {
                     'type': 'string',
                     'enum': ['ssh', 'x509']
@@ -83,6 +83,11 @@
     'additionalProperties': False,
 }
 
+create_v292 = copy.deepcopy(create_v210)
+create_v292['properties']['keypair']['properties']['name'] = (parameter_types.
+    keypair_name_special_chars_292)
+create_v292['properties']['keypair']['required'] = ['name', 'public_key']
+
 index_query_schema_v20 = {
     'type': 'object',
     'properties': {},
diff --git a/nova/api/validation/parameter_types.py b/nova/api/validation/parameter_types.py
@@ -290,7 +290,7 @@ def valid_char(char):
 
 name = {
     # NOTE: Nova v2.1 API contains some 'name' parameters such
-    # as keypair, server, flavor, aggregate and so on. They are
+    # as server, flavor, aggregate and so on. They are
     # stored in the DB and Nova specific parameters.
     # This definition is used for all their parameters.
     'type': 'string', 'minLength': 1, 'maxLength': 255,
@@ -304,6 +304,18 @@ def valid_char(char):
 }
 
 
+keypair_name_special_chars = {'allOf': [name, {
+    'type': 'string', 'minLength': 1, 'maxLength': 255,
+    'format': 'keypair_name_20'
+}]}
+
+
+keypair_name_special_chars_292 = {'allOf': [name, {
+    'type': 'string', 'minLength': 1, 'maxLength': 255,
+    'format': 'keypair_name_292'
+}]}
+
+
 az_name_with_leading_trailing_spaces = {
     'type': 'string', 'minLength': 1, 'maxLength': 255,
     'format': 'az_name_with_leading_trailing_spaces'
diff --git a/nova/api/validation/validators.py b/nova/api/validation/validators.py
@@ -17,6 +17,7 @@
 """
 
 import re
+import string
 
 import jsonschema
 from jsonschema import exceptions as jsonschema_exc
@@ -153,6 +154,28 @@ def _validate_az_name(instance):
     raise exception.InvalidName(reason=regex.reason)
 
 
+@jsonschema.FormatChecker.cls_checks('keypair_name_20',
+                                     exception.InvalidName)
+def _validate_keypair_name_20(keypair_name):
+    safe_chars = "_- " + string.digits + string.ascii_letters
+    return _validate_keypair_name(keypair_name, safe_chars)
+
+
+@jsonschema.FormatChecker.cls_checks('keypair_name_292',
+                                     exception.InvalidName)
+def _validate_keypair_name_292(keypair_name):
+    safe_chars = "@._- " + string.digits + string.ascii_letters
+    return _validate_keypair_name(keypair_name, safe_chars)
+
+
+def _validate_keypair_name(keypair_name, safe_chars):
+    clean_value = "".join(x for x in keypair_name if x in safe_chars)
+    if clean_value != keypair_name:
+        reason = _("Only expected characters: [%s]") % safe_chars
+        raise exception.InvalidName(reason=reason)
+    return True
+
+
 def _soft_validate_additional_properties(validator,
                                          additional_properties_value,
                                          instance,
diff --git a/nova/compute/api.py b/nova/compute/api.py
@@ -22,7 +22,6 @@
 import collections
 import functools
 import re
-import string
 import typing as ty
 
 from castellan import key_manager
@@ -6694,19 +6693,7 @@ def _notify(self, context, event_suffix, keypair_name):
         }
         self.notifier.info(context, 'keypair.%s' % event_suffix, payload)
 
-    def _validate_new_key_pair(self, context, user_id, key_name, key_type):
-        safe_chars = "_- " + string.digits + string.ascii_letters
-        clean_value = "".join(x for x in key_name if x in safe_chars)
-        if clean_value != key_name:
-            raise exception.InvalidKeypair(
-                reason=_("Keypair name contains unsafe characters"))
-
-        try:
-            utils.check_string_length(key_name, min_length=1, max_length=255)
-        except exception.InvalidInput:
-            raise exception.InvalidKeypair(
-                reason=_('Keypair name must be string and between '
-                         '1 and 255 characters long'))
+    def _check_key_pair_quotas(self, context, user_id, key_name, key_type):
         try:
             objects.Quotas.check_deltas(context, {'key_pairs': 1}, user_id)
             local_limit.enforce_db_limit(context, local_limit.KEY_PAIRS,
@@ -6720,7 +6707,7 @@ def _validate_new_key_pair(self, context, user_id, key_name, key_type):
     def import_key_pair(self, context, user_id, key_name, public_key,
                         key_type=keypair_obj.KEYPAIR_TYPE_SSH):
         """Import a key pair using an existing public key."""
-        self._validate_new_key_pair(context, user_id, key_name, key_type)
+        self._check_key_pair_quotas(context, user_id, key_name, key_type)
 
         self._notify(context, 'import.start', key_name)
 
@@ -6755,7 +6742,7 @@ def import_key_pair(self, context, user_id, key_name, public_key,
     def create_key_pair(self, context, user_id, key_name,
                         key_type=keypair_obj.KEYPAIR_TYPE_SSH):
         """Create a new key pair."""
-        self._validate_new_key_pair(context, user_id, key_name, key_type)
+        self._check_key_pair_quotas(context, user_id, key_name, key_type)
 
         keypair = objects.KeyPair(context)
         keypair.user_id = user_id
diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-keypairs/v2.92/keypairs-import-post-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-keypairs/v2.92/keypairs-import-post-req.json.tpl
@@ -0,0 +1,8 @@
+{
+    "keypair": {
+        "name": "%(keypair_name)s",
+        "type": "%(keypair_type)s",
+        "public_key": "%(public_key)s",
+        "user_id": "%(user_id)s"
+    }
+}
diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-keypairs/v2.92/keypairs-import-post-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-keypairs/v2.92/keypairs-import-post-resp.json.tpl
@@ -0,0 +1,9 @@
+{
+    "keypair": {
+        "fingerprint": "%(fingerprint)s",
+        "name": "%(keypair_name)s",
+        "type": "%(keypair_type)s",
+        "public_key": "%(public_key)s",
+        "user_id": "%(user_id)s"
+    }
+}
diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-keypairs/v2.92/keypairs-post-req.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-keypairs/v2.92/keypairs-post-req.json.tpl
@@ -0,0 +1,7 @@
+{
+    "keypair": {
+        "name": "%(keypair_name)s",
+        "type": "%(keypair_type)s",
+        "user_id": "%(user_id)s"
+    }
+}
diff --git a/nova/tests/functional/api_sample_tests/test_keypairs.py b/nova/tests/functional/api_sample_tests/test_keypairs.py
diff --git a/nova/tests/functional/notification_sample_tests/test_keypair.py b/nova/tests/functional/notification_sample_tests/test_keypair.py
diff --git a/nova/tests/functional/regressions/test_bug_1843708.py b/nova/tests/functional/regressions/test_bug_1843708.py
diff --git a/nova/tests/unit/api/openstack/compute/test_keypairs.py b/nova/tests/unit/api/openstack/compute/test_keypairs.py
diff --git a/nova/tests/unit/compute/test_keypairs.py b/nova/tests/unit/compute/test_keypairs.py
diff --git a/releasenotes/notes/bp-keypair-generation-removal-3004a8643dcd1fd9.yaml b/releasenotes/notes/bp-keypair-generation-removal-3004a8643dcd1fd9.yaml

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@`
`19`	`19`	`}`
`20`	`20`	`],`
`21`	`21`	`"status": "CURRENT",`
`22`		`- "version": "2.91",`
	`22`	`+ "version": "2.92",`
`23`	`23`	`"min_version": "2.1",`
`24`	`24`	`"updated": "2013-07-23T11:33:21Z"`
`25`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@`
`22`	`22`	`}`
`23`	`23`	`],`
`24`	`24`	`"status": "CURRENT",`
`25`		`- "version": "2.91",`
	`25`	`+ "version": "2.92",`
`26`	`26`	`"min_version": "2.1",`
`27`	`27`	`"updated": "2013-07-23T11:33:21Z"`
`28`	`28`	`}`