diff --git a/db/integrity.sql b/db/integrity.sql index 1db2c7b5..bfe70ecb 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 26d21fbc..a4ea1b28 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 a61566b9..d12125e1 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 db3f7041..53495d39 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 b27ece76..6ae5c958 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 f7d6eec0..3cd35ef7 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 b21a9743..ba4fae23 100644 --- a/serveradmin/serverdb/forms.py +++ b/serveradmin/serverdb/forms.py @@ -44,7 +44,10 @@ 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 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: 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 00000000..a03e7733 --- /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/0023_attribute_multi_target_servertype.py b/serveradmin/serverdb/migrations/0023_attribute_multi_target_servertype.py new file mode 100644 index 00000000..4a5fbdf7 --- /dev/null +++ b/serveradmin/serverdb/migrations/0023_attribute_multi_target_servertype.py @@ -0,0 +1,112 @@ +"""Convert Attribute.target_servertype from ForeignKey to ManyToManyField. + +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 +""" + +import django.db.models +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', '0022_attribute_relax_target_servertype_constraints') + ] + + operations = [ + # 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=[], + ), + + # Phase 2: Copy existing FK data into the M2M table. + migrations.RunPython(copy_fk_to_m2m, copy_m2m_to_fk), + + # 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 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', + ), + ), + ], + ), + ] diff --git a/serveradmin/serverdb/models.py b/serveradmin/serverdb/models.py index 0676660d..e91f0ccf 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 b4fafd5f..6ad76a3b 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 f440b42e..394a17d2 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,6 +208,17 @@ def _containment_filter_template(attribute, filt): return template.format(_raw_sql_escape(value)) +def _target_servertype_sql(alias: str, attribute: models.Attribute) -> str: + 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 +228,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 +276,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' ), )