From cac48e7cfb2d063f885a9d61d49284a98c92c43e Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Tue, 2 Apr 2019 15:40:56 -0400 Subject: [PATCH 1/8] Updated IsolatedManager to take a callback that captures the remote command --- awx/main/isolated/manager.py | 10 +++++++++- awx/main/tasks.py | 12 +++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/awx/main/isolated/manager.py b/awx/main/isolated/manager.py index 5a51e898e0..8e9d8ac7a2 100644 --- a/awx/main/isolated/manager.py +++ b/awx/main/isolated/manager.py @@ -29,13 +29,14 @@ def set_pythonpath(venv_libdir, env): class IsolatedManager(object): - def __init__(self, cancelled_callback=None): + def __init__(self, cancelled_callback=None, check_callback=None): """ :param cancelled_callback: a callable - which returns `True` or `False` - signifying if the job has been prematurely cancelled """ self.cancelled_callback = cancelled_callback + self.check_callback = check_callback self.idle_timeout = max(60, 2 * settings.AWX_ISOLATED_CONNECTION_TIMEOUT) self.started_at = None @@ -187,6 +188,13 @@ class IsolatedManager(object): self.private_data_dir, extravars=extravars) status, rc = runner_obj.status, runner_obj.rc + if self.check_callback is not None: + command_path = self.path_to('artifacts', self.ident, 'command') + # If the command artifact has been synced back, update the model + if os.path.exists(command_path): + with open(command_path, 'r') as f: + data = json.load(f) + self.check_callback(data['command'], data['cwd'], self.runner_params['envvars'].copy()) self.consume_events(dispatcher) last_check = time.time() diff --git a/awx/main/tasks.py b/awx/main/tasks.py index ede7e04de9..2f580a894e 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1077,6 +1077,15 @@ class BaseTask(object): self.instance = self.update_model(self.instance.pk, job_args=json.dumps(runner_config.command), job_cwd=runner_config.cwd, job_env=job_env) + def check_handler(self, command, cwd, env): + ''' + IsolatedManager callback triggered by the repeated checks of the isolated node + ''' + self.instance = self.update_model(self.instance.pk, + job_args=command, + job_cwd=cwd, + job_env=build_safe_env(env)) + @with_path_cleanup def run(self, pk, **kwargs): @@ -1212,7 +1221,8 @@ class BaseTask(object): ) ansible_runner.utils.dump_artifacts(params) isolated_manager_instance = isolated_manager.IsolatedManager( - cancelled_callback=lambda: self.update_model(self.instance.pk).cancel_flag + cancelled_callback=lambda: self.update_model(self.instance.pk).cancel_flag, + check_callback=self.check_callback, ) status, rc = isolated_manager_instance.run(self.instance, private_data_dir, From 32286a9d49ae7ee22cebba03d758576fabbd8922 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Tue, 2 Apr 2019 16:49:40 -0400 Subject: [PATCH 2/8] Change the artifact to also capture the actual envvars data --- awx/main/isolated/manager.py | 10 +++++----- awx/main/tasks.py | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/awx/main/isolated/manager.py b/awx/main/isolated/manager.py index 8e9d8ac7a2..779e9bade8 100644 --- a/awx/main/isolated/manager.py +++ b/awx/main/isolated/manager.py @@ -189,12 +189,12 @@ class IsolatedManager(object): extravars=extravars) status, rc = runner_obj.status, runner_obj.rc if self.check_callback is not None: - command_path = self.path_to('artifacts', self.ident, 'command') - # If the command artifact has been synced back, update the model - if os.path.exists(command_path): - with open(command_path, 'r') as f: + config_path = self.path_to('artifacts', self.ident, 'config') + # If the configuration artifact has been synced back, update the model + if os.path.exists(config_path): + with open(config_path, 'r') as f: data = json.load(f) - self.check_callback(data['command'], data['cwd'], self.runner_params['envvars'].copy()) + self.check_callback(data) self.consume_events(dispatcher) last_check = time.time() diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 2f580a894e..8eca563c5f 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1077,14 +1077,14 @@ class BaseTask(object): self.instance = self.update_model(self.instance.pk, job_args=json.dumps(runner_config.command), job_cwd=runner_config.cwd, job_env=job_env) - def check_handler(self, command, cwd, env): + def check_handler(self, config): ''' IsolatedManager callback triggered by the repeated checks of the isolated node ''' self.instance = self.update_model(self.instance.pk, - job_args=command, - job_cwd=cwd, - job_env=build_safe_env(env)) + job_args=json.dumps(config['command']), + job_cwd=config['cwd'], + job_env=config['env']) @with_path_cleanup From 4364e0011752de3a10a374d7fef226925d24c37f Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 3 Apr 2019 14:34:09 -0400 Subject: [PATCH 3/8] Do the env vars redaction for isolated nodes on this side --- awx/main/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 8eca563c5f..01bc1328bd 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1084,7 +1084,7 @@ class BaseTask(object): self.instance = self.update_model(self.instance.pk, job_args=json.dumps(config['command']), job_cwd=config['cwd'], - job_env=config['env']) + job_env=build_safe_env(config['env'])) @with_path_cleanup From b0f6d2214c0becf4f38b4b8c2519ad4bbe67c0d3 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 3 Apr 2019 14:57:02 -0400 Subject: [PATCH 4/8] Fix a typo: there is no method called check_callback on BaseTask --- awx/main/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 01bc1328bd..da444d87ce 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1222,7 +1222,7 @@ class BaseTask(object): ansible_runner.utils.dump_artifacts(params) isolated_manager_instance = isolated_manager.IsolatedManager( cancelled_callback=lambda: self.update_model(self.instance.pk).cancel_flag, - check_callback=self.check_callback, + check_callback=self.check_handler, ) status, rc = isolated_manager_instance.run(self.instance, private_data_dir, From b4e508f72adafc422049616ccd5e44c352e470e3 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 3 Apr 2019 15:12:29 -0400 Subject: [PATCH 5/8] Bring the check_callback call out of the loop We shouldn't need to call it multiple times. --- awx/main/isolated/manager.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/awx/main/isolated/manager.py b/awx/main/isolated/manager.py index 779e9bade8..a297e2ba98 100644 --- a/awx/main/isolated/manager.py +++ b/awx/main/isolated/manager.py @@ -188,17 +188,18 @@ class IsolatedManager(object): self.private_data_dir, extravars=extravars) status, rc = runner_obj.status, runner_obj.rc - if self.check_callback is not None: - config_path = self.path_to('artifacts', self.ident, 'config') - # If the configuration artifact has been synced back, update the model - if os.path.exists(config_path): - with open(config_path, 'r') as f: - data = json.load(f) - self.check_callback(data) self.consume_events(dispatcher) last_check = time.time() + if self.check_callback is not None: + config_path = self.path_to('artifacts', self.ident, 'config') + # If the configuration artifact has been synced back, update the model + if os.path.exists(config_path): + with open(config_path, 'r') as f: + data = json.load(f) + self.check_callback(data) + if status == 'successful': status_path = self.path_to('artifacts', self.ident, 'status') rc_path = self.path_to('artifacts', self.ident, 'rc') From 467700e4bb8a96ba3ffb1986cdf8054119673954 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Wed, 3 Apr 2019 16:04:07 -0400 Subject: [PATCH 6/8] Bring the check_callback back into the loop but try to process it only once. --- awx/main/isolated/manager.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/awx/main/isolated/manager.py b/awx/main/isolated/manager.py index a297e2ba98..7f728518b8 100644 --- a/awx/main/isolated/manager.py +++ b/awx/main/isolated/manager.py @@ -39,6 +39,7 @@ class IsolatedManager(object): self.check_callback = check_callback self.idle_timeout = max(60, 2 * settings.AWX_ISOLATED_CONNECTION_TIMEOUT) self.started_at = None + self.captured_config_artifact = False def build_runner_params(self, hosts, verbosity=1): env = dict(os.environ.items()) @@ -188,18 +189,23 @@ class IsolatedManager(object): self.private_data_dir, extravars=extravars) status, rc = runner_obj.status, runner_obj.rc + + if self.check_callback is not None and not self.captured_config_artifact: + config_path = self.path_to('artifacts', self.ident, 'config') + # If the configuration artifact has been synced back, update the model + if os.path.exists(config_path): + try: + with open(config_path, 'r') as f: + data = json.load(f) + self.check_callback(data) + self.captured_config_artifact = True + except json.decoder.JSONDecodeError: # Just in case it's not fully here yet. + pass + self.consume_events(dispatcher) last_check = time.time() - if self.check_callback is not None: - config_path = self.path_to('artifacts', self.ident, 'config') - # If the configuration artifact has been synced back, update the model - if os.path.exists(config_path): - with open(config_path, 'r') as f: - data = json.load(f) - self.check_callback(data) - if status == 'successful': status_path = self.path_to('artifacts', self.ident, 'status') rc_path = self.path_to('artifacts', self.ident, 'rc') From 3f6d3506c6124ee6db89f5f10fcdc3ff7022d521 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Thu, 4 Apr 2019 14:25:50 -0400 Subject: [PATCH 7/8] Change the artifact file convention for isolated nodes to 'command' since that's what landed in the ansible-runner PR. --- awx/main/isolated/manager.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/awx/main/isolated/manager.py b/awx/main/isolated/manager.py index 7f728518b8..04da23c8f3 100644 --- a/awx/main/isolated/manager.py +++ b/awx/main/isolated/manager.py @@ -39,7 +39,7 @@ class IsolatedManager(object): self.check_callback = check_callback self.idle_timeout = max(60, 2 * settings.AWX_ISOLATED_CONNECTION_TIMEOUT) self.started_at = None - self.captured_config_artifact = False + self.captured_command_artifact = False def build_runner_params(self, hosts, verbosity=1): env = dict(os.environ.items()) @@ -190,15 +190,15 @@ class IsolatedManager(object): extravars=extravars) status, rc = runner_obj.status, runner_obj.rc - if self.check_callback is not None and not self.captured_config_artifact: - config_path = self.path_to('artifacts', self.ident, 'config') + if self.check_callback is not None and not self.captured_command_artifact: + command_path = self.path_to('artifacts', self.ident, 'command') # If the configuration artifact has been synced back, update the model - if os.path.exists(config_path): + if os.path.exists(command_path): try: - with open(config_path, 'r') as f: + with open(command_path, 'r') as f: data = json.load(f) self.check_callback(data) - self.captured_config_artifact = True + self.captured_command_artifact = True except json.decoder.JSONDecodeError: # Just in case it's not fully here yet. pass From c6643946c52c9c616f0c29eaa45a5763fbc78144 Mon Sep 17 00:00:00 2001 From: Jeff Bradberry Date: Thu, 4 Apr 2019 15:22:27 -0400 Subject: [PATCH 8/8] Capture the redacted credential env vars separately and then make use of them specifically to make safe the env vars coming back from an isolated node. This will allow us to capture the safed versions of custom credential values, but without potentially clobbering normal env var values that vary between the controller and the node. --- awx/main/tasks.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/awx/main/tasks.py b/awx/main/tasks.py index da444d87ce..d80506377b 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -1081,10 +1081,14 @@ class BaseTask(object): ''' IsolatedManager callback triggered by the repeated checks of the isolated node ''' + job_env = build_safe_env(config['env']) + for k, v in self.safe_cred_env.items(): + if k in job_env: + job_env[k] = v self.instance = self.update_model(self.instance.pk, job_args=json.dumps(config['command']), job_cwd=config['cwd'], - job_env=build_safe_env(config['env'])) + job_env=job_env) @with_path_cleanup @@ -1107,6 +1111,7 @@ class BaseTask(object): Needs to be an object property because status_handler uses it in a callback context ''' self.safe_env = {} + self.safe_cred_env = {} private_data_dir = None isolated_manager_instance = None @@ -1159,8 +1164,11 @@ class BaseTask(object): for credential in credentials: if credential: credential.credential_type.inject_credential( - credential, env, self.safe_env, args, private_data_dir + credential, env, self.safe_cred_env, args, private_data_dir ) + + self.safe_env.update(self.safe_cred_env) + self.write_args_file(private_data_dir, args) password_prompts = self.get_password_prompts(passwords)