diff --git a/glanceclient/common/utils.py b/glanceclient/common/utils.py index 8daafff..304fcd4 100644 --- a/glanceclient/common/utils.py +++ b/glanceclient/common/utils.py @@ -36,11 +36,15 @@ from oslo_utils import encodeutils from oslo_utils import strutils import prettytable +from glanceclient import _i18n from glanceclient import exc +_ = _i18n._ + _memoized_property_lock = threading.Lock() SENSITIVE_HEADERS = ('X-Auth-Token', ) +REQUIRED_FIELDS_ON_DATA = ('disk_format', 'container_format') # Decorator for cli-args @@ -53,6 +57,47 @@ def arg(*args, **kwargs): return _decorator +def on_data_require_fields(data_fields, required=REQUIRED_FIELDS_ON_DATA): + """Decorator to check commands' validity + + This decorator checks that required fields are present when image + data has been supplied via command line arguments or via stdin + + On error throws CommandError exception with meaningful message. + + :param data_fields: Which fields' presence imply image data + :type data_fields: iter + :param required: Required fields + :type required: iter + :return: function decorator + """ + + def args_decorator(func): + def prepare_fields(fields): + args = ('--' + x.replace('_', '-') for x in fields) + return ', '.join(args) + + def func_wrapper(gc, args): + # Set of arguments with data + fields = set(a[0] for a in vars(args).items() if a[1]) + + # Fields the conditional requirements depend on + present = fields.intersection(data_fields) + + # How many conditional requirements are missing + missing = set(required) - fields + + # We use get_data_file to check if data is provided in stdin + if (present or get_data_file(args)) and missing: + msg = (_("error: Must provide %(req)s when using %(opt)s.") % + {'req': prepare_fields(missing), + 'opt': prepare_fields(present) or 'stdin'}) + raise exc.CommandError(msg) + return func(gc, args) + return func_wrapper + return args_decorator + + def schema_args(schema_getter, omit=None): omit = omit or [] typemap = { diff --git a/glanceclient/tests/unit/v1/test_shell.py b/glanceclient/tests/unit/v1/test_shell.py index a3a41c2..dd3e3cf 100644 --- a/glanceclient/tests/unit/v1/test_shell.py +++ b/glanceclient/tests/unit/v1/test_shell.py @@ -232,6 +232,9 @@ class ShellInvalidEndpointandParameterTest(utils.TestCase): 'OS_IMAGE_URL': 'http://is.invalid'} self.shell = shell.OpenStackImagesShell() + self.patched = mock.patch('glanceclient.common.utils.get_data_file', + autospec=True, return_value=None) + self.mock_get_data_file = self.patched.start() self.gc = self._mock_glance_client() @@ -254,6 +257,7 @@ class ShellInvalidEndpointandParameterTest(utils.TestCase): def tearDown(self): super(ShellInvalidEndpointandParameterTest, self).tearDown() os.environ = self.old_environment + self.patched.stop() def run_command(self, cmd): self.shell.main(cmd.split()) @@ -323,6 +327,46 @@ class ShellInvalidEndpointandParameterTest(utils.TestCase): SystemExit, self.run_command, 'image-create --min-disk 10gb') + @mock.patch('sys.stderr') + def test_image_create_missing_disk_format(self, __): + # We test for all possible sources + for origin in ('--file', '--location', '--copy-from'): + e = self.assertRaises(exc.CommandError, self.run_command, + '--os-image-api-version 1 image-create ' + + origin + ' fake_src --container-format bare') + self.assertEqual('error: Must provide --disk-format when using ' + + origin + '.', e.message) + + @mock.patch('sys.stderr') + def test_image_create_missing_container_format(self, __): + # We test for all possible sources + for origin in ('--file', '--location', '--copy-from'): + e = self.assertRaises(exc.CommandError, self.run_command, + '--os-image-api-version 1 image-create ' + + origin + ' fake_src --disk-format qcow2') + self.assertEqual('error: Must provide --container-format when ' + 'using ' + origin + '.', e.message) + + @mock.patch('sys.stderr') + def test_image_create_missing_container_format_stdin_data(self, __): + # Fake that get_data_file method returns data + self.mock_get_data_file.return_value = six.StringIO() + e = self.assertRaises(exc.CommandError, self.run_command, + '--os-image-api-version 1 image-create' + ' --disk-format qcow2') + self.assertEqual('error: Must provide --container-format when ' + 'using stdin.', e.message) + + @mock.patch('sys.stderr') + def test_image_create_missing_disk_format_stdin_data(self, __): + # Fake that get_data_file method returns data + self.mock_get_data_file.return_value = six.StringIO() + e = self.assertRaises(exc.CommandError, self.run_command, + '--os-image-api-version 1 image-create' + ' --container-format bare') + self.assertEqual('error: Must provide --disk-format when using stdin.', + e.message) + @mock.patch('sys.stderr') def test_image_update_invalid_size_parameter(self, __): self.assertRaises( diff --git a/glanceclient/tests/unit/v2/test_shell_v2.py b/glanceclient/tests/unit/v2/test_shell_v2.py index faa12a7..e2678af 100644 --- a/glanceclient/tests/unit/v2/test_shell_v2.py +++ b/glanceclient/tests/unit/v2/test_shell_v2.py @@ -16,18 +16,73 @@ import json import mock import os +import six import tempfile import testtools from glanceclient.common import utils +from glanceclient import exc +from glanceclient import shell + +# NOTE(geguileo): This is very nasty, but I can't find a better way to set +# command line arguments in glanceclient.v2.shell.do_image_create that are +# set by decorator utils.schema_args while preserving the spirits of the test + +# Backup original decorator +original_schema_args = utils.schema_args + + +# Set our own decorator that calls the original but with simulated schema +def schema_args(schema_getter, omit=None): + global original_schema_args + # We only add the 2 arguments that are required by image-create + my_schema_getter = lambda: { + 'properties': { + 'container_format': { + 'enum': [None, 'ami', 'ari', 'aki', 'bare', 'ovf', 'ova'], + 'type': 'string', + 'description': 'Format of the container'}, + 'disk_format': { + 'enum': [None, 'ami', 'ari', 'aki', 'vhd', 'vmdk', 'raw', + 'qcow2', 'vdi', 'iso'], + 'type': 'string', + 'description': 'Format of the disk'}, + 'location': {'type': 'string'}, + 'locations': {'type': 'string'}, + 'copy_from': {'type': 'string'}}} + return original_schema_args(my_schema_getter, omit) +utils.schema_args = schema_args + from glanceclient.v2 import shell as test_shell +# Return original decorator. +utils.schema_args = original_schema_args + class ShellV2Test(testtools.TestCase): def setUp(self): super(ShellV2Test, self).setUp() self._mock_utils() self.gc = self._mock_glance_client() + self.shell = shell.OpenStackImagesShell() + os.environ = { + 'OS_USERNAME': 'username', + 'OS_PASSWORD': 'password', + 'OS_TENANT_ID': 'tenant_id', + 'OS_TOKEN_ID': 'test', + 'OS_AUTH_URL': 'http://127.0.0.1:5000/v2.0/', + 'OS_AUTH_TOKEN': 'pass', + 'OS_IMAGE_API_VERSION': '1', + 'OS_REGION_NAME': 'test', + 'OS_IMAGE_URL': 'http://is.invalid'} + self.shell = shell.OpenStackImagesShell() + self.patched = mock.patch('glanceclient.common.utils.get_data_file', + autospec=True, return_value=None) + self.mock_get_data_file = self.patched.start() + + def tearDown(self): + super(ShellV2Test, self).tearDown() + self.patched.stop() def _make_args(self, args): # NOTE(venkatesh): this conversion from a dict to an object @@ -59,6 +114,49 @@ class ShellV2Test(testtools.TestCase): mocked_utils_exit.assert_called_once_with(err_msg) + def _run_command(self, cmd): + self.shell.main(cmd.split()) + + @mock.patch('sys.stderr') + def test_image_create_missing_disk_format(self, __): + # We test for all possible sources + for origin in ('--file', '--location', '--copy-from'): + e = self.assertRaises(exc.CommandError, self._run_command, + '--os-image-api-version 2 image-create ' + + origin + ' fake_src --container-format bare') + self.assertEqual('error: Must provide --disk-format when using ' + + origin + '.', e.message) + + @mock.patch('sys.stderr') + def test_image_create_missing_container_format(self, __): + # We test for all possible sources + for origin in ('--file', '--location', '--copy-from'): + e = self.assertRaises(exc.CommandError, self._run_command, + '--os-image-api-version 2 image-create ' + + origin + ' fake_src --disk-format qcow2') + self.assertEqual('error: Must provide --container-format when ' + 'using ' + origin + '.', e.message) + + @mock.patch('sys.stderr') + def test_image_create_missing_container_format_stdin_data(self, __): + # Fake that get_data_file method returns data + self.mock_get_data_file.return_value = six.StringIO() + e = self.assertRaises(exc.CommandError, self._run_command, + '--os-image-api-version 2 image-create' + ' --disk-format qcow2') + self.assertEqual('error: Must provide --container-format when ' + 'using stdin.', e.message) + + @mock.patch('sys.stderr') + def test_image_create_missing_disk_format_stdin_data(self, __): + # Fake that get_data_file method returns data + self.mock_get_data_file.return_value = six.StringIO() + e = self.assertRaises(exc.CommandError, self._run_command, + '--os-image-api-version 2 image-create' + ' --container-format bare') + self.assertEqual('error: Must provide --disk-format when using stdin.', + e.message) + def test_do_image_list(self): input = { 'limit': None, @@ -261,6 +359,7 @@ class ShellV2Test(testtools.TestCase): 'container_format': 'bare'}) def test_do_image_create_with_file(self): + self.mock_get_data_file.return_value = six.StringIO() try: file_name = None with open(tempfile.mktemp(), 'w+') as f: @@ -305,7 +404,9 @@ class ShellV2Test(testtools.TestCase): def test_do_image_create_with_user_props(self, mock_stdin): args = self._make_args({'name': 'IMG-01', 'property': ['myprop=myval'], - 'file': None}) + 'file': None, + 'container_format': 'bare', + 'disk_format': 'qcow2'}) with mock.patch.object(self.gc.images, 'create') as mocked_create: ignore_fields = ['self', 'access', 'file', 'schema'] expect_image = dict([(field, field) for field in ignore_fields]) @@ -320,7 +421,9 @@ class ShellV2Test(testtools.TestCase): test_shell.do_image_create(self.gc, args) mocked_create.assert_called_once_with(name='IMG-01', - myprop='myval') + myprop='myval', + container_format='bare', + disk_format='qcow2') utils.print_dict.assert_called_once_with({ 'id': 'pass', 'name': 'IMG-01', 'myprop': 'myval'}) diff --git a/glanceclient/v1/shell.py b/glanceclient/v1/shell.py index 83b8576..c28dc23 100644 --- a/glanceclient/v1/shell.py +++ b/glanceclient/v1/shell.py @@ -32,6 +32,7 @@ import glanceclient.v1.images CONTAINER_FORMATS = 'Acceptable formats: ami, ari, aki, bare, and ovf.' DISK_FORMATS = ('Acceptable formats: ami, ari, aki, vhd, vmdk, raw, ' 'qcow2, vdi, and iso.') +DATA_FIELDS = ('location', 'copy_from', 'file') _bool_strict = functools.partial(strutils.bool_from_string, strict=True) @@ -221,6 +222,7 @@ def do_image_download(gc, args): help='Print image size in a human-friendly format.') @utils.arg('--progress', action='store_true', default=False, help='Show upload progress bar.') +@utils.on_data_require_fields(DATA_FIELDS) def do_image_create(gc, args): """Create a new image.""" # Filter out None values diff --git a/glanceclient/v2/shell.py b/glanceclient/v2/shell.py index 2f6122a..deea24b 100644 --- a/glanceclient/v2/shell.py +++ b/glanceclient/v2/shell.py @@ -27,6 +27,7 @@ import os MEMBER_STATUS_VALUES = image_members.MEMBER_STATUS_VALUES IMAGE_SCHEMA = None +DATA_FIELDS = ('location', 'copy_from', 'file') def get_image_schema(): @@ -55,6 +56,7 @@ def get_image_schema(): 'to the client via stdin.') @utils.arg('--progress', action='store_true', default=False, help='Show upload progress bar.') +@utils.on_data_require_fields(DATA_FIELDS) def do_image_create(gc, args): """Create a new image.""" schema = gc.schemas.get("image")