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

using regex for path split #368

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

eberna
Copy link
Contributor

@eberna eberna commented Aug 21, 2024

SUMMARY

Fixes #349

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ansible.utils.update_fact

ADDITIONAL INFORMATION

If a key in the path contains a square bracket the module does not separate the path correctly and if fails. This request changes the parser part to use regular expressions.

ansible@lnx-controller:~/ansible_collections/ansible/utils$ ansible-playbook -vv  main.yaml
ansible-playbook [core 2.16.3]
  config file = /home/ansible/ansible_collections/ansible/utils/ansible.cfg
  configured module search path = ['/home/ansible/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/ansible/.local/pipx/venvs/ansible/lib/python3.11/site-packages/ansible
  ansible collection location = /home/ansible/ansible_collections
  executable location = /home/ansible/.local/bin/ansible-playbook
  python version = 3.11.2 (main, May  2 2024, 11:59:08) [GCC 12.2.0] (/home/ansible/.local/pipx/venvs/ansible/bin/python)
  jinja version = 3.1.2
  libyaml = True
Using /home/ansible/ansible_collections/ansible/utils/ansible.cfg as config file
[DEPRECATION WARNING]: [defaults]collections_paths option, does not fit var naming standard, use the singular form collections_path instead. This feature
 will be removed from ansible-core in version 2.19. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.
[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'
running playbook inside collection ansible.utils
Skipping callback 'default', as we already have a stdout callback.
Skipping callback 'minimal', as we already have a stdout callback.
Skipping callback 'oneline', as we already have a stdout callback.

PLAYBOOK: main.yaml **************************************************************************************************************************************
1 plays in main.yaml

PLAY [127.0.0.1] *****************************************************************************************************************************************

TASK [Gathering Facts] ***********************************************************************************************************************************
task path: /home/ansible/ansible_collections/ansible/utils/main.yaml:1
ok: [127.0.0.1]

TASK [set fact] ******************************************************************************************************************************************
task path: /home/ansible/ansible_collections/ansible/utils/main.yaml:4
ok: [127.0.0.1] => {"ansible_facts": {"my_dict": {"[runners]": "old value"}}, "changed": false}

TASK [update fact] ***************************************************************************************************************************************
task path: /home/ansible/ansible_collections/ansible/utils/main.yaml:8
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: KeyError: '[runners'
fatal: [127.0.0.1]: FAILED! => {"changed": false, "msg": "Error: the key '[runners' was not found in {'[runners]': 'old value'}."}

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

ansible@lnx-controller:~/ansible_collections/ansible/utils$ ansible-playbook -vv  main.yaml
ansible-playbook [core 2.16.3]
  config file = /home/ansible/ansible_collections/ansible/utils/ansible.cfg
  configured module search path = ['/home/ansible/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/ansible/.local/pipx/venvs/ansible/lib/python3.11/site-packages/ansible
  ansible collection location = /home/ansible/ansible_collections
  executable location = /home/ansible/.local/bin/ansible-playbook
  python version = 3.11.2 (main, May  2 2024, 11:59:08) [GCC 12.2.0] (/home/ansible/.local/pipx/venvs/ansible/bin/python)
  jinja version = 3.1.2
  libyaml = True
Using /home/ansible/ansible_collections/ansible/utils/ansible.cfg as config file
[DEPRECATION WARNING]: [defaults]collections_paths option, does not fit var naming standard, use the singular form collections_path instead. This feature
 will be removed from ansible-core in version 2.19. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.
[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'
running playbook inside collection ansible.utils
Skipping callback 'default', as we already have a stdout callback.
Skipping callback 'minimal', as we already have a stdout callback.
Skipping callback 'oneline', as we already have a stdout callback.

PLAYBOOK: main.yaml **************************************************************************************************************************************
1 plays in main.yaml

PLAY [127.0.0.1] *****************************************************************************************************************************************

TASK [Gathering Facts] ***********************************************************************************************************************************
task path: /home/ansible/ansible_collections/ansible/utils/main.yaml:1
ok: [127.0.0.1]

TASK [set fact] ******************************************************************************************************************************************
task path: /home/ansible/ansible_collections/ansible/utils/main.yaml:4
ok: [127.0.0.1] => {"ansible_facts": {"my_dict": {"[runners]": "old value"}}, "changed": false}

TASK [update fact] ***************************************************************************************************************************************
task path: /home/ansible/ansible_collections/ansible/utils/main.yaml:8
changed: [127.0.0.1] => {"changed": true, "my_dict": {"[runners]": "new value"}}

TASK [debug] *********************************************************************************************************************************************
task path: /home/ansible/ansible_collections/ansible/utils/main.yaml:15
ok: [127.0.0.1] => {
    "my_dict": {
        "[runners]": "old value"
    }
}

TASK [debug] *********************************************************************************************************************************************
task path: /home/ansible/ansible_collections/ansible/utils/main.yaml:16
ok: [127.0.0.1] => {
    "updated": {
        "changed": true,
        "failed": false,
        "my_dict": {
            "[runners]": "new value"
        }
    }
}

PLAY RECAP ***********************************************************************************************************************************************
127.0.0.1                  : ok=5    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

Hope this may help, thank you.

Copy link
Contributor

Copy link
Contributor

@Ruchip16
Copy link
Contributor

@eberna can you please add specific tests as well to validate the change?

@eberna
Copy link
Contributor Author

eberna commented Aug 28, 2024

@eberna can you please add specific tests as well to validate the change?

Integration or unit test? or both?
thanks

@Ruchip16
Copy link
Contributor

@eberna can you please add specific tests as well to validate the change?

Integration or unit test? or both? thanks

I am fine with both or anything that would validate this particular change.

Copy link
Contributor

@eberna
Copy link
Contributor Author

eberna commented Sep 12, 2024

@eberna can you please add specific tests as well to validate the change?

Integration or unit test? or both? thanks

I am fine with both or anything that would validate this particular change.

done, I hope it's okay, in case let me know and I'll edit.

Copy link
Contributor

Copy link
Contributor

Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update_fact fails when path contains a key with a bracket in its name
2 participants