Skip to content

Commit

Permalink
Merge branch '5.6.x'
Browse files Browse the repository at this point in the history
  • Loading branch information
lunkwill42 committed Sep 1, 2023
2 parents a2772e1 + 92acff9 commit e2b92de
Show file tree
Hide file tree
Showing 13 changed files with 205 additions and 19 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ found in the [HISTORY](HISTORY) file.
- Show help text instead of error when running `nav` command without arguments ([#2601](https://github.com/Uninett/nav/issues/2601), [#2603](https://github.com/Uninett/nav/pull/2603))
- Prevent users from entering invalid `sysObjectID` values when editing Netbox types in SeedDB ([#2584](https://github.com/Uninett/nav/pull/2584), [#2566](https://github.com/Uninett/nav/issues/2566))
- Removed upper version bound for *Pillow* image manipulation library, to fix security warnings ([#2567](https://github.com/Uninett/nav/pull/2567))
- Alerts that cannot be sent due to blacklisted media plugins will no longer fill up `alertengine.log` every 30 seconds, unless DEBUG level logging is enabled ([#1787](https://github.com/Uninett/nav/issues/1787), [#2652](https://github.com/Uninett/nav/pull/2652))
- DNS lookups in ipdevinfo are now properly case insensitive ([#2615](https://github.com/Uninett/nav/issues/2615), [#2650](https://github.com/Uninett/nav/pull/2650))
- Alert Profiles will now properly require Slack alert addresses to be valid URLs ([#2657](https://github.com/Uninett/nav/pull/2657))
- 5 minute and 15 minute load average values will now be collected correctly for Juniper devices ([#2671](https://github.com/Uninett/nav/issues/2671), [#2672](https://github.com/Uninett/nav/pull/2672))
- Fix cabling API, which broke due to internal refactorings ([#2621](https://github.com/Uninett/nav/pull/2621))
- Only install NAV's custom `epollreactor2` in ipdevpoll if running on Linux ([#2503](https://github.com/Uninett/nav/issues/2503), [#2604](https://github.com/Uninett/nav/pull/2604))
- Stops ipdevpoll from crashing on BSDs.

Expand Down
7 changes: 7 additions & 0 deletions python/nav/alertengine/dispatchers/slack_dispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import json
import time

from django.core.validators import URLValidator
from django.core.exceptions import ValidationError
from urllib.request import Request, urlopen
from urllib.error import HTTPError

Expand Down Expand Up @@ -86,4 +88,9 @@ def _is_still_backing_off_for(self, address):

@staticmethod
def is_valid_address(address):
validator = URLValidator()
try:
validator(address)
except ValidationError:
return False
return True
6 changes: 3 additions & 3 deletions python/nav/asyncdns.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class Resolver(object):

def __init__(self):
self._resolvers = cycle(
[client.Resolver('/etc/resolv.conf') for _i in range(3)]
[client.Resolver("/etc/resolv.conf") for _i in range(3)]
)
self.results = defaultdict(list)
self._finished = False
Expand Down Expand Up @@ -136,7 +136,7 @@ def lookup(self, name):
"""Returns a deferred object with all records related to hostname"""

if isinstance(name, str):
name = name.encode('idna')
name = name.encode("idna")

resolver = next(self._resolvers)
return [resolver.lookupAddress(name), resolver.lookupIPV6Address(name)]
Expand All @@ -148,7 +148,7 @@ def _extract_records(result, name):

for record_list in result:
for record in record_list:
if str(record.name) == name:
if str(record.name).lower() == name.lower():
if record.type == dns.A:
address_list.append(
socket.inet_ntop(socket.AF_INET, record.payload.address)
Expand Down
10 changes: 5 additions & 5 deletions python/nav/ipdevpoll/shadows/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from nav.ipdevpoll.storage import MetaShadow, Shadow, shadowify
from nav.ipdevpoll import descrparsers
from nav.ipdevpoll import utils
from nav.oids import get_enterprise_id

from .netbox import Netbox
from .interface import Interface, InterfaceStack, InterfaceAggregate
Expand Down Expand Up @@ -73,11 +74,10 @@ def get_enterprise_id(self):
otherwise.
"""
prefix = u"1.3.6.1.4.1."
if self.sysobjectid.startswith(prefix):
specific = self.sysobjectid[len(prefix) :]
enterprise = specific.split('.')[0]
return int(enterprise)
try:
return get_enterprise_id(self.sysobjectid)
except ValueError:
return None


class NetboxInfo(Shadow):
Expand Down
4 changes: 2 additions & 2 deletions python/nav/mibs/juniper_mib.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
OPERATING_DESCR = "jnxOperatingDescr"
OPERATING_CPU = "jnxOperatingCPU"
LOAD_AVG_1MIN = "jnxOperating1MinLoadAvg"
LOAD_AVG_5MIN = "jnxOperating1MinLoadAvg"
LOAD_AVG_15MIN = "jnxOperating1MinLoadAvg"
LOAD_AVG_5MIN = "jnxOperating5MinLoadAvg"
LOAD_AVG_15MIN = "jnxOperating15MinLoadAvg"
OPERATING_MEM = "jnxOperatingMemory"
OPERATING_BUF = "jnxOperatingBuffer"

Expand Down
10 changes: 5 additions & 5 deletions python/nav/models/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
from nav.models.fields import DateTimeInfinityField, VarcharField, PointField
from nav.models.fields import CIDRField
import nav.models.event
from nav.oids import get_enterprise_id


_logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -1344,11 +1345,10 @@ def get_enterprise_id(self):
specific to the vendor.
"""
prefix = u"1.3.6.1.4.1."
if self.sysobjectid.startswith(prefix):
specific = self.sysobjectid[len(prefix) :]
enterprise = specific.split('.')[0]
return int(enterprise)
try:
return get_enterprise_id(self.sysobjectid)
except ValueError:
return None


#######################################################################
Expand Down
2 changes: 1 addition & 1 deletion python/nav/models/profiles.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ def send(self, alert, subscription):
return True

if self.type.is_blacklisted():
_logger.warning(
_logger.debug(
'Not sending alert %s to %s as handler %s is blacklisted: %s',
alert.id,
self.address,
Expand Down
4 changes: 4 additions & 0 deletions python/nav/web/alertprofiles/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from crispy_forms_foundation.layout import Layout, Row, Column, Field, Submit, HTML

from nav.alertengine.dispatchers.email_dispatcher import Email
from nav.alertengine.dispatchers.slack_dispatcher import Slack
from nav.alertengine.dispatchers.sms_dispatcher import Sms

from nav.models.profiles import MatchField, Filter, Expression, FilterGroup
Expand Down Expand Up @@ -118,6 +119,9 @@ def clean(self):
elif type_.handler == 'email':
if not Email.is_valid_address(address):
error = 'Not a valid email address.'
elif type_.handler == 'slack':
if not Slack.is_valid_address(address):
error = 'Not a valid absolute url.'

if error:
self._errors['address'] = self.error_class([error])
Expand Down
10 changes: 9 additions & 1 deletion python/nav/web/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,6 @@ class NetboxViewSet(LoggerMixin, NAVAPIMixin, viewsets.ModelViewSet):
queryset = manage.Netbox.objects.all().prefetch_related("info_set")
serializer_class = serializers.NetboxSerializer
filterset_fields = (
'ip',
'sysname',
'room',
'organization',
Expand Down Expand Up @@ -442,6 +441,15 @@ def get_queryset(self):
qs = qs.filter(type__name__istartswith=value[1:])
else:
qs = qs.filter(type__name=value)
ip = params.get('ip', None)
if ip:
try:
addr = IP(ip)
except ValueError:
raise IPParseError
oper = '=' if addr.len() == 1 else '<<'
expr = "netbox.ip {} '{}'".format(oper, addr)
qs = qs.extra(where=[expr])

return qs

Expand Down
16 changes: 16 additions & 0 deletions tests/integration/api_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,22 @@ def test_update_group_on_org(db, api_client, token):
# Netbox specific tests


def test_filter_netbox_by_invalid_ip(db, api_client, token):
create_token_endpoint(token, 'netbox')
response = api_client.get('{}?ip=10'.format(ENDPOINTS['netbox']))
print(response)
assert response.status_code == 200


def test_filter_netbox_by_invalid_ip_that_cannot_be_converted_throws_error(
db, api_client, token
):
create_token_endpoint(token, 'netbox')
response = api_client.get('{}?ip=x'.format(ENDPOINTS['netbox']))
print(response)
assert response.status_code == 400


def test_update_netbox(db, api_client, token):
endpoint = 'netbox'
create_token_endpoint(token, endpoint)
Expand Down
43 changes: 43 additions & 0 deletions tests/integration/models/netbox_type_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
from nav.enterprise.ids import VENDOR_ID_H3C
from nav.models.manage import NetboxType


class TestNetboxType(object):
def test_get_enterprise_id_returns_correctly_for_sysobjectid_with_leading_period(
self,
):
sysobjectid = ".1.3.6.1.4.1.25506.1.517"
netbox_type = NetboxType(
vendor_id="hp",
name="A5120-48P-EI-2",
description="HP Procurve A5120 V2",
sysobjectid=sysobjectid,
)

assert netbox_type.get_enterprise_id() == VENDOR_ID_H3C

def test_get_enterprise_id_returns_correctly_for_sysobjectid_without_leading_period(
self,
):
sysobjectid = "1.3.6.1.4.1.25506.1.517"
netbox_type = NetboxType(
vendor_id="hp",
name="A5120-48P-EI-2",
description="HP Procurve A5120 V2",
sysobjectid=sysobjectid,
)

assert netbox_type.get_enterprise_id() == VENDOR_ID_H3C

def test_get_enterprise_id_returns_none_for_sysobjectid_of_wrong_format(
self,
):
sysobjectid = "abc"
netbox_type = NetboxType(
vendor_id="hp",
name="A5120-48P-EI-2",
description="HP Procurve A5120 V2",
sysobjectid=sysobjectid,
)

assert netbox_type.get_enterprise_id() is None
65 changes: 63 additions & 2 deletions tests/integration/web/alertprofiles_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,13 @@
from django.test.client import RequestFactory
from django.urls import reverse
from nav.compatibility import smart_str

from nav.models.profiles import AlertProfile, Account, AlertPreference
from nav.models.profiles import (
Account,
AlertAddress,
AlertPreference,
AlertProfile,
AlertSender,
)
from nav.web.alertprofiles.views import set_active_profile


Expand Down Expand Up @@ -171,6 +176,62 @@ def test_set_accountgroup_permissions_should_not_crash(db, client):
assert response.status_code == 200


def test_alertprofiles_add_slack_address_with_valid_url_should_succeed(client):
"""Tests that a slack address with a valid absolute url can be added"""
valid_url = "https://example.com"
slack = AlertSender.objects.get(name=AlertSender.SLACK)
url = reverse("alertprofiles-address-save")
data = {
"address": valid_url,
"type": slack.pk,
}
response = client.post(url, data=data, follow=True)
assert response.status_code == 200
assert AlertAddress.objects.filter(
type=slack,
address=valid_url,
).exists()
assert f"Saved address {valid_url}" in smart_str(response.content)


def test_alertprofiles_add_slack_address_with_a_valid_but_not_absolute_url_should_fail(
client,
):
"""Tests that a slack address with a not valid url cannot be added"""
non_absolute_url = "www.example.com"
slack = AlertSender.objects.get(name=AlertSender.SLACK)
url = reverse("alertprofiles-address-save")
data = {
"address": non_absolute_url,
"type": slack.pk,
}
response = client.post(url, data=data, follow=True)
assert response.status_code == 200
assert not AlertAddress.objects.filter(
type=slack,
address=non_absolute_url,
).exists()
assert "Not a valid absolute url." in smart_str(response.content)


def test_alertprofiles_add_slack_address_with_non_valid_url_should_fail(client):
"""Tests that a slack address with a not valid url cannot be added"""
invalid_url = "abc"
slack = AlertSender.objects.get(name=AlertSender.SLACK)
url = reverse("alertprofiles-address-save")
data = {
"address": invalid_url,
"type": slack.pk,
}
response = client.post(url, data=data, follow=True)
assert response.status_code == 200
assert not AlertAddress.objects.filter(
type=slack,
address=invalid_url,
).exists()
assert "Not a valid absolute url." in smart_str(response.content)


#
# fixtures and helpers
#
Expand Down
42 changes: 42 additions & 0 deletions tests/unittests/asyncdns_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
from mock import Mock
from twisted.names import dns

from nav.asyncdns import ForwardResolver

BUICK_NAME = "buick.lab.uninett.no"


def test_forward_lookup_should_work_with_ipv4_results():
record = Mock()
record.name = BUICK_NAME
record.type = dns.A
record.payload.address = b"\xac\x00\x00\x01"
result = [[record]]
result_name, address_list = ForwardResolver._extract_records(result, BUICK_NAME)

assert result_name == BUICK_NAME
assert address_list == ["172.0.0.1"]


def test_forward_lookup_should_work_with_ipv6_results():
record = Mock()
record.name = BUICK_NAME
record.type = dns.AAAA
record.payload.address = b" \x01\r\xb833DDUUffww\x88\x88"
result = [[record]]
result_name, address_list = ForwardResolver._extract_records(result, BUICK_NAME)

assert result_name == BUICK_NAME
assert address_list == ["2001:db8:3333:4444:5555:6666:7777:8888"]


def test_forward_lookup_should_work_with_different_case_name_results():
record = Mock()
record.name = "bUiCk.LaB.uNiNeTt.No"
record.type = dns.A
record.payload.address = b"\x9e&\x98\xa2"
result = [[record]]
result_name, address_list = ForwardResolver._extract_records(result, BUICK_NAME)

assert result_name == BUICK_NAME
assert address_list == ["158.38.152.162"]

0 comments on commit e2b92de

Please sign in to comment.