diff --git a/changelogs/fragments/222_pfsense_rule_tracker.yml b/changelogs/fragments/222_pfsense_rule_tracker.yml new file mode 100644 index 00000000..241b6b70 --- /dev/null +++ b/changelogs/fragments/222_pfsense_rule_tracker.yml @@ -0,0 +1,2 @@ +bugfixes: + - pfsense_rule - Prevent duplicate tracker IDs from being generated or added (https://github.com/pfsensible/core/issues/222). diff --git a/plugins/module_utils/rule.py b/plugins/module_utils/rule.py index 44f19b55..cc125e75 100644 --- a/plugins/module_utils/rule.py +++ b/plugins/module_utils/rule.py @@ -291,28 +291,36 @@ def _adjust_separators(self, start_idx, add=True, before=False): elif idx > start_idx: row_elt.text = 'fr' + str(idx - 1) - def _check_tracker(self): + def _check_tracker(self, tracker, fail=False): """ check the tracking used is unique and change it if required """ if not self.trackers: - trackers = self.root_elt.findall('tracker') - for tracker in trackers: - self.trackers.add(tracker.text) + tracker_elts = self.root_elt.findall('rule/tracker') + for t_elt in tracker_elts: + self.trackers.add(t_elt.text) - start = int(time.time()) - while self.obj['tracker'] in self.trackers: - start = start + 1 - self.obj['tracker'] = str(start) + if tracker in self.trackers: + if fail: + self.module.fail_json(msg=f"tracker {tracker} is not unique") - # keep the tracker for future calls if module is used with aggregate - self.trackers.add(self.obj['tracker']) + t = int(tracker) + while t in self.trackers: + t = t + 1 + tracker = str(t) + + # keep the tracker for future calls if module is used with aggregate + self.trackers.add(tracker) + + return tracker def _copy_and_add_target(self): """ create the XML target_elt """ timestamp = '%d' % int(time.time()) self.obj['id'] = '' if 'tracker' not in self.obj: - self.obj['tracker'] = timestamp - self._check_tracker() + self.obj['tracker'] = self._check_tracker(timestamp) + else: + # When creating a new rule with a given tracker, it must be unique + self._check_tracker(self.obj['tracker'], fail=True) self.obj['created'] = self.obj['updated'] = dict() self.obj['created']['time'] = self.obj['updated']['time'] = timestamp @@ -328,6 +336,9 @@ def _copy_and_update_target(self): before = self._rule_element_to_dict() if 'tracker' not in self.obj: self.obj['tracker'] = before['tracker'] + elif self.obj['tracker'] != before['tracker']: + # When updating the tracker of a rule, it must be unique + self._check_tracker(self.obj['tracker'], fail=True) if 'associated-rule-id' not in self.obj and 'associated-rule-id' in before and before['associated-rule-id'] != '': self.module.fail_json(msg='Target filter rule is associated with a NAT rule.') diff --git a/tests/unit/plugins/modules/test_pfsense_rule_create.py b/tests/unit/plugins/modules/test_pfsense_rule_create.py index f300e07f..98f79101 100644 --- a/tests/unit/plugins/modules/test_pfsense_rule_create.py +++ b/tests/unit/plugins/modules/test_pfsense_rule_create.py @@ -641,6 +641,12 @@ def test_rule_create_tracker_invalid(self): msg = 'tracker -1234 must be a positive integer' self.do_module_test(obj, failed=True, msg=msg) + def test_rule_create_tracker_duplicate(self): + """ test creation of a new rule with duplicate tracker """ + obj = dict(name='one_rule', source='any', destination='any', interface='lan', tracker='1545574416') + msg = 'tracker 1545574416 is not unique' + self.do_module_test(obj, failed=True, msg=msg) + def test_rule_create_schedule(self): """ test creation of a new rule with schedule """ obj = dict(name='one_rule', source='any', destination='any', interface='lan', sched='workdays')