Gate-unbreaking combo review
Fix argument precedence hack Working around issues in os-client-config <= 1.18.0 This is ugly because the issues in o-c-c 1.19.1 run even deeper than in 1.18.0, so we're going to use 1.19.0 get_one_cloud() that is known to work for OSC and fix o-c-c with an axe. Remove return values for set commands 'identity provider set' and 'service provider set' were still returning their show-like data, this is a fail for set commands now, don't know how this ever passed before... Constraints are ready to be used for tox.ini Per email[1] from Andreas, we don't need to hack at install_command any longer. [1] http://openstack.markmail.org/thread/a4l7tokbotwqvuoh Co-authorioed-by: Steve Martinelli <s.martinelli@gmail.com> Depends-On: I49313dc7d4f44ec897de7a375f25b7ed864226f1 Change-Id: I426548376fc7d3cdb36501310dafd8c44d22ae30
This commit is contained in:
Dean Troyer
parent
0b91368164
commit
2a1a174086
|
|
@ -0,0 +1,186 @@
|
|||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
#
|
||||
|
||||
"""OpenStackConfig subclass for argument compatibility"""
|
||||
|
||||
import logging
|
||||
|
||||
from os_client_config.config import OpenStackConfig
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
||||
# Sublcass OpenStackConfig in order to munge config values
|
||||
# before auth plugins are loaded
|
||||
class OSC_Config(OpenStackConfig):
|
||||
|
||||
def _auth_select_default_plugin(self, config):
|
||||
"""Select a default plugin based on supplied arguments
|
||||
|
||||
Migrated from auth.select_auth_plugin()
|
||||
"""
|
||||
|
||||
identity_version = config.get('identity_api_version', '')
|
||||
|
||||
if config.get('username', None) and not config.get('auth_type', None):
|
||||
if identity_version == '3':
|
||||
config['auth_type'] = 'v3password'
|
||||
elif identity_version.startswith('2'):
|
||||
config['auth_type'] = 'v2password'
|
||||
else:
|
||||
# let keystoneauth figure it out itself
|
||||
config['auth_type'] = 'password'
|
||||
elif config.get('token', None) and not config.get('auth_type', None):
|
||||
if identity_version == '3':
|
||||
config['auth_type'] = 'v3token'
|
||||
elif identity_version.startswith('2'):
|
||||
config['auth_type'] = 'v2token'
|
||||
else:
|
||||
# let keystoneauth figure it out itself
|
||||
config['auth_type'] = 'token'
|
||||
else:
|
||||
# The ultimate default is similar to the original behaviour,
|
||||
# but this time with version discovery
|
||||
if not config.get('auth_type', None):
|
||||
config['auth_type'] = 'password'
|
||||
|
||||
LOG.debug("Auth plugin %s selected" % config['auth_type'])
|
||||
return config
|
||||
|
||||
def _auth_v2_arguments(self, config):
|
||||
"""Set up v2-required arguments from v3 info
|
||||
|
||||
Migrated from auth.build_auth_params()
|
||||
"""
|
||||
|
||||
if ('auth_type' in config and config['auth_type'].startswith("v2")):
|
||||
if 'project_id' in config['auth']:
|
||||
config['auth']['tenant_id'] = config['auth']['project_id']
|
||||
if 'project_name' in config['auth']:
|
||||
config['auth']['tenant_name'] = config['auth']['project_name']
|
||||
return config
|
||||
|
||||
def _auth_v2_ignore_v3(self, config):
|
||||
"""Remove v3 arguemnts if present for v2 plugin
|
||||
|
||||
Migrated from clientmanager.setup_auth()
|
||||
"""
|
||||
|
||||
# NOTE(hieulq): If USER_DOMAIN_NAME, USER_DOMAIN_ID, PROJECT_DOMAIN_ID
|
||||
# or PROJECT_DOMAIN_NAME is present and API_VERSION is 2.0, then
|
||||
# ignore all domain related configs.
|
||||
if (config.get('identity_api_version', '').startswith('2') and
|
||||
config.get('auth_type', None).endswith('password')):
|
||||
domain_props = [
|
||||
'project_domain_id',
|
||||
'project_domain_name',
|
||||
'user_domain_id',
|
||||
'user_domain_name',
|
||||
]
|
||||
for prop in domain_props:
|
||||
if config['auth'].pop(prop, None) is not None:
|
||||
LOG.warning("Ignoring domain related config " +
|
||||
prop + " because identity API version is 2.0")
|
||||
return config
|
||||
|
||||
def _auth_default_domain(self, config):
|
||||
"""Set a default domain from available arguments
|
||||
|
||||
Migrated from clientmanager.setup_auth()
|
||||
"""
|
||||
|
||||
identity_version = config.get('identity_api_version', '')
|
||||
auth_type = config.get('auth_type', None)
|
||||
|
||||
# TODO(mordred): This is a usability improvement that's broadly useful
|
||||
# We should port it back up into os-client-config.
|
||||
default_domain = config.get('default_domain', None)
|
||||
if (identity_version == '3' and
|
||||
not auth_type.startswith('v2') and
|
||||
default_domain):
|
||||
|
||||
# NOTE(stevemar): If PROJECT_DOMAIN_ID or PROJECT_DOMAIN_NAME is
|
||||
# present, then do not change the behaviour. Otherwise, set the
|
||||
# PROJECT_DOMAIN_ID to 'OS_DEFAULT_DOMAIN' for better usability.
|
||||
if (
|
||||
not config['auth'].get('project_domain_id') and
|
||||
not config['auth'].get('project_domain_name')
|
||||
):
|
||||
config['auth']['project_domain_id'] = default_domain
|
||||
|
||||
# NOTE(stevemar): If USER_DOMAIN_ID or USER_DOMAIN_NAME is present,
|
||||
# then do not change the behaviour. Otherwise, set the
|
||||
# USER_DOMAIN_ID to 'OS_DEFAULT_DOMAIN' for better usability.
|
||||
# NOTE(aloga): this should only be set if there is a username.
|
||||
# TODO(dtroyer): Move this to os-client-config after the plugin has
|
||||
# been loaded so we can check directly if the options are accepted.
|
||||
if (
|
||||
auth_type in ("password", "v3password", "v3totp") and
|
||||
not config['auth'].get('user_domain_id') and
|
||||
not config['auth'].get('user_domain_name')
|
||||
):
|
||||
config['auth']['user_domain_id'] = default_domain
|
||||
return config
|
||||
|
||||
def auth_config_hook(self, config):
|
||||
"""Allow examination of config values before loading auth plugin
|
||||
|
||||
OpenStackClient will override this to perform additional chacks
|
||||
on auth_type.
|
||||
"""
|
||||
|
||||
config = self._auth_select_default_plugin(config)
|
||||
config = self._auth_v2_arguments(config)
|
||||
config = self._auth_v2_ignore_v3(config)
|
||||
config = self._auth_default_domain(config)
|
||||
|
||||
LOG.debug("auth_config_hook(): %s" % config)
|
||||
return config
|
||||
|
||||
def _validate_auth_ksc(self, config, cloud):
|
||||
"""Old compatibility hack for OSC, no longer needed/wanted"""
|
||||
return config
|
||||
|
||||
def _validate_auth(self, config, loader):
|
||||
"""Validate auth plugin arguments"""
|
||||
# May throw a keystoneauth1.exceptions.NoMatchingPlugin
|
||||
|
||||
plugin_options = loader.get_options()
|
||||
|
||||
for p_opt in plugin_options:
|
||||
# if it's in config, win, move it and kill it from config dict
|
||||
# if it's in config.auth but not in config we're good
|
||||
# deprecated loses to current
|
||||
# provided beats default, deprecated or not
|
||||
winning_value = self._find_winning_auth_value(p_opt, config)
|
||||
if not winning_value:
|
||||
winning_value = self._find_winning_auth_value(
|
||||
p_opt, config['auth'])
|
||||
|
||||
# Clean up after ourselves
|
||||
for opt in [p_opt.name] + [o.name for o in p_opt.deprecated]:
|
||||
opt = opt.replace('-', '_')
|
||||
config.pop(opt, None)
|
||||
config['auth'].pop(opt, None)
|
||||
|
||||
if winning_value:
|
||||
# Prefer the plugin configuration dest value if the value's key
|
||||
# is marked as depreciated.
|
||||
if p_opt.dest is None:
|
||||
config['auth'][p_opt.name.replace('-', '_')] = (
|
||||
winning_value)
|
||||
else:
|
||||
config['auth'][p_opt.dest] = winning_value
|
||||
|
||||
return config
|
||||
|
|
@ -204,11 +204,10 @@ class SetIdentityProvider(command.Command):
|
|||
if parsed_args.remote_id_file or parsed_args.remote_id:
|
||||
kwargs['remote_ids'] = remote_ids
|
||||
|
||||
identity_provider = federation_client.identity_providers.update(
|
||||
parsed_args.identity_provider, **kwargs)
|
||||
|
||||
identity_provider._info.pop('links', None)
|
||||
return zip(*sorted(six.iteritems(identity_provider._info)))
|
||||
federation_client.identity_providers.update(
|
||||
parsed_args.identity_provider,
|
||||
**kwargs
|
||||
)
|
||||
|
||||
|
||||
class ShowIdentityProvider(command.ShowOne):
|
||||
|
|
|
|||
|
|
@ -182,12 +182,13 @@ class SetServiceProvider(command.Command):
|
|||
elif parsed_args.disable is True:
|
||||
enabled = False
|
||||
|
||||
service_provider = federation_client.service_providers.update(
|
||||
parsed_args.service_provider, enabled=enabled,
|
||||
federation_client.service_providers.update(
|
||||
parsed_args.service_provider,
|
||||
enabled=enabled,
|
||||
description=parsed_args.description,
|
||||
auth_url=parsed_args.auth_url,
|
||||
sp_url=parsed_args.service_provider_url)
|
||||
return zip(*sorted(six.iteritems(service_provider._info)))
|
||||
sp_url=parsed_args.service_provider_url,
|
||||
)
|
||||
|
||||
|
||||
class ShowServiceProvider(command.ShowOne):
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ from oslo_utils import importutils
|
|||
import six
|
||||
|
||||
import openstackclient
|
||||
from openstackclient.common import client_config as cloud_config
|
||||
from openstackclient.common import clientmanager
|
||||
from openstackclient.common import commandmanager
|
||||
|
||||
|
|
@ -127,9 +128,33 @@ class OpenStackShell(shell.OpenStackShell):
|
|||
def initialize_app(self, argv):
|
||||
super(OpenStackShell, self).initialize_app(argv)
|
||||
|
||||
# For now we need to build our own ClientManager so re-do what
|
||||
# has already been done :(
|
||||
# TODO(dtroyer): remove when osc-lib is fixed
|
||||
# Argument precedence is really broken in multiple places
|
||||
# so we're just going to fix it here until o-c-c and osc-lib
|
||||
# get sorted out.
|
||||
# TODO(dtroyer): remove when os-client-config and osc-lib are fixed
|
||||
|
||||
# First, throw away what has already been done with o-c-c and
|
||||
# use our own.
|
||||
try:
|
||||
cc = cloud_config.OSC_Config(
|
||||
override_defaults={
|
||||
'interface': None,
|
||||
'auth_type': self._auth_type,
|
||||
},
|
||||
)
|
||||
except (IOError, OSError) as e:
|
||||
self.log.critical("Could not read clouds.yaml configuration file")
|
||||
self.print_help_if_requested()
|
||||
raise e
|
||||
|
||||
if not self.options.debug:
|
||||
self.options.debug = None
|
||||
self.cloud = cc.get_one_cloud(
|
||||
cloud=self.options.cloud,
|
||||
argparse=self.options,
|
||||
)
|
||||
|
||||
# Then, re-create the client_manager with the correct arguments
|
||||
self.client_manager = clientmanager.ClientManager(
|
||||
cli_options=self.cloud,
|
||||
api_version=self.api_version,
|
||||
|
|
|
|||
|
|
@ -356,19 +356,11 @@ class TestIdentityProviderSet(TestIdentityProvider):
|
|||
('remote_id', None)
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
self.cmd.take_action(parsed_args)
|
||||
self.identity_providers_mock.update.assert_called_with(
|
||||
identity_fakes.idp_id,
|
||||
description=new_description,
|
||||
)
|
||||
self.assertEqual(self.columns, columns)
|
||||
datalist = (
|
||||
identity_fakes.idp_description,
|
||||
False,
|
||||
identity_fakes.idp_id,
|
||||
identity_fakes.idp_remote_ids
|
||||
)
|
||||
self.assertEqual(datalist, data)
|
||||
|
||||
def test_identity_provider_disable(self):
|
||||
"""Disable Identity Provider
|
||||
|
|
@ -402,22 +394,13 @@ class TestIdentityProviderSet(TestIdentityProvider):
|
|||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
self.cmd.take_action(parsed_args)
|
||||
self.identity_providers_mock.update.assert_called_with(
|
||||
identity_fakes.idp_id,
|
||||
enabled=False,
|
||||
remote_ids=identity_fakes.idp_remote_ids
|
||||
)
|
||||
|
||||
self.assertEqual(self.columns, columns)
|
||||
datalist = (
|
||||
identity_fakes.idp_description,
|
||||
False,
|
||||
identity_fakes.idp_id,
|
||||
identity_fakes.idp_remote_ids
|
||||
)
|
||||
self.assertEqual(datalist, data)
|
||||
|
||||
def test_identity_provider_enable(self):
|
||||
"""Enable Identity Provider.
|
||||
|
||||
|
|
@ -448,12 +431,10 @@ class TestIdentityProviderSet(TestIdentityProvider):
|
|||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
self.cmd.take_action(parsed_args)
|
||||
self.identity_providers_mock.update.assert_called_with(
|
||||
identity_fakes.idp_id, enabled=True,
|
||||
remote_ids=identity_fakes.idp_remote_ids)
|
||||
self.assertEqual(self.columns, columns)
|
||||
self.assertEqual(self.datalist, data)
|
||||
|
||||
def test_identity_provider_replace_remote_ids(self):
|
||||
"""Enable Identity Provider.
|
||||
|
|
@ -488,18 +469,10 @@ class TestIdentityProviderSet(TestIdentityProvider):
|
|||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
self.cmd.take_action(parsed_args)
|
||||
self.identity_providers_mock.update.assert_called_with(
|
||||
identity_fakes.idp_id, enabled=True,
|
||||
remote_ids=[self.new_remote_id])
|
||||
self.assertEqual(self.columns, columns)
|
||||
datalist = (
|
||||
identity_fakes.idp_description,
|
||||
True,
|
||||
identity_fakes.idp_id,
|
||||
[self.new_remote_id]
|
||||
)
|
||||
self.assertEqual(datalist, data)
|
||||
|
||||
def test_identity_provider_replace_remote_ids_file(self):
|
||||
"""Enable Identity Provider.
|
||||
|
|
@ -538,18 +511,10 @@ class TestIdentityProviderSet(TestIdentityProvider):
|
|||
mocker.return_value = self.new_remote_id
|
||||
with mock.patch("openstackclient.identity.v3.identity_provider."
|
||||
"utils.read_blob_file_contents", mocker):
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
self.cmd.take_action(parsed_args)
|
||||
self.identity_providers_mock.update.assert_called_with(
|
||||
identity_fakes.idp_id, enabled=True,
|
||||
remote_ids=[self.new_remote_id])
|
||||
self.assertEqual(self.columns, columns)
|
||||
datalist = (
|
||||
identity_fakes.idp_description,
|
||||
True,
|
||||
identity_fakes.idp_id,
|
||||
[self.new_remote_id]
|
||||
)
|
||||
self.assertEqual(datalist, data)
|
||||
|
||||
def test_identity_provider_no_options(self):
|
||||
def prepare(self):
|
||||
|
|
@ -580,12 +545,7 @@ class TestIdentityProviderSet(TestIdentityProvider):
|
|||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
|
||||
# expect take_action() to return (None, None) as
|
||||
# neither --enable nor --disable was specified
|
||||
self.assertEqual(self.columns, columns)
|
||||
self.assertEqual(self.datalist, data)
|
||||
self.cmd.take_action(parsed_args)
|
||||
|
||||
|
||||
class TestIdentityProviderShow(TestIdentityProvider):
|
||||
|
|
|
|||
|
|
@ -289,7 +289,7 @@ class TestServiceProviderSet(TestServiceProvider):
|
|||
('disable', True),
|
||||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
self.cmd.take_action(parsed_args)
|
||||
self.service_providers_mock.update.assert_called_with(
|
||||
service_fakes.sp_id,
|
||||
enabled=False,
|
||||
|
|
@ -298,9 +298,6 @@ class TestServiceProviderSet(TestServiceProvider):
|
|||
sp_url=None
|
||||
)
|
||||
|
||||
self.assertEqual(self.columns, columns)
|
||||
self.assertEqual(self.datalist, data)
|
||||
|
||||
def test_service_provider_enable(self):
|
||||
"""Enable Service Provider.
|
||||
|
||||
|
|
@ -327,19 +324,10 @@ class TestServiceProviderSet(TestServiceProvider):
|
|||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
self.cmd.take_action(parsed_args)
|
||||
self.service_providers_mock.update.assert_called_with(
|
||||
service_fakes.sp_id, enabled=True, description=None,
|
||||
auth_url=None, sp_url=None)
|
||||
self.assertEqual(self.columns, columns)
|
||||
datalist = (
|
||||
service_fakes.sp_auth_url,
|
||||
service_fakes.sp_description,
|
||||
True,
|
||||
service_fakes.sp_id,
|
||||
service_fakes.service_provider_url
|
||||
)
|
||||
self.assertEqual(datalist, data)
|
||||
|
||||
def test_service_provider_no_options(self):
|
||||
def prepare(self):
|
||||
|
|
@ -372,20 +360,7 @@ class TestServiceProviderSet(TestServiceProvider):
|
|||
]
|
||||
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
|
||||
|
||||
columns, data = self.cmd.take_action(parsed_args)
|
||||
|
||||
# expect take_action() to return (None, None) as none of --disabled,
|
||||
# --enabled, --description, --service-provider-url, --auth_url option
|
||||
# was set.
|
||||
self.assertEqual(self.columns, columns)
|
||||
datalist = (
|
||||
service_fakes.sp_auth_url,
|
||||
service_fakes.sp_description,
|
||||
True,
|
||||
service_fakes.sp_id,
|
||||
service_fakes.service_provider_url
|
||||
)
|
||||
self.assertEqual(datalist, data)
|
||||
self.cmd.take_action(parsed_args)
|
||||
|
||||
|
||||
class TestServiceProviderShow(TestServiceProvider):
|
||||
|
|
|
|||
4
tox.ini
4
tox.ini
|
|
@ -55,8 +55,6 @@ setenv = OS_TEST_PATH=./functional/tests
|
|||
passenv = OS_*
|
||||
|
||||
[testenv:venv]
|
||||
# TODO(ihrachys): remove once infra supports constraints for this target
|
||||
install_command = pip install -U {opts} {packages}
|
||||
commands = {posargs}
|
||||
|
||||
[testenv:cover]
|
||||
|
|
@ -71,8 +69,6 @@ commands = oslo_debug_helper -t openstackclient/tests {posargs}
|
|||
commands = python setup.py build_sphinx
|
||||
|
||||
[testenv:releasenotes]
|
||||
# TODO(ihrachys): remove once infra supports constraints for this target
|
||||
install_command = pip install -U {opts} {packages}
|
||||
commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html
|
||||
|
||||
[flake8]
|
||||
|
|
|
|||
Loading…
Reference in New Issue