Remove SecretsHelper
This code hasn't worked for over two years when
9ed9ab68f7 was added
which does API version discovery, which requires auth,
which happens before the SecretsHelper is used. That was
back in the 2.27.0 release in the Liberty series.
Given how long this has been broken without anyone noticing,
and it's undocumented and untested (basically), and
python-openstackclient properly handles prompting for a password,
we should just remove this code rather than try to fix it.
Change-Id: I62188e73a48f6878ce920a3b4724dba101564aef
Closes-Bug: #1732744
This commit is contained in:
Matt Riedemann
parent
3637f6bf16
commit
8bfcd2b0e6
|
|
@ -20,7 +20,6 @@ Command-line interface to the OpenStack Nova API.
|
|||
|
||||
from __future__ import print_function
|
||||
import argparse
|
||||
import getpass
|
||||
import logging
|
||||
import sys
|
||||
|
||||
|
|
@ -32,14 +31,6 @@ import six
|
|||
|
||||
osprofiler_profiler = importutils.try_import("osprofiler.profiler")
|
||||
|
||||
HAS_KEYRING = False
|
||||
all_errors = ValueError
|
||||
try:
|
||||
import keyring
|
||||
HAS_KEYRING = True
|
||||
except ImportError:
|
||||
pass
|
||||
|
||||
import novaclient
|
||||
from novaclient import api_versions
|
||||
from novaclient import client
|
||||
|
|
@ -210,133 +201,6 @@ class DeprecatedAction(argparse.Action):
|
|||
action(parser, namespace, values, option_string)
|
||||
|
||||
|
||||
class SecretsHelper(object):
|
||||
def __init__(self, args, client):
|
||||
self.args = args
|
||||
self.client = client
|
||||
self.key = None
|
||||
self._password = None
|
||||
|
||||
def _validate_string(self, text):
|
||||
if text is None or len(text) == 0:
|
||||
return False
|
||||
return True
|
||||
|
||||
def _make_key(self):
|
||||
if self.key is not None:
|
||||
return self.key
|
||||
keys = [
|
||||
self.client.auth_url,
|
||||
self.client.projectid,
|
||||
self.client.user,
|
||||
self.client.region_name,
|
||||
self.client.endpoint_type,
|
||||
self.client.service_type,
|
||||
self.client.service_name,
|
||||
]
|
||||
for (index, key) in enumerate(keys):
|
||||
if key is None:
|
||||
keys[index] = '?'
|
||||
else:
|
||||
keys[index] = str(keys[index])
|
||||
self.key = "/".join(keys)
|
||||
return self.key
|
||||
|
||||
def _prompt_password(self, verify=True):
|
||||
pw = None
|
||||
if hasattr(sys.stdin, 'isatty') and sys.stdin.isatty():
|
||||
# Check for Ctl-D
|
||||
try:
|
||||
while True:
|
||||
pw1 = getpass.getpass('OS Password: ')
|
||||
if verify:
|
||||
pw2 = getpass.getpass('Please verify: ')
|
||||
else:
|
||||
pw2 = pw1
|
||||
if pw1 == pw2 and self._validate_string(pw1):
|
||||
pw = pw1
|
||||
break
|
||||
except EOFError:
|
||||
pass
|
||||
return pw
|
||||
|
||||
def save(self, auth_token, management_url, tenant_id):
|
||||
if not HAS_KEYRING or not self.args.os_cache:
|
||||
return
|
||||
if (auth_token == self.auth_token and
|
||||
management_url == self.management_url):
|
||||
# Nothing changed....
|
||||
return
|
||||
if not all([management_url, auth_token, tenant_id]):
|
||||
raise ValueError(_("Unable to save empty management url/auth "
|
||||
"token"))
|
||||
value = "|".join([str(auth_token),
|
||||
str(management_url),
|
||||
str(tenant_id)])
|
||||
keyring.set_password("novaclient_auth", self._make_key(), value)
|
||||
|
||||
@property
|
||||
def password(self):
|
||||
# Cache password so we prompt user at most once
|
||||
if self._password:
|
||||
pass
|
||||
elif self._validate_string(self.args.os_password):
|
||||
self._password = self.args.os_password
|
||||
else:
|
||||
verify_pass = strutils.bool_from_string(
|
||||
utils.env("OS_VERIFY_PASSWORD", default=False), True)
|
||||
self._password = self._prompt_password(verify_pass)
|
||||
if not self._password:
|
||||
raise exc.CommandError(
|
||||
'Expecting a password provided via either '
|
||||
'--os-password, env[OS_PASSWORD], or '
|
||||
'prompted response')
|
||||
return self._password
|
||||
|
||||
@property
|
||||
def management_url(self):
|
||||
if not HAS_KEYRING or not self.args.os_cache:
|
||||
return None
|
||||
management_url = None
|
||||
try:
|
||||
block = keyring.get_password('novaclient_auth', self._make_key())
|
||||
if block:
|
||||
_token, management_url, _tenant_id = block.split('|', 2)
|
||||
except all_errors:
|
||||
pass
|
||||
return management_url
|
||||
|
||||
@property
|
||||
def auth_token(self):
|
||||
# Now is where it gets complicated since we
|
||||
# want to look into the keyring module, if it
|
||||
# exists and see if anything was provided in that
|
||||
# file that we can use.
|
||||
if not HAS_KEYRING or not self.args.os_cache:
|
||||
return None
|
||||
token = None
|
||||
try:
|
||||
block = keyring.get_password('novaclient_auth', self._make_key())
|
||||
if block:
|
||||
token, _management_url, _tenant_id = block.split('|', 2)
|
||||
except all_errors:
|
||||
pass
|
||||
return token
|
||||
|
||||
@property
|
||||
def tenant_id(self):
|
||||
if not HAS_KEYRING or not self.args.os_cache:
|
||||
return None
|
||||
tenant_id = None
|
||||
try:
|
||||
block = keyring.get_password('novaclient_auth', self._make_key())
|
||||
if block:
|
||||
_token, _management_url, tenant_id = block.split('|', 2)
|
||||
except all_errors:
|
||||
pass
|
||||
return tenant_id
|
||||
|
||||
|
||||
class NovaClientArgumentParser(argparse.ArgumentParser):
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
|
|
@ -688,7 +552,6 @@ class OpenStackComputeShell(object):
|
|||
|
||||
# We may have either, both or none of these.
|
||||
# If we have both, we don't need USERNAME, PASSWORD etc.
|
||||
# Fill in the blanks from the SecretsHelper if possible.
|
||||
# Finally, authenticate unless we have both.
|
||||
# Note if we don't auth we probably don't have a tenant ID so we can't
|
||||
# cache the token.
|
||||
|
|
@ -847,27 +710,6 @@ class OpenStackComputeShell(object):
|
|||
user_domain_id=os_user_domain_id,
|
||||
user_domain_name=os_user_domain_name)
|
||||
|
||||
# Now check for the password/token of which pieces of the
|
||||
# identifying keyring key can come from the underlying client
|
||||
if must_auth:
|
||||
helper = SecretsHelper(args, self.cs.client)
|
||||
self.cs.client.keyring_saver = helper
|
||||
|
||||
tenant_id = helper.tenant_id
|
||||
# Allow commandline to override cache
|
||||
if not auth_token:
|
||||
auth_token = helper.auth_token
|
||||
endpoint_override = endpoint_override or helper.management_url
|
||||
if tenant_id and auth_token and endpoint_override:
|
||||
self.cs.client.tenant_id = tenant_id
|
||||
self.cs.client.auth_token = auth_token
|
||||
self.cs.client.management_url = endpoint_override
|
||||
self.cs.client.password_func = lambda: helper.password
|
||||
else:
|
||||
# We're missing something, so auth with user/pass and save
|
||||
# the result in our helper.
|
||||
self.cs.client.password = helper.password
|
||||
|
||||
args.func(self.cs, args)
|
||||
|
||||
if osprofiler_profiler and args.profile:
|
||||
|
|
|
|||
|
|
@ -576,20 +576,6 @@ class ShellTest(utils.TestCase):
|
|||
stdout, stderr = self.shell('list')
|
||||
self.assertEqual((stdout + stderr), ex)
|
||||
|
||||
@mock.patch('sys.stdin', side_effect=mock.MagicMock)
|
||||
@mock.patch('getpass.getpass', side_effect=EOFError)
|
||||
def test_no_password(self, mock_getpass, mock_stdin):
|
||||
required = ('Expecting a password provided'
|
||||
' via either --os-password, env[OS_PASSWORD],'
|
||||
' or prompted response',)
|
||||
self.make_env(exclude='OS_PASSWORD')
|
||||
try:
|
||||
self.shell('list')
|
||||
except exceptions.CommandError as message:
|
||||
self.assertEqual(required, message.args)
|
||||
else:
|
||||
self.fail('CommandError not raised')
|
||||
|
||||
def _test_service_type(self, version, service_type, mock_client):
|
||||
if version is None:
|
||||
cmd = 'list'
|
||||
|
|
@ -666,25 +652,6 @@ class ShellTest(utils.TestCase):
|
|||
self.assertIn('unrecognized arguments: --profile swordfish',
|
||||
stderr)
|
||||
|
||||
@mock.patch('novaclient.shell.SecretsHelper.tenant_id',
|
||||
return_value=True)
|
||||
@mock.patch('novaclient.shell.SecretsHelper.auth_token',
|
||||
return_value=True)
|
||||
@mock.patch('novaclient.shell.SecretsHelper.management_url',
|
||||
return_value=True)
|
||||
@requests_mock.Mocker()
|
||||
def test_keyring_saver_helper(self,
|
||||
sh_management_url_function,
|
||||
sh_auth_token_function,
|
||||
sh_tenant_id_function,
|
||||
m_requests):
|
||||
self.make_env(fake_env=FAKE_ENV)
|
||||
self.register_keystone_discovery_fixture(m_requests)
|
||||
self.shell('list')
|
||||
mock_client_instance = self.mock_client.return_value
|
||||
keyring_saver = mock_client_instance.client.keyring_saver
|
||||
self.assertIsInstance(keyring_saver, novaclient.shell.SecretsHelper)
|
||||
|
||||
def test_microversion_with_default_behaviour(self):
|
||||
self.make_env(fake_env=FAKE_ENV5)
|
||||
self.mock_server_version_range.return_value = (
|
||||
|
|
|
|||
Loading…
Reference in New Issue