firewalld: add zone target set #526

maxamillion · 2020-06-17T04:45:03Z

SUMMARY

Add Zone target setting

Fixes ansible/ansible#49232

Signed-off-by: Adam Miller admiller@redhat.com

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

firewalld

Fixes ansible/ansible#49232 Signed-off-by: Adam Miller <admiller@redhat.com>

ansibullbot · 2020-06-17T04:48:58Z

cc @MorrisA @bcoca @d-little @flynn1973 @gforster @kairoaraujo @marvin-sinister @mator @molekuul @ramooncamacho @wtcross
click here for bot help

ansibullbot · 2020-06-17T04:55:23Z

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

Command "ansible-doc -t module community.general.firewalld" returned exit status 1.
>>> Standard Error
ERROR! Unable to retrieve documentation from 'community.general.firewalld' due to: Expected string in description of target at index 1, got <class 'ansible.parsing.yaml.objects.AnsibleMapping'>

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

plugins/modules/system/firewalld.py:0:0: invalid-documentation: DOCUMENTATION.options.target.description.0: expected str @ data['options']['target']['description'][0]. Got {'firewalld Zone target, any one of': 'C(default), C(ACCEPT), C(DROP), C(REJECT)'}

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

plugins/modules/system/firewalld.py:576:1: E302: expected 2 blank lines, found 1
plugins/modules/system/firewalld.py:622:1: E302: expected 2 blank lines, found 1

click here for bot help

Signed-off-by: Adam Miller <admiller@redhat.com>

felixfontein

Did you run the tests manually? Right now, they are still disabled in CI.

plugins/modules/system/firewalld.py

changelogs/fragments/firewalld_zone_target.yml

…ettings Signed-off-by: Adam Miller <admiller@redhat.com>

maxamillion · 2020-06-17T14:13:26Z

Did you run the tests manually? Right now, they are still disabled in CI.

Yes, but the test is failing and I don't know why ... I'm going to mark the PR as [WIP] while I figure it out.

Signed-off-by: Adam Miller <admiller@redhat.com>

maxamillion · 2020-06-17T15:46:27Z

@felixfontein alright, got it fixed and ran integration tests successfully against RHEL 8.1

$(ansible)  ansible-test integration --remote rhel/8.1 firewalld --allow-disabled
Red Hat Enterprise Linux release 8.1 (Ootpa)
Running firewalld integration test role
[WARNING]: running playbook inside collection community.general

PLAY [testhost] ****************************************************************************************************************************

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

TASK [setup_pkg_mgr : set_fact] ************************************************************************************************************
skipping: [testhost]

TASK [setup_pkg_mgr : set_fact] ************************************************************************************************************
skipping: [testhost]

TASK [Ensure firewalld is installed] *******************************************************************************************************
ok: [testhost]

TASK [Check to make sure the firewalld python module is available.] ************************************************************************
changed: [testhost]

TASK [start firewalld] *********************************************************************************************************************
changed: [testhost]

TASK [Ensure /run/firewalld exists] ********************************************************************************************************
ok: [testhost]

TASK [firewalld : include_tasks] ***********************************************************************************************************
included: /root/ansible_collections/community/general/tests/output/.tmp/integration/firewalld-hpod3w84-ÅÑŚÌβŁÈ/tests/integration/targets/firewalld/tasks/service_test_cases.yml for testhost

TASK [firewalld service test permanent enabled] ********************************************************************************************
changed: [testhost]

TASK [assert firewalld service test permanent enabled worked] ******************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld service test permanent enabled rerun (verify not changed)] *****************************************************************
ok: [testhost]

TASK [assert firewalld service test permanent enabled rerun worked (verify not changed)] ***************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld service test permanent disabled] *******************************************************************************************
changed: [testhost]

TASK [assert firewalld service test permanent disabled worked] *****************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld service test permanent disabled rerun (verify not changed)] ****************************************************************
ok: [testhost]

TASK [assert firewalld service test permanent disabled rerun worked (verify not changed)] **************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld : include_tasks] ***********************************************************************************************************
included: /root/ansible_collections/community/general/tests/output/.tmp/integration/firewalld-hpod3w84-ÅÑŚÌβŁÈ/tests/integration/targets/firewalld/tasks/port_test_cases.yml for testhost

TASK [firewalld port test permanent enabled] ***********************************************************************************************
changed: [testhost]

TASK [assert firewalld port test permanent enabled worked] *********************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld port test permanent enabled rerun (verify not changed)] ********************************************************************
ok: [testhost]

TASK [assert firewalld port test permanent enabled rerun worked (verify not changed)] ******************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld port test permanent disabled] **********************************************************************************************
changed: [testhost]

TASK [assert firewalld port test permanent disabled worked] ********************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld port test permanent disabled rerun (verify not changed)] *******************************************************************
ok: [testhost]

TASK [assert firewalld port test permanent disabled rerun worked (verify not changed)] *****************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent enabled] *********************************************************************************************
changed: [testhost]

TASK [assert firewalld source test permanent enabled worked] *******************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent enabled rerun (verify not changed)] ******************************************************************
ok: [testhost]

TASK [assert firewalld source test permanent enabled rerun worked (verify not changed)] ****************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent disabled] ********************************************************************************************
changed: [testhost]

TASK [assert firewalld source test permanent disabled worked] ******************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent disabled rerun (verify not changed)] *****************************************************************
ok: [testhost]

TASK [assert firewalld source test permanent disabled rerun worked (verify not changed)] ***************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent enabled is exclusive (verify exclusive error)] *******************************************************
fatal: [testhost]: FAILED! => {"changed": false, "msg": "can only operate on port, service, rich_rule, masquerade, icmp_block, icmp_block_inversion, interface or source at once"}
...ignoring

TASK [assert firewalld source test permanent enabled is exclusive (verify exclusive error)] ************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld dmz zone target DROP] ******************************************************************************************************
changed: [testhost]

TASK [assert firewalld dmz zone target DROP present worked] ********************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld dmz zone target DROP rerun (verify not changed)] ***************************************************************************
ok: [testhost]

TASK [assert firewalld dmz zone target DROP present worked (verify not changed)] ***********************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld dmz zone target DROP absent] ***********************************************************************************************
changed: [testhost]

TASK [assert firewalld dmz zone target DROP absent worked] *********************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld dmz zone target DROP rerun (verify not changed)] ***************************************************************************
ok: [testhost]

TASK [assert firewalld dmz zone target DROP present worked (verify not changed)] ***********************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [stop firewalld] **********************************************************************************************************************
changed: [testhost]

TASK [Ensure /run/firewalld exists] ********************************************************************************************************
ok: [testhost]

TASK [firewalld : include_tasks] ***********************************************************************************************************
included: /root/ansible_collections/community/general/tests/output/.tmp/integration/firewalld-hpod3w84-ÅÑŚÌβŁÈ/tests/integration/targets/firewalld/tasks/service_test_cases.yml for testhost

TASK [firewalld service test permanent enabled] ********************************************************************************************
changed: [testhost]

TASK [assert firewalld service test permanent enabled worked] ******************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld service test permanent enabled rerun (verify not changed)] *****************************************************************
ok: [testhost]

TASK [assert firewalld service test permanent enabled rerun worked (verify not changed)] ***************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld service test permanent disabled] *******************************************************************************************
changed: [testhost]

TASK [assert firewalld service test permanent disabled worked] *****************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld service test permanent disabled rerun (verify not changed)] ****************************************************************
ok: [testhost]

TASK [assert firewalld service test permanent disabled rerun worked (verify not changed)] **************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld : include_tasks] ***********************************************************************************************************
included: /root/ansible_collections/community/general/tests/output/.tmp/integration/firewalld-hpod3w84-ÅÑŚÌβŁÈ/tests/integration/targets/firewalld/tasks/port_test_cases.yml for testhost

TASK [firewalld port test permanent enabled] ***********************************************************************************************
changed: [testhost]

TASK [assert firewalld port test permanent enabled worked] *********************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld port test permanent enabled rerun (verify not changed)] ********************************************************************
ok: [testhost]

TASK [assert firewalld port test permanent enabled rerun worked (verify not changed)] ******************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld port test permanent disabled] **********************************************************************************************
changed: [testhost]

TASK [assert firewalld port test permanent disabled worked] ********************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld port test permanent disabled rerun (verify not changed)] *******************************************************************
ok: [testhost]

TASK [assert firewalld port test permanent disabled rerun worked (verify not changed)] *****************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent enabled] *********************************************************************************************
changed: [testhost]

TASK [assert firewalld source test permanent enabled worked] *******************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent enabled rerun (verify not changed)] ******************************************************************
ok: [testhost]

TASK [assert firewalld source test permanent enabled rerun worked (verify not changed)] ****************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent disabled] ********************************************************************************************
changed: [testhost]

TASK [assert firewalld source test permanent disabled worked] ******************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent disabled rerun (verify not changed)] *****************************************************************
ok: [testhost]

TASK [assert firewalld source test permanent disabled rerun worked (verify not changed)] ***************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld source test permanent enabled is exclusive (verify exclusive error)] *******************************************************
fatal: [testhost]: FAILED! => {"changed": false, "msg": "can only operate on port, service, rich_rule, masquerade, icmp_block, icmp_block_inversion, interface or source at once"}
...ignoring

TASK [assert firewalld source test permanent enabled is exclusive (verify exclusive error)] ************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld dmz zone target DROP] ******************************************************************************************************
changed: [testhost]

TASK [assert firewalld dmz zone target DROP present worked] ********************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld dmz zone target DROP rerun (verify not changed)] ***************************************************************************
ok: [testhost]

TASK [assert firewalld dmz zone target DROP present worked (verify not changed)] ***********************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld dmz zone target DROP absent] ***********************************************************************************************
changed: [testhost]

TASK [assert firewalld dmz zone target DROP absent worked] *********************************************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

TASK [firewalld dmz zone target DROP rerun (verify not changed)] ***************************************************************************
ok: [testhost]

TASK [assert firewalld dmz zone target DROP present worked (verify not changed)] ***********************************************************
ok: [testhost] => {
    "changed": false,
    "msg": "All assertions passed"
}

PLAY RECAP *********************************************************************************************************************************
testhost                   : ok=79   changed=19   unreachable=0    failed=0    skipped=2    rescued=0    ignored=2

felixfontein · 2020-06-19T20:37:39Z

/rebuild

felixfontein · 2020-06-19T20:39:58Z

@maxamillion you marked the review comments of mine as resolved, but never pushed a change which adjusted anything in these lines. Did you forgot to push something? (I unresolved them.)

Co-authored-by: Felix Fontein <felix@fontein.de>

maxamillion · 2020-06-19T20:53:53Z

@maxamillion you marked the review comments of mine as resolved, but never pushed a change which adjusted anything in these lines. Did you forgot to push something? (I unresolved them.)

@felixfontein no sorry, the fix I was talking about ended up being this #499 ... shippable should be happy now.

felixfontein · 2020-06-19T21:04:27Z

@maxamillion tests passed, merged :)

firewalld: add zone target set

5f594bd

Fixes ansible/ansible#49232 Signed-off-by: Adam Miller <admiller@redhat.com>

ansibullbot added affects_2.10 community_review feature This issue/PR relates to a feature request integration tests/integration module module needs_triage plugins plugin (any type) system tests tests labels Jun 17, 2020

maxamillion mentioned this pull request Jun 17, 2020

firewalld module can not set target property of a zone ansible/ansible#49232

Closed

ansibullbot added the has_issue label Jun 17, 2020

fix sanity tests, add example of zone target setting

56d52e1

Signed-off-by: Adam Miller <admiller@redhat.com>

felixfontein reviewed Jun 17, 2020

View reviewed changes

plugins/modules/system/firewalld.py Show resolved Hide resolved

changelogs/fragments/firewalld_zone_target.yml Outdated Show resolved Hide resolved

test different zone/target combination as we're not hitting default s…

1eb9fc3

…ettings Signed-off-by: Adam Miller <admiller@redhat.com>

maxamillion changed the title ~~firewalld: add zone target set~~ [WIP] firewalld: add zone target set Jun 17, 2020

ansibullbot added the WIP Work in progress label Jun 17, 2020

fix enabled values for zone operations

5358a95

Signed-off-by: Adam Miller <admiller@redhat.com>

maxamillion force-pushed the firewalld/target_zone_property branch from 6ceacdb to 5358a95 Compare June 17, 2020 15:45

maxamillion changed the title ~~[WIP] firewalld: add zone target set~~ firewalld: add zone target set Jun 17, 2020

ansibullbot removed the WIP Work in progress label Jun 17, 2020

ansibullbot removed the needs_triage label Jun 19, 2020

Apply suggestions from code review

2bd60ff

Co-authored-by: Felix Fontein <felix@fontein.de>

felixfontein approved these changes Jun 19, 2020

View reviewed changes

felixfontein merged commit 42c5cdf into ansible-collections:master Jun 19, 2020

St0rmingBr4in mentioned this pull request Nov 16, 2020

firewalld module missing the target feature due to the migration from community.general ansible-collections/ansible.posix#112

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

firewalld: add zone target set #526

firewalld: add zone target set #526

maxamillion commented Jun 17, 2020

ansibullbot commented Jun 17, 2020

ansibullbot commented Jun 17, 2020

felixfontein left a comment

maxamillion commented Jun 17, 2020

maxamillion commented Jun 17, 2020

felixfontein commented Jun 19, 2020

felixfontein commented Jun 19, 2020

maxamillion commented Jun 19, 2020

felixfontein commented Jun 19, 2020

firewalld: add zone target set #526

firewalld: add zone target set #526

Conversation

maxamillion commented Jun 17, 2020

SUMMARY

ISSUE TYPE

COMPONENT NAME

ansibullbot commented Jun 17, 2020

ansibullbot commented Jun 17, 2020

felixfontein left a comment

Choose a reason for hiding this comment

maxamillion commented Jun 17, 2020

maxamillion commented Jun 17, 2020

felixfontein commented Jun 19, 2020

felixfontein commented Jun 19, 2020

maxamillion commented Jun 19, 2020

felixfontein commented Jun 19, 2020