Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Role Argument Spec Validation Improvements - String conversion, int ranges, Module validation features #77159

Closed
1 task done
Nubly opened this issue Feb 28, 2022 · 10 comments
Closed
1 task done
Labels
affects_2.12 bot_closed feature This issue/PR relates to a feature request. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@Nubly
Copy link

Nubly commented Feb 28, 2022

Summary

(All code previews are from the 2.12.3 release commit on the stable-2.12 branch.)

I was looking into valiadation methods for Ansible roles, and was excited to see that Ansible now has support for automatic role argument validation tasks through the meta/argument_specs.yml file.

Only the 'options' portion of the argument specification is considered in the creation of automatically inserted validation tasks:

return {
'action': {
'module': 'ansible.builtin.validate_argument_spec',
# Pass only the 'options' portion of the arg spec to the module.
'argument_spec': argument_spec.get('options', {}),
'provided_arguments': self._role_params,
'validate_args_context': {
'type': 'role',
'name': self._role_name,
'argument_spec_name': entrypoint_name,
'path': self._role_path
},
},
'name': task_name,
'tags': ['always'],
}

Many features which are available to AnsibleModule's argument_spec are not available to role argument specifications, such as options being mutually exclusive, and I believe this may be why.

String conversion is another issue. The STRING_CONVERSION_ACTION configuration option appears to not apply here, and it appears that there is currently no way from within YAML (or existing code for that matter) to control whether or not things get converted to strings:

def check_type_str(value, allow_conversion=True, param=None, prefix=''):

The only way strings are useful with this limitation in mind is to use the choices option for the spec and this is not applicable to every use case - even if it does support regular expressions.

Lastly, range support for the int data type would be nice. For example, when you'd like to specify a valid port number and not have to use regex on a string to validate.

The lack of these features is preventing me from using role argument spec validation within my roles versus other tools, particularly the default string conversion settings within the check_type_str() method. Being able to pass a list of dicts to my role when it expects a string and have it pass validation is something that I would like to be able to configure as a failure.

Please let me know if this is better off as a proposal instead.

Issue Type

Feature Idea

Component Name

Role Argument Spec Validation

Additional Information

Ansible Version Info

nubly@hostname ~
(ansible-2.12) > $ ansible --version
ansible [core 2.12.2]
  config file = /home/nubly/Git/ansible-repo-test/ansible.cfg
  configured module search path = ['/home/nubly/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/nubly/.venvs/ansible-2.12/lib/python3.9/site-packages/ansible
  ansible collection location = /home/nubly/Git/ansible-repo-test/collections
  executable location = /home/nubly/.venvs/ansible-2.12/bin/ansible
  python version = 3.9.2 (default, Jan 19 2022, 12:26:41) [GCC 11.1.0]
  jinja version = 3.0.3
  libyaml = True

Example Run

meta/argument_specs.yml:

---
argument_specs:
  main:
    short_description: "Main play/entrypoint of {{ role_name }}"
    options:
      test_option_one:
        description: "An option to test."
        type: "str"
      test_option_two:
        description: "This is meant to be a port number from 1000-65535"
        type: "int"
    mutually_exclusive:
      - "test_option_one"
      - "test_option_two"

test-playbook.yml:

---
- hosts: "localhost"
  vars:
    test_option_one:
      - name: "dict one"
        dict_one_var: 6.628
      - name: "dict two"
        dict_two_var:
          - "I am a list"
          - "of various values"
          - "nested inside a dict in a list!"
          - 1
          - 1.01
          - true
          - false
    test_option_two: -99999
  roles:
    - "test_role"

tasks/main.yml:

---
- name: "Print some info about the vars"
  debug:
    msg: "{{ test_option_one }} 
        {{ test_option_two }}"

ansible-playbook run:

nubly@hostname ~/ansible/roles/test_role
(ansible-2.12) > $ ansible-playbook -i "localhost," test-playbook.yml

PLAY [localhost] ******************************************************************************************************************************************

TASK [test_role : Validating arguments against arg spec 'main' - Main entrypoint/task of test_role] *******************************************************
ok: [localhost]

TASK [test_role : Print some info about the vars] *********************************************************************************************************
ok: [localhost] => {
    "msg": "[{'name': 'dict one', 'dict_one_var': 'hello'}, {'name': 'dict two', 'dict_two_var': ['I am a list', 'of various values', 'nested inside a dict in a list!', 1, 1.01, True, 'world']}] -99999"
}

PLAY RECAP ************************************************************************************************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibot
Copy link
Contributor

ansibot commented Feb 28, 2022

Files identified in the description:
None

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.12 feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Feb 28, 2022
@sivel
Copy link
Member

sivel commented Feb 28, 2022

String conversion is another issue. The STRING_CONVERSION_ACTION configuration option appears to not apply here, and it appears that there is currently no way from within YAML (or existing code for that matter) to control whether or not things get converted to strings:

STRING_CONVERSION_ACTION is effectively non-existent any more. It doesn't affect anything, and we need to remove it from the config. Additionally, not even python argument specs can take advantage of allow_conversion, so it should be deprecated. What we need is a str_strict which we talked about at one point.

Generally speaking the intention of our argspec types is coercion validation. If you say the value requires an int, try to make it an int if possible, and fail otherwise. The same with str, etc... This is largely due to how ansible interacts with jinja2, in that a templating action can never actually produce an integer unless jinja2 native is enabled.

Many features which are available to AnsibleModule's argument_spec are not available to role argument specifications, such as options being mutually exclusive, and I believe this may be why.

This was intentional, in that the MVP was only to implement certain aspects of the module argument spec functionality. Not that it cannot be extended at this point.

@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Mar 1, 2022
@sivel
Copy link
Member

sivel commented Mar 2, 2022

Just as an FYI, the argument spec will never be able to implement every possible feature. For things like ranges, you would need to do what we recommend for plugin authors, and implement the check external to the argument spec.

The argument spec would simply need to indicate it was an int, and then add an assert task to verify it falls within a range.

As for non-coercing validators, a X_strict version of each validator would need implemented. This goes for most all of them currently.

@sivel sivel added the waiting_on_contributor This would be accepted but there are no plans to actively work on it. label Mar 2, 2022
@kpfleming
Copy link
Contributor

There's another feature of module specs that doesn't appear to be supported in role specs: defaults. They can be specified, and ansible-doc -t role will display them, but they don't appear to actually be implemented.

@bcoca
Copy link
Member

bcoca commented Oct 20, 2022

roles already have a defaults/main.yml so it was not implemented in the spec to avoid conflicts

@kpfleming
Copy link
Contributor

Sure, that's logical, but they don't quite serve the same purpose :-)

@nkakouros
Copy link
Contributor

@kpfleming see also #77664

@kpfleming
Copy link
Contributor

Thanks for the link. This is definitely a complicated area, as I'd like to be able to set defaults for specific keys inside a dict argument, which I can't do in defaults/main.yml. I make use of that in modules today and it's really handy :-)

@ziegenberg
Copy link
Contributor

This is partly related to #74995.

@ansibot
Copy link
Contributor

ansibot commented Mar 11, 2023

Thank you very much for your submission to Ansible. It means a lot to us that you've taken time to contribute.

Unfortunately, this issue has been open for some time while waiting for a contributor to take it up but there does not seem to have been anyone that did so. So we are going to close this issue to clear up the queues and make it easier for contributors to browse possible implementation targets.

However, we're absolutely always up for discussion. Because this project is very active, we're unlikely to see comments made on closed tickets and we lock them after some time. If you or anyone else has any further questions, please let us know by using any of the communication methods listed in the page below:

In the future, sometimes starting a discussion on the development list prior to proposing or implementing a feature can make getting things included a little easier, but it's not always necessary.

Thank you once again for this and your interest in Ansible!

click here for bot help

@ansibot ansibot added bot_closed and removed waiting_on_contributor This would be accepted but there are no plans to actively work on it. labels Mar 11, 2023
@ansibot ansibot closed this as completed Mar 11, 2023
@ansible ansible locked and limited conversation to collaborators Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.12 bot_closed feature This issue/PR relates to a feature request. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

No branches or pull requests

8 participants