Treat endpoints with trailing slashes the same way as without them

We've been historically using endpoints without trailing slashes in
our API. Apparently, some libraries (like gophercloud) are quite
opinionated about it (see the story), so let's handle both.

The implementation could be simpler if we just added trailing slash
to all routes, but it would cause redirects for current users.

Change-Id: Icbd971a8e792f93f9c3fa66ba29bec055dcdee32
Story: #2007660
Task: #39749
This commit is contained in:
Dmitry Tantsur 2020-05-12 14:57:14 +02:00
parent b5d3214eb4
commit 5ccef9cd06
3 changed files with 25 additions and 8 deletions

View File

@ -172,8 +172,9 @@ def add_version_headers(res):
def create_link_object(urls): def create_link_object(urls):
links = [] links = []
for url in urls: for url in urls:
links.append({"rel": "self", links.append({
"href": os.path.join(flask.request.url_root, url)}) "rel": "self",
"href": os.path.join(flask.request.url_root, url).rstrip('/')})
return links return links
@ -181,7 +182,7 @@ def generate_resource_data(resources):
data = [] data = []
for resource in resources: for resource in resources:
item = {} item = {}
item['name'] = str(resource).split('/')[-1] item['name'] = str(resource).rstrip('/').split('/')[-1]
item['links'] = create_link_object([str(resource)[1:]]) item['links'] = create_link_object([str(resource)[1:]])
data.append(item) data.append(item)
return data return data
@ -226,6 +227,11 @@ def api(path, is_public_api=False, rule=None, verb_to_rule_map=None,
and strings to format the 'rule' string with and strings to format the 'rule' string with
:param kwargs: all the rest kwargs are passed to flask app.route :param kwargs: all the rest kwargs are passed to flask app.route
""" """
# Force uniform behavior with regards to trailing slashes
if not path.endswith('/'):
path = path + '/'
flask_kwargs['strict_slashes'] = False
def outer(func): def outer(func):
@_app.route(path, **flask_kwargs) @_app.route(path, **flask_kwargs)
@convert_exceptions @convert_exceptions
@ -264,7 +270,7 @@ def api_root():
@api('/<version>', rule='introspection:version', is_public_api=True, @api('/<version>', rule='introspection:version', is_public_api=True,
methods=['GET']) methods=['GET'])
def version_root(version): def version_root(version):
pat = re.compile(r'^\/%s\/[^\/]*?$' % version) pat = re.compile(r'^\/%s\/[^\/]*?/?$' % version)
resources = [] resources = []
for url in _app.url_map.iter_rules(): for url in _app.url_map.iter_rules():
@ -272,7 +278,7 @@ def version_root(version):
resources.append(url) resources.append(url)
if not resources: if not resources:
raise utils.Error(_('Version not found.'), code=404) raise utils.Error(_('Version %s not found.') % version, code=404)
return flask.jsonify(resources=generate_resource_data(resources)) return flask.jsonify(resources=generate_resource_data(resources))
@ -392,7 +398,7 @@ def api_introspection_reapply(node_id):
def rule_repr(rule, short): def rule_repr(rule, short):
result = rule.as_dict(short=short) result = rule.as_dict(short=short)
result['links'] = [{ result['links'] = [{
'href': flask.url_for('api_rule', uuid=result['uuid']), 'href': flask.url_for('api_rule', uuid=result['uuid']).rstrip('/'),
'rel': 'self' 'rel': 'self'
}] }]
return result return result

View File

@ -582,8 +582,8 @@ class TestApiVersions(BaseAPITest):
@mock.patch.object(main._app.url_map, "iter_rules", autospec=True) @mock.patch.object(main._app.url_map, "iter_rules", autospec=True)
def test_version_endpoint(self, mock_rules): def test_version_endpoint(self, mock_rules):
mock_rules.return_value = ["/v1/endpoint1", "/v1/endpoint2/<uuid>", mock_rules.return_value = ["/v1/endpoint1", "/v1/endpoint2/<uuid>",
"/v1/endpoint1/<name>", "/v1/endpoint1/<name>/",
"/v2/endpoint1", "/v1/endpoint3", "/v2/endpoint1", "/v1/endpoint3/",
"/v1/endpoint2/<uuid>/subpoint"] "/v1/endpoint2/<uuid>/subpoint"]
endpoint = "/v1" endpoint = "/v1"
res = self.app.get(endpoint) res = self.app.get(endpoint)
@ -606,6 +606,12 @@ class TestApiVersions(BaseAPITest):
]} ]}
self.assertEqual(expected, json_data) self.assertEqual(expected, json_data)
def test_version_endpoint_with_slash(self):
endpoint = "/v1/"
res = self.app.get(endpoint)
self.assertEqual(200, res.status_code)
self._check_version_present(res)
def test_version_endpoint_invalid(self): def test_version_endpoint_invalid(self):
endpoint = "/v-1" endpoint = "/v-1"
res = self.app.get(endpoint) res = self.app.get(endpoint)

View File

@ -0,0 +1,5 @@
---
fixes:
- |
Fixes accessing API endpoints with trailing slashes. Now they're treated
the same way as without slashes, although the latter remain canonical URLs.