community.general icon indicating copy to clipboard operation
community.general copied to clipboard

virtualbox: Fix crash when handling deeply nested hostvars

Open basicdays opened this issue 1 year ago • 1 comments

SUMMARY

Fixes #5332

Skip parsing values with keys that have both a value and nested data. Skip parsing values that are nested more than two keys deep.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

virtualbox

ADDITIONAL INFORMATION

Taken from the issue, calling ansible all -i vbox.yml --list -vvv produces an error that contains the following:

[WARNING]: * Failed to parse /home/etienne/playbook/vbox.yml with auto plugin: 'str' object does not support item assignment
File "/home/etienne/.local/lib/python3.8/site-packages/ansible/inventory/manager.py", line 290, in parse_source
plugin.parse(self._inventory, self._loader, source, cache=cache)
File "/home/etienne/.local/lib/python3.8/site-packages/ansible/plugins/inventory/auto.py", line 59, in parse
plugin.parse(inventory, loader, path, cache=cache)
File "/home/etienne/.ansible/collections/ansible_collections/community/general/plugins/inventory/virtualbox.py", line 281, in parse

This occurs when a virtualbox instance from the command VBoxManage showvminfo <uuid|vmname> has a key with both a value and nested data. This can be seen with the following vminfo segment:

Recording screens:           1
 Screen 0:
    Enabled:                 yes
    ID:                      0
    Record video:            yes
    Record audio:            no
    Destination:             File
    File:                    /Users/<USER>/VirtualBox VMs/<VBOX_INSTANCE>/<VBOX_INSTANCE>-screen0.webm
    Options:                 vc_enabled=true,ac_enabled=false,ac_profile=med
    Video dimensions:        1024x768
    Video rate:              512kbps
    Video FPS:               25fps

This fix now ignores the Recording screens value of 1. In addition, the nested information under Screen 0 is ignored.

This results in a hostvars dictionary of:

{
    "_meta": {
         hostvars": {
            "<VBOX_INSTANCE>" {
                ...
                "vbox_Recording_screens": {
                    "vbox_Screen_0": ""
                },
                ...
            }
        }
    }
}

Since the showvminfo is using a human readable output, I am not certain how nested data is formatted across the various Virtualbox versions. Because of that, I've decided to just skip deeply nested data. Another avenue to pull information about a vm instance could be using VBoxManage showvminfo --machinereadable <uuid|vmname> instead. However the data keys would change, meaning this would be a breaking change.

For reference, the following is the same information with the machinereadable option:

recording_screens=1
 rec_screen0
rec_screen_enabled="on"
rec_screen_id=0
rec_screen_video_enabled="on"
rec_screen_audio_enabled="off"
rec_screen_dest="File"
rec_screen_dest_filename="/Users/paulsanchez/VirtualBox VMs/rails-sandbox/rails-sandbox-screen0.webm"
rec_screen_opts="vc_enabled=true,ac_enabled=false,ac_profile=med"
rec_screen_video_res_xy="1024x768"
rec_screen_video_rate_kbps=512
rec_screen_video_fps=25

basicdays avatar Oct 12 '22 01:10 basicdays

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

github-actions[bot] avatar Oct 12 '22 16:10 github-actions[bot]

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/b0bb994c3e965381a7ce1f3b2dae6280d7e8b741/pr-5348

Backported as https://github.com/ansible-collections/community.general/pull/5381

🤖 @patchback I'm built with octomachinery and my source is open — https://github.com/sanitizers/patchback-github-app.

patchback[bot] avatar Oct 18 '22 07:10 patchback[bot]

@basicdays thanks for your contribution! @russoz thanks for reviewing!

felixfontein avatar Oct 18 '22 07:10 felixfontein