Root Zones fail Validation

[andy-shady-org]

inc @tobus3000

We run root zones, which have names of 'com', 'ch' and 'uk', etc. These are failing validation checks against invalid domain name.
I appreciate that validation should be done on the zone name, however perhaps an extra boolean field signifying if the zone is a root zone could bypass this validation.

What are your thoughts on this?

[peteeckel]

Hi @andy-shady-org, that makes perfect sense to me. Let me see how I can fix this short-term ... I think it shouldn't be too much of an issue to make TLDs (root zones would just be called . - should we make that a legal domain name as well?) pass validation.

[andy-shady-org]

we have both TLDs and a root zone '.'., both types are failing validation, it just so happens that i'm hitting a TLD first and not getting as far as the root :)

I appreciate you looking into this for us.

Thanks

[peteeckel]

Hm. I just tried to create a zone "com", which works perfectly well. However, I found an uncaught exception when entering "com." as the name of a zone when "com" already exists.

"." does currently not validate though.

[andy-shady-org]

Yes, root '.' is failing, just had to catch the specific zone that was failing:

Creating log entry for . with action update: Zone . failed validation: {'name': ['Not a valid DNS domain name']}

[peteeckel]

The problem is unfortunately less simple than it seemed, and the reasons lie in the dire past of NetBox DNS.

When I started contributing to the project, there was no validation whatsoever of anything - zones, records, whatever could be named completely arbitrarily, which was obviously not such a good idea. Values, too - nothing at all got validated.

At a much later point, validation was added, and while doing that it turned out that some normalisation of zone and record names was necessary as well - for example, the zones example.com and example.com. are essentially the same zone, and so they should raise a ValidationError when one of them was added when the other already was in place. The best way to handle this is to normalise the entries so both formats will essentially result in the same name.

Here's where the fall from grace happened, and a good part of it is my fault. Most zones in the real world had been entered without the trailing dot, so I decided to normalise zone names to that format - no dot at the end - to keep the disturbance as small as possible. Unfortunately, that results in the root zone having an empty string as its name, which creates a lot of trouble when it becomes a legal name - dnspython, for example, translates it to @, which is not a good idea at all.

So I'm afraid I'll have to sift through the entire code base and take care nothing bad happens when the domain name is empty (as opposed to None, which is an entirely different case). It can and will be done, but it will take some time until everything is working smoothly.

Sorry for that, in hindsight I might have done better if I had chosen the version with the trailing dot as the normalised form. That would however have created a host of other incompatibilities - there is no easy way.

[peteeckel]

OK, that said - I put together a branch that contains experimental support for managing root zones.

Essentially it creates a special case, allowing a zone name to be ., in which case no further validation takes place. It's not really a piece of beauty, but it saves major headaches and trouble for anyone relying on the old schema of zone normalisation. Please not that you still can't add records with the name ., so you'll need to handle the @ names dnspython creates for empty record names when you create zone files from the root zone.

Can you check out if it works for you? If it does, I'll probably hide the functionality after a feature flag so people who don't manage their own root zones don't get confused and push a new release.

[andy-shady-org]

Ive checked this this morning, and the fix works fine. I can add the root zone '.' without issue, in exactly the same way I can add a normal non-root zone.

For me this is great, so again, thank you so much, and for your quick response :)

[andy-shady-org]

Sure, I'll check it out in the morning and let you know ASAP. Thanks for your effort on this, very much appreciated.

[peteeckel]

Thanks! I just published 0.18.4, which should be available on PyPI in a few minutes.

Currently, unlike in yesterday's test branch, you'll need to enable that feature explicitly using

enable_root_zones = True

in configuration.py.

[andy-shady-org]

ok, understood.

I have another one for you also:

from IPy import IP
zone_name = IP('10.10.10.0/26').reverseName()[:-1]
zone_name
'0-63.10.10.10.in-addr.arpa'
from myinfoblox import IPHelpers
IPHelpers.is_valid_fqdn(zone_name)
True

zone = Zone.objects.update_or_create(view=view, name=zone_name, description=description, default_ttl=30, soa_ttl=86400, soa_mname=name_servers.get(custom_field_data__mserver=True), soa_rname="person@example.com", soa_serial=int(datetime.now().timestamp()), soa_refresh=10800, soa_retry=3600, soa_expire=2592000, soa_minimum=30, arpa_network=arpa_name)
Traceback (most recent call last):
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/query.py", line 929, in get_or_create
return self.get(**kwargs), False
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/query.py", line 650, in get
raise self.model.DoesNotExist(
netbox_dns.models.Zone.DoesNotExist: Zone matching query does not exist.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "", line 1, in
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/manager.py", line 85, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/query.py", line 962, in update_or_create
obj, created = self.select_for_update().get_or_create(defaults, **kwargs)
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/query.py", line 936, in get_or_create
return self.create(**params), True
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/query.py", line 671, in create
obj.save(force_insert=True, using=self.db)
File "/opt/packages/netbox-plugin-dns/netbox_dns/models.py", line 516, in save
zones = Zone.objects.filter(
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/manager.py", line 85, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/query.py", line 1421, in filter
return self._filter_or_exclude(False, args, kwargs)
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/query.py", line 1439, in _filter_or_exclude
clone._filter_or_exclude_inplace(negate, args, kwargs)
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/query.py", line 1446, in _filter_or_exclude_inplace
self._query.add_q(Q(*args, **kwargs))
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1532, in add_q
clause, _ = self._add_q(q_object, self.used_aliases)
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1562, in _add_q
child_clause, needed_inner = self.build_filter(
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1478, in build_filter
condition = self.build_lookup(lookups, col, value)
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/sql/query.py", line 1308, in build_lookup
raise ValueError("Cannot use None as a query value")
ValueError: Cannot use None as a query value

In theory, I would expect this to raise a validation error, if the zone_name variable was failing validation but validation passes:

from netbox_dns.validators import *
validate_fqdn(zone_name)
validate_domain_name(zone_name)

[peteeckel]

Hi @andy-shady-org,

that's another one that will get more attention in the (hopefully) near future ... zone_name = IP('10.10.10.0/26').reverseName()[:-1] creates an RFC2317 zone name, which strictly speaking isn't illegal but fails to work properly with NetBox DNS until RFC2317 is implemented.

I guess until that happens (it will be a major feature) I should make validation fail for RFC2317 zones. Let me see what I can do short-term.

[peteeckel]

My personal record - three releases in less than 24 hours :-)

0.18.5 is on its way.

This release does not support RFC2317, but it tolerates zone names that end in .in-addr.arpa and are not standard reverse zones, which was the case with your example.

[andy-shady-org]

Oh, Im sure there will be more to come, we are only just getting started with importing IPAM data to Netbox :)
Today we have approx 10k zones, 50k subnets, 1/2 mil host records, which is probably about to double through an acquisition, so we shall see where we get with all this.

[peteeckel]

This is definitely going to get interesting :-)

To elaborate a bit on this: I expect mass imports of zones and particularly records in that order of magnitude to fail in interesting ways. Probably some special measures are required to make them work, and I also expect one or the other improvement to the NetBox DNS code to be required. The most likely issues I expect are performance bottlenecks leading to timeouts, but I trust there will be others as well. NetBox DNS tries to do many things automatically in the background, but this is not really optimised for efficiency yet.

In other words: Ample opportunities for improvement and development towards a more heavy-duty tool for DNS management. I'm looking forward to it!

[andy-shady-org]

feel free to ping me offline and we can discuss further (andy at shady.org)

[andy-shady-org]

I found another issue with another niche case:

[{'name': '0.0.0.0/0',
'_ref': 'zone_auth/ZG5zLnpvbmUkLl9kZWZhdWx0LmFycGEuaW4tYWRkcg:0.0.0.0%2F0/default',
'description': 'Top level domain for reverse DNS zones',

We have a 0.0.0.0/0 zone, as per its description.

In [11]: IP('0.0.0.0/0').reverseName()[:-1]
Out[11]: '.in-addr.arpa'

name = dns_name.from_text('.in-addr.arpa', origin=None).to_unicode()
Traceback (most recent call last):
File "", line 1, in
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/dns/name.py", line 1008, in from_text
raise EmptyLabel
dns.name.EmptyLabel: A DNS label is empty.

This results in the 'add record' form failing to load (very slowly with now 8K+ zones) with (truncated):

File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/netbox_dns/models.py", line 296, in str
name = dns_name.from_text(self.name, origin=None).to_unicode()
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/dns/name.py", line 1008, in from_text
raise EmptyLabel
dns.name.EmptyLabel: A DNS label is empty.

[peteeckel]

Re. "EmptyLabel": Did you manage to add the zone .in-addr.arpa (including the leading dot) and now "Add Record" throws the exception? I'm asking because you shouldn't have been able to create a zone with this name in the first place - the correct name would be in-addr.arpa. Your version is missing a label, as you might have expected :-)

By the way, this looks like a bug in IPy to me. .in-addr.arpa isn't a valid DNS name after all.

The other issue is the slow performance of "Add Record". Are you using the GUI for this? I think the performance is so bad because the form contains a pulldown menu for the zone, and with 8k zones it needs to be read from the DB and (worse) transferred to the client in order to create the pulldown menu. Currently I'm not sure how to tackle that, but I'll try to think of something. That's one of the "problems I didn't expect" issues.

Another idea: If we're talking about the GUI here, could you try to use the "Add Record" button in the zone view instead of using "Add Record" from any other view? That one comes with the zone pre-selected, so it might (depending on how intelligent Django uses resources) avoid the cost of initialising the pulldown menu.

[andy-shady-org]

Ive raised the name problem as an issue with IPy. There are a few open issues on that project, so lets see if he gets round to fixing it.
Until then, I'll put a workaround in place. After that, Ill check that the Add Record button doesnt produce exception, and what the result is.

[peteeckel]

After that, Ill check that the Add Record button doesnt produce exception, and what the result is.

Can you describe the way you got the "Add Record" button to raise an exception? I wasn't able to enter the zone at all (correctly so, because its name is not valid), so how did you manage to click on "Add Record"?

The error is either that you were able to create the zone (in that case I'm interested on how you did that) or some weird problem with "Add Record" causing it to raise an exception - I'm not entirely clear on how that can happen.

An exception is never an expected and acceptable behaviour for GUI actions and that needs to be sorted out, IPy bug or not.

[andy-shady-org]

both by clicking the + button within the left hand side menu, and by clicking the btn-success class button on the records view. Both raised the same exception.

I was able to create the zone, and it still exists:

from netbox_dns.models import Zone
z = Zone.objects.get(name='.in-addr.arpa')
str(z)
Traceback (most recent call last):
File "", line 1, in
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/netbox_dns/models.py", line 296, in str
name = dns_name.from_text(self.name, origin=None).to_unicode()
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/dns/name.py", line 1008, in from_text
raise EmptyLabel
dns.name.EmptyLabel: A DNS label is empty.
z
Traceback (most recent call last):
File "", line 1, in
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/django/db/models/base.py", line 586, in repr
return "<%s: %s>" % (self.class.name, self)
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/netbox_dns/models.py", line 296, in str
name = dns_name.from_text(self.name, origin=None).to_unicode()
File "/opt/netbox-3.5.2/venv/lib/python3.10/site-packages/dns/name.py", line 1008, in from_text
raise EmptyLabel
dns.name.EmptyLabel: A DNS label is empty.
z.name
'.in-addr.arpa'

Obviously Ive not fixed it yet to strip the leading dot, which I will do.

I use the following approach, mainly for speed:

new_zones = list()
if not zone_exists(zone_name):
   new_zones.append(Zone(**zone_params))
else:
   does_zone_require_updating(zone_name)
Zone.objects.bulk_create(new_zones)

This is highly simplified, as everything is happening async, in batches, but its more efficient than using Zone.objects.update_or_create() on every zone. Using the bulk method, I can create 8k zones in approx 5 mins, and using the latter, it takes hours, and obviously gets progressively slower over time.

[peteeckel]

both by clicking the + button within the left hand side menu, and by clicking the btn-success class button on the records view. Both raised the same exception.

For the exception this is what I would expect - the zone is invalid, so whatever happens with it (including adding records) will have problems of that kind.

Zone.objects.bulk_create(new_zones)

That's our culprit. Unlike create(), bulk_create() does not call save() (which in turn calls full_clean(), where the validation takes place), so the invalid zone gets created even though it wouldn't pass validation. Performance vs. safety ... there are some more issues with bulk_create() that will likely interfere with NetBox DNS (and possibly NetBox itself as well). You can use it, but expect some things not to work without additional actions after the import.

Bottom line: Every class that runs additional stuff on save() or clean() will likely be affected by `bulk_create()'.

From the Django documentation:

This has a number of caveats though:

The model’s save() method will not be called, and the pre_save and post_save signals will not be sent.

It does not work with child models in a multi-table inheritance scenario.

If the model’s primary key is an AutoField, the primary key attribute can only be retrieved on certain databases (currently PostgreSQL, MariaDB 10.5+, and SQLite 3.35+). On other databases, it will not be set.

It does not work with many-to-many relationships.

It casts objs to a list, which fully evaluates objs if it’s a generator. The cast allows inspecting all objects so that any objects with a manually set primary key can be inserted first. If you want to insert objects in batches without evaluating the entire generator at once, you can use this technique as long as the objects don’t have any manually set primary keys:

from itertools import islice

batch_size = 100
objs = (Entry(headline="Test %s" % i) for i in range(1000))
while True:
    batch = list(islice(objs, batch_size))
    if not batch:
        break
    Entry.objects.bulk_create(batch, batch_size)

The batch_size parameter controls how many objects are created in a single query. The default is to create all objects in one batch, except for SQLite where the default is such that at most 999 variables per query are used.

On databases that support it (all but Oracle), setting the ignore_conflicts parameter to True tells the database to ignore failure to insert any rows that fail constraints such as duplicate unique values.

On databases that support it (all except Oracle and SQLite < 3.24), setting the update_conflicts parameter to True, tells the database to update update_fields when a row insertion fails on conflicts. On PostgreSQL and SQLite, in addition to update_fields, a list of unique_fields that may be in conflict must be provided.

Enabling the ignore_conflicts or update_conflicts parameter disable setting the primary key on each model instance (if the database normally support it).

Warning

On MySQL and MariaDB, setting the ignore_conflicts parameter to True turns certain types of errors, other than duplicate key, into warnings. Even with Strict Mode. For example: invalid values or non-nullable violations. See the MySQL documentation and MariaDB documentation for more details.

Some of them won't affect you because you're using PosgreSQL, but some can and at least one definitely will.

That's not to discourage using bulk_create(), just a heads up that it might not give you exactly the result you expect.

Using the bulk method, I can create 8k zones in approx 5 mins, and using the latter, it takes hours, and obviously gets progressively slower over time.

Yes, that is because Django creates one large query for inserting all the stuff in one go, while create/update handles one item after the other.

An interesting part is that it gets slower over time. That is most likely a matter of database performance, and the first idea I have is to look and the queries and the time they take. Some PostgreSQL tuning on the engine level and maybe a couple of indices could substantially help here.

[andy-shady-org]

Yes, my case is truly a case of speed v safety and its understood. The volume of data drives me towards speed, and even then timeouts on RQ have to be high, with expected timeouts until the complete set of data is imported.

If and when netbox django moves to 4.2, then usage of async model calls may help, but speed is a limitation of the ORM. Performing data inserts via raw SQL would make the issue even worse, so as much attention to the data that can be taken will be done.

In this case, its one object type at a time, and moving slowly through all the data that we ant to import.