KeyError raised if max hostname length exceeded.

Bug #1886905 reported by Ryan Drew
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack-Ansible
Fix Released
Undecided
Ryan Drew

Bug Description

When inventory is dynamically generated at playbook execution, the function osa_toolkit.generate.add_container_hosts will perform a sanity check to ensure that hostnames provided in openstack_user_config.yml don't exceed a hardcoded max length. The relevant code snippets for this check are as follows:

---python---
if len(type_and_name) > max_hostname_len and not properties['is_metal']:
---end---

---python---
elif len(host_type) > 63 and properties['is_metal']:
---end---

The properties dictionary doesn't always contain the 'is_metal' key, therefore we run into an issue where if the logical AND statements aren't short-circuited then a KeyError is raised. Steps that I used to reproduce are below.

* Clone https://opendev.org/openstack/openstack-ansible.git
* Copy openstack_user_config.yml.aio to openstack_user_config.yml
* Inside openstack_user_config.yml.aio replace "aio1" with "blahblahblahallinonehostwithalonghostname1"
* Create a debug playbook in (openstack-ansible root)/playbooks/debug.yml:

---yaml---
---
- name: Debug
  hosts: all
  tasks:
    - name: Print something
      debug:
        msg: "something"
---end---

* Replace the first line of inventory/dynamic_inventory.py to "#!/usr/bin/env python3"
* Set CWD to (openstack-ansible root)/inventory
* Create a python virtualenv with the following Pipfile file:

---Pipfile---
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

[dev-packages]

[packages]
netaddr = "*"
pyyaml = "*"

[requires]
python_version = "3.8"
---end---

* Run the following command:

---bash---
OSA_CONFIG_DIR=$(pwd)/../etc/openstack_deploy pipenv run ansible-playbook -i ./dynamic_inventory.py ../playbooks/debug.yml -vvvv
---end---

Output received:

ansible-playbook 2.9.9
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/rdrew/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.8/site-packages/ansible
  executable location = /usr/bin/ansible-playbook
  python version = 3.8.3 (default, May 29 2020, 00:00:00) [GCC 10.1.1 20200507 (Red Hat 10.1.1-1)]
Using /etc/ansible/ansible.cfg as config file
setting up inventory plugins
host_list declined parsing /home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py as it did not pass its verify_file() method
['/home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py', '--list']
auto declined parsing /home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py as it did not pass its verify_file() method
yaml declined parsing /home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py as it did not pass its verify_file() method
toml declined parsing /home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py as it did not pass its verify_file() method
[WARNING]: * Failed to parse /home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py with script plugin: Inventory script (/home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py) had an execution
error: Traceback (most recent call last): File "/home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py", line 80, in <module> output = generate.main(**all_args) File "/home/rdrew/Documents/ansible/osa/openstack-
ansible/inventory/../osa_toolkit/generate.py", line 1194, in main container_skel_load( File "/home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/../osa_toolkit/generate.py", line 723, in container_skel_load _add_container_hosts(
File "/home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/../osa_toolkit/generate.py", line 374, in _add_container_hosts not properties['is_metal']: KeyError: 'is_metal'
  File "/usr/lib/python3.8/site-packages/ansible/inventory/manager.py", line 280, in parse_source
    plugin.parse(self._inventory, self._loader, source, cache=cache)
  File "/usr/lib/python3.8/site-packages/ansible/plugins/inventory/script.py", line 163, in parse
    raise AnsibleParserError(to_native(e))
[WARNING]: * Failed to parse /home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py with ini plugin: /home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py:18: Expected key=value host variable
assignment, got: argparse
  File "/usr/lib/python3.8/site-packages/ansible/inventory/manager.py", line 280, in parse_source
    plugin.parse(self._inventory, self._loader, source, cache=cache)
  File "/usr/lib/python3.8/site-packages/ansible/plugins/inventory/ini.py", line 138, in parse
    raise AnsibleParserError(e)
[WARNING]: Unable to parse /home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py as an inventory source
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'
Loading callback plugin default of type stdout, v2.0 from /usr/lib/python3.8/site-packages/ansible/plugins/callback/default.py

PLAYBOOK: debug.yml ******************************************************************************************************************************************************************************************************************************************
Positional arguments: ../playbooks/debug.yml
verbosity: 4
connection: smart
timeout: 10
become_method: sudo
tags: ('all',)
inventory: ('/home/rdrew/Documents/ansible/osa/openstack-ansible/inventory/dynamic_inventory.py',)
forks: 5
1 plays in ../playbooks/debug.yml

PLAY [Debug] *************************************************************************************************************************************************************************************************************************************************
skipping: no hosts matched

PLAY RECAP ***************************************************************************************************************************************************************************************************************************************************

Output Expected:

Ansible at least tries to run the "Print something" task.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to openstack-ansible (master)

Fix proposed to branch: master
Review: https://review.opendev.org/740343

Changed in openstack-ansible:
assignee: nobody → Ryan Drew (learnitall0)
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to openstack-ansible (master)

Reviewed: https://review.opendev.org/740343
Committed: https://git.openstack.org/cgit/openstack/openstack-ansible/commit/?id=d2657174ceaf289ad60ba7291a8a9eed10c26687
Submitter: Zuul
Branch: master

commit d2657174ceaf289ad60ba7291a8a9eed10c26687
Author: Ryan Drew <email address hidden>
Date: Thu Jul 9 12:01:00 2020 -0600

    Fix KeyError raised when max hostname length exceeded

    The function osa_toolkit.generate._add_container_hosts contains a bug in
    the code used to check if provided hostnames have exceeded their max
    allowed length. The logic used to perform this check depends on the
    `is_metal` flag within each container's properties. Unfortunately the
    `is_metal` flag is accessed within the `properties` dictionary using
    bracket notation rather than the safer `dict.get` method, causing a
    `KeyError` to be raised when a host's properties dictionary does not
    contain the `is_metal` flag.

    It is not expected that a `KeyError` would be raised in the function if
    hostnames have exceeded their max length. It is instead expected that a
    `SystemExit` exception would be raised warning the user of their invalid
    hostname(s).

    This bug will impacts deployments where hostnames actually do exceed the
    max allowed length due to the short circuit logic used in the if-elif
    tree.

    Closes-Bug: #1886905

    Change-Id: Ic1acfea71f27f94e277aa443f0a53ef16b4eb417

Changed in openstack-ansible:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to openstack-ansible (stable/ussuri)

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/740441

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to openstack-ansible (stable/train)

Fix proposed to branch: stable/train
Review: https://review.opendev.org/740442

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to openstack-ansible (stable/stein)

Fix proposed to branch: stable/stein
Review: https://review.opendev.org/740443

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to openstack-ansible (stable/ussuri)

Reviewed: https://review.opendev.org/740441
Committed: https://git.openstack.org/cgit/openstack/openstack-ansible/commit/?id=59deac41ea535d66d98cad28f85cdf57572c1acb
Submitter: Zuul
Branch: stable/ussuri

commit 59deac41ea535d66d98cad28f85cdf57572c1acb
Author: Ryan Drew <email address hidden>
Date: Thu Jul 9 12:01:00 2020 -0600

    Fix KeyError raised when max hostname length exceeded

    The function osa_toolkit.generate._add_container_hosts contains a bug in
    the code used to check if provided hostnames have exceeded their max
    allowed length. The logic used to perform this check depends on the
    `is_metal` flag within each container's properties. Unfortunately the
    `is_metal` flag is accessed within the `properties` dictionary using
    bracket notation rather than the safer `dict.get` method, causing a
    `KeyError` to be raised when a host's properties dictionary does not
    contain the `is_metal` flag.

    It is not expected that a `KeyError` would be raised in the function if
    hostnames have exceeded their max length. It is instead expected that a
    `SystemExit` exception would be raised warning the user of their invalid
    hostname(s).

    This bug will impacts deployments where hostnames actually do exceed the
    max allowed length due to the short circuit logic used in the if-elif
    tree.

    Closes-Bug: #1886905

    Change-Id: Ic1acfea71f27f94e277aa443f0a53ef16b4eb417

tags: added: in-stable-ussuri
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to openstack-ansible (stable/stein)

Reviewed: https://review.opendev.org/740443
Committed: https://git.openstack.org/cgit/openstack/openstack-ansible/commit/?id=f59ee5bdb6dc8c0acda8cef4a70e38b7f19e9060
Submitter: Zuul
Branch: stable/stein

commit f59ee5bdb6dc8c0acda8cef4a70e38b7f19e9060
Author: Ryan Drew <email address hidden>
Date: Thu Jul 9 12:01:00 2020 -0600

    Fix KeyError raised when max hostname length exceeded

    The function osa_toolkit.generate._add_container_hosts contains a bug in
    the code used to check if provided hostnames have exceeded their max
    allowed length. The logic used to perform this check depends on the
    `is_metal` flag within each container's properties. Unfortunately the
    `is_metal` flag is accessed within the `properties` dictionary using
    bracket notation rather than the safer `dict.get` method, causing a
    `KeyError` to be raised when a host's properties dictionary does not
    contain the `is_metal` flag.

    It is not expected that a `KeyError` would be raised in the function if
    hostnames have exceeded their max length. It is instead expected that a
    `SystemExit` exception would be raised warning the user of their invalid
    hostname(s).

    This bug will impacts deployments where hostnames actually do exceed the
    max allowed length due to the short circuit logic used in the if-elif
    tree.

    Closes-Bug: #1886905

    Change-Id: Ic1acfea71f27f94e277aa443f0a53ef16b4eb417

tags: added: in-stable-stein
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to openstack-ansible (stable/train)

Reviewed: https://review.opendev.org/740442
Committed: https://git.openstack.org/cgit/openstack/openstack-ansible/commit/?id=664d9457f842b3c2b1e0cf6f90fffd727ea6011f
Submitter: Zuul
Branch: stable/train

commit 664d9457f842b3c2b1e0cf6f90fffd727ea6011f
Author: Ryan Drew <email address hidden>
Date: Thu Jul 9 12:01:00 2020 -0600

    Fix KeyError raised when max hostname length exceeded

    The function osa_toolkit.generate._add_container_hosts contains a bug in
    the code used to check if provided hostnames have exceeded their max
    allowed length. The logic used to perform this check depends on the
    `is_metal` flag within each container's properties. Unfortunately the
    `is_metal` flag is accessed within the `properties` dictionary using
    bracket notation rather than the safer `dict.get` method, causing a
    `KeyError` to be raised when a host's properties dictionary does not
    contain the `is_metal` flag.

    It is not expected that a `KeyError` would be raised in the function if
    hostnames have exceeded their max length. It is instead expected that a
    `SystemExit` exception would be raised warning the user of their invalid
    hostname(s).

    This bug will impacts deployments where hostnames actually do exceed the
    max allowed length due to the short circuit logic used in the if-elif
    tree.

    Closes-Bug: #1886905

    Change-Id: Ic1acfea71f27f94e277aa443f0a53ef16b4eb417

tags: added: in-stable-train
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.