Skip to content

Allow a selection of target servertypes for attributes#427

Open
kofrezo wants to merge 4 commits intomainfrom
dk_attribute_multi_target_servertype
Open

Allow a selection of target servertypes for attributes#427
kofrezo wants to merge 4 commits intomainfrom
dk_attribute_multi_target_servertype

Conversation

@kofrezo
Copy link
Contributor

@kofrezo kofrezo commented Feb 10, 2026

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)

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)
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.
@kofrezo
Copy link
Contributor Author

kofrezo commented Feb 19, 2026

I will also check that it does not break the DNS views as they rely on the domain attributes.

@kofrezo kofrezo self-assigned this Feb 19, 2026
@kofrezo kofrezo changed the title Allow multiple target servertypes for attributes Allow a selection of target servertypes for attributes Feb 19, 2026
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.
@kofrezo kofrezo requested a review from brainexe February 20, 2026 08:43
@kofrezo kofrezo marked this pull request as ready for review February 20, 2026 08:43
return template.format(_raw_sql_escape(value))


def _target_servertype_sql(alias, attribute):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we use typehint at least for new code, else it's alwas hard to see is the attribute is the plain string or some django model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments