networktocode/diffsync

Don't allow shortname conflict in store

jakubkrysl opened this issue · 1 comments

Environment

  • DiffSync version: 1.7.0
  • Python version: 3.10.8
  • nautobot-ssot version: 1.2.0

Context

I have an Nautobot SSoT Job.
There is an issue where DiffSync store handles objects by uid but later for diff switches uid for shortname as name of the element:

def get_shortname(self) -> Text:
"""Get the (not guaranteed-unique) shortname of an object, if any.
By default the shortname is built based on all the keys defined in `_shortname`.
If `_shortname` is not specified, then this function is equivalent to `get_unique_id()`.
Returns:
str: Shortname of this object
"""
if self._shortname:
return "__".join([str(getattr(self, key)) for key in self._shortname])
return self.get_unique_id()

But there is no way to check if given shortname is already used in DiffSync store while adding it, I can only check for uid. And when I run diff calculation, it fails on shortname conflict, but catching ObjectAlreadyExists at this point aborts whole diff calculation.

Expected Behavior

Fail on step 2, DiffSync should not allow to add object to store if given shortname is already stored.

Observed Behavior

Fail on diff calculation, which is too late to do anything about it.

Traceback (most recent call last):
  File "/usr/local/lib/python3.10/site-packages/nautobot_ssot/jobs/base.py", line 332, in run
    self.sync_data()
  File "/usr/local/lib/python3.10/site-packages/nautobot_ssot/jobs/base.py", line 155, in sync_data
    self.calculate_diff()
  File "/usr/local/lib/python3.10/site-packages/nautobot_ssot/jobs/base.py", line 82, in calculate_diff
    self.diff = self.source_adapter.diff_to(self.target_adapter, flags=self.diffsync_flags)
  File "/usr/local/lib/python3.10/site-packages/diffsync/__init__.py", line 644, in diff_to
    return target.diff_from(self, diff_class=diff_class, flags=flags, callback=callback)
  File "/usr/local/lib/python3.10/site-packages/diffsync/__init__.py", line 626, in diff_from
    return differ.calculate_diffs()
  File "/usr/local/lib/python3.10/site-packages/diffsync/helpers.py", line 93, in calculate_diffs
    self.diff.add(diff_element)
  File "/usr/local/lib/python3.10/site-packages/diffsync/diff.py", line 60, in add
    raise ObjectAlreadyExists(f"Already storing a {element.type} named {element.name}", element)
diffsync.exceptions.ObjectAlreadyExists: ('Already storing a virtualmachine named *SHORTNAME*', )

Steps to Reproduce

  1. Have a model with shortname set
  2. Add 2 objects for that model with different identifiers but same shortnames
  3. Run diff calculation

Thanks for submitting! This would need to be handled in

  • DiffSync.add
  • DiffSync.update
  • DiffSync.get_or_instantiate
  • DiffSync.get_or_add_model_instance
  • DiffSync.update_or_instantiate
  • DiffSync.update_or_add_model_instance

As such, its probably best to handle this in the classes inheriting from BaseStore as then it would only need to be handled for add and update.