diff --git a/coriolisclient/cli/deployments.py b/coriolisclient/cli/deployments.py index 848adc9..8b8ec00 100644 --- a/coriolisclient/cli/deployments.py +++ b/coriolisclient/cli/deployments.py @@ -161,37 +161,57 @@ class CreateDeployment(show.ShowOne): """Start a new deployment from an existing transfer""" def get_parser(self, prog_name): parser = super(CreateDeployment, self).get_parser(prog_name) - parser.add_argument('transfer', - help='The ID of the transfer to migrate') - parser.add_argument('--force', - help='Force the deployment in case of a transfer ' - 'with failed executions', action='store_true', - default=False) - parser.add_argument('--dont-clone-disks', - help='Retain the transfer disks by cloning them', - action='store_false', dest="clone_disks", - default=True) - parser.add_argument('--skip-os-morphing', - help='Skip the OS morphing process', - action='store_true', - default=False) - parser.add_argument('--user-script-global', action='append', - required=False, - dest="global_scripts", - help='A script that will run for a particular ' - 'os_type. This option can be used multiple ' - 'times. Use: linux=/path/to/script.sh or ' - 'windows=/path/to/script.ps1') - parser.add_argument('--user-script-instance', action='append', - required=False, - dest="instance_scripts", - help='A script that will run for a particular ' - 'instance specified by the --instance option. ' - 'This option can be used multiple times. ' - 'Use: "instance_name"=/path/to/script.sh.' - ' This option overwrites any OS specific script ' - 'specified in --user-script-global for this ' - 'instance') + parser.add_argument( + 'transfer', + help='The ID of the transfer to migrate') + parser.add_argument( + '--force', + help='Force the deployment in case of a transfer ' + 'with failed executions', + action='store_true', + default=False) + parser.add_argument( + '--dont-clone-disks', + help='Retain the transfer disks by cloning them', + action='store_false', + dest="clone_disks", + default=True) + parser.add_argument( + '--skip-os-morphing', + help='Skip the OS morphing process', + action='store_true', + default=False) + parser.add_argument( + '--user-script-global', + action='append', + required=False, + dest="global_scripts", + type=cli_utils.comma_separated_kv_to_dict, + help='A script that will run for a particular os_type. This ' + 'option can be used multiple times. ' + 'Use: linux=/path/to/script.sh or ' + 'windows=/path/to/script.ps1. ' + 'Can optionally include a script phase: ' + 'windows=/path/to/script.ps1,phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') + parser.add_argument( + '--user-script-instance', + action='append', + required=False, + dest="instance_scripts", + type=cli_utils.comma_separated_kv_to_dict, + help='A script that will run for a particular ' + 'instance specified by the --instance option. ' + 'This option can be used multiple times. ' + 'Use: "instance_name"=/path/to/script.sh.' + ' This option overwrites any OS specific script ' + 'specified in --user-script-global for this ' + 'instance. Can optionally include a script phase: ' + 'instance_name=/path/to/script.ps1,' + 'phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') cli_utils.add_minion_pool_args_to_parser( parser, include_origin_pool_arg=False, include_destination_pool_arg=False, diff --git a/coriolisclient/cli/transfers.py b/coriolisclient/cli/transfers.py index 7ff54ed..dacdce6 100644 --- a/coriolisclient/cli/transfers.py +++ b/coriolisclient/cli/transfers.py @@ -170,48 +170,72 @@ class CreateTransfer(show.ShowOne): """Create a new transfer""" def get_parser(self, prog_name): parser = super(CreateTransfer, self).get_parser(prog_name) - parser.add_argument('--origin-endpoint', required=True, - help='The origin endpoint id') - parser.add_argument('--destination-endpoint', required=True, - help='The destination endpoint id') - parser.add_argument('--instance', action='append', required=True, - dest="instances", metavar="INSTANCE_IDENTIFIER", - help='The identifier of a source instance to be ' - 'transferred. Can be specified multiple ' - 'times') - parser.add_argument('--scenario', - dest="scenario", metavar="SCENARIO", - choices=[ - TRANSFER_SCENARIO_REPLICA, - TRANSFER_SCENARIO_LIVE_MIGRATION], - default=TRANSFER_SCENARIO_REPLICA, - help='The type of scenario to use when creating ' - 'the Transfer. "replica" will create a ' - 'monthly-billed Replica which can be ' - 'executed and deployed as many times as ' - 'desired, while "live_migration" will ' - 'create a Transfer which can be synced ' - 'as many times as needed but only ' - 'deployed once.') - parser.add_argument('--notes', dest='notes', - help='Notes about the transfer') - parser.add_argument('--user-script-global', action='append', - required=False, - dest="global_scripts", - help='A script that will run for a particular ' - 'os_type. This option can be used multiple ' - 'times. Use: linux=/path/to/script.sh or ' - 'windows=/path/to/script.ps1') - parser.add_argument('--user-script-instance', action='append', - required=False, - dest="instance_scripts", - help='A script that will run for a particular ' - 'instance specified by the --instance option. ' - 'This option can be used multiple times. ' - 'Use: "instance_name"=/path/to/script.sh.' - ' This option overwrites any OS specific script ' - 'specified in --user-script-global for this ' - 'instance') + parser.add_argument( + '--origin-endpoint', + required=True, + help='The origin endpoint id') + parser.add_argument( + '--destination-endpoint', + required=True, + help='The destination endpoint id') + parser.add_argument( + '--instance', + action='append', + required=True, + dest="instances", + metavar="INSTANCE_IDENTIFIER", + help='The identifier of a source instance to be ' + 'transferred. Can be specified multiple times') + parser.add_argument( + '--scenario', + dest="scenario", + metavar="SCENARIO", + choices=[ + TRANSFER_SCENARIO_REPLICA, + TRANSFER_SCENARIO_LIVE_MIGRATION], + default=TRANSFER_SCENARIO_REPLICA, + help='The type of scenario to use when creating ' + 'the Transfer. "replica" will create a ' + 'monthly-billed Replica which can be ' + 'executed and deployed as many times as ' + 'desired, while "live_migration" will ' + 'create a Transfer which can be synced ' + 'as many times as needed but only ' + 'deployed once.') + parser.add_argument( + '--notes', + dest='notes', + help='Notes about the transfer') + parser.add_argument( + '--user-script-global', + action='append', + required=False, + dest="global_scripts", + type=cli_utils.comma_separated_kv_to_dict, + help='A script that will run for a particular ' + 'os_type. This option can be used multiple ' + 'times. Use: linux=/path/to/script.sh or ' + 'windows=/path/to/script.ps1.' + 'Can optionally include a script phase: ' + 'windows=/path/to/script.ps1,phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') + parser.add_argument( + '--user-script-instance', action='append', + required=False, + dest="instance_scripts", + type=cli_utils.comma_separated_kv_to_dict, + help='A script that will run for a particular ' + 'instance specified by the --instance option. ' + 'This option can be used multiple times. ' + 'Use: "instance_name"=/path/to/script.sh.' + ' This option overwrites any OS specific script ' + 'specified in --user-script-global for this ' + 'instance. Can optionally include a script phase: ' + 'instance_name=/path/to/script.ps1,' + 'phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') cli_utils.add_args_for_json_option_to_parser( parser, 'destination-environment') @@ -349,25 +373,42 @@ class UpdateTransfer(show.ShowOne): def get_parser(self, prog_name): parser = super(UpdateTransfer, self).get_parser(prog_name) parser.add_argument('id', help='The transfer\'s id') - parser.add_argument('--notes', dest='notes', - help='Notes about the transfer.') - parser.add_argument('--user-script-global', action='append', - required=False, - dest="global_scripts", - help='A script that will run for a particular ' - 'os_type. This option can be used multiple ' - 'times. Use: linux=/path/to/script.sh or ' - 'windows=/path/to/script.ps1') - parser.add_argument('--user-script-instance', action='append', - required=False, - dest="instance_scripts", - help='A script that will run for a particular ' - 'instance specified by the --instance option. ' - 'This option can be used multiple times. ' - 'Use: "instance_name"=/path/to/script.sh.' - ' This option overwrites any OS specific script ' - 'specified in --user-script-global for this ' - 'instance') + parser.add_argument( + '--notes', + dest='notes', + help='Notes about the transfer.') + parser.add_argument( + '--user-script-global', + action='append', + required=False, + dest="global_scripts", + type=cli_utils.comma_separated_kv_to_dict, + help='A script that will run for a particular ' + 'os_type. This option can be used multiple ' + 'times. Use: linux=/path/to/script.sh or ' + 'windows=/path/to/script.ps1. ' + 'Omit the path to unregister the script, e.g. "linux=".' + 'Can optionally include a script phase: ' + 'windows=/path/to/script.ps1,phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') + parser.add_argument( + '--user-script-instance', action='append', + required=False, + dest="instance_scripts", + type=cli_utils.comma_separated_kv_to_dict, + help='A script that will run for a particular ' + 'instance specified by the --instance option. ' + 'This option can be used multiple times. ' + 'Use: "instance_name"=/path/to/script.sh.' + ' This option overwrites any OS specific script ' + 'specified in --user-script-global for this instance.' + 'Omit the path to unregister the script, e.g. "linux=".' + 'Can optionally include a script phase: ' + 'instance_name=/path/to/script.ps1,' + 'phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') cli_utils.add_args_for_json_option_to_parser( parser, 'destination-environment') diff --git a/coriolisclient/cli/utils.py b/coriolisclient/cli/utils.py index 18c1f1d..7ea37b0 100644 --- a/coriolisclient/cli/utils.py +++ b/coriolisclient/cli/utils.py @@ -191,42 +191,120 @@ def get_option_value_from_args(args, option_name, error_on_no_value=True): return value -def compose_user_scripts(global_scripts, instance_scripts): +def comma_separated_kv_to_dict(input_string: str) -> dict: + """Convert a comma separated list of key=value pairs to dict. + + Example: some_key=some_val,some_other_key=some_other_val + -> {"some_key": "some_val", "some_other_key": "some_other_val"} + """ + out = {} + kv_pairs = input_string.split(",") + for kv_pair in kv_pairs: + try: + key, value = kv_pair.split("=") + except ValueError as err: + raise ValueError( + "Not a = pair: %s. " % kv_pair) from err + out[key] = value + return out + + +def compose_user_scripts( + global_scripts: list[dict], + instance_scripts: list[dict], +) -> dict: + """Process user script arguments. + + :param global_scripts: a list of dicts describing user scripts, the target + OS and the phase in which the scripts should be invoked. + The dicts are expected to contain the following keys: + * : one of the operating systems supported by Coriolis, + e.g. "windows" or "linux". The value will be a local script path. + * "phase": optional phase, defaults to "osmorphing_post_os_mount". + :param instance_scripts: a list of dicts describing user scripts, each + having a corresponding instance. + The dicts are expected to contain the following keys: + * : The name of the instance where the script must run. + * "phase": optional phase, defaults to "osmorphing_post_os_mount". + :returns: the processed list of scripts as expected by the Coriolis API. + """ ret = { "global": {}, "instances": {} } global_scripts = global_scripts or [] instance_scripts = instance_scripts or [] - for glb in global_scripts: - split = glb.split("=", 1) - if len(split) != 2: - continue - if split[0] not in constants.OS_LIST: + for params in global_scripts: + phase = params.pop("phase", constants.PHASE_OSMORPHING_POST_OS_MOUNT) + if phase not in constants.USER_SCRIPT_PHASES: + raise ValueError( + f"Invalid user script phase: {phase}. " + "Available options are: " + f"{', '.join(constants.USER_SCRIPT_PHASES)}.") + if not params: + raise ValueError( + "OS type not specified. " + "Available options are: %s" % ", ".join(constants.OS_LIST)) + if len(params.keys()) > 1: + raise ValueError("Too many parameters.") + os_type = list(params.keys())[0] + script_path = params[os_type] + if os_type not in constants.OS_LIST: raise ValueError( "Invalid OS %s. Available options are: %s" % ( - split[0], ", ".join(constants.OS_LIST))) - if not split[1]: - # removing script - ret["global"][split[0]] = None - continue - if os.path.isfile(split[1]) is False: - raise ValueError("Could not find %s" % split[1]) - with open(split[1]) as sc: - ret["global"][split[0]] = sc.read() - - for inst in instance_scripts: - split = inst.split("=", 1) - if len(split) != 2: + os_type, ", ".join(constants.OS_LIST))) + + payload = None + # The user may omit the script path in order to remove all script + # records. + if not script_path: + ret["global"][os_type] = None continue - if not split[1]: - # removing script - ret['instances'][split[0]] = None + + if not os.path.isfile(script_path): + raise ValueError("Could not find %s" % script_path) + with open(script_path) as sc: + payload = sc.read() + if os_type not in ret["global"]: + ret["global"][os_type] = [] + script_entry = { + "phase": phase, + "payload": payload, + } + ret["global"][os_type].append(script_entry) + + for params in instance_scripts: + phase = params.pop("phase", constants.PHASE_OSMORPHING_POST_OS_MOUNT) + if phase not in constants.USER_SCRIPT_PHASES: + raise ValueError( + f"Invalid user script phase: {phase}. " + "Available options are: " + f"{', '.join(constants.USER_SCRIPT_PHASES)}.") + if not params: + raise ValueError("Instance not specified.") + if len(params.keys()) > 1: + raise ValueError( + "Too many parameters. Expecting just one instance.") + instance = list(params.keys())[0] + script_path = params[instance] + payload = None + # The user may omit the script path in order to remove all script + # records. + if not script_path: + ret["instances"][instance] = None continue - if os.path.isfile(split[1]) is False: - raise ValueError("Could not find %s" % split[1]) - with open(split[1]) as sc: - ret["instances"][split[0]] = sc.read() + + if not os.path.isfile(script_path): + raise ValueError("Could not find %s" % script_path) + with open(script_path) as sc: + payload = sc.read() + if instance not in ret["instances"]: + ret["instances"][instance] = [] + script_entry = { + "phase": phase, + "payload": payload, + } + ret["instances"][instance].append(script_entry) return ret diff --git a/coriolisclient/constants.py b/coriolisclient/constants.py index 8ba47d2..10c1981 100644 --- a/coriolisclient/constants.py +++ b/coriolisclient/constants.py @@ -48,3 +48,20 @@ OS_TYPE_OTHER, OS_TYPE_UNKNOWN, ] + +# User script execution phases. +# +# Scripts that must be executed before the OS partition is mounted, for +# example scripts that unlock encrypted partitions. +PHASE_OSMORPHING_PRE_OS_MOUNT = "osmorphing_pre_os_mount" +# Scripts that are executed after the OS partition is mounted (the default). +PHASE_OSMORPHING_POST_OS_MOUNT = "osmorphing_post_os_mount" +# We may eventually add "PHASE_REPLICA_FIRST_BOOT" for convenience, although +# the users can already achieve this by using os-morphing scripts to schedule +# scripts that will be executed at the next boot. This may require import +# provider support. + +USER_SCRIPT_PHASES = [ + PHASE_OSMORPHING_PRE_OS_MOUNT, + PHASE_OSMORPHING_POST_OS_MOUNT, +] diff --git a/coriolisclient/tests/cli/test_deployments.py b/coriolisclient/tests/cli/test_deployments.py index 0aefa8a..f80f0ca 100644 --- a/coriolisclient/tests/cli/test_deployments.py +++ b/coriolisclient/tests/cli/test_deployments.py @@ -210,13 +210,16 @@ def test_get_parser(self): parser = self.cli.get_parser('coriolis') global_script = "linux=/linux/path" instance_script = "instance1=/instance1/path" + + processed_global_scripts = [{"linux": "/linux/path"}] + processed_instance_scripts = [{"instance1": "/instance1/path"}] args = parser.parse_args([ 'transfer_id', '--force', '--dont-clone-disks', '--skip-os-morphing', '--user-script-global', global_script, '--user-script-instance', instance_script]) self.assertEqual( - ('transfer_id', True, False, True, [global_script], - [instance_script]), + ('transfer_id', True, False, True, + processed_global_scripts, processed_instance_scripts), (args.transfer, args.force, args.clone_disks, args.skip_os_morphing, args.global_scripts, args.instance_scripts)) diff --git a/coriolisclient/tests/cli/test_utils.py b/coriolisclient/tests/cli/test_utils.py index 4dd6d63..ec1a834 100644 --- a/coriolisclient/tests/cli/test_utils.py +++ b/coriolisclient/tests/cli/test_utils.py @@ -9,6 +9,10 @@ from coriolisclient.cli import utils from coriolisclient.tests import test_base +_user_script_path = os.path.join( + os.path.dirname(os.path.realpath(__file__)), + 'data/user_scripts.yml') + @ddt.ddt class UtilsTestCase(test_base.CoriolisBaseTestCase): @@ -230,6 +234,17 @@ def test_get_option_value_from_args_json_value_error(self): "option-name" ) + @ddt.data( + "linux script", + "linux=script,phase", + ) + def test_comma_separated_kv_to_dict_invalid(self, params): + self.assertRaises( + ValueError, + utils.comma_separated_kv_to_dict, + params, + ) + @ddt.data( { "global_scripts": None, @@ -249,15 +264,6 @@ def test_get_option_value_from_args_json_value_error(self): "instances": {"instance_1": None} } }, - { - "global_scripts": ["linux script"], - "instance_scripts": ["linux script"], - "expected_result": - { - "global": {}, - "instances": {} - } - }, { "global_scripts": ["invalid_os=scrips"], "instance_scripts": None, @@ -273,10 +279,30 @@ def test_get_option_value_from_args_json_value_error(self): "instance_scripts": ["linux='invalid/file/path'"], "expected_result": None }, + { + "global_scripts": None, + # Too many parameters. + "instance_scripts": [ + f"linux={_user_script_path},windows={_user_script_path}"], + "expected_result": None + }, + { + "global_scripts": None, + # Invalid phase. + "instance_scripts": [ + f"linux={_user_script_path},phase=invalid-phase"], + "expected_result": None + } ) def test_compose_user_scripts(self, data): - global_scripts = data["global_scripts"] - instance_scripts = data["instance_scripts"] + global_scripts = [ + utils.comma_separated_kv_to_dict(params) + for params in data["global_scripts"] + ] if data["global_scripts"] else None + instance_scripts = [ + utils.comma_separated_kv_to_dict(params) + for params in data["instance_scripts"] + ] if data["instance_scripts"] else None expected_result = data["expected_result"] if expected_result: @@ -298,15 +324,69 @@ def test_compose_user_scripts(self, data): def test_compose_user_scripts_from_file(self): script_path = os.path.dirname(os.path.realpath(__file__)) script_path = os.path.join(script_path, 'data/user_scripts.yml') - global_scripts = ["linux=%s" % script_path] - instance_scripts = ["linux=%s" % script_path] + global_scripts = [ + f"linux={script_path}", + f"windows={script_path},phase=osmorphing_pre_os_mount", + f"windows={script_path},phase=osmorphing_post_os_mount", + ] + instance_scripts = [ + f"instance0={script_path}", + f"instance1={script_path}", + f"instance1={script_path},phase=osmorphing_pre_os_mount", + ] + + # We could've provided the dicts directly but we'll exercise + # comma_separated_kv_to_dict instead. + global_scripts = [ + utils.comma_separated_kv_to_dict(params) + for params in global_scripts + ] + instance_scripts = [ + utils.comma_separated_kv_to_dict(params) + for params in instance_scripts + ] result = utils.compose_user_scripts(global_scripts, instance_scripts) + payload = '"mock_script1"\n"mock_script2"\n' self.assertEqual( { - 'global': {'linux': '"mock_script1"\n"mock_script2"\n'}, - 'instances': {'linux': '"mock_script1"\n"mock_script2"\n'} + 'global': { + 'linux': [ + { + 'phase': "osmorphing_post_os_mount", + 'payload': payload, + }, + ], + 'windows': [ + { + 'phase': "osmorphing_pre_os_mount", + 'payload': payload, + }, + { + 'phase': "osmorphing_post_os_mount", + 'payload': payload, + }, + ], + }, + 'instances': { + 'instance0': [ + { + 'phase': "osmorphing_post_os_mount", + 'payload': payload, + }, + ], + 'instance1': [ + { + 'phase': "osmorphing_post_os_mount", + 'payload': payload, + }, + { + 'phase': "osmorphing_pre_os_mount", + 'payload': payload, + }, + ], + }, }, result ) diff --git a/test-requirements.txt b/test-requirements.txt index fea111a..bd292a0 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,7 +2,7 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -hacking>=6.0.1,<=6.0.1 # Apache-2.0 +hacking>=7.0.0,<7.1.0 # Apache-2.0 coverage!=4.4,>=4.0 # Apache-2.0 ddt>=1.2.1 # MIT oslotest>=3.8.0 # Apache-2.0