Dima Kuznetsov has uploaded a new change for review. Change subject: client: Add warning prompt on dangerous commands ...................................................................... client: Add warning prompt on dangerous commands Added a warning and confirmation prompt for the following commands: * deactivateStorageDomain * deleteImage * deleteVolume * deleteVolumeByDescr * destroyStoragePool * extendStorageDomain * extendVolume * forcedDetachStorageDomain * formatStorageDomain * mergeSnapshots * moveImage * moveMultiImage * reconstructMaster * releaseDomainLock * removeVG Also added '--force' flag to bypass warnings and prompts (intended for scripts). Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1005923 Change-Id: Idea34ad7c3d5b66993bcb56b61e7edb0549ef9fb Signed-off-by: Dima Kuznetsov <dkuznets(a)redhat.com&gt; --- M client/vdsClient.py M lib/vdsm/vdscli.py.in M tests/vdsClientTests.py 3 files changed, 95 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/28174/1 diff --git a/client/vdsClient.py b/client/vdsClient.py index 0235837..0eb4aac 100644 --- a/client/vdsClient.py +++ b/client/vdsClient.py @@ -85,6 +85,7 @@ print "-m\tList supported methods and their params (Short help)" print "-s [--truststore path]\tConnect to server with SSL." print "-o, --oneliner\tShow the key-val information in one line." + print "--force\tDo not prompt on dangerous commands (not recommended)." print "\tIf truststore path is not specified, use defaults." print "\nCommands" verbs = cmd.keys() @@ -2070,11 +2071,12 @@ 'Create a new VG from devices devlist (list of dev ' 'GUIDs)' )), - 'removeVG': (serv.removeVG, + 'removeVG': (vdscli.request_confirmation(serv.removeVG), ('<vgUUID>', 'remove the VG identified by its UUID' )), - 'extendStorageDomain': (serv.extendStorageDomain, + 'extendStorageDomain': (vdscli.request_confirmation( + serv.extendStorageDomain), ('<sdUUID> <spUUID> <devlist>', 'Extend the Storage Domain by adding devices' ' devlist (list of dev GUIDs)' @@ -2132,7 +2134,8 @@ ('<spUUID> <sdUUID>', 'acquire storage domain lock' )), - 'releaseDomainLock': (serv.releaseDomainLock, + 'releaseDomainLock': (vdscli.request_confirmation( + serv.releaseDomainLock), ('<spUUID> <sdUUID>', 'release storage domain lock' )), @@ -2154,7 +2157,8 @@ ('<spUUID> <vmUUID> [sdUUID]', 'Remove VM from pool or Backup domain' )), - 'reconstructMaster': (serv.reconstructMaster, + 'reconstructMaster': (vdscli.request_confirmation( + serv.reconstructMaster), ('<spUUID> <poolName> <masterDom> ' '<domDict>({sdUUID1=status,sdUUID2=status,...})' ' <masterVersion>, [<lockPolicy> ' @@ -2171,7 +2175,8 @@ 'Create new storage pool with single/multiple ' 'image data domain' )), - 'destroyStoragePool': (serv.destroyStoragePool, + 'destroyStoragePool': (vdscli.request_confirmation( + serv.destroyStoragePool), ('<spUUID> <id> <scsi-key>', 'Destroy storage pool', 'Parameter scsi-key is ignored (maintained ' @@ -2227,7 +2232,8 @@ 'Activate a storage domain that is already ' 'a member in a storage pool.' )), - 'deactivateStorageDomain': (serv.deactivateStorageDomain, + 'deactivateStorageDomain': (vdscli.request_confirmation( + serv.deactivateStorageDomain), ('<domain UUID> <pool UUID> <new master ' 'domain UUID> <masterVer>', 'Deactivate a storage domain. ' @@ -2241,12 +2247,14 @@ ' UUID> <masterVer>', 'Detach a storage domain from a storage pool.' )), - 'forcedDetachStorageDomain': (serv.forcedDetachStorageDomain, + 'forcedDetachStorageDomain': (vdscli.request_confirmation( + serv.forcedDetachStorageDomain), ('<domain UUID> <pool UUID>', 'Forced detach a storage domain from a ' 'storage pool.' )), - 'formatStorageDomain': (serv.formatStorageDomain, + 'formatStorageDomain': (vdscli.request_confirmation( + serv.formatStorageDomain), ('<domain UUID> [<autoDetach>]', 'Format detached storage domain.' )), @@ -2269,7 +2277,8 @@ '<srcImgUUID> <srcVolUUID>', 'Creates new volume or snapshot' )), - 'extendVolumeSize': (serv.extendVolumeSize, ( + 'extendVolumeSize': (vdscli.request_confirmation( + serv.extendVolumeSize), ( '<spUUID> <sdUUID> <imgUUID> <volUUID> <newSize>', 'Extend the volume size (virtual disk size seen by the guest).', )), @@ -2287,12 +2296,13 @@ '<Legality>', 'Set volume legality (ILLEGAL/LEGAL).' )), - 'deleteVolume': (serv.deleteVolume, + 'deleteVolume': (vdscli.request_confirmation(serv.deleteVolume), ('<sdUUID> <spUUID> <imgUUID> <volUUID>,...,<volUUID>' ' <postZero> [<force>]', 'Deletes an volume if its a leaf. Else returns error' )), - 'deleteVolumeByDescr': (serv.deleteVolumeByDescr, + 'deleteVolumeByDescr': (vdscli.request_confirmation( + serv.deleteVolumeByDescr), ('<part of description> <sdUUID> <spUUID> ' '<imgUUID>', 'Deletes list of volumes(only leafs) ' @@ -2391,11 +2401,11 @@ ' <enabled = true/false>', 'Enable or disable Hosted Engine HA' ' maintenance')), - 'deleteImage': (serv.deleteImage, + 'deleteImage': (vdscli.request_confirmation(serv.deleteImage), ('<sdUUID> <spUUID> <imgUUID> [<postZero>] [<force>]', 'Delete Image folder with all volumes.', )), - 'moveImage': (serv.moveImage, + 'moveImage': (vdscli.request_confirmation(serv.moveImage), ('<spUUID> <srcDomUUID> <dstDomUUID> <imgUUID> <vmUUID>' ' <op = COPY_OP/MOVE_OP> [<postZero>] [ <force>]', 'Move/Copy image between storage domains within same ' @@ -2423,7 +2433,7 @@ 'Download an image from a remote endpoint using the specified', 'methodArgs.' )), - 'moveMultiImage': (serv.moveMultiImage, + 'moveMultiImage': (vdscli.request_confirmation(serv.moveMultiImage), ('<spUUID> <srcDomUUID> <dstDomUUID> ' '<imgList>({imgUUID=postzero,' 'imgUUID=postzero,...}) <vmUUID> [<force>]', @@ -2439,7 +2449,7 @@ 'Do it by collapse and copy the whole chain ' '(baseVolUUID->srcVolUUID)' )), - 'mergeSnapshots': (serv.mergeSnapshots, + 'mergeSnapshots': (vdscli.request_confirmation(serv.mergeSnapshots), ('<sdUUID> <spUUID> <vmUUID> <imgUUID> <Ancestor ' 'Image uuid> <Successor Image uuid> [<postZero>]', 'Merge images from successor to ancestor.', @@ -2574,8 +2584,8 @@ try: opts, args = getopt.getopt(sys.argv[1:], "hmso", ["help", "methods", "SSL", "truststore=", - "oneliner"]) - + "oneliner", "force"]) + vdscli.REQUEST_CONFIRMATION = True for o, v in opts: if o == "-h" or o == "--help": usage(commands) @@ -2589,6 +2599,8 @@ serv.truststore = v if o == '-o' or o == '--oneliner': serv.pretty = False + if o == '--force': + vdscli.REQUEST_CONFIRMATION = False if len(args) < 2: raise Exception("Need at least two arguments") server, command = args[0:2] diff --git a/lib/vdsm/vdscli.py.in b/lib/vdsm/vdscli.py.in index 5fa7528..2ac21b2 100644 --- a/lib/vdsm/vdscli.py.in +++ b/lib/vdsm/vdscli.py.in @@ -21,6 +21,7 @@ import xmlrpclib import subprocess +import functools import os import re import sys @@ -165,3 +166,25 @@ d_useSSL, d_tsPath) server = connect() print server.getVdsCapabilities() + + +USER_WARNING = """ +Please note that vdsClient is not a supported tool and should be used only with +direct guidance from support representative. + +This operation might cause data corruption, would you like to proceed? +[n][Y] """.lstrip() + + +REQUEST_CONFIRMATION = False + + +def request_confirmation(f): + @functools.wraps(f) + def wrapper(*args, **kwargs): + if REQUEST_CONFIRMATION: + resp = raw_input(USER_WARNING).decode('string_escape') + if resp != 'Y': + sys.exit(1) + return f(*args, **kwargs) + return wrapper diff --git a/tests/vdsClientTests.py b/tests/vdsClientTests.py index 41e93fc..8d9b943 100644 --- a/tests/vdsClientTests.py +++ b/tests/vdsClientTests.py @@ -22,12 +22,15 @@ from tempfile import mkstemp from contextlib import contextmanager import subprocess +import sys +import StringIO from testrunner import VdsmTestCase as TestCaseBase from monkeypatch import MonkeyPatch import vdsClient from vdsm.vdscli import __getLocalVdsName as getLocalVdsName +import vdsm.vdscli as vdscli @contextmanager @@ -150,6 +153,7 @@ class vdscliTests(TestCaseBase): + @MonkeyPatch(subprocess, 'Popen', lambda *y, **x: _FakePopen( 'subject= /O=VDSM Certificate/CN=myhost\n')) def test__getLocalVdsName1(self): @@ -166,3 +170,42 @@ def test__getLocalVdsName3(self): cn = getLocalVdsName('fake') self.assertEquals('0', cn) + + +class ConfirmationTests(TestCaseBase): + def testDefaultOff(self): + @vdscli.request_confirmation + def foobar(): + return 101 + + self.assertEquals(foobar(), 101) + + @MonkeyPatch(vdscli, 'REQUEST_CONFIRMATION', True) + @MonkeyPatch(sys, 'stdin', StringIO.StringIO('n\n')) + def testDenied(self): + lst = [] + + @vdscli.request_confirmation + def appent_to_lst(): + lst.append(1) + + try: + appent_to_lst() + except SystemExit: + pass + self.assertEquals(lst, []) + + @MonkeyPatch(vdscli, 'REQUEST_CONFIRMATION', True) + @MonkeyPatch(sys, 'stdin', StringIO.StringIO('Y\n')) + def testConfirmed(self): + lst = [] + + @vdscli.request_confirmation + def appent_to_lst(): + lst.append(1) + + try: + appent_to_lst() + except SystemExit: + pass + self.assertEquals(lst, [1]) -- To view, visit http://gerrit.ovirt.org/28174 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idea34ad7c3d5b66993bcb56b61e7edb0549ef9fb Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dima Kuznetsov <dkuznets(a)redhat.com&gt;