Remove the need for project_id from API endpoints

Inclusion of a project_id in API URLs is now optional, and no longer
required. Removing the project_id requirement facilitates supporting
Secure RBAC's notion of system scope, in which an API method is not
associated with a specific project.

The API v3 routing is enhanced to provide duplicate routes for API
methods that traditionally required a project_id in the URL:
- The existing route for which a project_id is in the URL
- A new route for when the URL does not include a project_id

To test both routes and ensure there are no regresssions, the "API
samples" functional tests include a project_id in the URLs, and the
rest of the functional tests do not include the project_id. This is
handled by changing the 'noauth' WSGI middleware to no longer add the
project_id, and adding a new 'noauth_include_project_id' middleware
filter that implements the legacy behavior.

A new microversion V3.67 is introduced, but it only serves to inform
clients whether the project_id is optional or required. When an API
node supports mv 3.67, the project_id is optional in all API requests,
even when the request specifies a earlier microversion. See the spec
Ia44f199243be8f862520d7923007e7182b32f67d for more details on this
behavior.

Note: Much of the groundwork for this is based on manila's patch
I5127e150e8a71e621890f30dba6720b3932cf583.

DocImpact
APIImpact

Implements: blueprint project-id-optional-in-urls
Change-Id: I3729cbe1902ab4dc335451d13ed921ec236fb8fd

api-ref/source/v3/index.rst

@@ -4,6 +4,25 @@
 Block Storage API V3 (CURRENT)
 ==============================
 
+.. note::
+   The URL for most API methods includes a {project_id} placeholder that
+   represents the caller's project ID. As of V3.67, the project_id is optional
+   in the URL, and the following are equivalent:
+
+      * GET /v3/{project_id}/volumes
+      * GET /v3/volumes
+
+   In both instances, the actual project_id used by the API method is the one
+   in the caller's keystone context. For that reason, including a project_id
+   in the URL is redundant.
+
+   The V3.67 microversion is only used as an indicator that the API accepts a
+   URL without a project_id, and this applies to all requests regardless of
+   the microversion in the request. For example, an API node serving V3.67 or
+   greater will accept a URL without a project_id even if the request asks for
+   V3.0. Likewise, it will accept a URL containing a project_id even if the
+   request asks for V3.67.
+
 .. rest_expand_all::
 
 .. First thing we want to see is the version discovery document.

api-ref/source/v3/samples/versions/version-show-response.json

@@ -21,8 +21,8 @@
             ],
             "min_version": "3.0",
             "status": "CURRENT",
-            "updated": "2021-09-16T00:00:00Z",
-            "version": "3.66"
+            "updated": "2021-12-16T00:00:00Z",
+            "version": "3.67"
         }
     ]
 }

api-ref/source/v3/samples/versions/versions-response.json

@@ -21,8 +21,8 @@
             ],
             "min_version": "3.0",
             "status": "CURRENT",
-            "updated": "2021-09-16T00:00:00Z",
-            "version": "3.66"
+            "updated": "2021-12-16T00:00:00Z",
+            "version": "3.67"
         }
     ]
 }

cinder/api/common.py

@@ -229,6 +229,14 @@ class ViewBuilder(object):
 
     _collection_name = None
 
+    def _get_project_id_in_url(self, request):
+        project_id = request.environ["cinder.context"].project_id
+        if project_id and ("/v3/%s" % project_id in request.url):
+            # project_ids are not mandatory within v3 URLs, but links need
+            # to include them if the request does.
+            return project_id
+        return ''
+
     def _get_links(self, request, identifier):
         return [{"rel": "self",
                  "href": self._get_href_link(request, identifier), },
@@ -242,7 +250,7 @@ def _get_next_link(self, request, identifier, collection_name):
         prefix = self._update_link_prefix(get_request_url(request),
                                           CONF.public_endpoint)
         url = os.path.join(prefix,
-                           request.environ["cinder.context"].project_id,
+                           self._get_project_id_in_url(request),
                            collection_name)
         return "%s?%s" % (url, urllib.parse.urlencode(params))
 
@@ -251,7 +259,7 @@ def _get_href_link(self, request, identifier):
         prefix = self._update_link_prefix(get_request_url(request),
                                           CONF.public_endpoint)
         return os.path.join(prefix,
-                            request.environ["cinder.context"].project_id,
+                            self._get_project_id_in_url(request),
                             self._collection_name,
                             str(identifier))
 
@@ -261,7 +269,7 @@ def _get_bookmark_link(self, request, identifier):
         base_url = self._update_link_prefix(base_url,
                                             CONF.public_endpoint)
         return os.path.join(base_url,
-                            request.environ["cinder.context"].project_id,
+                            self._get_project_id_in_url(request),
                             self._collection_name,
                             str(identifier))
 

cinder/api/middleware/auth.py

@@ -125,15 +125,17 @@ def __call__(self, req):
         return self.application
 
 
-class NoAuthMiddleware(base_wsgi.Middleware):
+class NoAuthMiddlewareBase(base_wsgi.Middleware):
     """Return a fake token if one isn't specified."""
 
-    @webob.dec.wsgify(RequestClass=wsgi.Request)
-    def __call__(self, req):
+    def base_call(self, req, project_id_in_path=False):
         if 'X-Auth-Token' not in req.headers:
             user_id = req.headers.get('X-Auth-User', 'admin')
             project_id = req.headers.get('X-Auth-Project-Id', 'admin')
-            os_url = os.path.join(req.url, project_id)
+            if project_id_in_path:
+                os_url = os.path.join(req.url.rstrip('/'), project_id)
+            else:
+                os_url = req.url.rstrip('/')
             res = webob.Response()
             # NOTE(vish): This is expecting and returning Auth(1.1), whereas
             #             keystone uses 2.0 auth.  We should probably allow
@@ -157,3 +159,24 @@ def __call__(self, req):
 
         req.environ['cinder.context'] = ctx
         return self.application
+
+
+class NoAuthMiddleware(NoAuthMiddlewareBase):
+    """Return a fake token if one isn't specified.
+
+    Sets project_id in URLs.
+    """
+
+    @webob.dec.wsgify(RequestClass=wsgi.Request)
+    def __call__(self, req):
+        return self.base_call(req)
+
+
+class NoAuthMiddlewareIncludeProjectID(NoAuthMiddlewareBase):
+    """Return a fake token if one isn't specified.
+
+    Does not set project_id in URLs.
+    """
+    @webob.dec.wsgify(RequestClass=wsgi.Request)
+    def __call__(self, req):
+        return self.base_call(req, project_id_in_path=True)

cinder/api/openstack/__init__.py

@@ -18,6 +18,7 @@
 WSGI middleware for OpenStack API controllers.
 """
 
+from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_service import wsgi as base_wsgi
 import routes
@@ -26,6 +27,16 @@
 from cinder.i18n import _
 
 
+openstack_api_opts = [
+    cfg.StrOpt('project_id_regex',
+               default=r"[0-9a-f\-]+",
+               help=r'The validation regex for project_ids used in urls. '
+                    r'This defaults to [0-9a-f\\-]+ if not set, '
+                    r'which matches normal uuids created by keystone.'),
+]
+
+CONF = cfg.CONF
+CONF.register_opts(openstack_api_opts)
 LOG = logging.getLogger(__name__)
 
 
@@ -48,14 +59,42 @@ def connect(self, *args, **kwargs):
 
 class ProjectMapper(APIMapper):
     def resource(self, member_name, collection_name, **kwargs):
+        """Base resource path handler
+
+        This method is compatible with resource paths that include a
+        project_id and those that don't. Including project_id in the URLs
+        was a legacy API requirement; and making API requests against
+        such endpoints won't work for users that don't belong to a
+        particular project.
+        """
+        # NOTE: project_id parameter is only valid if its hex or hex + dashes
+        # (note, integers are a subset of this). This is required to handle
+        # our overlapping routes issues.
+        project_id_regex = CONF.project_id_regex
+        project_id_token = '{project_id:%s}' % project_id_regex
+        if 'parent_resource' not in kwargs:
+            kwargs['path_prefix'] = '%s/' % project_id_token
+        else:
+            parent_resource = kwargs['parent_resource']
+            p_collection = parent_resource['collection_name']
+            p_member = parent_resource['member_name']
+            kwargs['path_prefix'] = '%s/%s/:%s_id' % (project_id_token,
+                                                      p_collection,
+                                                      p_member)
+        routes.Mapper.resource(self,
+                               member_name,
+                               collection_name,
+                               **kwargs)
+
+        # Add additional routes without project_id.
         if 'parent_resource' not in kwargs:
-            kwargs['path_prefix'] = '{project_id}/'
+            del kwargs['path_prefix']
         else:
             parent_resource = kwargs['parent_resource']
             p_collection = parent_resource['collection_name']
             p_member = parent_resource['member_name']
-            kwargs['path_prefix'] = '{project_id}/%s/:%s_id' % (p_collection,
-                                                                p_member)
+            kwargs['path_prefix'] = '%s/:%s_id' % (p_collection,
+                                                   p_member)
         routes.Mapper.resource(self,
                                member_name,
                                collection_name,

cinder/api/openstack/api_version_request.py

@@ -152,14 +152,15 @@
            - Accept 'consumes_quota' filter in volume and snapshot list
              operation.
     * 3.66 - Allow snapshotting in-use volumes without force flag.
+    * 3.67 - API URLs no longer need to include a project_id parameter.
 """
 
 # The minimum and maximum versions of the API supported
 # The default api version request is defined to be the
 # minimum version of the API supported.
 _MIN_API_VERSION = "3.0"
-_MAX_API_VERSION = "3.66"
-UPDATED = "2021-09-16T00:00:00Z"
+_MAX_API_VERSION = "3.67"
+UPDATED = "2021-11-02T00:00:00Z"
 
 
 # NOTE(cyeoh): min and max versions declared as functions so we can

cinder/api/openstack/rest_api_version_history.rst

@@ -505,3 +505,11 @@ Volume snapshots of in-use volumes can be created without the 'force' flag.
 Although the 'force' flag is now considered invalid when passed in a volume
 snapshot request, for backward compatibility, the 'force' flag with a value
 evaluating to True is silently ignored.
+
+3.67
+----
+API URLs no longer need a "project_id" argument in them. For example, the API
+route: ``https://$(controller)s/volume/v3/$(project_id)s/volumes`` is
+equivalent to ``https://$(controller)s/volume/v3/volumes``. When interacting
+with the cinder service as system or domain scoped users, a project_id should
+not be specified in the API path.

cinder/api/v3/router.py

@@ -94,27 +94,31 @@ def _setup_routes(self, mapper, ext_mgr):
                         controller=self.resources['groups'],
                         collection={'detail': 'GET'},
                         member={'action': 'POST'})
-        mapper.connect("groups",
-                       "/{project_id}/groups/{id}/action",
-                       controller=self.resources["groups"],
-                       action="action",
-                       conditions={"method": ["POST"]})
-        mapper.connect("groups/action",
-                       "/{project_id}/groups/action",
-                       controller=self.resources["groups"],
-                       action="action",
-                       conditions={"method": ["POST"]})
+        for path_prefix in ['/{project_id}', '']:
+            # project_id is optional
+            mapper.connect("groups",
+                           "%s/groups/{id}/action" % path_prefix,
+                           controller=self.resources["groups"],
+                           action="action",
+                           conditions={"method": ["POST"]})
+            mapper.connect("groups/action",
+                           "%s/groups/action" % path_prefix,
+                           controller=self.resources["groups"],
+                           action="action",
+                           conditions={"method": ["POST"]})
 
         self.resources['group_snapshots'] = group_snapshots.create_resource()
         mapper.resource("group_snapshot", "group_snapshots",
                         controller=self.resources['group_snapshots'],
                         collection={'detail': 'GET'},
                         member={'action': 'POST'})
-        mapper.connect("group_snapshots",
-                       "/{project_id}/group_snapshots/{id}/action",
-                       controller=self.resources["group_snapshots"],
-                       action="action",
-                       conditions={"method": ["POST"]})
+        for path_prefix in ['/{project_id}', '']:
+            # project_id is optional
+            mapper.connect("group_snapshots",
+                           "%s/group_snapshots/{id}/action" % path_prefix,
+                           controller=self.resources["group_snapshots"],
+                           action="action",
+                           conditions={"method": ["POST"]})
         self.resources['snapshots'] = snapshots.create_resource(ext_mgr)
         mapper.resource("snapshot", "snapshots",
                         controller=self.resources['snapshots'],
@@ -134,11 +138,13 @@ def _setup_routes(self, mapper, ext_mgr):
                         parent_resource=dict(member_name='snapshot',
                                              collection_name='snapshots'))
 
-        mapper.connect("metadata",
-                       "/{project_id}/snapshots/{snapshot_id}/metadata",
-                       controller=snapshot_metadata_controller,
-                       action='update_all',
-                       conditions={"method": ['PUT']})
+        for path_prefix in ['/{project_id}', '']:
+            # project_id is optional
+            mapper.connect("metadata",
+                           "%s/snapshots/{snapshot_id}/metadata" % path_prefix,
+                           controller=snapshot_metadata_controller,
+                           action='update_all',
+                           conditions={"method": ['PUT']})
 
         self.resources['volume_metadata'] = volume_metadata.create_resource()
         volume_metadata_controller = self.resources['volume_metadata']
@@ -148,11 +154,13 @@ def _setup_routes(self, mapper, ext_mgr):
                         parent_resource=dict(member_name='volume',
                                              collection_name='volumes'))
 
-        mapper.connect("metadata",
-                       "/{project_id}/volumes/{volume_id}/metadata",
-                       controller=volume_metadata_controller,
-                       action='update_all',
-                       conditions={"method": ['PUT']})
+        for path_prefix in ['/{project_id}', '']:
+            # project_id is optional
+            mapper.connect("metadata",
+                           "%s/volumes/{volume_id}/metadata" % path_prefix,
+                           controller=volume_metadata_controller,
+                           action='update_all',
+                           conditions={"method": ['PUT']})
 
         self.resources['consistencygroups'] = (
             consistencygroups.create_resource())

cinder/common/config.py

@@ -147,9 +147,12 @@
     cfg.StrOpt('auth_strategy',
                default='keystone',
                choices=[('noauth', 'Do not perform authentication'),
+                        ('noauth_include_project_id',
+                         'Do not perform authentication, and include a'
+                         ' project_id in API URLs'),
                         ('keystone', 'Authenticate using keystone')],
-               help='The strategy to use for auth. Supports noauth or '
-                    'keystone.'),
+               help='The strategy to use for auth. Supports noauth,'
+                    ' noauth_include_project_id or keystone.'),
 ]
 
 backup_opts = [

cinder/opts.py

@@ -29,6 +29,7 @@
 objects.register_all()
 from cinder.api import common as cinder_api_common
 from cinder.api.middleware import auth as cinder_api_middleware_auth
+import cinder.api.openstack
 from cinder.api.views import versions as cinder_api_views_versions
 from cinder.backup import api as cinder_backup_api
 from cinder.backup import chunkeddriver as cinder_backup_chunkeddriver
@@ -216,6 +217,7 @@ def list_opts():
             itertools.chain(
                 cinder_api_common.api_common_opts,
                 [cinder_api_middleware_auth.use_forwarded_for_opt],
+                cinder.api.openstack.openstack_api_opts,
                 cinder_api_views_versions.versions_opts,
                 cinder_backup_api.backup_opts,
                 cinder_backup_chunkeddriver.backup_opts,

cinder/tests/functional/api_samples_test_base.py

@@ -78,6 +78,22 @@ def setUp(self):
         # this is used to generate sample docs
         self.generate_samples = os.getenv('GENERATE_SAMPLES') is not None
 
+    def _get_flags(self):
+        f = super()._get_flags()
+
+        # Use noauth_include_project_id so the API samples tests include a
+        # project_id in the API URLs. This is done for two reasons:
+        #
+        # 1. The API samples generated by the tests need to include a
+        # project_id because the API documentation includes the project_id.
+        #
+        # 2. It ensures there are no regressions, and the API functions
+        # correctly when a project_id is in the URL. The other functional
+        # tests do not include the project_id, so we cover both variants.
+        f['auth_strategy'] = {'v': 'noauth_include_project_id'}
+
+        return f
+
     @property
     def subs(self):
         return self._subs