diff --git a/CHANGELOG.md b/CHANGELOG.md index db325bb08..1d5ee0345 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,16 @@ NetBox now supports modeling physical cables for console, power, and interface c --- +v2.4.8 (FUTURE) + +## Bug Fixes + +* [#2473](https://github.com/digitalocean/netbox/issues/2473) - Fix encoding of long (>127 character) secrets +* [#2558](https://github.com/digitalocean/netbox/issues/2558) - Filter on all tags when multiple are passed +* [#2575](https://github.com/digitalocean/netbox/issues/2575) - Correct model specified for rack roles table + +--- + v2.4.7 (2018-11-06) ## Enhancements diff --git a/docs/development/extending-models.md b/docs/development/extending-models.md new file mode 100644 index 000000000..190fad5e0 --- /dev/null +++ b/docs/development/extending-models.md @@ -0,0 +1,70 @@ +# Extending Models + +Below is a list of items to consider when adding a new field to a model: + +### 1. Generate and run database migration + +Django migrations are used to express changes to the database schema. In most cases, Django can generate these automatically, however very complex changes may require manual intervention. Always remember to specify a short but descriptive name when generating a new migration. + +``` +./manage.py makemigrations -n +./manage.py migrate +``` + +Where possible, try to merge related changes into a single migration. For example, if three new fields are being added to different models within an app, these can be expressed in the same migration. You can merge a new migration with an existing one by combining their `operations` lists. + +!!! note + Migrations can only be merged within a release. Once a new release has been published, its migrations cannot be altered. + +### 2. Add validation logic to `clean()` + +If the new field introduces additional validation requirements (beyond what's included with the field itself), implement them in the model's `clean()` method. Remember to call the model's original method using `super()` before or agter your custom validation as appropriate: + +``` +class Foo(models.Model): + + def clean(self): + + super(DeviceCSVForm, self).clean() + + # Custom validation goes here + if self.bar is None: + raise ValidationError() +``` + +### 3. Add CSV helpers + +Add the name of the new field to `csv_headers` and included a CSV-friendly representation of its data in the model's `to_csv()` method. These will be used when exporting objects in CSV format. + +### 4. Update relevant querysets + +If you're adding a relational field (e.g. `ForeignKey`) and intend to include the data when retreiving a list of objects, be sure to include the field using `select_related()` or `prefetch_related()` as appropriate. This will optimize the view and avoid excessive database lookups. + +### 5. Update API serializer + +Extend the model's API serializer in `.api.serializers` to include the new field. In most cases, it will not be necessary to also extend the nested serializer, which produces a minimal represenation of the model. + +### 6. Add field to forms + +Extend any forms to include the new field as appropriate. Common forms include: + +* **Credit/edit** - Manipulating a single object +* **Bulk edit** - Performing a change on mnay objects at once +* **CSV import** - The form used when bulk importing objects in CSV format +* **Filter** - Displays the options available for filtering a list of objects (both UI and API) + +### 7. Extend object filter set + +If the new field should be filterable, add it to the `FilterSet` for the model. If the field should be searchable, remember to reference it in the FilterSet's `search()` method. + +### 8. Add column to object table + +If the new field will be included in the object list view, add a column to the model's table. For simple fields, adding the field name to `Meta.fields` will be sufficient. More complex fields may require explicitly declaring a new column. + +### 9. Update the UI templates + +Edit the object's view template to display the new field. There may also be a custom add/edit form template that needs to be updated. + +### 10. Adjust API and model tests + +Extend the model and/or API tests to verify that the new field and any accompanying validation logic perform as expected. This is especially important for relational fields. diff --git a/docs/development/index.md b/docs/development/index.md index 91086c61e..5830da765 100644 --- a/docs/development/index.md +++ b/docs/development/index.md @@ -28,10 +28,3 @@ NetBox components are arranged into functional subsections called _apps_ (a carr * `tenancy`: Tenants (such as customers) to which NetBox objects may be assigned * `utilities`: Resources which are not user-facing (extendable classes, etc.) * `virtualization`: Virtual machines and clusters - -## Style Guide - -NetBox generally follows the [Django style guide](https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/), which is itself based on [PEP 8](https://www.python.org/dev/peps/pep-0008/). The following exceptions are noted: - -* [Pycodestyle](https://github.com/pycqa/pycodestyle) is used to validate code formatting, ignoring certain violations. See `scripts/cibuild.sh`. -* Constants may be imported via wildcard (for example, `from .constants import *`). diff --git a/docs/development/style-guide.md b/docs/development/style-guide.md new file mode 100644 index 000000000..18dadd2d2 --- /dev/null +++ b/docs/development/style-guide.md @@ -0,0 +1,41 @@ +# Style Guide + +NetBox generally follows the [Django style guide](https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/), which is itself based on [PEP 8](https://www.python.org/dev/peps/pep-0008/). [Pycodestyle](https://github.com/pycqa/pycodestyle) is used to validate code formatting, ignoring certain violations. See `scripts/cibuild.sh`. + +## PEP 8 Exceptions + +* Wildcard imports (for example, `from .constants import *`) are acceptable under any of the following conditions: + * The library being import contains only constant declarations (`constants.py`) + * The library being imported explicitly defines `__all__` (e.g. `.api.nested_serializers`) + +* Maximum line length is 120 characters (E501) + * This does not apply to HTML templates or to automatically generated code (e.g. database migrations). + +* Line breaks are permitted following binary operators (W504) + +## Enforcing Code Style + +The `pycodestyle` utility (previously `pep8`) is used by the CI process to enforce code style. It is strongly recommended to include as part of your commit process. A git commit hook is provided in the source at `scripts/git-hooks/pre-commit`. Linking to this script from `.git/hooks/` will invoke `pycodestyle` prior to every commit attempt and abort if the validation fails. + +``` +$ cd .git/hooks/ +$ ln -s ../../scripts/git-hooks/pre-commit +``` + +To invoke `pycodestyle` manually, run: + +``` +pycodestyle --ignore=W504,E501 netbox/ +``` + +## General Guidance + +* When in doubt, remain consistent: It is better to be consistently incorrect than inconsistently correct. If you notice in the course of unrelated work a pattern that should be corrected, continue to follow the pattern for now and open a bug so that the entire code base can be evaluated at a later point. + +* No easter eggs. While they can be fun, NetBox must be considered as a business-critical tool. The potential, however minor, for introducing a bug caused by unnecessary logic is best avoided entirely. + +* Constants (variables which generally do not change) should be declared in `constants.py` within each app. Wildcard imports from the file are acceptable. + +* Every model should have a docstring. Every custom method should include an expalantion of its function. + +* Nested API serializers generate minimal representations of an object. These are stored separately from the primary serializers to avoid circular dependencies. Always import nested serializers from other apps directly. For example, from within the DCIM app you would write `from ipam.api.nested_serializers import NestedIPAddressSerializer`. diff --git a/mkdocs.yml b/mkdocs.yml index 87b7da254..cd718cc76 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -45,7 +45,9 @@ pages: - Examples: 'api/examples.md' - Development: - Introduction: 'development/index.md' + - Style Guide: 'development/style-guide.md' - Utility Views: 'development/utility-views.md' + - Extending Models: 'development/extending-models.md' - Release Checklist: 'development/release-checklist.md' markdown_extensions: diff --git a/netbox/circuits/filters.py b/netbox/circuits/filters.py index 61613bc6d..0982624d5 100644 --- a/netbox/circuits/filters.py +++ b/netbox/circuits/filters.py @@ -4,7 +4,7 @@ from django.db.models import Q from dcim.models import Site from extras.filters import CustomFieldFilterSet from tenancy.models import Tenant -from utilities.filters import NumericInFilter +from utilities.filters import NumericInFilter, TagFilter from .constants import CIRCUIT_STATUS_CHOICES from .models import Provider, Circuit, CircuitTermination, CircuitType @@ -29,9 +29,7 @@ class ProviderFilter(CustomFieldFilterSet, django_filters.FilterSet): to_field_name='slug', label='Site (slug)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Provider @@ -110,9 +108,7 @@ class CircuitFilter(CustomFieldFilterSet, django_filters.FilterSet): to_field_name='slug', label='Site (slug)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Circuit diff --git a/netbox/dcim/filters.py b/netbox/dcim/filters.py index bed641ff7..aa18f46c6 100644 --- a/netbox/dcim/filters.py +++ b/netbox/dcim/filters.py @@ -7,7 +7,7 @@ from netaddr.core import AddrFormatError from extras.filters import CustomFieldFilterSet from tenancy.models import Tenant -from utilities.filters import NullableCharFieldFilter, NumericInFilter +from utilities.filters import NullableCharFieldFilter, NumericInFilter, TagFilter from virtualization.models import Cluster from .constants import * from .models import ( @@ -81,9 +81,7 @@ class SiteFilter(CustomFieldFilterSet, django_filters.FilterSet): to_field_name='slug', label='Tenant (slug)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Site @@ -202,9 +200,7 @@ class RackFilter(CustomFieldFilterSet, django_filters.FilterSet): label='Role (slug)', ) asset_tag = NullableCharFieldFilter() - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Rack @@ -346,9 +342,7 @@ class DeviceTypeFilter(CustomFieldFilterSet, django_filters.FilterSet): method='_pass_through_ports', label='Has pass-through ports', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = DeviceType @@ -625,9 +619,7 @@ class DeviceFilter(CustomFieldFilterSet, django_filters.FilterSet): queryset=VirtualChassis.objects.all(), label='Virtual chassis (ID)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Device @@ -714,9 +706,7 @@ class DeviceComponentFilterSet(django_filters.FilterSet): to_field_name='name', label='Device (name)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class ConsolePortFilter(DeviceComponentFilterSet): @@ -775,9 +765,7 @@ class InterfaceFilter(django_filters.FilterSet): method='_mac_address', label='MAC address', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() vlan_id = django_filters.CharFilter( method='filter_vlan_id', label='Assigned VLAN' @@ -932,9 +920,7 @@ class VirtualChassisFilter(django_filters.FilterSet): to_field_name='slug', label='Tenant (slug)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = VirtualChassis diff --git a/netbox/dcim/tables.py b/netbox/dcim/tables.py index 6402958c2..547b0721d 100644 --- a/netbox/dcim/tables.py +++ b/netbox/dcim/tables.py @@ -7,7 +7,7 @@ from .models import ( Cable, ConsoleConnection, ConsolePort, ConsolePortTemplate, ConsoleServerPort, ConsoleServerPortTemplate, Device, DeviceBay, DeviceBayTemplate, DeviceRole, DeviceType, FrontPort, FrontPortTemplate, Interface, InterfaceConnection, InterfaceTemplate, InventoryItem, Manufacturer, Platform, PowerConnection, PowerOutlet, PowerOutletTemplate, - PowerPort, PowerPortTemplate, Rack, RackGroup, RackReservation, RearPort, RearPortTemplate, Region, Site, + PowerPort, PowerPortTemplate, Rack, RackGroup, RackReservation, RackRole, RearPort, RearPortTemplate, Region, Site, VirtualChassis, ) @@ -261,7 +261,7 @@ class RackRoleTable(BaseTable): verbose_name='') class Meta(BaseTable.Meta): - model = RackGroup + model = RackRole fields = ('pk', 'name', 'rack_count', 'color', 'slug', 'actions') diff --git a/netbox/ipam/filters.py b/netbox/ipam/filters.py index 7b9dbb6ca..abef95c45 100644 --- a/netbox/ipam/filters.py +++ b/netbox/ipam/filters.py @@ -7,7 +7,7 @@ from netaddr.core import AddrFormatError from dcim.models import Site, Device, Interface from extras.filters import CustomFieldFilterSet from tenancy.models import Tenant -from utilities.filters import NumericInFilter +from utilities.filters import NumericInFilter, TagFilter from virtualization.models import VirtualMachine from .constants import IPADDRESS_ROLE_CHOICES, IPADDRESS_STATUS_CHOICES, PREFIX_STATUS_CHOICES, VLAN_STATUS_CHOICES from .models import Aggregate, IPAddress, Prefix, RIR, Role, Service, VLAN, VLANGroup, VRF @@ -32,9 +32,7 @@ class VRFFilter(CustomFieldFilterSet, django_filters.FilterSet): to_field_name='slug', label='Tenant (slug)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() def search(self, queryset, name, value): if not value.strip(): @@ -80,9 +78,7 @@ class AggregateFilter(CustomFieldFilterSet, django_filters.FilterSet): to_field_name='slug', label='RIR (slug)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Aggregate @@ -184,9 +180,7 @@ class PrefixFilter(CustomFieldFilterSet, django_filters.FilterSet): choices=PREFIX_STATUS_CHOICES, null_value=None ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Prefix @@ -316,9 +310,7 @@ class IPAddressFilter(CustomFieldFilterSet, django_filters.FilterSet): role = django_filters.MultipleChoiceFilter( choices=IPADDRESS_ROLE_CHOICES ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = IPAddress @@ -438,9 +430,7 @@ class VLANFilter(CustomFieldFilterSet, django_filters.FilterSet): choices=VLAN_STATUS_CHOICES, null_value=None ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = VLAN @@ -482,9 +472,7 @@ class ServiceFilter(django_filters.FilterSet): to_field_name='name', label='Virtual machine (name)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Service diff --git a/netbox/netbox/views.py b/netbox/netbox/views.py index bfdeb8c28..263acb8ee 100644 --- a/netbox/netbox/views.py +++ b/netbox/netbox/views.py @@ -207,7 +207,7 @@ class HomeView(View): 'stats': stats, 'topology_maps': TopologyMap.objects.filter(site__isnull=True), 'report_results': ReportResult.objects.order_by('-created')[:10], - 'changelog': ObjectChange.objects.select_related('user')[:50] + 'changelog': ObjectChange.objects.select_related('user', 'changed_object_type')[:50] }) diff --git a/netbox/secrets/filters.py b/netbox/secrets/filters.py index 79ae3a115..5880fb9f9 100644 --- a/netbox/secrets/filters.py +++ b/netbox/secrets/filters.py @@ -3,7 +3,7 @@ from django.db.models import Q from dcim.models import Device from extras.filters import CustomFieldFilterSet -from utilities.filters import NumericInFilter +from utilities.filters import NumericInFilter, TagFilter from .models import Secret, SecretRole @@ -43,9 +43,7 @@ class SecretFilter(CustomFieldFilterSet, django_filters.FilterSet): to_field_name='name', label='Device (name)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Secret diff --git a/netbox/secrets/models.py b/netbox/secrets/models.py index dcdfcac73..72dbc10a1 100644 --- a/netbox/secrets/models.py +++ b/netbox/secrets/models.py @@ -1,4 +1,5 @@ import os +import sys from Crypto.Cipher import AES, PKCS1_OAEP from Crypto.PublicKey import RSA @@ -386,6 +387,7 @@ class Secret(ChangeLoggedModel, CustomFieldModel): s = s.encode('utf8') if len(s) > 65535: raise ValueError("Maximum plaintext size is 65535 bytes.") + # Minimum ciphertext size is 64 bytes to conceal the length of short secrets. if len(s) <= 62: pad_length = 62 - len(s) @@ -393,12 +395,14 @@ class Secret(ChangeLoggedModel, CustomFieldModel): pad_length = 16 - ((len(s) + 2) % 16) else: pad_length = 0 - return ( - chr(len(s) >> 8).encode() + - chr(len(s) % 256).encode() + - s + - os.urandom(pad_length) - ) + + # Python 2 compatibility + if sys.version_info[0] < 3: + header = chr(len(s) >> 8) + chr(len(s) % 256) + else: + header = bytes([len(s) >> 8]) + bytes([len(s) % 256]) + + return header + s + os.urandom(pad_length) def _unpad(self, s): """ diff --git a/netbox/secrets/tests/test_models.py b/netbox/secrets/tests/test_models.py index 0e5ad55ca..b3ba0cee1 100644 --- a/netbox/secrets/tests/test_models.py +++ b/netbox/secrets/tests/test_models.py @@ -1,3 +1,5 @@ +import string + from Crypto.PublicKey import RSA from django.conf import settings from django.contrib.auth.models import User @@ -86,7 +88,7 @@ class SecretTestCase(TestCase): """ Test basic encryption and decryption functionality using a random master key. """ - plaintext = "FooBar123" + plaintext = string.printable * 2 secret_key = generate_random_key() s = Secret(plaintext=plaintext) s.encrypt(secret_key) diff --git a/netbox/templates/dcim/inc/interface.html b/netbox/templates/dcim/inc/interface.html index 48dc67e39..a0ca6dcf9 100644 --- a/netbox/templates/dcim/inc/interface.html +++ b/netbox/templates/dcim/inc/interface.html @@ -18,7 +18,7 @@ {# LAG #} {% if iface.lag %} - {{ iface.lag }} + {{ iface.lag }} {% endif %} diff --git a/netbox/tenancy/filters.py b/netbox/tenancy/filters.py index e2bfd6632..5b3ec30d4 100644 --- a/netbox/tenancy/filters.py +++ b/netbox/tenancy/filters.py @@ -2,7 +2,7 @@ import django_filters from django.db.models import Q from extras.filters import CustomFieldFilterSet -from utilities.filters import NumericInFilter +from utilities.filters import NumericInFilter, TagFilter from .models import Tenant, TenantGroup @@ -32,9 +32,7 @@ class TenantFilter(CustomFieldFilterSet, django_filters.FilterSet): to_field_name='slug', label='Group (slug)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Tenant diff --git a/netbox/utilities/filters.py b/netbox/utilities/filters.py index 18161d465..424a2e47c 100644 --- a/netbox/utilities/filters.py +++ b/netbox/utilities/filters.py @@ -1,4 +1,5 @@ import django_filters +from taggit.models import Tag class NumericInFilter(django_filters.BaseInFilter, django_filters.NumberFilter): @@ -19,3 +20,18 @@ class NullableCharFieldFilter(django_filters.CharFilter): return super(NullableCharFieldFilter, self).filter(qs, value) qs = self.get_method(qs)(**{'{}__isnull'.format(self.name): True}) return qs.distinct() if self.distinct else qs + + +class TagFilter(django_filters.ModelMultipleChoiceFilter): + """ + Match on one or more assigned tags. If multiple tags are specified (e.g. ?tag=foo&tag=bar), the queryset is filtered + to objects matching all tags. + """ + def __init__(self, *args, **kwargs): + + kwargs.setdefault('field_name', 'tags__slug') + kwargs.setdefault('to_field_name', 'slug') + kwargs.setdefault('conjoined', True) + kwargs.setdefault('queryset', Tag.objects.all()) + + super(TagFilter, self).__init__(*args, **kwargs) diff --git a/netbox/virtualization/filters.py b/netbox/virtualization/filters.py index 3854fb132..a103e9b29 100644 --- a/netbox/virtualization/filters.py +++ b/netbox/virtualization/filters.py @@ -7,7 +7,7 @@ from netaddr.core import AddrFormatError from dcim.models import DeviceRole, Interface, Platform, Region, Site from extras.filters import CustomFieldFilterSet from tenancy.models import Tenant -from utilities.filters import NumericInFilter +from utilities.filters import NumericInFilter, TagFilter from .constants import VM_STATUS_CHOICES from .models import Cluster, ClusterGroup, ClusterType, VirtualMachine @@ -65,9 +65,7 @@ class ClusterFilter(CustomFieldFilterSet): to_field_name='slug', label='Site (slug)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = Cluster @@ -172,9 +170,7 @@ class VirtualMachineFilter(CustomFieldFilterSet): to_field_name='slug', label='Platform (slug)', ) - tag = django_filters.CharFilter( - field_name='tags__slug', - ) + tag = TagFilter() class Meta: model = VirtualMachine diff --git a/scripts/git-hooks/pre-commit b/scripts/git-hooks/pre-commit new file mode 100755 index 000000000..5974f91d8 --- /dev/null +++ b/scripts/git-hooks/pre-commit @@ -0,0 +1,14 @@ +#!/bin/sh +# Create a link to this file at .git/hooks/pre-commit to +# force PEP8 validation prior to committing +# +# Ignored violations: +# +# W504: Line break after binary operator +# E501: Line too long + +exec 1>&2 + +echo "Validating PEP8 compliance..." +pycodestyle --ignore=W504,E501 netbox/ +