Tenant-Aware Report filters and ValidatedSW.#574
Conversation
…ce Tenant, and related UI updates. modified: nautobot_device_lifecycle_mgmt/filter_extensions.py modified: nautobot_device_lifecycle_mgmt/filters.py modified: nautobot_device_lifecycle_mgmt/forms.py new file: nautobot_device_lifecycle_mgmt/migrations/0030_validatedsoftwarelcm_device_tenant.py modified: nautobot_device_lifecycle_mgmt/models.py modified: nautobot_device_lifecycle_mgmt/software_filters.py modified: nautobot_device_lifecycle_mgmt/tables.py modified: nautobot_device_lifecycle_mgmt/views.py
modified: nautobot_device_lifecycle_mgmt/software_filters.py
modified: nautobot_device_lifecycle_mgmt/filters.py modified: nautobot_device_lifecycle_mgmt/forms.py modified: nautobot_device_lifecycle_mgmt/migrations/0030_validatedsoftwarelcm_device_tenant.py modified: nautobot_device_lifecycle_mgmt/models.py modified: nautobot_device_lifecycle_mgmt/software_filters.py modified: nautobot_device_lifecycle_mgmt/tables.py
…ement. modified: nautobot_device_lifecycle_mgmt/software_filters.py modified: nautobot_device_lifecycle_mgmt/tests/test_forms.py modified: nautobot_device_lifecycle_mgmt/tests/test_model.py
new file: changes/573.added
modified: nautobot_device_lifecycle_mgmt/filters.py modified: nautobot_device_lifecycle_mgmt/forms.py modified: nautobot_device_lifecycle_mgmt/migrations/0030_validatedsoftwarelcm_device_tenant.py modified: nautobot_device_lifecycle_mgmt/software_filters.py modified: nautobot_device_lifecycle_mgmt/tests/test_forms.py modified: nautobot_device_lifecycle_mgmt/tests/test_model.py modified: nautobot_device_lifecycle_mgmt/views.py
modified: nautobot_device_lifecycle_mgmt/filters.py modified: nautobot_device_lifecycle_mgmt/forms.py
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
modified: nautobot_device_lifecycle_mgmt/software_filters.py
| When(device_roles=self.item_obj.role.pk, preferred=True, then=Value(40)), | ||
| When(device_roles=self.item_obj.role.pk, preferred=False, then=Value(1040)), | ||
| When( | ||
| device_tenants=self.item_obj.tenant, |
There was a problem hiding this comment.
What happens when tenant is not defined for the device?
There was a problem hiding this comment.
if device has no tenant, it only sees validated software with no tenant assigned.
There was a problem hiding this comment.
Apologies, let me rephrase. What happens to this query specifically if the tenant is not defined? Does it work? Does it error out? What will it return?
There was a problem hiding this comment.
it will work:
device without tenant defined:
(AND: (OR: RelatedIn(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_devices, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_devices.device), [UUID('217b8dda-b719-4680-a2d2-5a3e184c1487')]), (AND: RelatedExact(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_3fbc, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_roles.role), UUID('d91b7e68-dadb-4fbb-840c-5b4739575d73')), RelatedExact(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_7ebc, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_types.devicetype), UUID('21475479-aa38-40c2-bed7-4e33ac7a4640'))), (AND: RelatedIsNull(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_3fbc, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_roles.role), True), RelatedExact(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_7ebc, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_types.devicetype), UUID('21475479-aa38-40c2-bed7-4e33ac7a4640'))), (AND: RelatedExact(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_3fbc, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_roles.role), UUID('d91b7e68-dadb-4fbb-840c-5b4739575d73')), RelatedIsNull(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_7ebc, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_types.devicetype), True)), RelatedIn(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_object_tags, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_object_tags.tag), <django.db.models.sql.query.Query object at 0x7ad37df44500>)), RelatedIn(Col(T10, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_devices.device), [UUID('217b8dda-b719-4680-a2d2-5a3e184c1487')]))
device with tenant defined:
(AND: (OR: RelatedIn(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_devices, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_devices.device), [UUID('e46734cb-cbea-4d9f-ad9e-728af5f584a9')]), (AND: RelatedExact(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_a4e6, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_tenants.tenant), UUID('33b7f29e-ca9c-412f-83d6-c37fc8f96abc')), RelatedExact(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_7ebc, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_types.devicetype), UUID('da439205-b475-4cb9-a025-ff01b7223fb0'))), (AND: RelatedExact(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_3fbc, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_roles.role), UUID('c018c03a-e764-40c1-b56d-19e327a4b8d0')), RelatedExact(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_a4e6, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_tenants.tenant), UUID('33b7f29e-ca9c-412f-83d6-c37fc8f96abc'))), (AND: RelatedExact(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_a4e6, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_tenants.tenant), UUID('33b7f29e-ca9c-412f-83d6-c37fc8f96abc')), RelatedIsNull(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_device_7ebc, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_device_types.devicetype), True)), RelatedIn(Col(nautobot_device_lifecycle_mgmt_validatedsoftwarelcm_object_tags, nautobot_device_lifecycle_mgmt.ValidatedSoftwareLCM_object_tags.tag), <django.db.models.sql.query.Query object at 0x7ad37de06e90>)))
| When( | ||
| device_tenants=self.item_obj.tenant, | ||
| device_types=self.item_obj.device_type.pk, |
There was a problem hiding this comment.
Do we need to have (tenant, device_type) and (tenant, role) combinations, or is (tenant, device_type) enough?
There was a problem hiding this comment.
I did not receive role as a requirement, that's why it is not implemented ... do you see it as a potential case? roles are shared across tenants so it did not made a lot of sense to me, but maybe there could be some useful case?
| if self.item_obj.validated_software.exists(): | ||
| self.validated_software_qs = self.validated_software_qs.filter(devices__in=[self.item_obj.pk]).distinct() |
There was a problem hiding this comment.
The queries above already have a distinct() applied to them. This would essentially repeat the computation we've done up to this point.
There was a problem hiding this comment.
this essentially overrides the previous queryset, that's why I added distinct(). Computation-wise should be ok as it is all lazy-eval.
| if self.item_obj.validated_software.exists(): | ||
| self.validated_software_qs = self.validated_software_qs.filter(devices__in=[self.item_obj.pk]).distinct() | ||
| self.validated_software_qs = self._add_weights().order_by("weight", "start") | ||
| self.validated_software_qs = self.validated_software_qs.order_by("weight", "start") |
There was a problem hiding this comment.
We already ordered it on the previous line.
There was a problem hiding this comment.
good catch! missed tshoot remnant, getting rid of it.
| software=self.software, | ||
| start=date(2013, 11, 2), | ||
| ) | ||
| lcm_global.device_tenants.set([self.tenant_1]) |
There was a problem hiding this comment.
If we are testing for fallback, then shouldn't the lcm_global.device_tenants be an empty set?
There was a problem hiding this comment.
need to modify this, I disabled global fallback, so should be def test_get_for_object_device_tenant_no_fallback_to_global(self):
| Q(devices=self.item_obj.pk) | ||
| | Q(device_tenants=self.item_obj.tenant, device_types=self.item_obj.device_type.pk) | ||
| | Q(device_tenants=self.item_obj.tenant, device_roles=self.item_obj.role.pk) | ||
| | Q(device_tenants=self.item_obj.tenant, device_types=None, software__platform=self.item_obj.platform) |
There was a problem hiding this comment.
Why do we include software__platform=self.item_obj.platform ? This changes the behavior, is only included when the tenant is defined, and is negated by the line that follows Q(device_tenants=self.item_obj.tenant, device_types=None).
There was a problem hiding this comment.
this is a good point, wanted to add validation so if a software from a different platform than device is added, it will not be accepted. However, current app behavior allows for these cases. Going to get rid of it for now, so we stay consistent across tenant and non-tenant cases.
| queryset=Platform.objects.all(), | ||
| label="Platform", | ||
| ) | ||
| tenant = django_filters.ModelMultipleChoiceFilter( |
There was a problem hiding this comment.
We need to add a tenant_id equivalent and add tests.
| queryset=Platform.objects.all(), | ||
| label="Platform", | ||
| ) | ||
| tenant = django_filters.ModelMultipleChoiceFilter( |
There was a problem hiding this comment.
We need to add a tenant_id equivalent and add tests.
| "start": {"lookup_expr": "icontains", "preprocessor": str.strip}, | ||
| "end": {"lookup_expr": "icontains", "preprocessor": str.strip}, | ||
| "devices__name": {"lookup_expr": "icontains", "preprocessor": str.strip}, | ||
| "devices__tenant__name": {"lookup_expr": "icontains", "preprocessor": str.strip}, |
There was a problem hiding this comment.
You probably also want this:
device_tenants__name": {"lookup_expr": "icontains", "preprocessor": str.strip},
There was a problem hiding this comment.
eagle's eye, fumbled that one!
modified: nautobot_device_lifecycle_mgmt/filters.py modified: nautobot_device_lifecycle_mgmt/software_filters.py modified: nautobot_device_lifecycle_mgmt/tests/test_filters.py modified: nautobot_device_lifecycle_mgmt/tests/test_model.py
…s. Removal platform lookup for tenant-specific filters. modified: nautobot_device_lifecycle_mgmt/software_filters.py
modified: nautobot_device_lifecycle_mgmt/software_filters.py modified: nautobot_device_lifecycle_mgmt/tests/test_filters.py
modified: nautobot_device_lifecycle_mgmt/filters.py
…agement. modified: docs/user/software_lifecycle.md
Co-authored-by: Josh VanDeraa <jv@networktocode.com>
modified: nautobot_device_lifecycle_mgmt/filters.py modified: nautobot_device_lifecycle_mgmt/forms.py
| ) | ||
| devices = models.ManyToManyField(to="dcim.Device", related_name="validated_software", blank=True) | ||
| device_types = models.ManyToManyField(to="dcim.DeviceType", related_name="validated_software", blank=True) | ||
| device_tenants = models.ManyToManyField(to="tenancy.Tenant", related_name="validated_software_tenants", blank=True) |
There was a problem hiding this comment.
Probably just have related_name="validated_software"
| ).distinct() | ||
|
|
||
| # 1. tenant relationship exists. | ||
| if self.item_obj.tenant: |
There was a problem hiding this comment.
Thinking about this more. This is breaking backwards compatibility. We are applying this new logic to any device with a tenant defined. Validation logic will break if an operator has an existing ValidatedSoftware record without a tenant defined (all of them, since this is a new addition) that worked for his devices, whether they have a tenant or not.
I think we'll need a workflow diagram illustrating different use cases. We need to ensure the existing behavior is not broken. This means adding regression tests.
Closes: #573
What's Changed
What's Changed
This PR introduces Multi-Tenancy awareness to the Device Lifecycle Management application. It enables organizations to define hardware and software validation policies that are scoped to specific Tenants, preventing "validation leakage" in shared or service-provider environments.
Multi-Tenant Software Validation
Enhanced Reporting & Filtering
To Do