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

plugins: filter: ipaddr: Add AddrFormatError exeption when hwaddr value is not macaddr #39140

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
4 participants
@k0ste
Contributor

k0ste commented Apr 23, 2018

SUMMARY

netaddr.AddrFormatError is not handled by hwaddr() filter.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ansible/lib/ansible/plugins/filter/ipaddr.py

ANSIBLE VERSION
ansible 2.4.3.0
  config file = /home/k0ste/ansible-fork/ansible.cfg
  configured module search path = [u'/home/k0ste/ansible/my_modules', u'/home/k0ste/ceph-ansible/library']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.14 (default, Jan  5 2018, 10:41:29) [GCC 7.2.1 20171224]
ADDITIONAL INFORMATION
Test data:
---
- name: hwaddr validation
  become: false
  gather_facts: false
  remote_user: k0ste
  no_log: false
  strategy: linear
  hosts: localhost
  vars:
    maddr: 'd0:50:99:1c:78:'
  tasks:
  - assert:
      that: "vars['maddr'] | hwaddr()"
      msg: "'maddr' is not maddr"
Expected results:

msg: vars['maddr'] is not maddr.

Actual results:
PLAY [hwaddr validation] *******************************************************************************************************************************************************************************************

TASK [assert] ******************************************************************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"msg": "The conditional check 'vars['maddr'] | hwaddr()' failed. The error was: local variable 'v' referenced before assignment"}

PLAY RECAP *********************************************************************************************************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1   
Fixed via this PR:
PLAY [hwaddr validation] *******************************************************************************************************************************************************************************************

TASK [assert] ******************************************************************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"msg": "The conditional check 'vars['maddr'] | hwaddr()' failed. The error was: hwaddr: not a hardware address: d0:50:99:1c:78:"}

PLAY RECAP *********************************************************************************************************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1   
@k0ste

This comment has been minimized.

Contributor

k0ste commented Apr 23, 2018

ready_for_review

@webknjaz webknjaz requested a review from jimi-c Apr 24, 2018

@webknjaz webknjaz removed the needs_triage label Apr 24, 2018

@ansibot ansibot added the stale_ci label May 2, 2018

@ansibot ansibot added the affects_2.6 label May 18, 2018

@ansibot ansibot added the small_patch label Jun 27, 2018

@k0ste k0ste force-pushed the k0ste:newfeature branch Aug 9, 2018

@ansibot ansibot removed the stale_ci label Aug 9, 2018

@k0ste

This comment has been minimized.

Contributor

k0ste commented Aug 9, 2018

@ansibot ansibot added stale_ci and removed needs_revision labels Aug 17, 2018

@k0ste k0ste force-pushed the k0ste:newfeature branch to 689bb08 Aug 20, 2018

@ansibot ansibot removed the stale_ci label Aug 20, 2018

@ansibot ansibot added the stale_ci label Aug 28, 2018

@k0ste k0ste force-pushed the k0ste:newfeature branch 2 times, most recently from 99e8291 to b5773dc Oct 22, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Oct 22, 2018

The test ansible-test sanity --test changelog [explain] failed with the error:

Command "/usr/bin/python test/sanity/code-smell/changelog.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
  File "packaging/release/changelogs/changelog.py", line 814, in <module>
    main()
  File "packaging/release/changelogs/changelog.py", line 98, in main
    args.func(args)
  File "packaging/release/changelogs/changelog.py", line 109, in command_lint
    lint_fragments(fragments, exceptions)
  File "packaging/release/changelogs/changelog.py", line 227, in lint_fragments
    errors += linter.lint(fragment)
  File "packaging/release/changelogs/changelog.py", line 307, in lint
    errors += [(fragment.path, 0, 0, result[1]) for result in results]
  File "packaging/release/changelogs/changelog.py", line 307, in <listcomp>
    errors += [(fragment.path, 0, 0, result[1]) for result in results]
  File "/usr/local/lib/python3.6/dist-packages/rstcheck.py", line 169, in check
    find_ignored_languages(source)
  File "/usr/local/lib/python3.6/dist-packages/rstcheck.py", line 235, in find_ignored_languages
    for (index, line) in enumerate(source.splitlines()):
AttributeError: 'dict' object has no attribute 'splitlines'
Traceback (most recent call last):
  File "test/sanity/code-smell/changelog.py", line 14, in <module>
    main()
  File "test/sanity/code-smell/changelog.py", line 10, in main
    subprocess.check_call(cmd)
  File "/usr/lib/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['packaging/release/changelogs/changelog.py', 'lint', 'changelogs/fragments/2.8-core-deprecations.yaml', 'changelogs/fragments/2.8-removed-modules.yaml', 'changelogs/fragments/42866-galaxy-search-unicode.yaml', 'changelogs/fragments/43123-add_support_for_per_host_no_stats.yaml', 'changelogs/fragments/43874-docker_container-stop_timeout.yaml', 'changelogs/fragments/44278-pamd_valid_simple_controls.yaml', 'changelogs/fragments/44789-docker_container-comparisons.yaml', 'changelogs/fragments/45628-fetch_url-error-headers.yaml', 'changelogs/fragments/46322-docker_container-image-not-given.yaml', 'changelogs/fragments/46594-docker_container-publish-all-ports.yml', 'changelogs/fragments/46595-docker_container-expected_ports.yml', 'changelogs/fragments/46596-docker_container-published_ports.yml', 'changelogs/fragments/46598-docker_container-volume-modes.yml', 'changelogs/fragments/46739-gcp-compute-instance-metadata.yaml', 'changelogs/fragments/46743-fix-native-jinja-newlines.yaml', 'changelogs/fragments/46961_fix_aws_ec2_cache.yaml', 'changelogs/fragments/47247-docker_container-add-runtime-option.yaml', 'changelogs/fragments/47281-pamd-dont-delete-named_temporary_file_on_close.yaml', 'changelogs/fragments/47307-handler-include-task.yml', 'changelogs/fragments/add-elapsed-return-value-to-select-modules.yaml', 'changelogs/fragments/agnostic-become-prompt.yaml', 'changelogs/fragments/ajson-nested-decode.yaml', 'changelogs/fragments/ansible-doc-fixes.yml', 'changelogs/fragments/async-dir.yaml', 'changelogs/fragments/async_statys_pyx_compat_fix.yml', 'changelogs/fragments/azure_rm_appgateway-probe.yaml', 'changelogs/fragments/azure_rm_deployment_fix_45941.yaml', 'changelogs/fragments/blockinfile-bytes-fix.yaml', 'changelogs/fragments/code-cleanup-no-get-exception.yaml', 'changelogs/fragments/copy-diff-text.yaml', 'changelogs/fragments/copy-recursive-remote-src.yml', 'changelogs/fragments/dd-put-empty-files.yaml', 'changelogs/fragments/delegate_to_loop_hostvars.yaml', 'changelogs/fragments/dnf-group-removal.yaml', 'changelogs/fragments/docker-image-ids.yaml', 'changelogs/fragments/docker_container-idempotency.yaml', 'changelogs/fragments/drop-pkg_resources.yaml', 'changelogs/fragments/ec2_asg-launch-template-support.yml', 'changelogs/fragments/ec2_group_fix_target_containing_list_within_list.yaml', 'changelogs/fragments/elb_target_group_fix_KeyError.yaml', 'changelogs/fragments/fix_ec2_group_target_vpc_precedence.yaml', 'changelogs/fragments/fix_ec2_group_vpc_precedence_classic.yaml', 'changelogs/fragments/free-strategy-include-var-tags.yaml', 'changelogs/fragments/get-url-fix-idempotency.yaml', 'changelogs/fragments/get_url.yaml', 'changelogs/fragments/inv_fixes.yml', 'changelogs/fragments/lineinfile-insertbefore-index-out-of-range.yaml', 'changelogs/fragments/loop-empty-literal-list.yaml', 'changelogs/fragments/loop_undefined_delegate_to.yaml', 'changelogs/fragments/macports-upgrade-selfupdate.yml', 'changelogs/fragments/mysql-migrate_to_pymysql.yaml', 'changelogs/fragments/no-mutable-fieldattribute-defaults.yaml', 'changelogs/fragments/no_empty_groups.yml', 'changelogs/fragments/openssl-python3.yaml', 'changelogs/fragments/openstack_inventory_fix.yml', 'changelogs/fragments/piped-transfer-empty-files.yaml', 'changelogs/fragments/plugin-docs-list-fix.yaml', 'changelogs/fragments/plugin-filters-cfg.yaml', 'changelogs/fragments/postgresql_user-not-sup-error.yaml', 'changelogs/fragments/psexec-handle-socket-errors.yaml', 'changelogs/fragments/psexec-imp-error.yaml', 'changelogs/fragments/psrp-utf8.yaml', 'changelogs/fragments/reboot-show-timeout.yaml', 'changelogs/fragments/reboot-unicode-string.yaml', 'changelogs/fragments/reboot_missing_parameter.yaml', 'changelogs/fragments/reboot_openbsd_support.yaml', 'changelogs/fragments/restore_sigpipe_dfl.yml', 'changelogs/fragments/run-command-expand-shell.yaml', 'changelogs/fragments/s3_bucket_fix_non_str_tags.yaml', 'changelogs/fragments/script-module-no-file-path.yaml', 'changelogs/fragments/sns-boto3.yaml', 'changelogs/fragments/solaris-prtdiag-path.yaml', 'changelogs/fragments/tower_credential_ssh_key_data.yaml', 'changelogs/fragments/unsafe-set-wrap.yaml', 'changelogs/fragments/user-do-not-pass-ssh_key_passphrase-on-cmdline.yaml', 'changelogs/fragments/user-docs-underlying-tools.yaml', 'changelogs/fragments/v2.8.0-initial-commit.yaml', 'changelogs/fragments/win_copy-dest-quote.yaml', 'changelogs/fragments/win_group_membership-com-marshal.yaml', 'changelogs/fragments/win_package_chdir.yaml', 'changelogs/fragments/win_say-fix.yaml', 'changelogs/fragments/win_scheduled_task-repetition.yaml', 'changelogs/fragments/win_script-become.yaml', 'changelogs/fragments/windows-deprecated-functionality.yaml', 'changelogs/fragments/windows-exec-changes.yaml', 'changelogs/fragments/windows-psrp-unreachable.yaml', 'changelogs/fragments/winrm_pexpect.yaml', 'changelogs/fragments/yumdnf-update-cache.yaml']' returned non-zero exit status 1.

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/system/user.py:876:21: E210 subprocess.Popen call found. Should be module.run_command

click here for bot help

@ansibot ansibot added needs_revision and removed stale_ci labels Oct 22, 2018

@k0ste k0ste force-pushed the k0ste:newfeature branch 4 times, most recently from 604de78 to 73e5481 Oct 23, 2018

@ansibot ansibot removed the needs_revision label Nov 10, 2018

@k0ste k0ste force-pushed the k0ste:newfeature branch from 73e5481 to 94910e6 Nov 22, 2018

@k0ste k0ste force-pushed the k0ste:newfeature branch 2 times, most recently from c9ca2ae to ff6a098 Nov 29, 2018

@Akasurde

Add a unit test for this change here

@@ -1059,6 +1059,8 @@ def hwaddr(value, query='', alias='hwaddr'):
try:
v = netaddr.EUI(value)
except netaddr.AddrFormatError:
raise errors.AnsibleFilterError(alias + ': not a hardware address: %s' % value)

This comment has been minimized.

@Akasurde

Akasurde Nov 30, 2018

Member

How about returning False instead of raising error. As per documentation if string is not a valid mac address then we should return False

This comment has been minimized.

@k0ste

k0ste Nov 30, 2018

Contributor

Okay.

PLAY [hwaddr validation] *****************************************************************************************************************************

TASK [assert] ****************************************************************************************************************************************
fatal: [localhost]: FAILED! => {
    "assertion": "vars['maddr'] | hwaddr()",
    "changed": false,
    "evaluated_to": false,
    "msg": "'maddr' is not maddr"
}

PLAY RECAP *******************************************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1   

@k0ste k0ste force-pushed the k0ste:newfeature branch 2 times, most recently from dd21304 to 5d00610 Nov 30, 2018

@k0ste k0ste force-pushed the k0ste:newfeature branch from 5d00610 to c2a37dd Dec 1, 2018

@k0ste k0ste force-pushed the k0ste:newfeature branch from c2a37dd to 570ebae Dec 5, 2018

@ansibot ansibot added stale_ci and removed support:core labels Dec 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment