Fix same subnet in different sections bug (#33)

The lookup for subnets now respect the provided section.
Now a subnet can be used in different sections.

To test more complex situations we refactored the test data and
test scripts to be more common for all test cases.

The condition in the `run` method for deleting children has not workded
properly. We fix these condition to use the correct method to resolve
parent.

Add changelog fragment.

Author: cmeissner

plugins/module_utils/phpipam_helper.py

@@ -127,23 +127,25 @@ def find_entity(self, controller, path, params=None):
             return None
 
         if type(result) == list:
-            if len(result) == 1:
-                result = result[0]
+            if len(result) >= 1:
+                result = result
             else:
-                if len(result) > 1:
-                    error_msg = "too many ({0})".format(len(result))
-                else:
-                    error_msg = "no"
-                self.fail_json(msg="Found {0} results while searching for {1} at {2}".format(error_msg, controller, path))
+                self.fail_json(msg="Found no results while searching for {0} at {1}".format(controller, path))
 
         return result
 
-    def find_subnet(self, subnet, mask):
+    def find_subnet(self, subnet, mask, section):
         # lookups for subnets need a separate find method
         # We only support cidr format to simplify the task.
         # CIDR is valid for ipv4 and ipv6 too.
         path = 'cidr/{0}/{1}'.format(subnet, mask)
-        return self.find_entity('subnets', path)
+
+        lookup_params = {
+            'filter_by': 'section',
+            'filter_value': section,
+        }
+
+        return self.find_entity('subnets', path, params=lookup_params)
 
     def find_address(self, address):
         path = 'search/{0}'.format(address)
@@ -181,7 +183,7 @@ def find_by_key(self, controller, value, key='name'):
 
     def find_current_entity(self):
         if self.controller_name == 'subnet':
-            entity = self.find_subnet(self.phpipam_params['subnet'], self.phpipam_params['mask'])
+            entity = self.find_subnet(self.phpipam_params['subnet'], self.phpipam_params['mask'], self.phpipam_params['section'])
         elif self.controller_name == 'address':
             entity = self.find_address(self.phpipam_params['ipaddress'])
         elif self.controller_name == 'device':
@@ -209,7 +211,7 @@ def _resolve_entity(self, key):
         result = None
         if controller == 'subnets':
             subnet, mask = self.phpipam_params[key].split('/')
-            result = self.find_subnet(subnet, mask)
+            result = self.find_subnet(subnet, mask, self.phpipam_params['section'])
         elif controller == 'tools/device_types':
             result = self.find_device_type(self.phpipam_params[key])
         elif 'tools' in controller or controller in ['vlan', 'l2domains', 'vrf']:
@@ -507,7 +509,7 @@ def run(self, **kwargs):
         Manage entities. Lookup, ensure, sanitize and manage parameters.
         """
         if ('parent' in self.phpipam_spec and self.phpipam_spec['parent'].get('type') == 'entity'
-                and self.desired_absent and 'parent' in self.phpipam_params and self.loopup_entity('parent') is None):
+                and self.desired_absent and 'parent' in self.phpipam_params and self._resolve_entity('parent') is None):
             return None
 
         desired_entity = self._auto_resolve_entities()

tests/test_playbooks/example_setup.yml

@@ -0,0 +1,53 @@
+---
+- hosts: localhost
+  collections:
+    - codeaffen.phpipam
+  gather_facts: false
+  vars_files:
+    - vars/server.yml
+    - vars/section.yml
+    - vars/subnet.yml
+  tasks:
+    - name: setup data
+      block:
+      - name: create sections
+        include: tasks/section.yml
+        vars:
+          name: "{{ section.name }}"
+        loop: "{{ sections }}"
+        loop_control:
+          loop_var: section
+
+      - name: create subnets
+        include: tasks/subnet.yml
+        vars:
+          name: "{{ subnet.cidr }}"
+        loop: "{{ subnets }}"
+        loop_control:
+          loop_var: subnet
+      when: cleanup is not defined
+
+    - name: cleanup data
+      block:
+        - name: remove subnets
+          include: tasks/subnet.yml
+          vars:
+            name: "{{ subnet_loop.cidr }}"
+            override:
+              state: absent
+            subnet: "{{ subnet_loop | combine(override) }}"
+          loop: "{{ subnets | reverse | list }}"
+          loop_control:
+            loop_var: subnet_loop
+
+        - name: remove sections
+          include: tasks/section.yml
+          vars:
+            name: "{{ section_loop.name }}"
+            override:
+              state: absent
+            section: "{{ section_loop | combine(override) }}"
+          loop: "{{ sections | reverse | list }}"
+          loop_control:
+            loop_var: section_loop
+      when: cleanup is defined

tests/test_playbooks/section.yml

@@ -11,20 +11,26 @@
       include: tasks/section.yml
       vars:
         name: create section
+        section: "{{ base_section_data }}"
 
     - name: create section again, no change
       include: tasks/section.yml
       vars:
         name: create section again, no change
+        section: "{{ base_section_data }}"
 
     - name: update section
       include: tasks/section.yml
       vars:
         name: update section
-        section_description: section updated
+        override:
+          description: section updated
+        section: "{{ base_section_data | combine(override) }}"
 
     - name: delete section
       include: tasks/section.yml
       vars:
         name: delete section
-        section_state: absent
+        override:
+          state: absent
+        section: "{{ base_section_data | combine(override) }}"

tests/test_playbooks/subnet.yml

@@ -11,14 +11,18 @@
       include: tasks/subnet.yml
       vars:
         name: create subnet
+        subnet: "{{ base_subnet_data }}"
 
     - name: create subnet again, no change
       include: tasks/subnet.yml
       vars:
         name: create subnet again, no change
+        subnet: "{{ base_subnet_data }}"
 
     - name: delete subnet
       include: tasks/subnet.yml
       vars:
         name: delete subnet
-        subnet_state: absent
+        override:
+          state: absent
+        subnet: "{{ base_subnet_data | combine(override) }}"

tests/test_playbooks/tasks/section.yml

@@ -6,15 +6,15 @@
     username: "{{ phpipam_username }}"
     password: "{{ phpipam_password }}"
     name: "{{ section.name }}"
-    description: "{{ section_description | default(omit) }}"
-    parent: "{{ section_parent | default(omit) }}"
-    permissions: "{{ section_permissions | default(omit) }}"
-    strict_mode: "{{ section_strict_mode | default(omit) }}"
-    subnet_ordering: "{{ section_subnet_ordering | default(omit) }}"
-    list_order: "{{ section_list_order | default(omit) }}"
-    show_vlan: "{{ section_show_vlan | default(omit) }}"
-    show_vrf: "{{ section_show_vrf | default(omit) }}"
-    show_supernets_only: "{{ section_show_supernets_only | default(omit) }}"
-    dns_resolver: "{{ section_dns_resolver | default(omit) }}"
-    validate_certs: "{{ section_validate_certs | default('no') }}"
-    state: "{{ section_state | default('present') }}"
+    description: "{{ section.description | default(omit) }}"
+    parent: "{{ section.parent | default(omit) }}"
+    permissions: "{{ section.permissions | default(omit) }}"
+    strict_mode: "{{ section.strict_mode | default(omit) }}"
+    subnet_ordering: "{{ section.subnet_ordering | default(omit) }}"
+    list_order: "{{ section.list_order | default(omit) }}"
+    show_vlan: "{{ section.show_vlan | default(omit) }}"
+    show_vrf: "{{ section.show_vrf | default(omit) }}"
+    show_supernets_only: "{{ section.show_supernets_only | default(omit) }}"
+    dns_resolver: "{{ section.dns_resolver | default(omit) }}"
+    validate_certs: "{{ section.validate_certs | default('no') }}"
+    state: "{{ section.state | default('present') }}"

tests/test_playbooks/tasks/subnet.yml

@@ -10,22 +10,22 @@
     mask: "{{ subnet.mask | default(omit) }}"
     description: "{{ subnet.description | default(omit) }}"
     section: "{{ subnet.section | default(omit) }}"
-    linked_subnet: "{{ subnet_linked_subnet | default(omit) }}"
-    vlan_id: "{{ subnet_vlan_id | default(omit) }}"
-    vrf_id: "{{ subnet_vrf_id | default(omit) }}"
-    master_subnet_cidr: "{{ subnet_master_subnet_cidr | default(omit) }}"
-    nameserver: "{{ subnet_nameserver | default(omit) }}"
-    show_as_name: "{{ subnet_show_as_name | default(omit) }}"
-    permissions: "{{ subnet_permissions | default(omit) }}"
-    dns_recursive: "{{ subnet_dns_recursive | default(omit) }}"
-    dns_records: "{{ subnet_dns_records | default(omit) }}"
-    allow_requests: "{{ subnet_allow_requests | default(omit) }}"
-    scan_agent: "{{ subnet_scan_agent | default(omit) }}"
-    ping_subnet: "{{ subnet_ping_subnet | default(omit) }}"
-    discover_subnet: "{{ subnet_discover_subnet | default(omit) }}"
-    is_folder: "{{ subnet_is_folder | default(omit) }}"
-    is_full: "{{ subnet_is_full | default(omit) }}"
-    subnet_state: "{{ subnet_subnet_state | default(omit) }}"
-    threshold: "{{ subnet_threshold | default(omit) }}"
-    location: "{{ subnet_location | default(omit) }}"
-    state: "{{ subnet_state | default('present') }}"
+    linked_subnet: "{{ subnet.linked_subnet | default(omit) }}"
+    vlan_id: "{{ subnet.vlan_id | default(omit) }}"
+    vrf_id: "{{ subnet.vrf_id | default(omit) }}"
+    master_subnet.cidr: "{{ subnet.master_subnet.cidr | default(omit) }}"
+    nameserver: "{{ subnet.nameserver | default(omit) }}"
+    show_as_name: "{{ subnet.show_as_name | default(omit) }}"
+    permissions: "{{ subnet.permissions | default(omit) }}"
+    dns_recursive: "{{ subnet.dns_recursive | default(omit) }}"
+    dns_records: "{{ subnet.dns_records | default(omit) }}"
+    allow_requests: "{{ subnet.allow_requests | default(omit) }}"
+    scan_agent: "{{ subnet.scan_agent | default(omit) }}"
+    ping_subnet: "{{ subnet.ping_subnet | default(omit) }}"
+    discover_subnet: "{{ subnet.discover_subnet | default(omit) }}"
+    is_folder: "{{ subnet.is_folder | default(omit) }}"
+    is_full: "{{ subnet.is_full | default(omit) }}"
+    subnet.state: "{{ subnet.subnet.state | default(omit) }}"
+    threshold: "{{ subnet.threshold | default(omit) }}"
+    location: "{{ subnet.location | default(omit) }}"
+    state: "{{ subnet.state | default('present') }}"

tests/test_playbooks/vars/section.yml

@@ -1,3 +1,10 @@
 ---
-section:
+base_section_data:
   name: "Example Inc."
+
+sections:
+  - name: "ACME Inc."
+  - name: "DevOps Department"
+    parent: "ACME Inc."
+  - name: "Infrastructure Department"
+    parent: "ACME Inc."

tests/test_playbooks/vars/subnet.yml

@@ -1,4 +1,12 @@
 ---
-subnet:
+base_subnet_data:
   cidr: 10.0.0.0/24
   section: "Customers"
+
+subnets:
+  - cidr: 192.0.2.0/24
+    section: "ACME Inc."
+  - cidr: 192.0.2.0/25
+    section: "DevOps Department"
+  - cidr: 192.0.2.0/25
+    section: "Infrastructure Department"