From 286a70f2cac304aad7122160c96528c2cfc7b2c1 Mon Sep 17 00:00:00 2001 From: Jim Ladd Date: Tue, 14 Nov 2017 17:26:58 -0500 Subject: [PATCH 1/6] Add support for multi-file injection in custom creds --- awx/main/fields.py | 5 ++--- awx/main/models/credential/__init__.py | 17 ++++++++++------- awx/main/tests/functional/test_credential.py | 2 ++ 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 777836ebf3..893051492f 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -695,11 +695,10 @@ class CredentialTypeInjectorField(JSONSchemaField): 'properties': { 'file': { 'type': 'object', - 'properties': { - 'template': {'type': 'string'}, + 'patternProperties': { + '^template\.[a-zA-Z_]+$': {'type': 'string'}, }, 'additionalProperties': False, - 'required': ['template'], }, 'env': { 'type': 'object', diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index face2befdb..5f82c79a91 100644 --- a/awx/main/models/credential/__init__.py +++ b/awx/main/models/credential/__init__.py @@ -594,9 +594,11 @@ class CredentialType(CommonModelNameNotUnique): return class TowerNamespace: - filename = None + pass tower_namespace = TowerNamespace() + filename_namespace = TowerNamespace() + tower_namespace.filename = filename_namespace # maintain a normal namespace for building the ansible-playbook arguments (env and args) namespace = {'tower': tower_namespace} @@ -622,17 +624,18 @@ class CredentialType(CommonModelNameNotUnique): if len(value): namespace[field_name] = value - file_tmpl = self.injectors.get('file', {}).get('template') - if file_tmpl is not None: - # If a file template is provided, render the file and update the - # special `tower` template namespace so the filename can be - # referenced in other injectors + file_tmpls = self.injectors.get('file', {}) + # If any file templates are provided, render the files and update the + # special `tower` template namespace so the filename can be + # referenced in other injectors + for file_label, file_tmpl in file_tmpls.items(): data = Template(file_tmpl).render(**namespace) _, path = tempfile.mkstemp(dir=private_data_dir) with open(path, 'w') as f: f.write(data) os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) - namespace['tower'].filename = path + file_label = file_label.split('.')[1] + setattr(namespace['tower'].filename, file_label, path) for env_var, tmpl in self.injectors.get('env', {}).items(): if env_var.startswith('ANSIBLE_') or env_var in self.ENV_BLACKLIST: diff --git a/awx/main/tests/functional/test_credential.py b/awx/main/tests/functional/test_credential.py index 1fe909c092..2bf15b3f8e 100644 --- a/awx/main/tests/functional/test_credential.py +++ b/awx/main/tests/functional/test_credential.py @@ -109,6 +109,8 @@ def test_cred_type_input_schema_validity(input_, valid): ({'file': 123}, False), ({'file': {}}, False), ({'file': {'template': '{{username}}'}}, True), + ({'file': {'template.username': '{{username}}'}}, True), + ({'file': {'template.username': '{{username}}', 'template.password': '{{pass}}'}}, True), ({'file': {'foo': 'bar'}}, False), ({'env': 123}, False), ({'env': {}}, True), From 7aa1ae69b3974167a99dabbf12eeda6f4eafdbbc Mon Sep 17 00:00:00 2001 From: Jim Ladd Date: Tue, 21 Nov 2017 11:00:31 -0500 Subject: [PATCH 2/6] Add backwards compatibility for injecting single file --- awx/main/fields.py | 2 +- awx/main/models/credential/__init__.py | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 893051492f..005ed85362 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -696,7 +696,7 @@ class CredentialTypeInjectorField(JSONSchemaField): 'file': { 'type': 'object', 'patternProperties': { - '^template\.[a-zA-Z_]+$': {'type': 'string'}, + '^template(\.[a-zA-Z_]+)?$': {'type': 'string'}, }, 'additionalProperties': False, }, diff --git a/awx/main/models/credential/__init__.py b/awx/main/models/credential/__init__.py index 5f82c79a91..86c3930299 100644 --- a/awx/main/models/credential/__init__.py +++ b/awx/main/models/credential/__init__.py @@ -597,8 +597,6 @@ class CredentialType(CommonModelNameNotUnique): pass tower_namespace = TowerNamespace() - filename_namespace = TowerNamespace() - tower_namespace.filename = filename_namespace # maintain a normal namespace for building the ansible-playbook arguments (env and args) namespace = {'tower': tower_namespace} @@ -634,8 +632,15 @@ class CredentialType(CommonModelNameNotUnique): with open(path, 'w') as f: f.write(data) os.chmod(path, stat.S_IRUSR | stat.S_IWUSR) - file_label = file_label.split('.')[1] - setattr(namespace['tower'].filename, file_label, path) + + # determine if filename indicates single file or many + if file_label.find('.') == -1: + tower_namespace.filename = path + else: + if not hasattr(tower_namespace, 'filename'): + tower_namespace.filename = TowerNamespace() + file_label = file_label.split('.')[1] + setattr(tower_namespace.filename, file_label, path) for env_var, tmpl in self.injectors.get('env', {}).items(): if env_var.startswith('ANSIBLE_') or env_var in self.ENV_BLACKLIST: From 18178c83b3c0909e2254b13e0d1e0d871a8c75ef Mon Sep 17 00:00:00 2001 From: Jim Ladd Date: Tue, 21 Nov 2017 13:11:14 -0500 Subject: [PATCH 3/6] Validate single and multi-file injection --- awx/main/fields.py | 18 +++++++++++++++++- docs/custom_credential_types.md | 5 ++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 005ed85362..6e8d3bb1f3 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -748,8 +748,24 @@ class CredentialTypeInjectorField(JSONSchemaField): class TowerNamespace: filename = None - valid_namespace['tower'] = TowerNamespace() + + # ensure either single file or multi-file syntax is used (but not both) + template_names = set(key for type_, injector in value.items() + for key, tmpl in injector.items() + if key.startswith('template')) + if 'template' in template_names and len(template_names) > 1: + raise django_exceptions.ValidationError( + _('Must use multi-file syntax when injecting multiple files'), + code='invalid', + params={'value': value}, + ) + if 'template' not in template_names: + valid_namespace['tower'].filename = TowerNamespace() + for template_name in template_names: + template_name = template_name[9:] + setattr(valid_namespace['tower'].filename, template_name, 'EXAMPLE') + for type_, injector in value.items(): for key, tmpl in injector.items(): try: diff --git a/docs/custom_credential_types.md b/docs/custom_credential_types.md index c1b5387565..33426c2011 100644 --- a/docs/custom_credential_types.md +++ b/docs/custom_credential_types.md @@ -194,7 +194,8 @@ certificate/key data: } } - +Note that the single and multi-file syntax cannot be mixed within the same +``Credential Type``. Job and Job Template Credential Assignment ------------------------------------------ @@ -326,6 +327,8 @@ When verifying acceptance we should ensure the following statements are true: * Custom `Credential Types` should support injecting both single and multiple files. (Furthermore, the new syntax for injecting multiple files should work properly even if only a single file is injected). +* Users should not be able to use the syntax for injecting single and + multiple files in the same custom credential. * The default `Credential Types` included with Tower in 3.2 should be non-editable/readonly and cannot be deleted by any user. * Stored `Credential` values for _all_ types should be consistent before and From 4b13bcdce2363ca91b476035a5ba4a7bd14812cc Mon Sep 17 00:00:00 2001 From: Jim Ladd Date: Tue, 21 Nov 2017 14:43:10 -0500 Subject: [PATCH 4/6] Update tests for custom credentials --- awx/main/fields.py | 2 +- awx/main/tests/functional/test_credential.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index 6e8d3bb1f3..d63d1a30b8 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -763,7 +763,7 @@ class CredentialTypeInjectorField(JSONSchemaField): if 'template' not in template_names: valid_namespace['tower'].filename = TowerNamespace() for template_name in template_names: - template_name = template_name[9:] + template_name = template_name.split('.')[1] setattr(valid_namespace['tower'].filename, template_name, 'EXAMPLE') for type_, injector in value.items(): diff --git a/awx/main/tests/functional/test_credential.py b/awx/main/tests/functional/test_credential.py index 2bf15b3f8e..37609cd222 100644 --- a/awx/main/tests/functional/test_credential.py +++ b/awx/main/tests/functional/test_credential.py @@ -107,10 +107,11 @@ def test_cred_type_input_schema_validity(input_, valid): ({}, True), ({'invalid-injector': {}}, False), ({'file': 123}, False), - ({'file': {}}, False), + ({'file': {}}, True), ({'file': {'template': '{{username}}'}}, True), ({'file': {'template.username': '{{username}}'}}, True), ({'file': {'template.username': '{{username}}', 'template.password': '{{pass}}'}}, True), + ({'file': {'template': '{{username}}', 'template.password': '{{pass}}'}}, False), ({'file': {'foo': 'bar'}}, False), ({'env': 123}, False), ({'env': {}}, True), From 4c1dddcaf98d1909a696698d11da246e71264ecf Mon Sep 17 00:00:00 2001 From: Jim Ladd Date: Wed, 31 Jan 2018 11:22:01 -0500 Subject: [PATCH 5/6] Respond to PR feedback --- awx/main/fields.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/awx/main/fields.py b/awx/main/fields.py index d63d1a30b8..9bb257faba 100644 --- a/awx/main/fields.py +++ b/awx/main/fields.py @@ -696,7 +696,7 @@ class CredentialTypeInjectorField(JSONSchemaField): 'file': { 'type': 'object', 'patternProperties': { - '^template(\.[a-zA-Z_]+)?$': {'type': 'string'}, + '^template(\.[a-zA-Z_]+[a-zA-Z0-9_]*)?$': {'type': 'string'}, }, 'additionalProperties': False, }, @@ -751,9 +751,7 @@ class CredentialTypeInjectorField(JSONSchemaField): valid_namespace['tower'] = TowerNamespace() # ensure either single file or multi-file syntax is used (but not both) - template_names = set(key for type_, injector in value.items() - for key, tmpl in injector.items() - if key.startswith('template')) + template_names = [x for x in value.get('file', {}).keys() if x.startswith('template')] if 'template' in template_names and len(template_names) > 1: raise django_exceptions.ValidationError( _('Must use multi-file syntax when injecting multiple files'), From d558299b1f935835cdf905b625e79386b0d0e87c Mon Sep 17 00:00:00 2001 From: Jim Ladd Date: Fri, 2 Feb 2018 11:07:13 -0500 Subject: [PATCH 6/6] Add test for injecting multiple files --- awx/main/tests/unit/test_tasks.py | 44 +++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/awx/main/tests/unit/test_tasks.py b/awx/main/tests/unit/test_tasks.py index 2b24c18325..030db3958d 100644 --- a/awx/main/tests/unit/test_tasks.py +++ b/awx/main/tests/unit/test_tasks.py @@ -1230,6 +1230,50 @@ class TestJobCredentials(TestJobExecution): self.run_pexpect.side_effect = run_pexpect_side_effect self.task.run(self.pk) + def test_custom_environment_injectors_with_files(self): + some_cloud = CredentialType( + kind='cloud', + name='SomeCloud', + managed_by_tower=False, + inputs={ + 'fields': [{ + 'id': 'cert', + 'label': 'Certificate', + 'type': 'string' + }, { + 'id': 'key', + 'label': 'Key', + 'type': 'string' + }] + }, + injectors={ + 'file': { + 'template.cert': '[mycert]\n{{cert}}', + 'template.key': '[mykey]\n{{key}}' + }, + 'env': { + 'MY_CERT_INI_FILE': '{{tower.filename.cert}}', + 'MY_KEY_INI_FILE': '{{tower.filename.key}}' + } + } + ) + credential = Credential( + pk=1, + credential_type=some_cloud, + inputs = {'cert': 'CERT123', 'key': 'KEY123'} + ) + self.instance.credentials.add(credential) + self.task.run(self.pk) + + def run_pexpect_side_effect(*args, **kwargs): + args, cwd, env, stdout = args + assert open(env['MY_CERT_INI_FILE'], 'rb').read() == '[mycert]\nCERT123' + assert open(env['MY_KEY_INI_FILE'], 'rb').read() == '[mykey]\nKEY123' + return ['successful', 0] + + self.run_pexpect.side_effect = run_pexpect_side_effect + self.task.run(self.pk) + def test_multi_cloud(self): gce = CredentialType.defaults['gce']() gce_credential = Credential(