Skip to content

Commit

Permalink
Do not try to use /v1/v1 when endpoint_override is used
Browse files Browse the repository at this point in the history
In one of the places where endpoint_override is used we did not strip
the /v1 suffix, resulting in doube /v1/v1 sometimes.

This patch also corrects the way we strip this suffix (str.rstrip accepts
a set of symbols, not a substring; use regex instead).

It remains unclear why the breakage passed the gate initially. I assume
it was not on the active code path until some Nova change.

Story: #2005723
Task: #31051
Change-Id: I3b25f4fb170aa93159ffa8074dc74fa6f50671b7
(cherry picked from commit 4565af8)
  • Loading branch information
dtantsur committed May 17, 2019
1 parent 575ad1b commit 51fc3a7
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 9 deletions.
16 changes: 10 additions & 6 deletions ironicclient/common/http.py
Expand Up @@ -18,6 +18,7 @@
import hashlib
import logging
import os
import re
import socket
import ssl
import textwrap
Expand Down Expand Up @@ -50,7 +51,8 @@
USER_AGENT = 'python-ironicclient'
CHUNKSIZE = 1024 * 64 # 64kB

API_VERSION = '/v1'
_MAJOR_VERSION = 1
API_VERSION = '/v%d' % _MAJOR_VERSION
API_VERSION_SELECTED_STATES = ('user', 'negotiated', 'cached', 'default')


Expand All @@ -61,10 +63,12 @@

SUPPORTED_ENDPOINT_SCHEME = ('http', 'https')

_API_VERSION_RE = re.compile(r'/+(v%d)?/*$' % _MAJOR_VERSION)


def _trim_endpoint_api_version(url):
"""Trim API version and trailing slash from endpoint."""
return url.rstrip('/').rstrip(API_VERSION).rstrip('/')
return re.sub(_API_VERSION_RE, '', url)


def _extract_error_json(body):
Expand Down Expand Up @@ -604,6 +608,9 @@ def __init__(self,
'removed in Stein. Please use "endpoint_override" '
'instead.')
self.endpoint = endpoint
if isinstance(kwargs.get('endpoint_override'), six.string_types):
kwargs['endpoint_override'] = _trim_endpoint_api_version(
kwargs['endpoint_override'])

super(SessionClient, self).__init__(**kwargs)

Expand Down Expand Up @@ -634,10 +641,7 @@ def _http_request(self, url, method, **kwargs):
kwargs.setdefault('user_agent', USER_AGENT)
kwargs.setdefault('auth', self.auth)
if isinstance(self.endpoint_override, six.string_types):
kwargs.setdefault(
'endpoint_override',
_trim_endpoint_api_version(self.endpoint_override)
)
kwargs.setdefault('endpoint_override', self.endpoint_override)

if getattr(self, 'os_ironic_api_version', None):
kwargs['headers'].setdefault('X-OpenStack-Ironic-API-Version',
Expand Down
6 changes: 3 additions & 3 deletions ironicclient/tests/unit/test_client.py
Expand Up @@ -93,14 +93,14 @@ def test_get_client_only_ironic_url(self):
client = self._test_get_client(auth='none',
warn_mock_call_count=1, **kwargs)
self.assertIsInstance(client.http_client, http.SessionClient)
self.assertEqual('http://localhost:6385/v1',
self.assertEqual('http://localhost:6385',
client.http_client.endpoint_override)

def test_get_client_only_endpoint(self):
kwargs = {'endpoint': 'http://localhost:6385/v1'}
client = self._test_get_client(auth='none', **kwargs)
self.assertIsInstance(client.http_client, http.SessionClient)
self.assertEqual('http://localhost:6385/v1',
self.assertEqual('http://localhost:6385',
client.http_client.endpoint_override)

def test_get_client_with_auth_token_ironic_url(self):
Expand All @@ -113,7 +113,7 @@ def test_get_client_with_auth_token_ironic_url(self):
warn_mock_call_count=2, **kwargs)

self.assertIsInstance(client.http_client, http.SessionClient)
self.assertEqual('http://localhost:6385/v1',
self.assertEqual('http://localhost:6385',
client.http_client.endpoint_override)

def test_get_client_no_auth_token(self):
Expand Down
5 changes: 5 additions & 0 deletions releasenotes/notes/endpoint-strip-dea59ccb05628a35.yaml
@@ -0,0 +1,5 @@
---
fixes:
- |
Prevent trying to access endpoints with ``/v1/v1`` when using endpoint
overrides containing ``/v1``.

0 comments on commit 51fc3a7

Please sign in to comment.