[bind] missing state fails zone/view configuration creation

[BerkhanBerkdemir]

In the current master branch of the documentation and the master branch of the code, BIND role documentation examples suggest to have views and zones without state attribute. However, this behavior is not acceptable because of the following code piece:

debops/ansible/roles/bind/tasks/main.yml

Lines 85 to 102 in a4aa669

	- name: Generate list of toplevel views
	ansible.builtin.set_fact:
	bind__tmp_top_views: '{{ bind__combined_zones
	| debops.debops.parse_kv_items(name="view")
	| selectattr("state", "eq", "present") }}'
	tags: [ 'role::bind:config' ]
	- name: Generate list of toplevel zones
	ansible.builtin.set_fact:
	bind__tmp_top_zones: '{{ bind__combined_zones
	| debops.debops.parse_kv_items(name="name")
	| selectattr("state", "eq", "present") }}'
	tags: [ 'role::bind:config' ]
	- name: Make sure either zones or views are defined
	ansible.builtin.assert:
	that: bind__tmp_top_views | length == 0 or bind__tmp_top_zones | length == 0
	tags: [ 'role::bind:config' ]

I don't see anything wrong with the code, but missing documentation can be improved in the future. This issue is created both for future references regarding "Make sure either zones or views are defined" error and possible documentation improvements in debops.bind role.

[Alphix]

No state is usually interpreted as "present" in DebOps roles, and the above logic doesn't really reflect that. But as far as I can remember, debops.debops.parse_kv_items should set an undefined state to present. @BerkhanBerkdemir: what did your inventory look like when you triggered this behaviour?

Anyway, I guess the logic above could be improved. A first thought would be to replace selectattr("state", "eq", "present") by rejectattr("state", "in", "[init, absent, ignore]")...buuut that doesn't work because rejectattr blows up when state is undefined.

I think a better option might be to replace this:

ansible.builtin.set_fact: 
  bind__tmp_top_views: '{{ bind__combined_zones 
                           | debops.debops.parse_kv_items(name="view") 
                           | selectattr("state", "eq", "present") }}'
...
ansible.builtin.set_fact: 
  bind__tmp_top_zones: '{{ bind__combined_zones 
                           | debops.debops.parse_kv_items(name="name") 
                           | selectattr("state", "eq", "present") }}'

With something like this:

ansible.builtin.set_fact: 
  bind__tmp_top_views: '{{ bind__tmp_top_views | d([]) + [ item ] }}'
loop: '{{ bind__combined_zones 
          | debops.debops.parse_kv_items(name="view") }}'
loop_control:
  label: '{{ item.view }}'
when: item.state | d("present") == "present"
...
ansible.builtin.set_fact: 
  bind__tmp_top_zones: '{{ bind__tmp_top_zones | d([]) + [ item ] }}'
loop: '{{ bind__combined_zones 
          | debops.debops.parse_kv_items(name="name") }}'
loop_control:
  label: '{{ item.name }}'
when: item.state | d("present") == "present"

[BerkhanBerkdemir]

What did your inventory look like when you triggered this behavior?

I have used the suggested inventory which is in the documentation. Currently there is nothing in the production system but I'm testing everything in a virtual environment before I make this DebOps role available to my network.

I liked your approach @Alphix, and I would like to try that in my own development environment.

[BerkhanBerkdemir]

I am actually thinking different in this bug at this point. Since both variables are already filled with data, we should be checking only if it is NOT. So, why are we checking if it is == 0? Let's check if it's > 0.