Ns1 zone diff show by mattschwen · Pull Request #36 · ns1/ns1-ansible-modules

mattschwen · 2021-01-06T23:08:22Z

Allow Ansible module to show diffs in zone updates.

RESULTS

⇒  ./docker_run_cmd.sh ansible-playbook -i inventories/production/netbox-adsrvr-org.yaml playbooks/ns1/custom-ssp-endpoints.yaml --tags=zone_testing  --diff
Warning: Permanently added '[127.0.0.1]:9222' (ECDSA) to the list of known hosts.

PLAY [localhost] *********************************************************************************************************************************************************************************************************************************************************

TASK [Gathering Facts] ***************************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [ns1/custom-ssp-endpoints : testing NS1 zones] **********************************************************************************************************************************************************************************************************************
--- before
+++ after
@@ -31,8 +31,8 @@
   tier: 1
   ttl: 3600
   type: NS
-refresh: 1232
+refresh: 6777
 retry: 7200
 serial: 1609974157
-ttl: 3600
+ttl: 698
 zone: mattstestzone.org

changed: [localhost]

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

UNIT TESTING

matthew.schwen@mschwen:~/projects/ns1-ansible-modules|ns1-zone-diff-show
⇒  pytest -v
========================================================================================================================== test session starts ===========================================================================================================================
platform darwin -- Python 3.7.7, pytest-5.1.2, py-1.8.0, pluggy-0.12.0 -- /usr/local/opt/python/bin/python3.7
cachedir: .pytest_cache
rootdir: /Users/matthew.schwen/projects/ns1-ansible-modules
plugins: testinfra-3.2.0, celery-4.4.5
collected 46 items

tests/unit/test_ns1_datasources.py::test_get_datasources PASSED                                                                                                                                                                                                    [  2%]
tests/unit/test_ns1_datasources.py::test_get_datasources_failure PASSED                                                                                                                                                                                            [  4%]
tests/unit/test_ns1_zone.py::test_module_fail_when_required_args_missing PASSED                                                                                                                                                                                    [  6%]
tests/unit/test_ns1_zone.py::test_diff_params[missing_param] PASSED                                                                                                                                                                                                [  8%]
tests/unit/test_ns1_zone.py::test_diff_params[missing_set_param] PASSED                                                                                                                                                                                            [ 10%]
tests/unit/test_ns1_zone.py::test_diff_params[missing_dict_param] PASSED                                                                                                                                                                                           [ 13%]
tests/unit/test_ns1_zone.py::test_diff_params[updated_param] PASSED                                                                                                                                                                                                [ 15%]
tests/unit/test_ns1_zone.py::test_diff_params[updated_set_param] PASSED                                                                                                                                                                                            [ 17%]
tests/unit/test_ns1_zone.py::test_diff_params[updated_dict_param] PASSED                                                                                                                                                                                           [ 19%]
tests/unit/test_ns1_zone.py::test_diff_params[updated_nested_dict_param] PASSED                                                                                                                                                                                    [ 21%]
tests/unit/test_ns1_zone.py::test_diff_params[removed_set_param] PASSED                                                                                                                                                                                            [ 23%]
tests/unit/test_ns1_zone.py::test_diff_params[no_diff_param] PASSED                                                                                                                                                                                                [ 26%]
tests/unit/test_ns1_zone.py::test_diff_params[ignore_set_param_order] PASSED                                                                                                                                                                                       [ 28%]
tests/unit/test_ns1_zone.py::test_diff_params[no_diff_dict_param] PASSED                                                                                                                                                                                           [ 30%]
tests/unit/test_ns1_zone.py::test_get_changed_params PASSED                                                                                                                                                                                                        [ 32%]
tests/unit/test_ns1_zone.py::test_diff_in_secondaries[ignore_secondary_order] PASSED                                                                                                                                                                               [ 34%]
tests/unit/test_ns1_zone.py::test_diff_in_secondaries[ignore_networks_order] PASSED                                                                                                                                                                                [ 36%]
tests/unit/test_ns1_zone.py::test_diff_in_secondaries[ignore_extra_key_in_have] PASSED                                                                                                                                                                             [ 39%]
tests/unit/test_ns1_zone.py::test_diff_in_secondaries[removed_secondary] PASSED                                                                                                                                                                                    [ 41%]
tests/unit/test_ns1_zone.py::test_diff_in_secondaries[added_secondary] PASSED                                                                                                                                                                                      [ 43%]
tests/unit/test_ns1_zone.py::test_diff_in_secondaries[updated_param] PASSED                                                                                                                                                                                        [ 45%]
tests/unit/test_ns1_zone.py::test_convert_secondaries_to_dict PASSED                                                                                                                                                                                               [ 47%]
tests/unit/test_ns1_zone.py::test_update[update_param] PASSED                                                                                                                                                                                                      [ 50%]
tests/unit/test_ns1_zone.py::test_update[update_list] PASSED                                                                                                                                                                                                       [ 52%]
tests/unit/test_ns1_zone.py::test_update[update_suboption] PASSED                                                                                                                                                                                                  [ 54%]
tests/unit/test_ns1_zone.py::test_check_mode[True] PASSED                                                                                                                                                                                                          [ 56%]
tests/unit/test_ns1_zone.py::test_check_mode[False] PASSED                                                                                                                                                                                                         [ 58%]
tests/unit/test_ns1_zone.py::test_sanitize_params[single_param] PASSED                                                                                                                                                                                             [ 60%]
tests/unit/test_ns1_zone.py::test_sanitize_params[none_param] PASSED                                                                                                                                                                                               [ 63%]
tests/unit/test_ns1_zone.py::test_sanitize_params[ignored_param] PASSED                                                                                                                                                                                            [ 65%]
tests/unit/test_ns1_zone.py::test_sanitize_params[suboption] PASSED                                                                                                                                                                                                [ 67%]
tests/unit/test_ns1_zone.py::test_sanitize_params[nested_suboption] PASSED                                                                                                                                                                                         [ 69%]
tests/unit/test_ns1_zone.py::test_sanitize_params[none_suboption] PASSED                                                                                                                                                                                           [ 71%]
tests/unit/test_ns1_zone.py::test_sanitize_params[all_none_suboption] PASSED                                                                                                                                                                                       [ 73%]
tests/unit/test_ns1_zone.py::test_get_zone PASSED                                                                                                                                                                                                                  [ 76%]
tests/unit/test_ns1_zone.py::test_delete PASSED                                                                                                                                                                                                                    [ 78%]
tests/unit/test_ns1_zone.py::test_create PASSED                                                                                                                                                                                                                    [ 80%]
tests/unit/test_ns1_zone.py::test_present[update] PASSED                                                                                                                                                                                                           [ 82%]
tests/unit/test_ns1_zone.py::test_present[create] PASSED                                                                                                                                                                                                           [ 84%]
tests/unit/test_ns1_zone.py::test_update_on_change[diff] PASSED                                                                                                                                                                                                    [ 86%]
tests/unit/test_ns1_zone.py::test_update_on_change[no_diff] PASSED                                                                                                                                                                                                 [ 89%]
tests/unit/test_ns1_zone.py::test_absent[deletion] PASSED                                                                                                                                                                                                          [ 91%]
tests/unit/test_ns1_zone.py::test_absent[no_deletion] PASSED                                                                                                                                                                                                       [ 93%]
tests/unit/test_ns1_zone.py::test_build_result[no_change] PASSED                                                                                                                                                                                                   [ 95%]
tests/unit/test_ns1_zone.py::test_build_result[change] PASSED                                                                                                                                                                                                      [ 97%]
tests/unit/test_ns1_zone.py::test_build_result[change_check_mode] PASSED                                                                                                                                                                                           [100%]

=========================================================================================================================== 46 passed in 0.37s ===========================================================================================================================

ANSIBLE PLAYBOOK TEST RUN

matthew.schwen@mschwen:~/projects/ns1-ansible-modules|ns1-zone-diff-show⚡
⇒  ansible-playbook -i local tests/ns1_zone.yaml --extra-vars ns1_token=############## --extra-vars test_zone="mattstestzone12345.org"
[WARNING]: Unable to parse /Users/matthew.schwen/projects/ns1-ansible-modules/local 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'

PLAY [localhost] ******************************************************************************************************************************************************************************************************************************************************************************

TASK [Gathering Facts] ************************************************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [setup] **********************************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => (item=mattstestzone12345.org)
ok: [localhost] => (item=linked-mattstestzone12345.org)
ok: [localhost] => (item=secondary-mattstestzone12345.org)
ok: [localhost] => (item=primary-mattstestzone12345.org)

TASK [Verify setup] ***************************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Test zone creation] *********************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify zone creation] *******************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Test zone update] ***********************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify zone update] *********************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Test enabling dnssec] *******************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify dnssec enabled] ******************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Test disabling dnssec] ******************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify dnssec disabled] *****************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Test linked zone creation] **************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify linked zone creation] ************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Delete linked zone] *********************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Create secondary zone] ******************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify secondary zone creation] *********************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Disable secondary zone] *****************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify secondary zone disabled] *********************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Delete secondary zone] ******************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify secondary zone deleted] **********************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Create primary zone] ********************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify primary zone creation] ***********************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Update primary zone] ********************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify primary zone update] *************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Verify idempotency] *********************************************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [Verify idempotency] *********************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Delete secondaries] *********************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify secondaries deleted] *************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Delete primary zone] ********************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify primary zone deleted] ************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [Test zone deletion] *********************************************************************************************************************************************************************************************************************************************************************
changed: [localhost]

TASK [Verify zone deletion] *******************************************************************************************************************************************************************************************************************************************************************
ok: [localhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

PLAY RECAP ************************************************************************************************************************************************************************************************************************************************************************************
localhost                  : ok=32   changed=14   unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

rupa · 2021-01-11T20:13:12Z

Hi, thanks for this, I've got my eye on it, and will do some careful review ASAP

rupa

hey, this looks pretty good. A minor quibble I wouldn't mention unless you were already gonna be in there, and one request:

The ruamel.yaml dependency seems common and fine, but could you add a requirements.txt with it pinned to whatever version (implementer's choice).

library/ns1_zone.py

…equested by my teammate

rupa · 2021-02-01T20:08:15Z

library/ns1_zone.py

+                        want[param] = value
+                else:
+                    if want[param] != value:
+                        want[param] = value


unfortunately i think comparison to params needs to be done recursively, or another approach, as value can be a complex object (dictionary). The create secondary zone integration is failing, as the secondary param's value is an object, and its uninitialized values (eg primary_port) are None, and our API is complaining about it.

this pattern seems applicable to records/other stuff, so it would make sense to me to factor this out into a common function.

I took a whack - haven't really tested the corners, but maybe it's a start http://ix.io/2O0T/diff

(i should have used isinstance instead of type)

Thank you @rupa! I appreciate the due diligence, you are correct and that is something I didn't notice. Your code looks good, I'll go through the proposed changes and make an update to this MR.

matthew.schwen added 2 commits January 6, 2021 14:56

Adding options to show diffs when diff flag is true

61b9e6a

minor modifications to allow for changes and diffs to be displayed

93b435d

Kirstinslevin requested a review from rupa January 7, 2021 17:22

matthew.schwen added 4 commits January 7, 2021 11:32

Solved Unit Testing Problems

cafbddb

removed line used for testing

aea8783

removed un-necessary if then statement

76b271f

additions to allow test playbook to pass

607baca

rupa suggested changes Jan 15, 2021

View reviewed changes

library/ns1_zone.py Outdated Show resolved Hide resolved

fixed suggestions from NS1 and added a diff for new zone records as r…

bda884c

…equested by my teammate

mattschwen requested a review from rupa January 25, 2021 19:05

matthew.schwen added 2 commits January 25, 2021 12:06

empty line on first line fix

aeb461e

fixed error where None Value was causing issues with the NS1 API

49cd0a5

rupa suggested changes Feb 1, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ns1 zone diff show#36

Ns1 zone diff show#36
mattschwen wants to merge 9 commits intons1:masterfrom
mattschwen:ns1-zone-diff-show

mattschwen commented Jan 6, 2021 •

edited

Loading

Uh oh!

rupa commented Jan 11, 2021

Uh oh!

rupa left a comment

Uh oh!

Uh oh!

rupa Feb 1, 2021 •

edited

Loading

Uh oh!

rupa Feb 1, 2021

Uh oh!

rupa Feb 2, 2021

Uh oh!

mattschwen Feb 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mattschwen commented Jan 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rupa commented Jan 11, 2021

Uh oh!

rupa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rupa Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rupa Feb 1, 2021

Choose a reason for hiding this comment

Uh oh!

rupa Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

mattschwen Feb 2, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mattschwen commented Jan 6, 2021 •

edited

Loading

rupa Feb 1, 2021 •

edited

Loading