openstack/python-openstackclient
Code Issues Proposed changes

Merge "image: Simplify handling of data provided via stdin"

Browse Source
This commit is contained in:
Zuul 2022-11-16 12:37:58 +00:00 committed by Gerrit Code Review
commit 222ea8def2
2 changed files with 82 additions and 103 deletions
openstackclient
image/v2
image.py
tests/unit/image/v2
test_image.py

91
openstackclient/image/v2/image.py

@ -134,34 +134,32 @@ def _get_member_columns(item):
)
def get_data_file(args):
if args.file:
return (open(args.file, 'rb'), args.file)
else:
# distinguish cases where:
# (1) stdin is not valid (as in cron jobs):
# openstack ... <&-
# (2) image data is provided through stdin:
# openstack ... < /tmp/file
# (3) no image data provided
# openstack ...
try:
os.fstat(0)
except OSError:
# (1) stdin is not valid
return (None, None)
if not sys.stdin.isatty():
# (2) image data is provided through stdin
image = sys.stdin
if hasattr(sys.stdin, 'buffer'):
image = sys.stdin.buffer
if msvcrt:
msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY)
def get_data_from_stdin():
# distinguish cases where:
# (1) stdin is not valid (as in cron jobs):
# openstack ... <&-
# (2) image data is provided through stdin:
# openstack ... < /tmp/file
# (3) no image data provided
# openstack ...
try:
os.fstat(0)
except OSError:
# (1) stdin is not valid
return None
return (image, None)
else:
# (3)
return (None, None)
if not sys.stdin.isatty():
# (2) image data is provided through stdin
image = sys.stdin
if hasattr(sys.stdin, 'buffer'):
image = sys.stdin.buffer
if msvcrt:
msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY)
return image
else:
# (3)
return None
class AddProjectToImage(command.ShowOne):
@ -277,6 +275,7 @@ class CreateImage(command.ShowOne):
source_group = parser.add_mutually_exclusive_group()
source_group.add_argument(
"--file",
dest="filename",
metavar="<file>",
help=_("Upload image from local file"),
)
@ -299,7 +298,10 @@ class CreateImage(command.ShowOne):
"--progress",
action="store_true",
default=False,
help=_("Show upload progress bar."),
help=_(
"Show upload progress bar "
"(ignored if passing data via stdin)"
),
)
parser.add_argument(
'--sign-key-path',
@ -463,7 +465,15 @@ class CreateImage(command.ShowOne):
# open the file first to ensure any failures are handled before the
# image is created. Get the file name (if it is file, and not stdin)
# for easier further handling.
fp, fname = get_data_file(parsed_args)
if parsed_args.filename:
try:
fp = open(parsed_args.filename, 'rb')
except FileNotFoundError:
raise exceptions.CommandError(
'%r is not a valid file' % parsed_args.filename,
)
else:
fp = get_data_from_stdin()
if fp is not None and parsed_args.volume:
msg = _(
@ -472,26 +482,24 @@ class CreateImage(command.ShowOne):
)
raise exceptions.CommandError(msg)
if fp is None and parsed_args.file:
LOG.warning(_("Failed to get an image file."))
return {}, {}
if parsed_args.progress and parsed_args.file:
if parsed_args.progress and parsed_args.filename:
# NOTE(stephenfin): we only show a progress bar if the user
# requested it *and* we're reading from a file (not stdin)
filesize = os.path.getsize(fname)
filesize = os.path.getsize(parsed_args.filename)
if filesize is not None:
kwargs['validate_checksum'] = False
kwargs['data'] = progressbar.VerboseFileWrapper(fp, filesize)
elif fname:
kwargs['filename'] = fname
else:
kwargs['data'] = fp
elif parsed_args.filename:
kwargs['filename'] = parsed_args.filename
elif fp:
kwargs['validate_checksum'] = False
kwargs['data'] = fp
# sign an image using a given local private key file
if parsed_args.sign_key_path or parsed_args.sign_cert_id:
if not parsed_args.file:
if not parsed_args.filename:
msg = _(
"signing an image requires the --file option, "
"passing files via stdin when signing is not "
@ -546,6 +554,10 @@ class CreateImage(command.ShowOne):
kwargs['img_signature_key_type'] = signer.padding_method
image = image_client.create_image(**kwargs)
if parsed_args.filename:
fp.close()
return _format_image(image)
def _take_action_volume(self, parsed_args):
@ -980,6 +992,7 @@ class SaveImage(command.Command):
parser.add_argument(
"--file",
metavar="<filename>",
dest="filename",
help=_("Downloaded image save filename (default: stdout)"),
)
parser.add_argument(
@ -993,7 +1006,7 @@ class SaveImage(command.Command):
image_client = self.app.client_manager.image
image = image_client.find_image(parsed_args.image)
output_file = parsed_args.file
output_file = parsed_args.filename
if output_file is None:
output_file = getattr(sys.stdout, "buffer", sys.stdout)

94
openstackclient/tests/unit/image/v2/test_image.py

@ -14,7 +14,6 @@
import copy
import io
import os
import tempfile
from unittest import mock
@ -104,15 +103,8 @@ class TestImageCreate(TestImage):
disk_format=image.DEFAULT_DISK_FORMAT,
)
# Verify update() was not called, if it was show the args
self.assertEqual(self.client.update_image.call_args_list, [])
self.assertEqual(
self.expected_columns,
columns)
self.assertCountEqual(
self.expected_data,
data)
self.assertEqual(self.expected_columns, columns)
self.assertCountEqual(self.expected_data, data)
@mock.patch('sys.stdin', side_effect=[None])
def test_image_reserve_options(self, raw_input):
@ -121,10 +113,11 @@ class TestImageCreate(TestImage):
'--disk-format', 'ami',
'--min-disk', '10',
'--min-ram', '4',
('--protected'
if self.new_image.is_protected else '--unprotected'),
('--private'
if self.new_image.visibility == 'private' else '--public'),
'--protected' if self.new_image.is_protected else '--unprotected',
(
'--private'
if self.new_image.visibility == 'private' else '--public'
),
'--project', self.new_image.owner_id,
'--project-domain', self.domain.id,
self.new_image.name,
@ -160,12 +153,8 @@ class TestImageCreate(TestImage):
visibility=self.new_image.visibility,
)
self.assertEqual(
self.expected_columns,
columns)
self.assertCountEqual(
self.expected_data,
data)
self.assertEqual(self.expected_columns, columns)
self.assertCountEqual(self.expected_data, data)
def test_image_create_with_unexist_project(self):
self.project_mock.get.side_effect = exceptions.NotFound(None)
@ -217,7 +206,7 @@ class TestImageCreate(TestImage):
self.new_image.name,
]
verifylist = [
('file', imagefile.name),
('filename', imagefile.name),
('is_protected', self.new_image.is_protected),
('visibility', self.new_image.visibility),
('properties', {'Alpha': '1', 'Beta': '2'}),
@ -252,12 +241,12 @@ class TestImageCreate(TestImage):
self.expected_data,
data)
@mock.patch('openstackclient.image.v2.image.get_data_file')
@mock.patch('openstackclient.image.v2.image.get_data_from_stdin')
def test_image_create__progress_ignore_with_stdin(
self, mock_get_data_file,
self, mock_get_data_from_stdin,
):
fake_stdin = io.StringIO('fake-image-data')
mock_get_data_file.return_value = (fake_stdin, None)
mock_get_data_from_stdin.return_value = fake_stdin
arglist = [
'--progress',
@ -322,11 +311,11 @@ class TestImageCreate(TestImage):
)
@mock.patch('osc_lib.utils.find_resource')
@mock.patch('openstackclient.image.v2.image.get_data_file')
@mock.patch('openstackclient.image.v2.image.get_data_from_stdin')
def test_image_create_from_volume(self, mock_get_data_f, mock_get_vol):
fake_vol_id = 'fake-volume-id'
mock_get_data_f.return_value = (None, None)
mock_get_data_f.return_value = None
class FakeVolume:
id = fake_vol_id
@ -353,12 +342,12 @@ class TestImageCreate(TestImage):
)
@mock.patch('osc_lib.utils.find_resource')
@mock.patch('openstackclient.image.v2.image.get_data_file')
@mock.patch('openstackclient.image.v2.image.get_data_from_stdin')
def test_image_create_from_volume_fail(self, mock_get_data_f,
mock_get_vol):
fake_vol_id = 'fake-volume-id'
mock_get_data_f.return_value = (None, None)
mock_get_data_f.return_value = None
class FakeVolume:
id = fake_vol_id
@ -379,7 +368,7 @@ class TestImageCreate(TestImage):
parsed_args)
@mock.patch('osc_lib.utils.find_resource')
@mock.patch('openstackclient.image.v2.image.get_data_file')
@mock.patch('openstackclient.image.v2.image.get_data_from_stdin')
def test_image_create_from_volume_v31(self, mock_get_data_f,
mock_get_vol):
@ -387,7 +376,7 @@ class TestImageCreate(TestImage):
api_versions.APIVersion('3.1'))
fake_vol_id = 'fake-volume-id'
mock_get_data_f.return_value = (None, None)
mock_get_data_f.return_value = None
class FakeVolume:
id = fake_vol_id
@ -1798,7 +1787,7 @@ class TestImageSave(TestImage):
arglist = ['--file', '/path/to/file', self.image.id]
verifylist = [
('file', '/path/to/file'),
('filename', '/path/to/file'),
('image', self.image.id),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@ -1813,49 +1802,26 @@ class TestImageSave(TestImage):
class TestImageGetData(TestImage):
def setUp(self):
super().setUp()
self.args = mock.Mock()
def test_get_data_file_file(self):
(fd, fname) = tempfile.mkstemp(prefix='osc_test_image')
self.args.file = fname
(test_fd, test_name) = image.get_data_file(self.args)
self.assertEqual(fname, test_name)
test_fd.close()
os.unlink(fname)
def test_get_data_file_2(self):
self.args.file = None
f = io.BytesIO(b"some initial binary data: \x00\x01")
def test_get_data_from_stdin(self):
fd = io.BytesIO(b"some initial binary data: \x00\x01")
with mock.patch('sys.stdin') as stdin:
stdin.return_value = f
stdin.return_value = fd
stdin.isatty.return_value = False
stdin.buffer = f
stdin.buffer = fd
(test_fd, test_name) = image.get_data_file(self.args)
test_fd = image.get_data_from_stdin()
# Ensure data written to temp file is correct
self.assertEqual(f, test_fd)
self.assertIsNone(test_name)
self.assertEqual(fd, test_fd)
def test_get_data_file_3(self):
self.args.file = None
f = io.BytesIO(b"some initial binary data: \x00\x01")
def test_get_data_from_stdin__interactive(self):
fd = io.BytesIO(b"some initial binary data: \x00\x01")
with mock.patch('sys.stdin') as stdin:
# There is stdin, but interactive
stdin.return_value = f
stdin.return_value = fd
(test_fd, test_fname) = image.get_data_file(self.args)
test_fd = image.get_data_from_stdin()
self.assertIsNone(test_fd)
self.assertIsNone(test_fname)