From 3501a0ff0bb8db0b500f77c665cb8a0bc3762103 Mon Sep 17 00:00:00 2001 From: pederhan Date: Thu, 21 Nov 2024 10:25:27 +0100 Subject: [PATCH 1/8] Fix `host info` for host with multiple IPs on same MAC --- mreg_cli/utilities/api.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mreg_cli/utilities/api.py b/mreg_cli/utilities/api.py index d32ce7fc..0eb81278 100644 --- a/mreg_cli/utilities/api.py +++ b/mreg_cli/utilities/api.py @@ -17,10 +17,11 @@ import requests from prompt_toolkit import prompt -from pydantic import BaseModel, TypeAdapter, field_validator +from pydantic import TypeAdapter, field_validator from requests import Response from mreg_cli.__about__ import __version__ +from mreg_cli.base_model import BaseModel from mreg_cli.config import MregCliConfig from mreg_cli.exceptions import ( APINotOk, @@ -540,7 +541,7 @@ def get_list_generic( if expect_one_result: if len(ret) == 0: return {} - if len(ret) != 1: + if len(ret) > 1 and not all(ret[0] == x for x in ret): raise MultipleEntititesFound(f"Expected exactly one result, got {len(ret)}.") return ret[0] return ret From 10d8201d2e7939a742894baf835f5d2a6603de3c Mon Sep 17 00:00:00 2001 From: pederhan Date: Thu, 21 Nov 2024 10:28:18 +0100 Subject: [PATCH 2/8] Remove WIP stuff oops --- mreg_cli/utilities/api.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mreg_cli/utilities/api.py b/mreg_cli/utilities/api.py index 0eb81278..e310da95 100644 --- a/mreg_cli/utilities/api.py +++ b/mreg_cli/utilities/api.py @@ -17,11 +17,10 @@ import requests from prompt_toolkit import prompt -from pydantic import TypeAdapter, field_validator +from pydantic import BaseModel, TypeAdapter, field_validator from requests import Response from mreg_cli.__about__ import __version__ -from mreg_cli.base_model import BaseModel from mreg_cli.config import MregCliConfig from mreg_cli.exceptions import ( APINotOk, From 579a260e533539ee3629520829ca0dca7219e3ce Mon Sep 17 00:00:00 2001 From: pederhan Date: Thu, 21 Nov 2024 10:44:55 +0100 Subject: [PATCH 3/8] Add `host info` test for host with ipv4 and ipv6 --- ci/testsuite | 1 + ci/testsuite-result.json | 121 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) diff --git a/ci/testsuite b/ci/testsuite index 8987491c..658e3f20 100644 --- a/ci/testsuite +++ b/ci/testsuite @@ -203,6 +203,7 @@ dhcp assoc foo aa:bb:cc:dd:ee:ff # should fail, the host has two IPs of differen host aaaa_remove foo 2001:db9::5 host aaaa_add foo 2001:db8::5 -f dhcp assoc foo aa:bb:cc:dd:ee:ff # should work, the host now has two IPs of different types on the same VLAN. +host info foo # should work and show both IPs host remove foo -f network remove 10.0.0.0/24 -f network remove 2001:db8::/64 -f diff --git a/ci/testsuite-result.json b/ci/testsuite-result.json index b13b78a7..25cb30b0 100644 --- a/ci/testsuite-result.json +++ b/ci/testsuite-result.json @@ -20022,6 +20022,127 @@ ], "time": null }, + { + "command": "host info foo", + "command_filter": null, + "command_filter_negate": false, + "command_issued": "host info foo # should work and show both IPs", + "ok": [], + "warning": [], + "error": [], + "output": [ + "Name: foo.example.org", + "Contact: ", + "A_Records IP MAC", + " 10.0.0.5 aa:bb:cc:dd:ee:ff", + "AAAA_Records IP MAC", + " 2001:db8::5 ", + "TTL: (Default)", + "TXT: v=spf1 -all", + "Created: Thu Nov 21 10:34:55 2024", + "Updated: Thu Nov 21 10:34:55 2024" + ], + "api_requests": [ + { + "method": "GET", + "url": "/api/v1/hosts/foo.example.org", + "data": {}, + "status": 200, + "response": { + "ipaddresses": [ + { + "macaddress": "aa:bb:cc:dd:ee:ff", + "ipaddress": "10.0.0.5", + "host": 13 + }, + { + "macaddress": "", + "ipaddress": "2001:db8::5", + "host": 13 + } + ], + "cnames": [], + "mxs": [], + "txts": [ + { + "txt": "v=spf1 -all", + "host": 13 + } + ], + "ptr_overrides": [], + "hinfo": null, + "loc": null, + "bacnetid": null, + "name": "foo.example.org", + "contact": "", + "ttl": null, + "comment": "", + "zone": 1 + } + }, + { + "method": "GET", + "url": "/api/v1/srvs/?host=13", + "data": {}, + "status": 200, + "response": { + "count": 0, + "next": null, + "previous": null, + "results": [] + } + }, + { + "method": "GET", + "url": "/api/v1/naptrs/?host=13", + "data": {}, + "status": 200, + "response": { + "count": 0, + "next": null, + "previous": null, + "results": [] + } + }, + { + "method": "GET", + "url": "/api/v1/sshfps/?host=13", + "data": {}, + "status": 200, + "response": { + "count": 0, + "next": null, + "previous": null, + "results": [] + } + }, + { + "method": "GET", + "url": "/api/v1/hostpolicy/roles/?hosts=13", + "data": {}, + "status": 200, + "response": { + "count": 0, + "next": null, + "previous": null, + "results": [] + } + }, + { + "method": "GET", + "url": "/api/v1/hostgroups/?hosts=13", + "data": {}, + "status": 200, + "response": { + "count": 0, + "next": null, + "previous": null, + "results": [] + } + } + ], + "time": null + }, { "command": "host remove foo -f", "command_filter": null, From 2082d36d3e97874f30a80c83c5589afb0b6a915e Mon Sep 17 00:00:00 2001 From: pederhan Date: Thu, 21 Nov 2024 10:48:36 +0100 Subject: [PATCH 4/8] Improve exception message --- mreg_cli/utilities/api.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mreg_cli/utilities/api.py b/mreg_cli/utilities/api.py index e310da95..4d4c4a3a 100644 --- a/mreg_cli/utilities/api.py +++ b/mreg_cli/utilities/api.py @@ -541,7 +541,9 @@ def get_list_generic( if len(ret) == 0: return {} if len(ret) > 1 and not all(ret[0] == x for x in ret): - raise MultipleEntititesFound(f"Expected exactly one result, got {len(ret)}.") + raise MultipleEntititesFound( + f"Expected a unique result, got {len(ret)} distinct results." + ) return ret[0] return ret From bd18e3a3108abef6175b496b86465ab5d48b5358 Mon Sep 17 00:00:00 2001 From: pederhan Date: Thu, 21 Nov 2024 10:52:07 +0100 Subject: [PATCH 5/8] Short circuit faster in equality check --- mreg_cli/utilities/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mreg_cli/utilities/api.py b/mreg_cli/utilities/api.py index 4d4c4a3a..a6fe67f7 100644 --- a/mreg_cli/utilities/api.py +++ b/mreg_cli/utilities/api.py @@ -540,7 +540,7 @@ def get_list_generic( if expect_one_result: if len(ret) == 0: return {} - if len(ret) > 1 and not all(ret[0] == x for x in ret): + if len(ret) > 1 and any(ret[0] != x for x in ret): raise MultipleEntititesFound( f"Expected a unique result, got {len(ret)} distinct results." ) From c3fd562fa471ca732a60ad9b14c6eecd6ba8a441 Mon Sep 17 00:00:00 2001 From: pederhan Date: Thu, 21 Nov 2024 11:04:45 +0100 Subject: [PATCH 6/8] Fix tests, exception name, add test for duplicate results --- mreg_cli/api/models.py | 6 +++--- mreg_cli/exceptions.py | 2 +- mreg_cli/utilities/api.py | 4 ++-- tests/utilities/test_api.py | 32 ++++++++++++++++++++++++++++---- 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/mreg_cli/api/models.py b/mreg_cli/api/models.py index 947257c9..ab192040 100644 --- a/mreg_cli/api/models.py +++ b/mreg_cli/api/models.py @@ -38,7 +38,7 @@ InvalidIPv6Address, InvalidNetwork, IPNetworkWarning, - MultipleEntititesFound, + MultipleEntitiesFound, PatchError, UnexpectedDataError, ValidationError, @@ -2185,7 +2185,7 @@ def get_by_host_and_name(cls, host: HostT | int, name: HostT) -> CNAME: raise EntityNotFound(f"CNAME record for {name} not found for {target_hostname}.") if len(results) > 1: - raise MultipleEntititesFound(f"Multiple CNAME records found for {host} with {name}!") + raise MultipleEntitiesFound(f"Multiple CNAME records found for {host} with {name}!") return results[0] @@ -2701,7 +2701,7 @@ def get_by_any_means( return hosts[0] if len(hosts) > 1: - raise MultipleEntititesFound( + raise MultipleEntitiesFound( f"Multiple hosts found with IP address or PTR {identifier}." ) diff --git a/mreg_cli/exceptions.py b/mreg_cli/exceptions.py index ceb45c8d..978225b9 100644 --- a/mreg_cli/exceptions.py +++ b/mreg_cli/exceptions.py @@ -171,7 +171,7 @@ class EntityAlreadyExists(CliWarning): pass -class MultipleEntititesFound(CliWarning): +class MultipleEntitiesFound(CliWarning): """Warning class for multiple entities found.""" pass diff --git a/mreg_cli/utilities/api.py b/mreg_cli/utilities/api.py index a6fe67f7..6e0cc154 100644 --- a/mreg_cli/utilities/api.py +++ b/mreg_cli/utilities/api.py @@ -26,7 +26,7 @@ APINotOk, CliError, LoginFailedError, - MultipleEntititesFound, + MultipleEntitiesFound, TooManyResults, ValidationError, ) @@ -541,7 +541,7 @@ def get_list_generic( if len(ret) == 0: return {} if len(ret) > 1 and any(ret[0] != x for x in ret): - raise MultipleEntititesFound( + raise MultipleEntitiesFound( f"Expected a unique result, got {len(ret)} distinct results." ) return ret[0] diff --git a/tests/utilities/test_api.py b/tests/utilities/test_api.py index e9cd9ed8..20710fd3 100644 --- a/tests/utilities/test_api.py +++ b/tests/utilities/test_api.py @@ -7,7 +7,7 @@ from pytest_httpserver import HTTPServer from werkzeug import Response -from mreg_cli.exceptions import MultipleEntititesFound, ValidationError +from mreg_cli.exceptions import MultipleEntitiesFound, ValidationError from mreg_cli.utilities.api import _strip_none, get_list, get_list_unique # type: ignore @@ -200,7 +200,7 @@ def test_get_list_unique_paginated(httpserver: HTTPServer) -> None: def test_get_list_unique_paginated_too_many_results(httpserver: HTTPServer) -> None: - """Non-paginated response with invalid JSON is an error.""" + """get_list_unique with multiple unique results is an error.""" httpserver.expect_oneshot_request("/foobar").respond_with_json( { "results": [{"foo": "bar"}], @@ -217,9 +217,33 @@ def test_get_list_unique_paginated_too_many_results(httpserver: HTTPServer) -> N "previous": "/foobar", } ) - with pytest.raises(MultipleEntititesFound) as exc_info: + with pytest.raises(MultipleEntitiesFound) as exc_info: get_list_unique("/foobar", params={}) - assert "Expected exactly one result, got 2" in exc_info.exconly() + assert exc_info.exconly() == snapshot( + "mreg_cli.exceptions.MultipleEntitiesFound: Expected a unique result, got 2 distinct results." + ) + + +def test_get_list_unique_paginated_duplicate_result_ok(httpserver: HTTPServer) -> None: + """get_list_unique with _only_ duplicate results is ok.""" + httpserver.expect_oneshot_request("/foobar").respond_with_json( + { + "results": [{"foo": "bar"}], + "count": 1, + "next": "/foobar?page=2", + "previous": None, + } + ) + httpserver.expect_oneshot_request("/foobar", query_string="page=2").respond_with_json( + { + "results": [{"foo": "bar"}], + "count": 1, + "next": None, + "previous": "/foobar", + } + ) + resp = get_list_unique("/foobar", params={}) + assert resp == snapshot({"foo": "bar"}) def test_get_list_unique_paginated_no_result(httpserver: HTTPServer) -> None: From 5ad215806aaa2f13329852fa53c13e6b61bf1cbf Mon Sep 17 00:00:00 2001 From: pederhan Date: Thu, 21 Nov 2024 13:34:14 +0100 Subject: [PATCH 7/8] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9216c76..eef95573 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - REPL command completion not showing descriptions for commands. +- `host info` with MAC address argument not working for hosts with multiple IPs associated with the same MAC address. ## [1.1.0](https://github.com/unioslo/mreg-cli/releases/tag/1.1.0) - 2024-11-19 From 4ceb2aae1759939a4c6439b70424febbec16c618 Mon Sep 17 00:00:00 2001 From: pederhan Date: Thu, 21 Nov 2024 15:17:50 +0100 Subject: [PATCH 8/8] Refactor `Host.get_by_any_means`, fall back on existing `MultipleEntitiesFound` --- ci/testsuite-result.json | 2 +- mreg_cli/api/models.py | 31 +++++++++++++------------------ mreg_cli/exceptions.py | 2 +- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/ci/testsuite-result.json b/ci/testsuite-result.json index 25cb30b0..b176806b 100644 --- a/ci/testsuite-result.json +++ b/ci/testsuite-result.json @@ -25049,7 +25049,7 @@ "api_requests": [ { "method": "GET", - "url": "/api/v1/hosts/?ipaddresses__ipaddress=10.0.0.20&ordering=name", + "url": "/api/v1/hosts/?ipaddresses__ipaddress=10.0.0.20", "data": {}, "status": 200, "response": { diff --git a/mreg_cli/api/models.py b/mreg_cli/api/models.py index ab192040..3b580598 100644 --- a/mreg_cli/api/models.py +++ b/mreg_cli/api/models.py @@ -118,6 +118,7 @@ def parse(cls, value: Any, mode: IPNetMode | None = None) -> IP_AddressT | IP_Ne :param value:The value to parse. :param mode: The mode to validate the input as. :returns: The parsed value as an IP address or network. + :raises IPNetworkWarning: If the value is not an IP address or network. """ ipnet = cls.validate(value) funcmap: dict[IPNetMode, Callable[..., IP_AddressT | IP_NetworkT]] = { @@ -2684,28 +2685,22 @@ def get_by_any_means( try: ptr = False ipaddress.ip_address(identifier) + NetworkOrIP.parse(identifier, mode="ip") - hosts = Host.get_list_by_field( - "ipaddresses__ipaddress", identifier, ordering="name" - ) - - if not hosts: - hosts = Host.get_list_by_field("ptr_overrides__ipaddress", identifier) + host = Host.get_by_field("ipaddresses__ipaddress", identifier) + if not host: + host = Host.get_by_field("ptr_overrides__ipaddress", identifier) ptr = True - if len(hosts) == 1: + if host: if ptr and inform_as_ptr: - OutputManager().add_line( - f"{identifier} is a PTR override for {hosts[0].name}" - ) - return hosts[0] - - if len(hosts) > 1: - raise MultipleEntitiesFound( - f"Multiple hosts found with IP address or PTR {identifier}." - ) - - except ValueError: + OutputManager().add_line(f"{identifier} is a PTR override for {host.name}") + return host + except MultipleEntitiesFound as e: + raise MultipleEntitiesFound( + f"Multiple hosts found with IP address or PTR {identifier}." + ) from e + except ValueError: # invalid IP pass try: diff --git a/mreg_cli/exceptions.py b/mreg_cli/exceptions.py index 978225b9..4d5095d6 100644 --- a/mreg_cli/exceptions.py +++ b/mreg_cli/exceptions.py @@ -195,7 +195,7 @@ class ForceMissing(CliWarning): pass -class IPNetworkWarning(CliWarning): +class IPNetworkWarning(ValueError, CliWarning): """Warning class for IP network/address warnings.""" pass