Merge "Remove SecretsHelper"
This commit is contained in:
Zuul
Gerrit Code Review
commit
5c8faafff3
@ -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
Block a user