openstack/python-cinderclient
Code Issues Proposed changes

Fix max version handling for help output

Browse Source 
Commit fefe331f218d73ba6d1d7acf81b5eb02609c953e incorrectly set the
default API version to the max the client knew about in order to get the
full help output, including all microversioned commands.

The original intent was to have help print out information for all
versions, but still require the user to specify a version if they wanted
to use any microversioned version of an API. The code accidentally made
it so all commands would request the max version the client knew about.
This meant an unspecified request would get the newer functionality
rather than the default v3 functionality, and also meant the client
could request a microversion higher than what the server knew about,
resulting in an unexpected error being returned.

To keep the originally intended functionality, but keep all help output,
this only uses the max API version for the help command unless the user
specifies otherwise.

Closes-bug: #1813967

Change-Id: I20f6c5471ffefe5524a4d48c967e2e8db53233f1
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
This commit is contained in:
Sean McGinnis 2019-01-30 12:20:26 -06:00
parent cca017218a
commit bccbd510a7
2 changed files with 17 additions and 2 deletions
cinderclient
shell.py
tests/unit
test_shell.py

7
cinderclient/shell.py

@ -46,6 +46,7 @@ from cinderclient import exceptions as exc
from cinderclient import utils
DEFAULT_MAJOR_OS_VOLUME_API_VERSION = "3"
DEFAULT_CINDER_ENDPOINT_TYPE = 'publicURL'
V1_SHELL = 'cinderclient.v1.shell'
V2_SHELL = 'cinderclient.v2.shell'
@ -527,8 +528,10 @@ class OpenStackCinderShell(object):
'--help' in argv) or ('-h' in argv) or not argv
if not options.os_volume_api_version:
api_version = api_versions.get_api_version(
api_versions.MAX_VERSION)
use_version = DEFAULT_MAJOR_OS_VOLUME_API_VERSION
if do_help:
use_version = api_versions.MAX_VERSION
api_version = api_versions.get_api_version(use_version)
else:
api_version = api_versions.get_api_version(
options.os_volume_api_version)

12
cinderclient/tests/unit/test_shell.py

@ -114,9 +114,11 @@ class ShellTest(utils.TestCase):
self.assertRaises(exceptions.CommandError, self.shell, 'help foofoo')
def test_help(self):
# Some expected help output, including microversioned commands
required = [
'.*?^usage: ',
'.*?(?m)^\s+create\s+Creates a volume.',
'.*?(?m)^\s+summary\s+Get volumes summary.',
'.*?(?m)^Run "cinder help SUBCOMMAND" for help on a subcommand.',
]
help_text = self.shell('help')
@ -134,6 +136,16 @@ class ShellTest(utils.TestCase):
self.assertThat(help_text,
matchers.MatchesRegex(r, re.DOTALL | re.MULTILINE))
def test_help_on_subcommand_mv(self):
required = [
'.*?^usage: cinder summary',
'.*?(?m)^Get volumes summary.',
]
help_text = self.shell('help summary')
for r in required:
self.assertThat(help_text,
matchers.MatchesRegex(r, re.DOTALL | re.MULTILINE))
@ddt.data('backup-create --help', '--help backup-create')
def test_dash_dash_help_on_subcommand(self, cmd):
required = ['.*?^Creates a volume backup.']