Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure default_ip_version is set correctly. Fix add_ip_address logic. #427

Merged
merged 4 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ v5.3.1
Minor Changes
-------------
- (#422) Fixed `admin_permission` module to properly convert list of groups to UUIDs
- (#427) Fixed setting of `default_ip_version` option. Fixed logic in `add_ip_address` that sets Ansible `host` values

v5.3.0
======
Expand Down
1 change: 1 addition & 0 deletions changelogs/changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -481,3 +481,4 @@ releases:
changes:
minor_changes:
- (#422) Fixed `admin_permission` module to properly convert list of groups to UUIDs
- (#427) Fixed setting of `default_ip_version` option. Fixed logic in `add_ip_address` that sets Ansible `host` values
10 changes: 7 additions & 3 deletions plugins/inventory/gql_inventory.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@
"role": "name",
"location": "id",
}
DEFAULT_IP_VERSION_CHOICES = ["IPv4", "ipv4", "IPv6", "ipv6"]


class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
Expand Down Expand Up @@ -278,15 +279,15 @@ def add_ip_address(self, device, default_ip_version="ipv4"):
# Check to see what the primary IP host addition is, first case is IPv4, which is the default
order_of_preference = ["primary_ip4"]

# if default_ip_version is IPv4, prepend, else add to the end
# if default_ip_version is IPv6, prepend, else add to the end
if default_ip_version.lower() == "ipv6":
order_of_preference.insert(0, "primary_ip6")
else:
order_of_preference.append("primary_ip6")

# Check of the address types in the order preference and if it find the first one, add that primary IP to the host
# Check of the address types in the order of preference and if it finds the first one, add that primary IP to the host
for address_type in order_of_preference:
if address_type in device and device[address_type].get("host"):
if address_type in device and device[address_type] and device[address_type].get("host"):
self.add_variable(device["name"], device[address_type]["host"], "ansible_host")
return

Expand Down Expand Up @@ -457,6 +458,9 @@ def parse(self, inventory, loader, path, cache=True):
if token:
self.headers.update({"Authorization": "Token %s" % token})

self.default_ip_version = self.get_option("default_ip_version")
if self.default_ip_version not in DEFAULT_IP_VERSION_CHOICES:
raise AnsibleError(f"Invalid choice for default_ip_version: {self.default_ip_version}")
self.gql_query = self.get_option("query")
self.group_by = self.get_option("group_by")
self.follow_redirects = self.get_option("follow_redirects")
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/targets/inventory/files/test_2.3-3.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "172.16.0.0/12",
"display": "172.16.0.0/12: Global",
"ip_version": 4,
"namespace": {
"custom_fields": {},
Expand Down Expand Up @@ -274,7 +274,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "172.16.0.0/12",
"display": "172.16.0.0/12: Global",
"ip_version": 4,
"namespace": {
"custom_fields": {},
Expand Down Expand Up @@ -742,7 +742,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "172.16.0.0/12",
"display": "172.16.0.0/12: Global",
"ip_version": 4,
"namespace": {
"custom_fields": {},
Expand Down Expand Up @@ -903,7 +903,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "2001::/64",
"display": "2001::/64: Global",
"ip_version": 6,
"namespace": {
"custom_fields": {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "172.16.0.0/12",
"display": "172.16.0.0/12: Global",
"ip_version": 4,
"namespace": {
"custom_fields": {},
Expand Down Expand Up @@ -270,7 +270,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "172.16.0.0/12",
"display": "172.16.0.0/12: Global",
"ip_version": 4,
"namespace": {
"custom_fields": {},
Expand Down Expand Up @@ -728,7 +728,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "172.16.0.0/12",
"display": "172.16.0.0/12: Global",
"ip_version": 4,
"namespace": {
"custom_fields": {},
Expand Down Expand Up @@ -889,7 +889,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "2001::/64",
"display": "2001::/64: Global",
"ip_version": 6,
"namespace": {
"custom_fields": {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "172.16.0.0/12",
"display": "172.16.0.0/12: Global",
"ip_version": 4,
"namespace": {
"custom_fields": {},
Expand Down Expand Up @@ -294,7 +294,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "172.16.0.0/12",
"display": "172.16.0.0/12: Global",
"ip_version": 4,
"namespace": {
"custom_fields": {},
Expand Down Expand Up @@ -791,7 +791,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "172.16.0.0/12",
"display": "172.16.0.0/12: Global",
"ip_version": 4,
"namespace": {
"custom_fields": {},
Expand Down Expand Up @@ -952,7 +952,7 @@
"custom_fields": {},
"date_allocated": null,
"description": "",
"display": "2001::/64",
"display": "2001::/64: Global",
"ip_version": 6,
"namespace": {
"custom_fields": {},
Expand Down
20 changes: 20 additions & 0 deletions tests/unit/inventory/test_graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,26 @@ def test_add_ip_address_no_ipv6(inventory_fixture, device_data):
assert mydevice_host.vars.get("ansible_host") == "10.10.10.10"


def test_add_ip_address_ipv4_none(inventory_fixture, device_data):
"""Regression bug test for issue #426."""
# Set the primary_ip4 to None
device_data["primary_ip4"] = None
try:
inventory_fixture.add_ip_address(device_data, default_ip_version="ipv4")
except AttributeError:
pytest.fail("Hit regression bug, see issue #426.")


def test_add_ip_address_ipv6_none(inventory_fixture, device_data):
"""Regression bug test for issue #426."""
# Set the primary_ip6 to None
device_data["primary_ip6"] = None
try:
inventory_fixture.add_ip_address(device_data, default_ip_version="ipv6")
except AttributeError:
pytest.fail("Hit regression bug, see issue #426.")


def test_ansible_platform(inventory_fixture, device_data):
inventory_fixture.group_by = ["location"]
inventory_fixture.create_groups(device_data)
Expand Down