From 41666fe4bb4e8fb61b26af3a9932a3ff90f93649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Tue, 10 Feb 2026 14:54:18 +0100 Subject: [PATCH 1/6] Allow multiple target servertypes for attributes It is currently only possible to select one or none (all) target servertype for attribtues of type relation, supernet and domain. We have a use case where we want to allow some servertypes as target but not all. - serverdb.models.Attribute: Replace FK with ManyToManyField - serverdb.sql_generator: Replace '=' with 'IN' operator - serverdb.query_materializer: Replace '=' with 'ANY' operator - serverdb.query_materializer: Use Django '__in' instead of '=' for ORM - serverdb.admin: Add filter_horizontal widget, remove from readonly_fields - serverdb.admin: Explain what happens if no servertype is selected - serverdb.forms: Adapt validation from is-not-None to queryset .exists() - Add migration to adjust schema and migrate existing data - Adjust fixtures for tests - Adjust db/integrity.sql: Updated to query M2M join table (likely orphaned) --- db/integrity.sql | 9 +- .../access_control/fixtures/serverdb.json | 4 +- serveradmin/serverdb/admin.py | 3 +- serveradmin/serverdb/fixtures/attribute.json | 4 +- .../serverdb/fixtures/ip_addr_type.json | 18 ++-- .../serverdb/fixtures/test_dataset.json | 22 ++--- serveradmin/serverdb/forms.py | 3 +- .../0022_attribute_multi_target_servertype.py | 83 +++++++++++++++++++ serveradmin/serverdb/models.py | 12 +-- serveradmin/serverdb/query_materializer.py | 10 ++- serveradmin/serverdb/sql_generator.py | 17 +++- 11 files changed, 146 insertions(+), 39 deletions(-) create mode 100644 serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py diff --git a/db/integrity.sql b/db/integrity.sql index 1db2c7b53..bfe70ecb1 100644 --- a/db/integrity.sql +++ b/db/integrity.sql @@ -140,10 +140,15 @@ returning *; -- delete from server_relation_attribute as extra -using attribute, server as target +using attribute +join attribute_target_servertype as ats on ats.attribute_id = attribute.attribute_id, +server as target where attribute.attribute_id = extra.attribute_id and target.server_id = extra.value -and attribute.target_servertype_id != target.servertype_id +and target.servertype_id not in ( + select ats2.servertype_id from attribute_target_servertype as ats2 + where ats2.attribute_id = attribute.attribute_id +) returning attribute.attribute_id, target.hostname; commit; diff --git a/serveradmin/access_control/fixtures/serverdb.json b/serveradmin/access_control/fixtures/serverdb.json index 26d21fbca..a4ea1b288 100644 --- a/serveradmin/access_control/fixtures/serverdb.json +++ b/serveradmin/access_control/fixtures/serverdb.json @@ -26,7 +26,7 @@ "help_link": null, "inet_address_family": "", "readonly": false, - "target_servertype": "hv", + "target_servertype": ["hv"], "reversed_attribute": null, "clone": false, "history": true, @@ -44,7 +44,7 @@ "help_link": null, "inet_address_family": "", "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "history": true, diff --git a/serveradmin/serverdb/admin.py b/serveradmin/serverdb/admin.py index a61566b90..d12125e1b 100644 --- a/serveradmin/serverdb/admin.py +++ b/serveradmin/serverdb/admin.py @@ -69,6 +69,7 @@ class ServerAdmin(admin.ModelAdmin): class AttributeAdmin(admin.ModelAdmin): form = AttributeAdminForm + filter_horizontal = ('target_servertype',) list_display = [ 'attribute_id', 'type', @@ -90,7 +91,7 @@ def get_readonly_fields(self, request, obj=None): # support it. if obj: fields += ( - 'type', 'attribute_id', 'target_servertype', + 'type', 'attribute_id', 'reversed_attribute' ) diff --git a/serveradmin/serverdb/fixtures/attribute.json b/serveradmin/serverdb/fixtures/attribute.json index db3f70410..53495d39f 100644 --- a/serveradmin/serverdb/fixtures/attribute.json +++ b/serveradmin/serverdb/fixtures/attribute.json @@ -17,7 +17,7 @@ "group": "project", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "history": false, @@ -34,7 +34,7 @@ "group": "project", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "history": true, diff --git a/serveradmin/serverdb/fixtures/ip_addr_type.json b/serveradmin/serverdb/fixtures/ip_addr_type.json index b27ece76f..6ae5c958f 100644 --- a/serveradmin/serverdb/fixtures/ip_addr_type.json +++ b/serveradmin/serverdb/fixtures/ip_addr_type.json @@ -50,7 +50,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -67,7 +67,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -83,7 +83,7 @@ "group": "other", "help_link": null, "readonly": true, - "target_servertype": "route_network", + "target_servertype": ["route_network"], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -100,7 +100,7 @@ "group": "other", "help_link": null, "readonly": true, - "target_servertype": "provider_network", + "target_servertype": ["provider_network"], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -117,7 +117,7 @@ "group": "other", "help_link": null, "readonly": true, - "target_servertype": "provider_network", + "target_servertype": ["provider_network"], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -133,7 +133,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -149,7 +149,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -165,7 +165,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -181,7 +181,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" diff --git a/serveradmin/serverdb/fixtures/test_dataset.json b/serveradmin/serverdb/fixtures/test_dataset.json index f7d6eec05..3cd35ef74 100644 --- a/serveradmin/serverdb/fixtures/test_dataset.json +++ b/serveradmin/serverdb/fixtures/test_dataset.json @@ -49,7 +49,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -65,7 +65,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -81,7 +81,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -97,7 +97,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A(0|[1-9][0-9]*)\\Z" @@ -113,7 +113,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -129,7 +129,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": "hypervisor", + "target_servertype": ["hypervisor"], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -145,7 +145,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -161,7 +161,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -177,7 +177,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A.*\\Z" @@ -193,7 +193,7 @@ "group": "other", "help_link": null, "readonly": false, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": null, "clone": false, "regexp": "\\A(wheezy|squeeze|jessie|buster|bullseye)\\Z" @@ -209,7 +209,7 @@ "group": "other", "help_link": null, "readonly": true, - "target_servertype": null, + "target_servertype": [], "reversed_attribute": "hypervisor", "clone": false, "regexp": "\\A.*\\Z" diff --git a/serveradmin/serverdb/forms.py b/serveradmin/serverdb/forms.py index b21a97439..7484f4523 100644 --- a/serveradmin/serverdb/forms.py +++ b/serveradmin/serverdb/forms.py @@ -44,7 +44,8 @@ class Meta: def clean(self): attr_type = self.cleaned_data.get('type') or self.instance.type # New or existing attribute ? - if attr_type != 'relation' and self.cleaned_data.get('target_servertype') is not None: + target_servertypes = self.cleaned_data.get('target_servertype') + if attr_type != 'relation' and target_servertypes and target_servertypes.exists(): raise ValidationError('Attribute type must be relation when target servertype is selected!') if attr_type == 'inet' and self.cleaned_data.get('multi') is True: diff --git a/serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py b/serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py new file mode 100644 index 000000000..100d08d69 --- /dev/null +++ b/serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py @@ -0,0 +1,83 @@ +"""Convert Attribute.target_servertype from ForeignKey to ManyToManyField. + +This migration: +1. Creates the M2M join table +2. Copies existing FK data into the join table +3. Drops the old FK column and its constraints +""" + +from django.db import migrations, models + + +def copy_fk_to_m2m(apps, schema_editor): + """Copy existing target_servertype_id FK values to the new M2M table.""" + with schema_editor.connection.cursor() as cursor: + cursor.execute( + "INSERT INTO attribute_target_servertype (attribute_id, servertype_id) " + "SELECT attribute_id, target_servertype_id FROM attribute " + "WHERE target_servertype_id IS NOT NULL" + ) + + +def copy_m2m_to_fk(apps, schema_editor): + """Reverse: copy M2M values back into the FK column (takes first value).""" + with schema_editor.connection.cursor() as cursor: + cursor.execute( + "UPDATE attribute SET target_servertype_id = m2m.servertype_id " + "FROM attribute_target_servertype AS m2m " + "WHERE attribute.attribute_id = m2m.attribute_id" + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('serverdb', '0021_serverinetattribute_server_inet_attribute_value_idx'), + ] + + operations = [ + # Step 1: Create the M2M join table manually so we can control + # the order of operations (data migration before dropping FK). + migrations.RunSQL( + sql=( + "CREATE TABLE attribute_target_servertype (" + " id BIGSERIAL PRIMARY KEY," + " attribute_id VARCHAR(32) NOT NULL REFERENCES attribute(attribute_id) ON DELETE CASCADE," + " servertype_id VARCHAR(32) NOT NULL REFERENCES servertype(servertype_id) ON DELETE CASCADE," + " UNIQUE (attribute_id, servertype_id)" + ")" + ), + reverse_sql="DROP TABLE IF EXISTS attribute_target_servertype", + ), + + # Step 2: Copy existing FK data into the M2M table. + migrations.RunPython(copy_fk_to_m2m, copy_m2m_to_fk), + + # Step 3: Drop the old FK constraint and CHECK constraint, then + # drop the column. + migrations.RunSQL( + sql=( + "ALTER TABLE attribute " + "DROP CONSTRAINT IF EXISTS attribute_target_servertype_id_0eab2dcc_fk_servertyp" + ), + reverse_sql=migrations.RunSQL.noop, + ), + migrations.RunSQL( + sql=( + "ALTER TABLE attribute " + "DROP CONSTRAINT IF EXISTS attribute_target_servertype_id_check" + ), + reverse_sql=( + "ALTER TABLE attribute ADD CONSTRAINT attribute_target_servertype_id_check " + "CHECK((type IN ('domain', 'supernet', 'relation')) = " + "(target_servertype_id IS NOT NULL OR type = 'relation'))" + ), + ), + migrations.RunSQL( + sql="ALTER TABLE attribute DROP COLUMN IF EXISTS target_servertype_id", + reverse_sql=( + "ALTER TABLE attribute ADD COLUMN target_servertype_id " + "VARCHAR(32) REFERENCES servertype(servertype_id) ON DELETE CASCADE" + ), + ), + ] diff --git a/serveradmin/serverdb/models.py b/serveradmin/serverdb/models.py index 0676660df..e91f0ccfa 100644 --- a/serveradmin/serverdb/models.py +++ b/serveradmin/serverdb/models.py @@ -310,8 +310,9 @@ def __init__(self, *args, **kwargs): choices=InetAddressFamilyChoice.choices, max_length=5, blank=True ) readonly = models.BooleanField(null=False, default=False) - target_servertype = models.ForeignKey( - Servertype, on_delete=models.CASCADE, db_index=False, null=True, blank=True + target_servertype = models.ManyToManyField( + Servertype, blank=True, + help_text="Selecting no servertype allows all servertypes.", ) reversed_attribute = models.ForeignKey( "self", @@ -666,11 +667,12 @@ def save_value(self, value): except Server.DoesNotExist: raise ValidationError('No server with hostname "{0}" exist.'.format(value)) - target_servertype = self.attribute.target_servertype - if target_servertype and target_server.servertype != target_servertype: + target_servertypes = self.attribute.target_servertype.all() + if target_servertypes.exists() and target_server.servertype not in target_servertypes: raise ValidationError( 'Attribute "{0}" has to be from servertype "{1}".'.format( - self.attribute, self.attribute.target_servertype + self.attribute, + ', '.join(str(st) for st in target_servertypes), ) ) diff --git a/serveradmin/serverdb/query_materializer.py b/serveradmin/serverdb/query_materializer.py index b4fafd5fa..6ad76a3b6 100644 --- a/serveradmin/serverdb/query_materializer.py +++ b/serveradmin/serverdb/query_materializer.py @@ -185,7 +185,7 @@ def _add_domain_attribute(self, attribute, servers): domain_lookup = { domain.hostname: domain for domain in Server.objects.filter( - servertype=attribute.target_servertype, + servertype__in=attribute.target_servertype.all(), hostname__in=domain_names, ) } @@ -221,7 +221,7 @@ def _add_supernet_attribute(self, attribute: Attribute, servers_in): JOIN {Attribute._meta.db_table} AS net_attr ON (net_attr.attribute_id = net_addr.attribute_id) JOIN {Attribute._meta.db_table} AS host_attr ON (host_attr.attribute_id = host_addr.attribute_id) WHERE - net.servertype_id = %(target_servertype)s + net.servertype_id = ANY(%(target_servertypes)s) AND host.server_id = any(%(hosts)s) """ @@ -236,7 +236,11 @@ def _add_supernet_attribute(self, attribute: Attribute, servers_in): supernets = Server.objects.raw( q, { - "target_servertype": attribute.target_servertype_id, + "target_servertypes": list( + attribute.target_servertype.values_list( + 'servertype_id', flat=True + ) + ), "address_family": attribute.inet_address_family, "hosts": list(servers_by_id.keys()), }, diff --git a/serveradmin/serverdb/sql_generator.py b/serveradmin/serverdb/sql_generator.py index f440b42eb..b2b0b6a6b 100644 --- a/serveradmin/serverdb/sql_generator.py +++ b/serveradmin/serverdb/sql_generator.py @@ -207,6 +207,17 @@ def _containment_filter_template(attribute, filt): return template.format(_raw_sql_escape(value)) +def _target_servertype_sql(alias, attribute): + ids = list( + attribute.target_servertype.values_list('servertype_id', flat=True) + ) + if len(ids) == 1: + return f"{alias}.servertype_id = '{ids[0]}'" + return "{}.servertype_id IN ({})".format( + alias, ', '.join("'{}'".format(i) for i in ids) + ) + + def _condition_sql(attribute, template, related_vias): if attribute.special: return template.format('server.' + attribute.special.field) @@ -216,13 +227,13 @@ def _condition_sql(attribute, template, related_vias): attribute, 'supernet', '>>= server_addr.value', ( - f"supernet.servertype_id = '{attribute.target_servertype_id}'", + _target_servertype_sql('supernet', attribute), template.format('supernet.server_id'), ) ) if attribute.type == 'domain': return _exists_sql(Server, 'sub', ( - "sub.servertype_id = '{0}'".format(attribute.target_servertype_id), + _target_servertype_sql('sub', attribute), "server.hostname ~ ('\\A[^\.]+\.' || regexp_replace(" "sub.hostname, '(\*|\-|\.)', '\\\1', 'g') || '\\Z')", template.format('sub.server_id'), @@ -264,7 +275,7 @@ def _real_condition_sql(attribute, template, related_vias): related_via_attribute, 'supernet', '>>= server_addr.value', ( - f"supernet.servertype_id = '{related_via_attribute.target_servertype_id}'", + _target_servertype_sql('supernet', related_via_attribute), 'supernet.server_id = sub.server_id' ), ) From bb47473bc0b3b2ff0a77c8ae33a6c4b9346f5640 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Tue, 10 Feb 2026 15:29:54 +0100 Subject: [PATCH 2/6] Make migration visible to Django's state tracker --- .../0022_attribute_multi_target_servertype.py | 128 ++++++++++++------ 1 file changed, 85 insertions(+), 43 deletions(-) diff --git a/serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py b/serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py index 100d08d69..fb6146125 100644 --- a/serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py +++ b/serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py @@ -1,11 +1,17 @@ """Convert Attribute.target_servertype from ForeignKey to ManyToManyField. -This migration: -1. Creates the M2M join table -2. Copies existing FK data into the join table -3. Drops the old FK column and its constraints +Uses SeparateDatabaseAndState so that: +- Django's state tracker sees standard RemoveField + AddField operations +- The actual DB operations are controlled manually to allow a data + migration between creating the M2M table and dropping the FK column + +Steps: +1. Create the M2M join table +2. Copy existing FK data into the join table +3. Drop the old FK column and its custom CHECK constraints """ +import django.db.models from django.db import migrations, models @@ -36,48 +42,84 @@ class Migration(migrations.Migration): ] operations = [ - # Step 1: Create the M2M join table manually so we can control - # the order of operations (data migration before dropping FK). - migrations.RunSQL( - sql=( - "CREATE TABLE attribute_target_servertype (" - " id BIGSERIAL PRIMARY KEY," - " attribute_id VARCHAR(32) NOT NULL REFERENCES attribute(attribute_id) ON DELETE CASCADE," - " servertype_id VARCHAR(32) NOT NULL REFERENCES servertype(servertype_id) ON DELETE CASCADE," - " UNIQUE (attribute_id, servertype_id)" - ")" - ), - reverse_sql="DROP TABLE IF EXISTS attribute_target_servertype", + # Phase 1: Create the M2M join table (DB only, state updated later). + migrations.SeparateDatabaseAndState( + database_operations=[ + migrations.RunSQL( + sql=( + "CREATE TABLE attribute_target_servertype (" + " id BIGSERIAL PRIMARY KEY," + " attribute_id VARCHAR(32) NOT NULL" + " REFERENCES attribute(attribute_id) ON DELETE CASCADE," + " servertype_id VARCHAR(32) NOT NULL" + " REFERENCES servertype(servertype_id) ON DELETE CASCADE," + " UNIQUE (attribute_id, servertype_id)" + ")" + ), + reverse_sql="DROP TABLE IF EXISTS attribute_target_servertype", + ), + ], + state_operations=[], ), - # Step 2: Copy existing FK data into the M2M table. + # Phase 2: Copy existing FK data into the M2M table. migrations.RunPython(copy_fk_to_m2m, copy_m2m_to_fk), - # Step 3: Drop the old FK constraint and CHECK constraint, then - # drop the column. - migrations.RunSQL( - sql=( - "ALTER TABLE attribute " - "DROP CONSTRAINT IF EXISTS attribute_target_servertype_id_0eab2dcc_fk_servertyp" - ), - reverse_sql=migrations.RunSQL.noop, - ), - migrations.RunSQL( - sql=( - "ALTER TABLE attribute " - "DROP CONSTRAINT IF EXISTS attribute_target_servertype_id_check" - ), - reverse_sql=( - "ALTER TABLE attribute ADD CONSTRAINT attribute_target_servertype_id_check " - "CHECK((type IN ('domain', 'supernet', 'relation')) = " - "(target_servertype_id IS NOT NULL OR type = 'relation'))" - ), - ), - migrations.RunSQL( - sql="ALTER TABLE attribute DROP COLUMN IF EXISTS target_servertype_id", - reverse_sql=( - "ALTER TABLE attribute ADD COLUMN target_servertype_id " - "VARCHAR(32) REFERENCES servertype(servertype_id) ON DELETE CASCADE" - ), + # Phase 3: Drop old FK column + constraints; update Django state. + migrations.SeparateDatabaseAndState( + database_operations=[ + migrations.RunSQL( + sql=( + "ALTER TABLE attribute " + "DROP CONSTRAINT IF EXISTS" + " attribute_target_servertype_id_0eab2dcc_fk_servertyp" + ), + reverse_sql=( + "ALTER TABLE attribute ADD CONSTRAINT" + " attribute_target_servertype_id_0eab2dcc_fk_servertyp" + " FOREIGN KEY (target_servertype_id)" + " REFERENCES servertype(servertype_id)" + " DEFERRABLE INITIALLY DEFERRED" + ), + ), + migrations.RunSQL( + sql=( + "ALTER TABLE attribute " + "DROP CONSTRAINT IF EXISTS" + " attribute_target_servertype_id_check" + ), + reverse_sql=( + "ALTER TABLE attribute ADD CONSTRAINT" + " attribute_target_servertype_id_check " + "CHECK((type IN ('domain', 'supernet', 'relation')) = " + "(target_servertype_id IS NOT NULL OR type = 'relation'))" + ), + ), + migrations.RunSQL( + sql=( + "ALTER TABLE attribute " + "DROP COLUMN IF EXISTS target_servertype_id" + ), + reverse_sql=( + "ALTER TABLE attribute ADD COLUMN target_servertype_id " + "VARCHAR(32)" + ), + ), + ], + state_operations=[ + migrations.RemoveField( + model_name='attribute', + name='target_servertype', + ), + migrations.AddField( + model_name='attribute', + name='target_servertype', + field=models.ManyToManyField( + blank=True, + help_text='Selecting no servertype allows all servertypes.', + to='serverdb.servertype', + ), + ), + ], ), ] From 1ecada56aab4df11a49d91e6329f1ec4b282c906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Thu, 19 Feb 2026 14:46:24 +0100 Subject: [PATCH 3/6] domain and supernet attribute must have a target servertype Whilst dropping the attribute_target_servertype_id_check we lost the enforcement via database constraint that domain and supernet must have a target servertype. This is not easily possible anymore because we now have a dedicated table (m2m). While otherwise (e.g. database triggers) are possible a simple validation in the admin form should suffice because this should be the only place where we manage attributes. So we accept the risk that somebody creates them by writing code and by passes this check. --- serveradmin/serverdb/forms.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/serveradmin/serverdb/forms.py b/serveradmin/serverdb/forms.py index 7484f4523..ba4fae23c 100644 --- a/serveradmin/serverdb/forms.py +++ b/serveradmin/serverdb/forms.py @@ -45,7 +45,9 @@ def clean(self): attr_type = self.cleaned_data.get('type') or self.instance.type # New or existing attribute ? target_servertypes = self.cleaned_data.get('target_servertype') - if attr_type != 'relation' and target_servertypes and target_servertypes.exists(): + if attr_type in ('domain', 'supernet') and not (target_servertypes and target_servertypes.exists()): + raise ValidationError('Attributes of type domain or supernet must have at least one target servertype!') + if attr_type not in ('domain', 'supernet', 'relation') and target_servertypes and target_servertypes.exists(): raise ValidationError('Attribute type must be relation when target servertype is selected!') if attr_type == 'inet' and self.cleaned_data.get('multi') is True: From c6760eb079e83940a44f0bae1b2953df57e04a6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Thu, 19 Feb 2026 16:43:57 +0100 Subject: [PATCH 4/6] Split database migrations to allow reverse migration Lift database check constraints first and then do the actual migration in a second one to allow reversing the migration. This is required because if we would reverse the migration in one the check constraint breaks it because the column cannot be null. Unfortunately check constraints cannot be deferred. --- ...ute_relax_target_servertype_constraints.py | 23 +++++++++++++++++++ ...te_relax_target_servertype_constraints.py} | 17 ++------------ 2 files changed, 25 insertions(+), 15 deletions(-) create mode 100644 serveradmin/serverdb/migrations/0022_attribute_relax_target_servertype_constraints.py rename serveradmin/serverdb/migrations/{0022_attribute_multi_target_servertype.py => 0023_attribute_relax_target_servertype_constraints.py} (84%) diff --git a/serveradmin/serverdb/migrations/0022_attribute_relax_target_servertype_constraints.py b/serveradmin/serverdb/migrations/0022_attribute_relax_target_servertype_constraints.py new file mode 100644 index 000000000..a03e7733b --- /dev/null +++ b/serveradmin/serverdb/migrations/0022_attribute_relax_target_servertype_constraints.py @@ -0,0 +1,23 @@ +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ('serverdb', '0021_serverinetattribute_server_inet_attribute_value_idx'), + ] + + operations = [ + migrations.RunSQL( + sql=( + "ALTER TABLE attribute " + "DROP CONSTRAINT IF EXISTS" + " attribute_target_servertype_id_check" + ), + reverse_sql=( + "ALTER TABLE attribute ADD CONSTRAINT" + " attribute_target_servertype_id_check " + "CHECK((type IN ('domain', 'supernet', 'relation')) = " + "(target_servertype_id IS NOT NULL OR type = 'relation'))" + ), + ), + ] diff --git a/serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py b/serveradmin/serverdb/migrations/0023_attribute_relax_target_servertype_constraints.py similarity index 84% rename from serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py rename to serveradmin/serverdb/migrations/0023_attribute_relax_target_servertype_constraints.py index fb6146125..4a5fbdf76 100644 --- a/serveradmin/serverdb/migrations/0022_attribute_multi_target_servertype.py +++ b/serveradmin/serverdb/migrations/0023_attribute_relax_target_servertype_constraints.py @@ -8,7 +8,7 @@ Steps: 1. Create the M2M join table 2. Copy existing FK data into the join table -3. Drop the old FK column and its custom CHECK constraints +3. Drop the old FK column """ import django.db.models @@ -38,7 +38,7 @@ def copy_m2m_to_fk(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ('serverdb', '0021_serverinetattribute_server_inet_attribute_value_idx'), + ('serverdb', '0022_attribute_relax_target_servertype_constraints') ] operations = [ @@ -82,19 +82,6 @@ class Migration(migrations.Migration): " DEFERRABLE INITIALLY DEFERRED" ), ), - migrations.RunSQL( - sql=( - "ALTER TABLE attribute " - "DROP CONSTRAINT IF EXISTS" - " attribute_target_servertype_id_check" - ), - reverse_sql=( - "ALTER TABLE attribute ADD CONSTRAINT" - " attribute_target_servertype_id_check " - "CHECK((type IN ('domain', 'supernet', 'relation')) = " - "(target_servertype_id IS NOT NULL OR type = 'relation'))" - ), - ), migrations.RunSQL( sql=( "ALTER TABLE attribute " From 4319272dc7d7fbeb7e6180a42f550ada27ec49bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Mon, 23 Feb 2026 14:17:20 +0100 Subject: [PATCH 5/6] Rename migration for clarification --- ...e_constraints.py => 0023_attribute_multi_target_servertype.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename serveradmin/serverdb/migrations/{0023_attribute_relax_target_servertype_constraints.py => 0023_attribute_multi_target_servertype.py} (100%) diff --git a/serveradmin/serverdb/migrations/0023_attribute_relax_target_servertype_constraints.py b/serveradmin/serverdb/migrations/0023_attribute_multi_target_servertype.py similarity index 100% rename from serveradmin/serverdb/migrations/0023_attribute_relax_target_servertype_constraints.py rename to serveradmin/serverdb/migrations/0023_attribute_multi_target_servertype.py From ea278b42e7b610ee2b92f4676f23b05a13daa2eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Kr=C3=B6ger?= Date: Mon, 23 Feb 2026 14:22:15 +0100 Subject: [PATCH 6/6] Add type hints --- serveradmin/serverdb/sql_generator.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/serveradmin/serverdb/sql_generator.py b/serveradmin/serverdb/sql_generator.py index b2b0b6a6b..394a17d22 100644 --- a/serveradmin/serverdb/sql_generator.py +++ b/serveradmin/serverdb/sql_generator.py @@ -25,6 +25,7 @@ StartsWith, Not, ) +from serveradmin.serverdb import models from serveradmin.serverdb.models import ( Server, ServerAttribute, @@ -207,7 +208,7 @@ def _containment_filter_template(attribute, filt): return template.format(_raw_sql_escape(value)) -def _target_servertype_sql(alias, attribute): +def _target_servertype_sql(alias: str, attribute: models.Attribute) -> str: ids = list( attribute.target_servertype.values_list('servertype_id', flat=True) )