add diff support to ufw module

[wytmttft-cmd]

SUMMARY

minor_changes:

• ufw - now reports diff

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

ufw

ADDITIONAL INFORMATION

before this code change:

changed: [myhost] => (item={'rule': 'allow', 'direction': 'in', 'name': 'OpenSSH', 'from_ip': '10.10.10.8/32', 'comment': 'mgmt - XXXXX'})

after this code change:

--- before
+++ after
@@ -16,6 +16,9 @@
 :ufw-user-limit - [0:0]
 :ufw-user-limit-accept - [0:0]
 ### RULES ###
+
+### tuple ### allow tcp 22 0.0.0.0/0 any 10.10.10.10 OpenSSH - in comment='[Ansible managed] mgmt - XXXXX'
+-A ufw-user-input -p tcp --dport 22 -s 10.10.10.10 -j ACCEPT -m comment --comment 'dapp_OpenSSH'
 
 ### END RULES ###
 

changed: [myhost] => (item={'rule': 'allow', 'direction': 'in', 'name': 'OpenSSH', 'from_ip': '10.10.10.8/32', 'comment': 'mgmt - XXXXX'})

[ansibullbot]

cc @ahtik @ovcharenko @pyykkis
click here for bot help

[felixfontein]

Thanks for your contribution! Please add a changelog fragment. Thanks.

I've added some first comments below. Please note that right now, the tests are failing.

[felixfontein]

Please keep the dictionary literal syntax.

[felixfontein]

These hard-coded values look wrong. What is this supposed to do?

[felixfontein]

Suggested change

	if (not changed):
	if not changed:

[felixfontein]

It's unusual to use these many brackets in Python:

Suggested change

	if (diff is not None) and (diff["before"] == diff["after"]):
	diff = None
	if (changed) and (diff is None):
	raise Exception("ufw module reported a change, but no differences detected")
	elif (changed is False) and (diff is not None):
	if diff is not None and diff["before"] == diff["after"]:
	diff = None
	if changed and diff is None:
	raise Exception("ufw module reported a change, but no differences detected")
	elif changed is False and diff is not None:

[wytmttft-cmd]

Hi Felix, Thank you for your email, and your suggestions. I will take them all on board and fix them, as well as fixing the tests. I need to dedicate some time to this, I hope to do so this coming weekend. Regarding the change log fragment, where should I put that exactly? The hard coded rule is there to get ufw to print out the current ruleset, even if ufw is disabled. I'll probably change this to a 'delete rule'. Please bear with me. Jort

…

On Sat, 16 Aug 2025, 08:38 Felix Fontein, ***@***.***> wrote: ***@***.**** commented on this pull request. Thanks for your contribution! Please add a changelog fragment <https://docs.ansible.com/ansible/devel/community/collection_development_process.html#creating-a-changelog-fragment>. Thanks. I've added some first comments below. Please note that right now, the tests are failing. ------------------------------ In plugins/modules/ufw.py <#10666 (comment)> : > params = module.params - commands = {key: params[key] for key in command_keys if params[key]} + commands = dict((key, params[key]) for key in command_keys if params[key]) Please keep the dictionary literal syntax. ------------------------------ In plugins/modules/ufw.py <#10666 (comment)> : > @@ -564,6 +599,15 @@ def ufw_version(): cmd.append([params['comment'], "comment '%s'" % params['comment']]) rules_dry = execute(cmd) + cmd = [[ufw_bin], ["--dry-run route allow in on foo out on bar from 1.1.1.2 port 7000 to 8.8.8.8 port 7001 proto tcp"]] These hard-coded values look wrong. What is this supposed to do? ------------------------------ In plugins/modules/ufw.py <#10666 (comment)> : > @@ -581,16 +625,32 @@ def ufw_version(): changed = True elif pre_rules != rules_dry: changed = True + else: + diff = None + + if not module.check_mode: + post_state = execute([[ufw_bin], ['status'], ['verbose']]) + post_rules = get_current_rules() + if (not changed): ⬇️ Suggested change - if (not changed): + if not changed: ------------------------------ In plugins/modules/ufw.py <#10666 (comment)> : > + if (diff is not None) and (diff["before"] == diff["after"]): + diff = None + + if (changed) and (diff is None): + raise Exception("ufw module reported a change, but no differences detected") + elif (changed is False) and (diff is not None): It's unusual to use these many brackets in Python: ⬇️ Suggested change - if (diff is not None) and (diff["before"] == diff["after"]): - diff = None - - if (changed) and (diff is None): - raise Exception("ufw module reported a change, but no differences detected") - elif (changed is False) and (diff is not None): + if diff is not None and diff["before"] == diff["after"]: + diff = None + + if changed and diff is None: + raise Exception("ufw module reported a change, but no differences detected") + elif changed is False and diff is not None: — Reply to this email directly, view it on GitHub <#10666 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BV7NOU67276R3JGB57FIHDT3NZALZAVCNFSM6AAAAACD3E2BOOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTCMRVGA3TINBXGE> . You are receiving this because you authored the thread.Message ID: ***@***.*** .com>