From 2034eac620d87be298788af5b49f2004313a607d Mon Sep 17 00:00:00 2001 From: Philip Douglass Date: Thu, 1 Dec 2022 15:22:41 -0500 Subject: [PATCH 01/10] Add function to walk the extra_vars and render the results --- awx/main/models/credential/__init__.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index cee657da01..72a49b7d1c 100644 --- a/awx/main/models/credential/__init__.py +++ b/awx/main/models/credential/__init__.py @@ -528,9 +528,13 @@ class CredentialType(CommonModelNameNotUnique): if 'INVENTORY_UPDATE_ID' not in env: # awx-manage inventory_update does not support extra_vars via -e - extra_vars = {} - for var_name, tmpl in self.injectors.get('extra_vars', {}).items(): - extra_vars[var_name] = sandbox_env.from_string(tmpl).render(**namespace) + def build_extra_vars(node): + if isinstance(node, dict): + return {k: build_extra_vars(v) for k, v in node.items()} + elif isinstance(node, list): + return [build_extra_vars(x) for x in node] + else: + return sandbox_env.from_string(node).render(**namespace) def build_extra_vars_file(vars, private_dir): handle, path = tempfile.mkstemp(dir=os.path.join(private_dir, 'env')) @@ -540,6 +544,7 @@ class CredentialType(CommonModelNameNotUnique): os.chmod(path, stat.S_IRUSR) return path + extra_vars = build_extra_vars(self.injectors.get('extra_vars', {})) if extra_vars: path = build_extra_vars_file(extra_vars, private_data_dir) container_path = to_container_path(path, private_data_dir) From d75f12c0010f64e382132a557ddf5bca64de71ea Mon Sep 17 00:00:00 2001 From: Philip Douglass Date: Thu, 1 Dec 2022 15:25:19 -0500 Subject: [PATCH 02/10] Render keys while walking extra_vars in addition to values --- awx/main/models/credential/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index 72a49b7d1c..d4c013ea63 100644 --- a/awx/main/models/credential/__init__.py +++ b/awx/main/models/credential/__init__.py @@ -530,7 +530,7 @@ class CredentialType(CommonModelNameNotUnique): # awx-manage inventory_update does not support extra_vars via -e def build_extra_vars(node): if isinstance(node, dict): - return {k: build_extra_vars(v) for k, v in node.items()} + return {build_extra_vars(k): build_extra_vars(v) for k, v in node.items()} elif isinstance(node, list): return [build_extra_vars(x) for x in node] else: From 128a130b841cce46557dcafe22eb07fac61199e6 Mon Sep 17 00:00:00 2001 From: Philip Douglass Date: Fri, 2 Dec 2022 14:23:28 -0500 Subject: [PATCH 03/10] Update documentation to include subkey injection --- docs/credentials/custom_credential_types.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/credentials/custom_credential_types.md b/docs/credentials/custom_credential_types.md index ed55847803..b1fba43372 100644 --- a/docs/credentials/custom_credential_types.md +++ b/docs/credentials/custom_credential_types.md @@ -172,7 +172,11 @@ of the [Jinja templating language](https://jinja.palletsprojects.com/en/2.10.x/) "THIRD_PARTY_CLOUD_API_TOKEN": "{{api_token}}" }, "extra_vars": { - "some_extra_var": "{{username}}:{{password}" + "some_extra_var": "{{username}}:{{password}}", + "auth": { + "username": "{{username}}", + "password": "{{password}}" + } } } From 27ea239c00cb52d6f17a417d81c3a3be803fc8fb Mon Sep 17 00:00:00 2001 From: Philip Douglass Date: Tue, 6 Dec 2022 10:47:42 -0500 Subject: [PATCH 04/10] Add two tests for nested and templated extra_vars keys --- awx/main/tests/unit/test_tasks.py | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index 9a59e091d1..bfec59b616 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -1209,6 +1209,42 @@ class TestJobCredentials(TestJobExecution): assert extra_vars["turbo_button"] == "True" return ['successful', 0] + def test_custom_environment_injectors_with_nested_extra_vars(self, private_data_dir, job, mock_me): + task = jobs.RunJob() + some_cloud = CredentialType( + kind='cloud', + name='SomeCloud', + managed=False, + inputs={'fields': [{'id': 'host', 'label': 'Host', 'type': 'string'}]}, + injectors={'extra_vars': {'auth': {'host': '{{host}}'}}}, + ) + credential = Credential(pk=1, credential_type=some_cloud, inputs={'host': 'example.com'}) + job.credentials.add(credential) + + args = task.build_args(job, private_data_dir, {}) + credential.credential_type.inject_credential(credential, {}, {}, args, private_data_dir) + extra_vars = parse_extra_vars(args, private_data_dir) + + assert extra_vars["auth"]["host"] == "example.com" + + def test_custom_environment_injectors_with_templated_extra_vars_key(self, private_data_dir, job, mock_me): + task = jobs.RunJob() + some_cloud = CredentialType( + kind='cloud', + name='SomeCloud', + managed=False, + inputs={'fields': [{'id': 'environment', 'label': 'Environment', 'type': 'string'}, {'id': 'host', 'label': 'Host', 'type': 'string'}]}, + injectors={'extra_vars': {'{{environment}}_auth': {'host': '{{host}}'}}}, + ) + credential = Credential(pk=1, credential_type=some_cloud, inputs={'environment': 'test', 'host': 'example.com'}) + job.credentials.add(credential) + + args = task.build_args(job, private_data_dir, {}) + credential.credential_type.inject_credential(credential, {}, {}, args, private_data_dir) + extra_vars = parse_extra_vars(args, private_data_dir) + + assert extra_vars["test_auth"]["host"] == "example.com" + def test_custom_environment_injectors_with_complicated_boolean_template(self, job, private_data_dir, mock_me): task = jobs.RunJob() some_cloud = CredentialType( From fcf56950b391d413b8436550b8183bf9f4e193de Mon Sep 17 00:00:00 2001 From: Philip Douglass Date: Wed, 14 Dec 2022 16:28:50 -0500 Subject: [PATCH 05/10] Add recursive properties to injectors jsonschema for extra_vars --- awx/main/fields.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 3372627f91..d319b40b1f 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -790,13 +790,11 @@ class CredentialTypeInjectorField(JSONSchemaField): 'extra_vars': { 'type': 'object', 'patternProperties': { - # http://docs.ansible.com/ansible/playbooks_variables.html#what-makes-a-valid-variable-name - '^[a-zA-Z_]+[a-zA-Z0-9_]*$': {'type': 'string'}, + r'^(?:(?:{{[^{}]*?}})|(?:[a-zA-Z_]+[a-zA-Z0-9_]*)+)+$': {"anyOf": [{'type': 'string'}, {'$ref': '#/properties/extra_vars'}]} }, 'additionalProperties': False, }, }, - 'additionalProperties': False, } def validate_env_var_allowed(self, env_var): From ad4e257fdb8dafa0bb2b766bd46e3a0a6b037def Mon Sep 17 00:00:00 2001 From: Philip Douglass Date: Thu, 15 Dec 2022 09:46:42 -0500 Subject: [PATCH 06/10] Add functions to support recursive validation for extra_vars --- awx/main/fields.py | 48 ++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index d319b40b1f..10f184691f 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -856,27 +856,41 @@ class CredentialTypeInjectorField(JSONSchemaField): template_name = template_name.split('.')[1] setattr(valid_namespace['tower'].filename, template_name, 'EXAMPLE_FILENAME') + def validate_template_string(tmpl): + try: + sandbox.ImmutableSandboxedEnvironment(undefined=StrictUndefined).from_string(tmpl).render(valid_namespace) + except UndefinedError as e: + raise django_exceptions.ValidationError( + _('{sub_key} uses an undefined field ({error_msg})').format(sub_key=key, error_msg=e), + code='invalid', + params={'value': value}, + ) + except SecurityError as e: + raise django_exceptions.ValidationError(_('Encountered unsafe code execution: {}').format(e)) + except TemplateSyntaxError as e: + raise django_exceptions.ValidationError( + _('Syntax error rendering template for {sub_key} inside of {type} ({error_msg})').format(sub_key=key, type=type_, error_msg=e), + code='invalid', + params={'value': value}, + ) + + def validate_extra_vars(node): + if isinstance(node, dict): + return {validate_extra_vars(k): validate_extra_vars(v) for k, v in node.items()} + elif isinstance(node, list): + return [validate_extra_vars(x) for x in node] + else: + validate_template_string(node) + for type_, injector in value.items(): if type_ == 'env': for key in injector.keys(): self.validate_env_var_allowed(key) - for key, tmpl in injector.items(): - try: - sandbox.ImmutableSandboxedEnvironment(undefined=StrictUndefined).from_string(tmpl).render(valid_namespace) - except UndefinedError as e: - raise django_exceptions.ValidationError( - _('{sub_key} uses an undefined field ({error_msg})').format(sub_key=key, error_msg=e), - code='invalid', - params={'value': value}, - ) - except SecurityError as e: - raise django_exceptions.ValidationError(_('Encountered unsafe code execution: {}').format(e)) - except TemplateSyntaxError as e: - raise django_exceptions.ValidationError( - _('Syntax error rendering template for {sub_key} inside of {type} ({error_msg})').format(sub_key=key, type=type_, error_msg=e), - code='invalid', - params={'value': value}, - ) + if type_ == 'extra_vars': + validate_extra_vars(injector) + else: + for key, tmpl in injector.items(): + validate_template_string(tmpl) class AskForField(models.BooleanField): From 51e244e18360fd65988f1bb60811e75ca4450c72 Mon Sep 17 00:00:00 2001 From: Philip Douglass Date: Thu, 15 Dec 2022 11:44:14 -0500 Subject: [PATCH 07/10] Expand pattern to support use of Jinja2 block delimiters --- awx/main/fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 10f184691f..2bc1e9e3bd 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -790,7 +790,7 @@ class CredentialTypeInjectorField(JSONSchemaField): 'extra_vars': { 'type': 'object', 'patternProperties': { - r'^(?:(?:{{[^{}]*?}})|(?:[a-zA-Z_]+[a-zA-Z0-9_]*)+)+$': {"anyOf": [{'type': 'string'}, {'$ref': '#/properties/extra_vars'}]} + r'^(?:(?:{(?:{|%)[^{}]*?(?:%|})})|(?:[a-zA-Z_]+[a-zA-Z0-9_]*)+)+$': {"anyOf": [{'type': 'string'}, {'$ref': '#/properties/extra_vars'}]} }, 'additionalProperties': False, }, From ae92f8292fe30605131a317b2244a7d990fb27a8 Mon Sep 17 00:00:00 2001 From: Philip Douglass Date: Fri, 16 Dec 2022 11:08:26 -0500 Subject: [PATCH 08/10] Account for validation context --- awx/main/fields.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 2bc1e9e3bd..77cebb0203 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -856,7 +856,7 @@ class CredentialTypeInjectorField(JSONSchemaField): template_name = template_name.split('.')[1] setattr(valid_namespace['tower'].filename, template_name, 'EXAMPLE_FILENAME') - def validate_template_string(tmpl): + def validate_template_string(type_, key, tmpl): try: sandbox.ImmutableSandboxedEnvironment(undefined=StrictUndefined).from_string(tmpl).render(valid_namespace) except UndefinedError as e: @@ -874,23 +874,23 @@ class CredentialTypeInjectorField(JSONSchemaField): params={'value': value}, ) - def validate_extra_vars(node): + def validate_extra_vars(key, node): if isinstance(node, dict): - return {validate_extra_vars(k): validate_extra_vars(v) for k, v in node.items()} + return {validate_extra_vars(key, k): validate_extra_vars(k, v) for k, v in node.items()} elif isinstance(node, list): - return [validate_extra_vars(x) for x in node] + return [validate_extra_vars(key, x) for x in node] else: - validate_template_string(node) + validate_template_string("extra_vars", key, node) for type_, injector in value.items(): if type_ == 'env': for key in injector.keys(): self.validate_env_var_allowed(key) if type_ == 'extra_vars': - validate_extra_vars(injector) + validate_extra_vars(None, injector) else: for key, tmpl in injector.items(): - validate_template_string(tmpl) + validate_template_string(type_, key, tmpl) class AskForField(models.BooleanField): From 7f6f57bfeed44e09f9354fe27cedee48c6d6aa45 Mon Sep 17 00:00:00 2001 From: Philip Douglass Date: Mon, 9 Jan 2023 14:58:20 -0500 Subject: [PATCH 09/10] Maintain nested context for validation error messages --- awx/main/fields.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 77cebb0203..85fd560c10 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -790,7 +790,9 @@ class CredentialTypeInjectorField(JSONSchemaField): 'extra_vars': { 'type': 'object', 'patternProperties': { - r'^(?:(?:{(?:{|%)[^{}]*?(?:%|})})|(?:[a-zA-Z_]+[a-zA-Z0-9_]*)+)+$': {"anyOf": [{'type': 'string'}, {'$ref': '#/properties/extra_vars'}]} + r'^(?:(?:{(?:{|%)[^{}]*?(?:%|})})|(?:[a-zA-Z_]+[a-zA-Z0-9_]*)+)+$': { + "anyOf": [{'type': 'string'}, {'type': 'array'}, {'$ref': '#/properties/extra_vars'}] + } }, 'additionalProperties': False, }, @@ -876,9 +878,9 @@ class CredentialTypeInjectorField(JSONSchemaField): def validate_extra_vars(key, node): if isinstance(node, dict): - return {validate_extra_vars(key, k): validate_extra_vars(k, v) for k, v in node.items()} + return {validate_extra_vars(key, k): validate_extra_vars("{key}.{k}".format(key=key, k=k), v) for k, v in node.items()} elif isinstance(node, list): - return [validate_extra_vars(key, x) for x in node] + return [validate_extra_vars("{key}[{i}]".format(key=key, i=i), x) for i, x in enumerate(node)] else: validate_template_string("extra_vars", key, node) From 9777ce7fb87d30a62356ad6f56686f9a9eae309f Mon Sep 17 00:00:00 2001 From: Alan Rominger Date: Wed, 18 Jan 2023 15:56:09 -0500 Subject: [PATCH 10/10] Touchup of validation logic from testing --- awx/main/fields.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 85fd560c10..95d5436fc8 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -790,13 +790,14 @@ class CredentialTypeInjectorField(JSONSchemaField): 'extra_vars': { 'type': 'object', 'patternProperties': { - r'^(?:(?:{(?:{|%)[^{}]*?(?:%|})})|(?:[a-zA-Z_]+[a-zA-Z0-9_]*)+)+$': { - "anyOf": [{'type': 'string'}, {'type': 'array'}, {'$ref': '#/properties/extra_vars'}] - } + # http://docs.ansible.com/ansible/playbooks_variables.html#what-makes-a-valid-variable-name + # plus, add ability to template + r'^[a-zA-Z_\{\}]+[a-zA-Z0-9_\{\}]*$': {"anyOf": [{'type': 'string'}, {'type': 'array'}, {'$ref': '#/properties/extra_vars'}]} }, 'additionalProperties': False, }, }, + 'additionalProperties': False, } def validate_env_var_allowed(self, env_var): @@ -878,9 +879,12 @@ class CredentialTypeInjectorField(JSONSchemaField): def validate_extra_vars(key, node): if isinstance(node, dict): - return {validate_extra_vars(key, k): validate_extra_vars("{key}.{k}".format(key=key, k=k), v) for k, v in node.items()} + for k, v in node.items(): + validate_template_string("extra_vars", 'a key' if key is None else key, k) + validate_extra_vars(k if key is None else "{key}.{k}".format(key=key, k=k), v) elif isinstance(node, list): - return [validate_extra_vars("{key}[{i}]".format(key=key, i=i), x) for i, x in enumerate(node)] + for i, x in enumerate(node): + validate_extra_vars("{key}[{i}]".format(key=key, i=i), x) else: validate_template_string("extra_vars", key, node)