Error message is missing target and port_forward parameters

[strarsis]

SUMMARY

The error message for changing more than one field that can only be exclusively changed at once in a firewalld resource doesn't include the target and port_forward parameters.

ISSUE TYPE

• Bug Report

COMPONENT NAME

ansible.posix.firewalld

ANSIBLE VERSION

ansible 2.9.10
  config file = None
  configured module search path = ['/home/build/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.8/dist-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.8.10 (default, Jun  2 2021, 10:49:15) [GCC 9.4.0]

COLLECTION VERSION

1.3.0

CONFIGURATION

(empty)

OS / ENVIRONMENT

Ubuntu 20.04 LTS on WSL 2 on Windows 10 x64.

STEPS TO REPRODUCE

Apply an ansible playbook with a firewalld resource changing more than one allowed parameter at once.

(empty)

EXPECTED RESULTS

The warning message should also include the target field.

ACTUAL RESULTS

fatal: [web]: FAILED! => {"changed": false, "msg": "can only operate on port, service, rich_rule, masquerade, icmp_block, icmp_block_inversion, interface or source at once"}

DISCUSSION

ansible.posix/plugins/modules/firewalld.py

Lines 815 to 839 in 7f16f56

	modification_count = 0
	if icmp_block is not None:
	modification_count += 1
	if icmp_block_inversion is not None:
	modification_count += 1
	if service is not None:
	modification_count += 1
	if port is not None:
	modification_count += 1
	if port_forward is not None:
	modification_count += 1
	if rich_rule is not None:
	modification_count += 1
	if interface is not None:
	modification_count += 1
	if masquerade is not None:
	modification_count += 1
	if source is not None:
	modification_count += 1
	if target is not None:
	modification_count += 1
	if modification_count > 1:
	module.fail_json(
	msg='can only operate on port, service, rich_rule, masquerade, icmp_block, icmp_block_inversion, interface or source at once'

Currently a (admittedly easily readable and understandable) if structure is used to determine the number of parameters of a set of parameters that can only be exclusively set per firewalld resource.
The error message is separately constructed, hence it can easily go out of sync with that counting code above.
An alternative could be defining a set of these only exclusively settable fields and count these and generate the error message by using that set as a single source of truth.

[saito-hideki]

I think your opinion is reasonable. It would be better including the targets in the error message. Thank you for reporting this! :)