linux-system-roles / firewall

Configure firewalld and system-config-firewall
https://linux-system-roles.github.io/firewall/
GNU General Public License v2.0
57 stars 32 forks source link

fix: error when running with check mode and previous: replaced #163

Closed richm closed 1 year ago

richm commented 1 year ago

Cause: The role was not checking to see if any files would be changed when using previous: replaced in check mode. The variables that should have been set were not set in check mode.

Consequence: The role would give an error about undefined variables.

Fix: A new check has been added for check mode, to see if any files would be changed by previous: replaced. The check for firewalld.conf looks at the rpm database to determine if the file has been changed from the version shipped in the package. If files would not be changed by using previous: replaced, then the role will also now correctly report if any settings would be changed.

Result: The role works in check mode when using previous: replaced

Signed-off-by: Rich Megginson rmeggins@redhat.com

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (9986937) 54.92% compared to head (9bfb005) 54.92%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #163 +/- ## ======================================= Coverage 54.92% 54.92% ======================================= Files 2 2 Lines 823 823 ======================================= Hits 452 452 Misses 371 371 ``` | Flag | Coverage Δ | | |---|---|---| | sanity | `∅ <ø> (∅)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=linux-system-roles#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

richm commented 1 year ago

[citest]

richm commented 1 year ago

[citest]

richm commented 1 year ago

[citest]

richm commented 1 year ago

ok - tests look good - I can verify locally that check mode appears to be working

for testing check mode:

this will ensure packages and services are present and leave the vm running

find the tmp inventory, then use that to run ansible-playbook against the running vm using check mode

kill the vm and clean up resources when done

BrennanPaciorek commented 1 year ago

Works well for me but a test like this fails with the same error as in #151:

---
- hosts: all
  become: true
  gather_facts: false

  tasks:
    - name: Install firewalld
      ansible.builtin.include_role:
        name: linux-system-roles.firewall

    - name: Enter check mode
      check_mode: true
      block:
        - name: Previous replaced
          ansible.builtin.include_role:
            name: linux-system-roles.firewall
          vars:
            firewall:
              - previous: replaced

This test seems to fail, making a suggestion now to fix that, though I'm not sure why it seems to not fail with ansible-playbook --check set