Incorrect group_name validation in lxd dynamic inventory plugin

[kaharlichenko]

Summary

I'm trying to set up my own home lab Kubernetes cluster using playbooks from amazing Kubespray project.

The aforementioned playbooks heavily rely on group names containing underscores like kube_node, kube_control_plane, etc. Alas, when trying to build the dynamic inventory with the lxd inventory plugin I get crashes due to the group name validation.

According to Inventory basics: formats, hosts, and groups:

Group names should follow the same guidelines as Creating valid variable names.

From Creating valid variable names we can learn that:

A variable name can only include letters, numbers, and underscores. Python keywords or playbook keywords are not valid variable names. A variable name cannot begin with a number.

That makes me think that both kube_node and kube_control_plane should be accepted as valid group names.

---

I believe the culprit resides here: https://github.com/ansible-collections/community.general/blob/42f7531f212c6401ca351de62bde0369916d8e58/plugins/inventory/lxd.py#L978-L982

isalnum gives both false negative and false positive validation results as can be seen below:

"γöфウ".isalnum()  # True, even though there are "funny" non-ASCII letters
"kube_control_plane".isalnum()  # False, even though an underscore is a perfectly valid symbol

I suggest changing the validation to something like this:

if self.groupby:
    var_name_pattern = re.compile(r"^[a-z_][a-z0-9_]*$", flags=re.IGNORECASE)

    for group_name in self.groupby:
        if not var_name_pattern.match(group_name):
            raise AnsibleParserError('Invalid character(s) in groupname: {0}'.format(to_native(group_name)))
        group_type(group_name)

This is still not ideal as it doesn't filter out neither Python nor Ansible keywords, but at least it one step closer to the variable definition from the docs.

Issue Type

Bug Report

Component Name

lxd

Ansible Version

$ ./venv/bin/ansible --version
ansible [core 2.15.0]
  config file = /tmp/zxc/ansible.cfg
  configured module search path = ['/home/ihor/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /tmp/zxc/venv/lib/python3.10/site-packages/ansible
  ansible collection location = /tmp/zxc/collections
  executable location = ./venv/bin/ansible
  python version = 3.10.6 (main, May 29 2023, 11:10:38) [GCC 11.3.0] (/tmp/zxc/venv/bin/python3)
  jinja version = 3.1.2
  libyaml = True

Community.general Version

$ ./venv/bin/ansible-galaxy collection list community.general

# /tmp/zxc/collections/ansible_collections
Collection        Version
----------------- -------
community.general 7.0.1

Configuration

$ ./venv/bin/ansible-config dump --only-changed
COLLECTIONS_PATHS(/tmp/zxc/ansible.cfg) = ['/tmp/zxc/collections']
CONFIG_FILE() = /tmp/zxc/ansible.cfg
DEFAULT_INVENTORY_PLUGIN_PATH(/tmp/zxc/ansible.cfg) = ['/tmp/zxc']
PAGER(env: PAGER) = less

OS / Environment

$ cat /etc/lsb-release 
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=22.04
DISTRIB_CODENAME=jammy
DISTRIB_DESCRIPTION="Ubuntu 22.04.2 LTS"

Steps to Reproduce

Given the hosts-lxd.yml inventory file with the following contents:

plugin: community.general.lxd
project: k8s
type_filter: both
groupby:
  kube_node:
    type: type
    attribute: virtual-machine

When I try to list the dynamic inventory with ansible-inventory --inventory hosts-lxd.yml --list I got a crash due to the group name validation. If I change kube_node to kubenode in the yaml file above everything works like a charm.

Expected Results

A proper dynamic inventory is expected to be built from the introspection of the LXD server instance. The group names must have underscores included in them.

Actual Results

$ ./venv/bin/ansible-inventory --inventory hosts-lxd.yml --list
[WARNING]:  * Failed to parse /tmp/zxc/hosts-lxd.yml with auto plugin: Invalid character(s) in groupname: kube_node
[WARNING]:  * Failed to parse /tmp/zxc/hosts-lxd.yml with yaml plugin: Plugin configuration YAML file, not YAML inventory
[WARNING]:  * Failed to parse /tmp/zxc/hosts-lxd.yml with ini plugin: Invalid host pattern 'plugin:' supplied, ending in ':' is not allowed, this character is reserved to
provide a port.
[WARNING]: Unable to parse /tmp/zxc/hosts-lxd.yml as an inventory source
[WARNING]: No inventory was parsed, only implicit localhost is available
{
    "_meta": {
        "hostvars": {}
    },
    "all": {
        "children": [
            "ungrouped"
        ]
    }
}

Code of Conduct

• [X] I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/connection/lxd.py

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

[ansibullbot]

cc @mattclay click here for bot help

[kaharlichenko]

!component =plugins/inventory/lxd.py

[ansibullbot]

Files identified in the description:

• plugins/inventory/lxd.py

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

[ansibullbot]

cc @conloos click here for bot help

[felixfontein]

Maybe it's better to remove the valiation completely? In any case, if it is changed, it needs to accept the names it accepted before (even though it also accepted pretty strange ones).

[kaharlichenko]

it needs to accept the names it accepted before (even though it also accepted pretty strange ones)

There's no guarantee that ansible itself will work well with "strange" names, so I believe it would be safe to forbid them.

As for allowing the new "good" ones it should be safe as well, since they are "good" by definition :)

On the other hand, it might be a good idea to be consistent with the rest of the community inventory plugins: if nobody does any validation, then maybe it's reasonable to drop it here too.

Another option might be using some kind of validate_variable_name function from ansible-core itself, provided such a function exists. I'm not that familiar with the code base.

[felixfontein]

ansible-core does some validation and allows sanitization of group names itself, see https://docs.ansible.com/ansible/latest/reference_appendices/config.html#transform-invalid-group-chars. I don't think it is a good idea to repeat this in all kind of different manners in all inventory plugins.