Enable support for injecting complex extra vars #13267

philipsd6 · 2022-12-01T20:56:19Z

SUMMARY

Until now, it's only been possible to inject flat strings with Jinja templating into extra vars from Custom Credential Types. However, it's quite desirable to be able to pass a data structure containing authentication information for certain collections/modules. For example, the Infoblox collection accepts authentication under the nios_provider key.

This PR enables that data structure to be created by an injector config.

related #1965 #13070

ISSUE TYPE

New or Enhanced Feature

COMPONENT NAME

Other

AWX VERSION

PYTHON=python3 make VERSION
awx: 21.9.1.dev32+g4af80777dd

ADDITIONAL INFORMATION

This PR also makes it possible to template the keys of an injector configuration, as requested in #13070.

For example, given an input configuration providing values for environment, host, apikey, username, it's possible to use an extra_vars injector configuration of:

"{{ environment }}_auth":
  host: "{{ host }}"
  apikey: "{{ apikey }}"
  username: "{{ username }}"
  verify_ssl: "{{ verify_ssl }}"

resulting in the following extra var available to the playbooks:

prod_auth:
  host: http://production.server
  apikey: 5hfthisj4hisj5hgnotj6hrealgj6
  username: philipsd6
  verify_ssl: True

This would lay the groundwork for allowing multiple credentials of the same credential type, as another credential might supply values for a different environment.

philipsd6 · 2022-12-02T12:18:51Z

I'm opening this PR to generate some discussion -- I'll update with tests and documentation if this passes muster.

AlanCoding · 2022-12-02T16:53:43Z

Let's get your example documented in docs/credentials/custom_credential_types.md

philipsd6 · 2022-12-02T19:14:18Z

Let's get your example documented in docs/credentials/custom_credential_types.md

I'll document the major part of this PR, but not the ability to template the key (yet) as that needs some discussion.

awx/main/models/credential/__init__.py

shanemcd · 2022-12-03T13:40:30Z

This is awesome 😄 Before we merge this we'll need to get some tests added. Check out test_tasks.py - there's a lot of examples of tests calling inject_credential.

philipsd6 · 2022-12-06T15:50:48Z

Added two tests for nested and templated extra vars keys, but still TBD: Updating the jsonschema for the CredentialTypeInjectorField model (the tests will fail until this is done.)

john-westcott-iv · 2022-12-07T20:12:16Z

@philipsd6 if you are all set with your changes can you convert this from a draft PR to an opened PR?

philipsd6 · 2022-12-07T20:16:43Z

@philipsd6 if you are all set with your changes can you convert this from a draft PR to an opened PR?

I don't think it's ready yet -- I'm struggling with the jsonschema that will allow recursively nested injector vars. Currently in my test env, I'm still getting this error when trying to create a nested extra_vars injector:

OrderedDict([('nested_auth', OrderedDict([('username', '{{ username }}'), ('password', '{{ password }}')]))]) is not valid under any of the given schemas

philipsd6 · 2022-12-14T21:44:07Z

I updated awx/main/fields.py:CredentialTypeInjectorField to try to make the extra_vars property recursive, but while the schema is ok with an online validator: https://www.jsonschemavalidator.net/s/Yr7ExLBf it still fails in AWX when trying to update a credential with a nested extra_var injector with:

TypeError: Can't compile non template nodes
10.0.2.100 PATCH /api/v2/credential_types/28/ - HTTP/1.1 500

philipsd6 · 2022-12-15T15:00:23Z

I updated awx/main/fields.py:CredentialTypeInjectorField to try to make the extra_vars property recursive, but while the schema is ok with an online validator: https://www.jsonschemavalidator.net/s/Yr7ExLBf it still fails in AWX when trying to update a credential with a nested extra_var injector with:
TypeError: Can't compile non template nodes
10.0.2.100 PATCH /api/v2/credential_types/28/ - HTTP/1.1 500

My bad, the jsonschema update worked fine, it was the validator that was failing on the nested part. Fixed with latest commit.

philipsd6 · 2022-12-16T13:05:57Z

awx/main/fields.py

@@ -790,13 +790,11 @@ def schema(self, model_instance):
                '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'}]}


This regex is ridiculous, but the only alternative I can see is to just make it r'^.+$'

AlanCoding · 2022-12-16T15:44:07Z

awx/main/fields.py

+                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),


I'm getting a server error due to the fact that key is undefined here.

yup, valid -- and not only that, so is type_. I'll have to revise a little deeper after closer inspection. Thanks for the review.

AlanCoding · 2022-12-16T16:25:29Z

awx/main/fields.py

+
+        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()}


We really don't want to lose context in error messages. The key passed in should always be a string right? So what if you combined k and key for the value here, so instead of validate_extra_vars(k, v) do validate_extra_vars(f'{key}:{k}', v), or whatever you'd prefer for delimiter. That would keep stacking layers of context on a rolling basis.

I used a '.' (dot) as the dict key delimiter, seemed appropriate for this.

AlanCoding · 2022-12-16T16:26:48Z

awx/main/fields.py

+            if isinstance(node, dict):
+                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(key, x) for x in node]


I'd like to change for x in node to for i, x in enumerate(node) and then stack onto key, like f'{key}:{i}. It doesn't distinguish between dicts and lists... but it shouldn't matter because the user know what injectors they submitted.

I think the context is fully maintained now in the validation error messages. I used key[i][i] style for the validation error message, as between the dict dot style and this, it should be pseudo jq/jmespath style.

@AlanCoding: running podman exec -it awx-tools_awx_1 make test_unit works passes the new unit tests, But I'm not sure how to actually test those validation error paths.

AlanCoding · 2023-01-10T19:34:29Z

awx/main/fields.py

-                        params={'value': value},
-                    )
+            if type_ == 'extra_vars':
+                validate_extra_vars(None, injector)


This None is coming through in the error message with a grammar that's not really desirable. I've actually been using the docs as an error case, because the existing example is badly formatted. That does need to be changed... but the value is:

"injectors": { "env": { "THIRD_PARTY_CLOUD_API_TOKEN": "{{api_token}}" }, "extra_vars": { "some_extra_var": "{{username}}:{{password}", "auth": { "username": "{{username}}", "password": "{{password}}" } } }

A bit here is missing a closing }. The currently produced error message is:

{'injectors': ["Syntax error rendering template for None.auth.username inside of extra_vars (unexpected '}')"]}

You know, in this line, injector must be a dictionary. This is fixable, but I think it will take an extra line or two of string formatting inside of validate_extra_vars.

I am going to write down a few validation test cases and save them for later.

AlanCoding · 2023-01-19T03:29:19Z

I poked at this some more and put up philipsd6#571, which is mainly concerned with error messages.

github-actions bot added component:api community labels Dec 1, 2022

philipsd6 mentioned this pull request Dec 2, 2022

AWX Office Hours Agenda - Dec 13 2022 #13240

Closed

github-actions bot added the component:docs label Dec 2, 2022

AlanCoding reviewed Dec 2, 2022

View reviewed changes

awx/main/models/credential/__init__.py Outdated Show resolved Hide resolved

AlanCoding reviewed Dec 2, 2022

View reviewed changes

awx/main/models/credential/__init__.py Outdated Show resolved Hide resolved

philipsd6 force-pushed the feature/complex_extra_vars branch from 2f7a3d0 to 9629119 Compare December 2, 2022 20:10

philipsd6 force-pushed the feature/complex_extra_vars branch from 9629119 to c00c6dc Compare December 6, 2022 15:49

philipsd6 force-pushed the feature/complex_extra_vars branch 4 times, most recently from d243279 to 34ea161 Compare December 14, 2022 21:40

philipsd6 force-pushed the feature/complex_extra_vars branch from b80a1b5 to 26f55f3 Compare December 15, 2022 15:10

philipsd6 marked this pull request as ready for review December 15, 2022 15:13

philipsd6 force-pushed the feature/complex_extra_vars branch from 26f55f3 to 628ab81 Compare December 15, 2022 15:31

philipsd6 requested a review from AlanCoding December 15, 2022 15:40

philipsd6 commented Dec 16, 2022

View reviewed changes

AlanCoding reviewed Dec 16, 2022

View reviewed changes

philipsd6 force-pushed the feature/complex_extra_vars branch from 454ed23 to 35433de Compare January 9, 2023 19:59

philipsd6 requested a review from AlanCoding January 9, 2023 20:03

philipsd6 force-pushed the feature/complex_extra_vars branch 2 times, most recently from 891fe8d to 3c51d01 Compare January 9, 2023 21:21

AlanCoding reviewed Jan 10, 2023

View reviewed changes

jay-steurer self-assigned this Jan 10, 2023

jay-steurer approved these changes Jan 11, 2023

View reviewed changes

philipsd6 force-pushed the feature/complex_extra_vars branch from 3c51d01 to 4073d0e Compare January 17, 2023 20:38

philipsd6 added 9 commits January 17, 2023 19:03

Add function to walk the extra_vars and render the results

2034eac

Render keys while walking extra_vars in addition to values

d75f12c

Update documentation to include subkey injection

128a130

Add two tests for nested and templated extra_vars keys

27ea239

Add recursive properties to injectors jsonschema for extra_vars

fcf5695

Add functions to support recursive validation for extra_vars

ad4e257

Expand pattern to support use of Jinja2 block delimiters

51e244e

Account for validation context

ae92f82

Maintain nested context for validation error messages

7f6f57b

philipsd6 force-pushed the feature/complex_extra_vars branch from 4073d0e to 7f6f57b Compare January 18, 2023 00:03

Touchup of validation logic from testing

9777ce7

fosterseth assigned AlanCoding Feb 1, 2023

AlanCoding approved these changes Feb 2, 2023

View reviewed changes

AlanCoding merged commit 1af955d into ansible:devel Feb 2, 2023

AlanCoding mentioned this pull request Feb 2, 2023

Custom credentials injector does not support hash result, only flat #1965

Closed

fosterseth mentioned this pull request Feb 14, 2023

AWX Community Meeting Agenda - Mar 14 2023 #13563

Closed

philipsd6 deleted the feature/complex_extra_vars branch March 21, 2023 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable support for injecting complex extra vars #13267

Enable support for injecting complex extra vars #13267

philipsd6 commented Dec 1, 2022 •

edited

philipsd6 commented Dec 2, 2022

AlanCoding commented Dec 2, 2022

philipsd6 commented Dec 2, 2022 •

edited

shanemcd commented Dec 3, 2022

philipsd6 commented Dec 6, 2022 •

edited

john-westcott-iv commented Dec 7, 2022

philipsd6 commented Dec 7, 2022

philipsd6 commented Dec 14, 2022

philipsd6 commented Dec 15, 2022

philipsd6 Dec 16, 2022

AlanCoding Dec 16, 2022

philipsd6 Dec 16, 2022

AlanCoding Dec 16, 2022

philipsd6 Jan 9, 2023

AlanCoding Dec 16, 2022

philipsd6 Jan 9, 2023

AlanCoding Jan 10, 2023

AlanCoding commented Jan 19, 2023

Enable support for injecting complex extra vars #13267

Enable support for injecting complex extra vars #13267

Conversation

philipsd6 commented Dec 1, 2022 • edited

SUMMARY

ISSUE TYPE

COMPONENT NAME

AWX VERSION

ADDITIONAL INFORMATION

philipsd6 commented Dec 2, 2022

AlanCoding commented Dec 2, 2022

philipsd6 commented Dec 2, 2022 • edited

shanemcd commented Dec 3, 2022

philipsd6 commented Dec 6, 2022 • edited

john-westcott-iv commented Dec 7, 2022

philipsd6 commented Dec 7, 2022

philipsd6 commented Dec 14, 2022

philipsd6 commented Dec 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlanCoding commented Jan 19, 2023

philipsd6 commented Dec 1, 2022 •

edited

philipsd6 commented Dec 2, 2022 •

edited

philipsd6 commented Dec 6, 2022 •

edited