From eb85486e38669677ad4cb7289fd6ba562520b7f3 Mon Sep 17 00:00:00 2001 From: william Date: Thu, 15 Nov 2018 18:20:25 +0000 Subject: [PATCH 01/11] Add a ShellVariable database table/model Shell variables are essentially key value pairs that are associated with a `Batch`. It isn't a great idea to store lists within a db and as such each variable will be stored in its own table. The basic design of ShellVariable has been modelled of Job (w/o the running code) --- src/models/batch.py | 7 +++++++ src/models/shell_variable.py | 13 +++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 src/models/shell_variable.py diff --git a/src/models/batch.py b/src/models/batch.py index 5ea78db..b68b0c8 100644 --- a/src/models/batch.py +++ b/src/models/batch.py @@ -4,6 +4,7 @@ from database import Base from models.config import Config from models.job import Job +from models.shell_variable import ShellVariable class Batch(Base): @@ -31,3 +32,9 @@ def is_interactive(self): def build_jobs(self, *nodes): return list(map(lambda n: Job(node = n, batch = self), nodes)) + + def build_shell_variables(self, **variables): + def build(key): + args = { 'key': key, 'value': variables[key], 'batch': self } + return ShellVariable(**args) + return list(map(lambda k: build(k), variables.keys())) diff --git a/src/models/shell_variable.py b/src/models/shell_variable.py new file mode 100644 index 0000000..7dbe3b6 --- /dev/null +++ b/src/models/shell_variable.py @@ -0,0 +1,13 @@ + +from sqlalchemy import Column, String, Integer, ForeignKey +from sqlalchemy.orm import relationship + +from database import Base + +class ShellVariable(Base): + + + key = Column(String) + value = Column(String) + batch_id = Column(Integer, ForeignKey('batches.id')) + batch = relationship("Batch", backref="shell_variables") From 17a0185b9aa4ba736c24f8b3745ea2f3db7bc91d Mon Sep 17 00:00:00 2001 From: william Date: Fri, 16 Nov 2018 13:29:15 +0000 Subject: [PATCH 02/11] Add config arguments into CLI To allow for greater flexibility in the arguments passing, the individual config files now dictate their arguments list. Note: The `config.args()` method is the safe version which handles the case when no arguments are specified. The `config.arguments` attribute returns the raw version from the config BUG FIX: It should have been `arg_n` not `arg_name` --- src/appliance_cli/command_generation.py | 2 +- src/commands/run.py | 3 ++- src/models/config.py | 3 +++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/appliance_cli/command_generation.py b/src/appliance_cli/command_generation.py index b6d9357..78d47b5 100644 --- a/src/appliance_cli/command_generation.py +++ b/src/appliance_cli/command_generation.py @@ -173,7 +173,7 @@ def add_argument(name): is_required = False for n in arg_n: add_argument(n) else: - add_argument(arg_name) + add_argument(arg_n) return args_map diff --git a/src/commands/run.py b/src/commands/run.py index 2310659..1843bee 100644 --- a/src/commands/run.py +++ b/src/commands/run.py @@ -33,7 +33,8 @@ def run(): runner_cmd = { 'help': Config.help, - **run_options + **run_options, + 'arguments': Config.args } runner_group = { 'help': (lambda names: "Run tools in {}".format(' '.join(names))), diff --git a/src/models/config.py b/src/models/config.py index 08fd42b..6b85a02 100644 --- a/src/models/config.py +++ b/src/models/config.py @@ -50,6 +50,9 @@ def help(self): if not self.data['help']: self.data['help'] = default return self.data['help'] + def args(self): + return self.arguments or [] + # TODO: Deprecated, avoid usage def interactive_only(self): return self.interactive() From 1b52fab759f941ea1f325810f0e811ebd74256ac Mon Sep 17 00:00:00 2001 From: william Date: Fri, 16 Nov 2018 17:05:44 +0000 Subject: [PATCH 03/11] Extract the batch building to an inner function There are going to be a couple of steps involved when creating a `batch`. Thus it is easier if they are abstracted away into a single function --- src/commands/run.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/commands/run.py b/src/commands/run.py index 1843bee..5aac37c 100644 --- a/src/commands/run.py +++ b/src/commands/run.py @@ -45,7 +45,7 @@ def run(): @command_creator.tools(run, command = runner_cmd, group = runner_group) @cli_utils.with__node__group @cli_utils.ignore_parent_commands - def runner(_ctx, configs, _a, options, nodes): + def runner(_ctx, configs, argv, options, nodes): def error_if_invalid_interactive_batch(): matches = [c for c in configs if c.interactive()] if matches and (len(configs) > 1 or len(nodes) > 1): @@ -67,12 +67,20 @@ def is_quiet(): if first.interactive() or first.report: return True else: return False + def build_batches(): + def build(config): + batch = Batch(config = config.path) + batch.build_jobs(*nodes) + return batch + return list(map(lambda c: build(c), configs)) + error_if_no_nodes() - batches = list(map(lambda c: Batch(config = c.path), configs)) error_if_invalid_interactive_batch() + + batches = build_batches() + if not (options['yes'].value or get_confirmation(batches, nodes)): return - for batch in batches: batch.build_jobs(*nodes) execute_batches(batches, quiet = is_quiet()) def execute_batches(batches, quiet = False): From e169aec0ec51b3ac65f73b66c9bf9c4958b7ba1a Mon Sep 17 00:00:00 2001 From: william Date: Fri, 16 Nov 2018 17:10:27 +0000 Subject: [PATCH 04/11] Make the `get_confirmation` an inner function It is only going to be used within the run method and as such should be located with it --- src/commands/run.py | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/commands/run.py b/src/commands/run.py index 5aac37c..184275a 100644 --- a/src/commands/run.py +++ b/src/commands/run.py @@ -74,14 +74,28 @@ def build(config): return batch return list(map(lambda c: build(c), configs)) + def get_confirmation(batches, nodes): + tool_names = '\n '.join([b.config_model.name() for b in batches]) + node_names = groups_util.compress_nodes(nodes).replace('],', ']\n ') + node_tag = 'node' if len(nodes) == 1 else 'nodes' + click.echo(""" +You are about to run: + {} +Over {}: + {} +""".strip().format(tool_names, node_tag, node_names)) + question = "Please enter [y/n] to confirm" + affirmatives = ['y', 'ye', 'yes'] + reply = click.prompt(question).lower() + if reply in affirmatives: return True + error_if_no_nodes() error_if_invalid_interactive_batch() batches = build_batches() - if not (options['yes'].value or get_confirmation(batches, nodes)): - return - execute_batches(batches, quiet = is_quiet()) + if (options['yes'].value or get_confirmation(batches, nodes)): + execute_batches(batches, quiet = is_quiet()) def execute_batches(batches, quiet = False): def run_print(string): @@ -136,19 +150,3 @@ def remove_done_tasks(): session.commit() Session.remove() run_print('Done') - - def get_confirmation(batches, nodes): - tool_names = '\n '.join([b.config_model.name() for b in batches]) - node_names = groups_util.compress_nodes(nodes).replace('],', ']\n ') - node_tag = 'node' if len(nodes) == 1 else 'nodes' - click.echo(""" -You are about to run: - {} -Over {}: - {} -""".strip().format(tool_names, node_tag, node_names)) - question = "Please enter [y/n] to confirm" - affirmatives = ['y', 'ye', 'yes'] - reply = click.prompt(question).lower() - if reply in affirmatives: - return True From c0d23cd4aedbd4dd9524d932e4e81f3e15f816cb Mon Sep 17 00:00:00 2001 From: william Date: Fri, 16 Nov 2018 17:30:49 +0000 Subject: [PATCH 05/11] Build the shell variables on the batches The ShellVariable models need to be added to the batches in a similar way to Jobs. This does take some effort to combined the shell variables as key value pairs --- src/commands/run.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/commands/run.py b/src/commands/run.py index 184275a..bca9162 100644 --- a/src/commands/run.py +++ b/src/commands/run.py @@ -67,10 +67,18 @@ def is_quiet(): if first.interactive() or first.report: return True else: return False + def argument_hash(config): + keys = config.args() + arg_hash = {} + for index, value in enumerate(argv): + arg_hash[keys[index]] = value + return arg_hash + def build_batches(): def build(config): batch = Batch(config = config.path) batch.build_jobs(*nodes) + batch.build_shell_variables(**argument_hash(config)) return batch return list(map(lambda c: build(c), configs)) From dcc05d288f86d43da106dfd70656ccbc84a36945 Mon Sep 17 00:00:00 2001 From: william Date: Mon, 19 Nov 2018 12:14:08 +0000 Subject: [PATCH 06/11] Ensure the models are loaded in the main thread This will prevent threading issue with SQLAlchemy --- src/commands/run.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/commands/run.py b/src/commands/run.py index bca9162..028da2e 100644 --- a/src/commands/run.py +++ b/src/commands/run.py @@ -147,6 +147,9 @@ def remove_done_tasks(): for batch in batches: session.add(batch) session.commit() + # Ensure the models are loaded from the db + batch.jobs + batch.shell_variables run_print('Executing: {}'.format(batch.name())) tasks = map(lambda j: j.task(thread_pool = pool), batch.jobs) loop.run_until_complete(start_tasks(tasks)) From 624ad626f1a62db9e5a0797d5f2a33e5a3ab445e Mon Sep 17 00:00:00 2001 From: william Date: Mon, 19 Nov 2018 12:21:01 +0000 Subject: [PATCH 07/11] Set the shell variables in the command The shell variables are set as bash variables before the command is ran. This allows then to be substituted into the config command. --- src/models/batch.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/models/batch.py b/src/models/batch.py index b68b0c8..1b7aa09 100644 --- a/src/models/batch.py +++ b/src/models/batch.py @@ -22,7 +22,12 @@ def help(self): return self.config_model.help() def command(self): - return self.config_model.command() + var_models = self.shell_variables + var_map = map(lambda v: '='.join([v.key, v.value]), var_models) + var_str = ' && '.join(var_map) + cmd = self.config_model.command() + if var_str: cmd = ' && '.join([var_str, cmd]) + return cmd def command_exists(self): return self.config_model.command_exists() From 214494f2b6475cc5ea163223b4cb8403288e6e51 Mon Sep 17 00:00:00 2001 From: william Date: Mon, 19 Nov 2018 12:35:13 +0000 Subject: [PATCH 08/11] Ensure the argument values are alphanumeric This is to prevent a injection attack through the shell variables --- src/models/shell_variable.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/models/shell_variable.py b/src/models/shell_variable.py index 7dbe3b6..fcd7714 100644 --- a/src/models/shell_variable.py +++ b/src/models/shell_variable.py @@ -1,6 +1,6 @@ from sqlalchemy import Column, String, Integer, ForeignKey -from sqlalchemy.orm import relationship +from sqlalchemy.orm import relationship, validates from database import Base @@ -11,3 +11,9 @@ class ShellVariable(Base): value = Column(String) batch_id = Column(Integer, ForeignKey('batches.id')) batch = relationship("Batch", backref="shell_variables") + + + @validates('value') + def validate_value(self, _, value): + assert value.isalnum() + return value From 13439c30d548d8df859caeab33560c82b95c1595 Mon Sep 17 00:00:00 2001 From: william Date: Mon, 19 Nov 2018 13:54:58 +0000 Subject: [PATCH 09/11] Catch the validation error as a click error This allows the error to be caught by clicks top level error handler --- src/models/batch.py | 1 + src/models/shell_variable.py | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/models/batch.py b/src/models/batch.py index 1b7aa09..093ef45 100644 --- a/src/models/batch.py +++ b/src/models/batch.py @@ -42,4 +42,5 @@ def build_shell_variables(self, **variables): def build(key): args = { 'key': key, 'value': variables[key], 'batch': self } return ShellVariable(**args) + return list(map(lambda k: build(k), variables.keys())) diff --git a/src/models/shell_variable.py b/src/models/shell_variable.py index fcd7714..dbf9ed1 100644 --- a/src/models/shell_variable.py +++ b/src/models/shell_variable.py @@ -4,6 +4,8 @@ from database import Base +import click + class ShellVariable(Base): @@ -15,5 +17,8 @@ class ShellVariable(Base): @validates('value') def validate_value(self, _, value): - assert value.isalnum() - return value + try: + assert value.isalnum() + return value + except AssertionError: + raise click.ClickException('The arguments must be alphanumeric') From 65912a943146ec350be3877beee8fa633704982f Mon Sep 17 00:00:00 2001 From: william Date: Mon, 19 Nov 2018 14:09:46 +0000 Subject: [PATCH 10/11] Update the README --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 028a013..58c28a1 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,14 @@ interactive: True # Runs the command in reporting mode. The standard output of the remote command will # be dumped to the screen instead of the exit code. report: True + +# The following list form the arguments to the command line. They are set as bash shell +# variables when running the command. This allows them to be used within the command +# NOTE: this key is optional +arguments: + - arg1 + - arg2 + ... ``` # Configuring Run Parameters From e7a7726910586fd962ec5d2b07b1c9e550b1e4c8 Mon Sep 17 00:00:00 2001 From: william Date: Mon, 19 Nov 2018 16:34:06 +0000 Subject: [PATCH 11/11] Tweak the readme comment --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 58c28a1..f73eae0 100644 --- a/README.md +++ b/README.md @@ -124,8 +124,9 @@ interactive: True # be dumped to the screen instead of the exit code. report: True -# The following list form the arguments to the command line. They are set as bash shell -# variables when running the command. This allows them to be used within the command +# The following list forms the arguments for the command line. They are set as +# bash shell variables when running the command. This allows them to be used +# within the command # NOTE: this key is optional arguments: - arg1