-
Notifications
You must be signed in to change notification settings - Fork 36
Tenant-Aware Report filters and ValidatedSW. #574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 14 commits
8412759
974c4e5
76c5880
c7f558c
691f589
58e633c
d5f1187
3157579
34f26e9
ff90014
04b7452
9a8d142
103a8df
c0f663f
363a2e9
c0cf665
658c099
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| feature: | ||
| - "Added multi-tenancy support to ValidatedSoftwareLCM, allowing software validations to be scoped to specific Tenants." | ||
| - "Added Tenant filtering to Hardware Notice and Validated Software reports." | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Generated by Django 4.2.26 on 2026-03-13 13:33 | ||
|
|
||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| dependencies = [ | ||
| ("tenancy", "0009_update_all_charfields_max_length_to_255"), | ||
| ("nautobot_device_lifecycle_mgmt", "0029_contractlcm_status"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( | ||
| model_name="validatedsoftwarelcm", | ||
| name="device_tenants", | ||
| field=models.ManyToManyField(blank=True, related_name="validated_software_tenants", to="tenancy.tenant"), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -298,6 +298,7 @@ class ValidatedSoftwareLCM(PrimaryModel): | |
| ) | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably just have |
||
| device_roles = models.ManyToManyField(to="extras.Role", related_name="validated_software", blank=True) | ||
| inventory_items = models.ManyToManyField(to="dcim.InventoryItem", related_name="validated_software", blank=True) | ||
| object_tags = models.ManyToManyField(to="extras.Tag", related_name="validated_software", blank=True) | ||
|
|
@@ -315,6 +316,7 @@ class Meta: | |
| clone_fields = ( | ||
| "software", | ||
| "devices", | ||
| "device_tenants", | ||
| "device_types", | ||
| "device_roles", | ||
| "inventory_items", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,14 +53,31 @@ def __init__(self, qs, item_obj): # pylint: disable=invalid-name | |
|
|
||
| def filter_qs(self): | ||
| """Returns filtered ValidatedSoftwareLCM query set.""" | ||
| self.validated_software_qs = self.validated_software_qs.filter( | ||
| Q(devices=self.item_obj.pk) | ||
| | Q(device_types=self.item_obj.device_type.pk, device_roles=self.item_obj.role.pk) | ||
| | Q(device_types=self.item_obj.device_type.pk, device_roles=None) | ||
| | Q(device_types=None, device_roles=self.item_obj.role.pk) | ||
| | Q(object_tags__in=self.item_obj.tags.all()) | ||
| ).distinct() | ||
|
|
||
| # 1. tenant relationship exists. | ||
| if self.item_obj.tenant: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @progala modified the behavior so it is fully backwards compatible, plus added regression tests for scenarios 1-5: Scenario 1: Device WITH tenant, only global records exist (the upgrade scenario) Scenario 2: Device WITHOUT tenant, only global records exist Scenario 3: Device WITH tenant, both tenant-specific AND global records exist Scenario 4: Device WITH Tenant A, records exist for Tenant B only + global Scenario 5: Device WITH tenant, tenant-specific records exist, no global Scenario 6: Device directly assigned to ValidatedSoftwareLCM |
||
| self.validated_software_qs = self.validated_software_qs.filter( | ||
| Q(devices__in=[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) | ||
| | Q(object_tags__in=self.item_obj.tags.all()) | ||
| ).distinct() | ||
|
Comment on lines
+57
to
+73
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not include
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fallback happens in lines 67-77 |
||
| # 2. No tenant relationship exists, filter based on device type, role, and tags. | ||
| else: | ||
| self.validated_software_qs = self.validated_software_qs.filter( | ||
| Q(devices__in=[self.item_obj.pk]) | ||
| | Q( | ||
| device_types=self.item_obj.device_type.pk, | ||
| device_roles=self.item_obj.role.pk, | ||
| device_tenants__isnull=True, | ||
| ) | ||
| | Q(device_types=self.item_obj.device_type.pk, device_roles=None, device_tenants__isnull=True) | ||
| | Q(device_types=None, device_roles=self.item_obj.role.pk, device_tenants__isnull=True) | ||
| | Q(object_tags__in=self.item_obj.tags.all()) | ||
| ).distinct() | ||
| # 3. Override qs when direct device assignments exist so no duplicates are returned. | ||
| if self.item_obj.validated_software.exists(): | ||
| self.validated_software_qs = self.validated_software_qs.filter(devices__in=[self.item_obj.pk]).distinct() | ||
|
Comment on lines
+88
to
+89
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The queries above already have a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this essentially overrides the previous queryset, that's why I added distinct(). Computation-wise should be ok as it is all lazy-eval. |
||
| self.validated_software_qs = self._add_weights().order_by("weight", "start") | ||
|
|
||
| return self.validated_software_qs | ||
|
|
@@ -87,6 +104,30 @@ def _add_weights(self): | |
| When(device_types=self.item_obj.device_type.pk, device_roles=None, preferred=False, then=Value(1030)), | ||
| 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, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens when tenant is not defined for the device?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if device has no tenant, it only
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it will work: device without tenant defined: device with tenant defined: |
||
| device_types=self.item_obj.device_type.pk, | ||
| preferred=True, | ||
| then=Value(50), | ||
| ), | ||
| When( | ||
| device_tenants=self.item_obj.tenant, | ||
| device_types=self.item_obj.device_type.pk, | ||
|
Comment on lines
+140
to
+142
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to have (tenant, device_type) and (tenant, role) combinations, or is (tenant, device_type) enough?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did not receive |
||
| preferred=False, | ||
| then=Value(1050), | ||
| ), | ||
| When( | ||
| device_tenants=self.item_obj.tenant, | ||
| device_roles=self.item_obj.role.pk, | ||
| preferred=True, | ||
| then=Value(60), | ||
| ), | ||
| When( | ||
| device_tenants=self.item_obj.tenant, | ||
| device_roles=self.item_obj.role.pk, | ||
| preferred=False, | ||
| then=Value(1060), | ||
| ), | ||
| When(preferred=True, then=Value(990)), | ||
| default=Value(1990), | ||
| output_field=IntegerField(), | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.