openstack
/
ironic
Code Issues Proposed changes

Fix swiftclient creation 

in the change I52f1386df45ebe0a43b11fe1583e012dfa3af532
we lost most of swiftclient options in a belief that those are handled
by the keystoneauth session passed to the swiftclient.

In fact though, swiftclient only uses this session to get itself an
endpoint and a token, but it has no SessionClient, and does not use that
passed in session to make further requests to swift itself.

This patch restores all the logic that we had to decompose the session
object loaded from config to options that are passed to swiftclient
explicitly.

Change-Id: I08f382aa9d2ad22f7dbd65f7b54a8dd0a765ba44
Partial-Bug: #1699547
Closes-Bug: #1736158
This commit is contained in:
Pavlo Shchelokovskyy 2017-06-29 09:03:18 +00:00 committed by Sam Betts
parent 27ce77142b
commit 9ee28252a4
7 changed files with 99 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
devstack/lib/ironic
View File

@ -1113,7 +1113,7 @@ function configure_ironic_conductor {
# TODO(pas-ha) this block is for transition period only,
# after all clients are moved to use keystoneauth adapters,
# it will be deleted
local sections_with_adapter="service_catalog glance cinder inspector"
local sections_with_adapter="service_catalog glance cinder inspector swift"
for conf_section in $sections_with_adapter; do
configure_adapter_for $conf_section
done

37
etc/ironic/ironic.conf.sample
View File

@ -3894,12 +3894,28 @@
# Domain name to scope to (string value)
#domain_name = <None>
# Always use this endpoint URL for requests for this client.
# (string value)
#endpoint_override = <None>
# Verify HTTPS connections. (boolean value)
#insecure = false
# PEM encoded client certificate key file (string value)
#keyfile = <None>
# The maximum major version of a given API, intended to be
# used as the upper bound of a range with min_version.
# Mutually exclusive with version. (string value)
#max_version = <None>
# The minimum major version of a given API, intended to be
# used as the lower bound of a range with max_version.
# Mutually exclusive with version. If min_version is given
# with no max_version it is as if max version is "latest".
# (string value)
#min_version = <None>
# User's password (string value)
#password = <None>
@ -3917,6 +3933,18 @@
# Deprecated group/name - [swift]/tenant_name
#project_name = <None>
# The default region_name for endpoint URL discovery. (string
# value)
#region_name = <None>
# The default service_name for endpoint URL discovery. (string
# value)
#service_name = <None>
# The default service_type for endpoint URL discovery. (string
# value)
#service_type = object-store
# Maximum number of times to retry a Swift request, before
# failing. (integer value)
#swift_max_retries = 2
@ -3945,3 +3973,12 @@
# Username (string value)
# Deprecated group/name - [swift]/user_name
#username = <None>
# List of interfaces, in order of preference, for endpoint
# URL. (list value)
#valid_interfaces = internal,public
# Minimum Major API version within a given Major API version
# for endpoint URL discovery. Mutually exclusive with
# min_version and max_version (string value)
#version = <None>

42
ironic/common/swift.py
View File

@ -14,6 +14,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import six
from six.moves import http_client
from six.moves.urllib import parse
from swiftclient import client as swift_client
@ -45,16 +46,41 @@ class SwiftAPI(object):
:raises: ConfigInvalid if required keystone authorization credentials
with swift are missing.
"""
params = {}
params = {'retries': CONF.swift.swift_max_retries}
if CONF.deploy.object_store_endpoint_type == 'radosgw':
params = {'authurl': CONF.swift.auth_url,
'user': CONF.swift.username,
'key': CONF.swift.password}
params.update({'authurl': CONF.swift.auth_url,
'user': CONF.swift.username,
'key': CONF.swift.password})
else:
# NOTE(aNuposic): Session will be initiated only when connection
# with swift is initialized. Since v3.2.0 swiftclient supports
# instantiating the API client from keystoneauth session.
params = {'session': _get_swift_session()}
# NOTE(pas-ha) swiftclient still (as of 3.3.0) does not use
# (adapter-based) SessionClient, and uses the passed in session
# only to resolve endpoint and get a token,
# but not to make further requests to Swift itself (LP 1736135).
# Thus we need to deconstruct back all the adapter- and
# session-related args as loaded by keystoneauth from config
# to pass them to the client explicitly.
# TODO(pas-ha) re-write this when swiftclient is brought on par
# with other OS clients re auth plugins, sessions and adapters
# support.
# TODO(pas-ha) pass the context here and use token from context
# with service auth
params['session'] = session = _get_swift_session()
adapter = keystone.get_adapter('swift', session=session)
params['os_options'] = {
'object_storage_url': adapter.get_endpoint()}
# deconstruct back session-related options
params['timeout'] = session.timeout
if session.verify is False:
params['insecure'] = True
elif isinstance(session.verify, six.string_types):
params['cacert'] = session.verify
if session.cert:
# NOTE(pas-ha) although setting cert as path to single file
# with both client cert and key is supported by Session,
# keystoneauth loading always sets the session.cert
# as tuple of cert and key.
params['cert'], params['cert_key'] = session.cert
self.connection = swift_client.Connection(**params)
def create_object(self, container, obj, filename,

4
ironic/conf/swift.py
View File

@ -29,8 +29,8 @@ opts = [
def register_opts(conf):
conf.register_opts(opts, group='swift')
auth.register_auth_opts(conf, 'swift')
auth.register_auth_opts(conf, 'swift', service_type='object-store')
def list_opts():
return auth.add_auth_opts(opts)
return auth.add_auth_opts(opts, service_type='object-store')

19
ironic/tests/unit/common/test_swift.py
View File

@ -32,7 +32,9 @@ if six.PY3:
file = io.BytesIO
@mock.patch.object(swift, '_get_swift_session', autospec=True)
@mock.patch.object(swift, '_get_swift_session', autospec=True,
return_value=mock.Mock(verify=False, cert=('spam', 'ham'),
timeout=42))
@mock.patch.object(swift_client, 'Connection', autospec=True)
class SwiftTestCase(base.TestCase):
@ -42,10 +44,18 @@ class SwiftTestCase(base.TestCase):
def test___init__(self, connection_mock, keystone_mock):
"""Check if client is properly initialized with swift"""
self.config(group='swift',
endpoint_override='http://example.com/objects')
swift.SwiftAPI()
connection_mock.assert_called_once_with(
session=keystone_mock.return_value)
retries=2,
session=keystone_mock.return_value,
timeout=42,
insecure=True,
cert='spam',
cert_key='ham',
os_options={'object_storage_url': 'http://example.com/objects'}
)
def test___init___radosgw(self, connection_mock, swift_session_mock):
"""Check if client is properly initialized with radosgw"""
@ -66,7 +76,8 @@ class SwiftTestCase(base.TestCase):
swift.SwiftAPI()
params = {'authurl': auth_url,
'user': username,
'key': password}
'key': password,
'retries': 2}
connection_mock.assert_called_once_with(**params)
self.assertFalse(swift_session_mock.called)

6
releasenotes/notes/fix-swift-ssl-options-d93d653dcd404960.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
Fixes a bug when SSL-related options in ``[swift]`` section of ironic
configuration file were ignored when performing API requests to Swift.
See https://launchpad.net/bugs/1736158 for more information.

4
releasenotes/notes/keystoneauth-config-1baa45a0a2dd93b4.yaml
View File

@ -12,3 +12,7 @@ features:
Adds the ability to set keystoneauth settings in the
``[inspector]`` configuration section for service automatic
discovery.
- |
Adds the ability to set keystoneauth settings in the
``[swift]`` configuration section for service automatic
discovery.