mirror of
https://github.com/containers/ansible-podman-collections.git
synced 2026-06-29 15:39:22 +00:00
* Fix podman_network not idempotent with IPv6 addresses, fix #1041 Normalize IPv6 addresses and subnets before comparing in diff methods using Python's ipaddress module. Different notations of the same IPv6 address (e.g. "::1" vs "0:1") are now correctly recognized as equal, preventing unnecessary network recreation. - Guard _normalize_ip/_normalize_subnet with HAS_IP_ADDRESS_MODULE check - Normalize user-supplied route strings in diffparam_route - Keep _lease_range_to_str as @staticmethod - Catch TypeError in _normalize_ip/_normalize_subnet for None inputs - Use .get("gateway") with filter in multi-gateway join to avoid KeyError - Fix implicit string concatenation lint warnings --------- Signed-off-by: Sagi Shnaidman <sshnaidm@redhat.com>
This commit is contained in:
Sergey
GitHub
parent
78ee8ca20e
commit
efa337c536
3 changed files with 249 additions and 15 deletions
|
|
@ -483,7 +483,9 @@ class PodmanNetworkDiff:
|
|||
@staticmethod
|
||||
def _lease_range_to_str(lease_range):
|
||||
"""Convert lease_range dict to normalized start-end string."""
|
||||
return f'{lease_range["start_ip"]}-{lease_range["end_ip"]}'
|
||||
start = PodmanNetworkDiff._normalize_ip(lease_range["start_ip"])
|
||||
end = PodmanNetworkDiff._normalize_ip(lease_range["end_ip"])
|
||||
return f"{start}-{end}"
|
||||
|
||||
@staticmethod
|
||||
def _ip_range_to_str(ip_range_cidr):
|
||||
|
|
@ -497,6 +499,26 @@ class PodmanNetworkDiff:
|
|||
end = net.broadcast_address
|
||||
return f"{start}-{end}"
|
||||
|
||||
@staticmethod
|
||||
def _normalize_ip(addr):
|
||||
"""Normalize an IP address string to its compressed canonical form."""
|
||||
if not HAS_IP_ADDRESS_MODULE:
|
||||
return addr
|
||||
try:
|
||||
return str(ipaddress.ip_address(addr))
|
||||
except (ValueError, TypeError):
|
||||
return addr
|
||||
|
||||
@staticmethod
|
||||
def _normalize_subnet(subnet):
|
||||
"""Normalize a subnet string to its compressed canonical form."""
|
||||
if not HAS_IP_ADDRESS_MODULE:
|
||||
return subnet
|
||||
try:
|
||||
return str(ipaddress.ip_network(subnet, strict=False))
|
||||
except (ValueError, TypeError):
|
||||
return subnet
|
||||
|
||||
def diffparam_disable_dns(self):
|
||||
# For v3 it's impossible to find out DNS settings.
|
||||
if LooseVersion(self.version) >= LooseVersion("4.0.0"):
|
||||
|
|
@ -534,6 +556,10 @@ class PodmanNetworkDiff:
|
|||
after = before
|
||||
if self.params["gateway"] is not None:
|
||||
after = self.params["gateway"]
|
||||
if before:
|
||||
before = self._normalize_ip(before)
|
||||
if after:
|
||||
after = self._normalize_ip(after)
|
||||
return self._diff_update_and_compare("gateway", before, after)
|
||||
else:
|
||||
before_subs = self.info.get("subnets")
|
||||
|
|
@ -542,12 +568,17 @@ class PodmanNetworkDiff:
|
|||
before = None
|
||||
if before_subs:
|
||||
if len(before_subs) > 1 and after:
|
||||
return self._diff_update_and_compare(
|
||||
"gateway", ",".join([i["gateway"] for i in before_subs]), after
|
||||
before_gws = ",".join(
|
||||
[self._normalize_ip(i.get("gateway")) for i in before_subs if i.get("gateway")]
|
||||
)
|
||||
return self._diff_update_and_compare("gateway", before_gws, self._normalize_ip(after))
|
||||
before = [i.get("gateway") for i in before_subs][0]
|
||||
if not after:
|
||||
after = before
|
||||
if before:
|
||||
before = self._normalize_ip(before)
|
||||
if after:
|
||||
after = self._normalize_ip(after)
|
||||
return self._diff_update_and_compare("gateway", before, after)
|
||||
|
||||
def diffparam_internal(self):
|
||||
|
|
@ -564,9 +595,7 @@ class PodmanNetworkDiff:
|
|||
|
||||
def diffparam_ip_range(self):
|
||||
if not HAS_IP_ADDRESS_MODULE:
|
||||
self.module.warn(
|
||||
"Python 'ipaddress' module is not available. "
|
||||
"Skipping ip_range idempotency check.")
|
||||
self.module.warn("Python 'ipaddress' module is not available. Skipping ip_range idempotency check.")
|
||||
return False
|
||||
before = ""
|
||||
after = self.params["ip_range"] or ""
|
||||
|
|
@ -594,7 +623,9 @@ class PodmanNetworkDiff:
|
|||
if before_subs:
|
||||
before_parts = []
|
||||
for i in before_subs:
|
||||
parts = [i["subnet"], i.get("gateway") or ""]
|
||||
subnet = self._normalize_subnet(i["subnet"])
|
||||
gateway = self._normalize_ip(i["gateway"]) if i.get("gateway") else ""
|
||||
parts = [subnet, gateway]
|
||||
lr = i.get("lease_range")
|
||||
if lr:
|
||||
parts.append(self._lease_range_to_str(lr))
|
||||
|
|
@ -604,14 +635,17 @@ class PodmanNetworkDiff:
|
|||
before = ""
|
||||
after_parts = []
|
||||
for i in after:
|
||||
parts = [i["subnet"], i.get("gateway") or ""]
|
||||
subnet = self._normalize_subnet(i["subnet"])
|
||||
gateway = self._normalize_ip(i["gateway"]) if i.get("gateway") else ""
|
||||
parts = [subnet, gateway]
|
||||
if i.get("ip_range"):
|
||||
if HAS_IP_ADDRESS_MODULE:
|
||||
parts.append(self._ip_range_to_str(i["ip_range"]))
|
||||
else:
|
||||
self.module.warn(
|
||||
"Python 'ipaddress' module is not available. "
|
||||
"Skipping ip_range idempotency check in net_config.")
|
||||
"Skipping ip_range idempotency check in net_config."
|
||||
)
|
||||
after_parts.append(",".join(parts))
|
||||
after = ":".join(sorted(after_parts))
|
||||
return self._diff_update_and_compare("net_config", before, after)
|
||||
|
|
@ -619,10 +653,25 @@ class PodmanNetworkDiff:
|
|||
def diffparam_route(self):
|
||||
routes = self.info.get("routes", [])
|
||||
if routes:
|
||||
before = [",".join([r["destination"], r["gateway"], str(r.get("metric", ""))]).rstrip(",") for r in routes]
|
||||
before = [
|
||||
",".join(
|
||||
[
|
||||
self._normalize_subnet(r["destination"]),
|
||||
self._normalize_ip(r["gateway"]),
|
||||
str(r.get("metric", "")),
|
||||
]
|
||||
).rstrip(",")
|
||||
for r in routes
|
||||
]
|
||||
else:
|
||||
before = []
|
||||
after = self.params["route"] or []
|
||||
after = []
|
||||
for r in self.params["route"] or []:
|
||||
parts = r.split(",")
|
||||
if len(parts) >= 2:
|
||||
parts[0] = self._normalize_subnet(parts[0])
|
||||
parts[1] = self._normalize_ip(parts[1])
|
||||
after.append(",".join(parts))
|
||||
return self._diff_update_and_compare("route", sorted(before), sorted(after))
|
||||
|
||||
def diffparam_subnet(self):
|
||||
|
|
@ -635,8 +684,10 @@ class PodmanNetworkDiff:
|
|||
after = before
|
||||
if self.params["subnet"] is not None:
|
||||
after = self.params["subnet"]
|
||||
if HAS_IP_ADDRESS_MODULE:
|
||||
after = ipaddress.ip_network(after).compressed
|
||||
if before:
|
||||
before = self._normalize_subnet(before)
|
||||
if after:
|
||||
after = self._normalize_subnet(after)
|
||||
return self._diff_update_and_compare("subnet", before, after)
|
||||
else:
|
||||
if self.params["ipv6"] is not None:
|
||||
|
|
@ -649,8 +700,13 @@ class PodmanNetworkDiff:
|
|||
before = self.info.get("subnets")
|
||||
if before:
|
||||
if len(before) > 1 and after:
|
||||
return self._diff_update_and_compare("subnet", ",".join([i["subnet"] for i in before]), after)
|
||||
before_subs = ",".join([self._normalize_subnet(i["subnet"]) for i in before])
|
||||
return self._diff_update_and_compare("subnet", before_subs, self._normalize_subnet(after))
|
||||
before = [i["subnet"] for i in before][0]
|
||||
if before:
|
||||
before = self._normalize_subnet(before)
|
||||
if after:
|
||||
after = self._normalize_subnet(after)
|
||||
return self._diff_update_and_compare("subnet", before, after)
|
||||
|
||||
def diffparam_macvlan(self):
|
||||
|
|
@ -859,7 +915,7 @@ class PodmanNetworkManager:
|
|||
}
|
||||
process_action = states_map[self.state]
|
||||
process_action()
|
||||
self.module.fail_json(msg="Unexpected logic error happened, " "please contact maintainers ASAP!")
|
||||
self.module.fail_json(msg="Unexpected logic error happened, please contact maintainers ASAP!")
|
||||
|
||||
def make_present(self):
|
||||
"""Run actions if desired state is 'started'."""
|
||||
|
|
|
|||
|
|
@ -832,6 +832,9 @@
|
|||
- name: Test ip_range diff
|
||||
include_tasks: test_ip_range_diff.yml
|
||||
|
||||
- name: Test IPv6 normalization idempotency (issue 1041)
|
||||
include_tasks: test_issue_1041.yml
|
||||
|
||||
always:
|
||||
|
||||
- name: Cleanup
|
||||
|
|
|
|||
|
|
@ -0,0 +1,175 @@
|
|||
- name: Test IPv6 address normalization idempotency (issue 1041)
|
||||
block:
|
||||
|
||||
- name: Make sure network doesn't exist
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
state: absent
|
||||
|
||||
# Test 1: net_config with compressed IPv6 gateway (the reported bug)
|
||||
- name: Create network with IPv6 gateway using compressed notation
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
net_config:
|
||||
- subnet: "fcb8:b02:5b0e:bc43:aa32:8d2c::/96"
|
||||
gateway: "fcb8:b02:5b0e:bc43:aa32:8d2c::1"
|
||||
- subnet: "10.89.55.0/24"
|
||||
gateway: "10.89.55.1"
|
||||
register: ipv6_gw_create
|
||||
|
||||
- name: Check network was created
|
||||
assert:
|
||||
that:
|
||||
- ipv6_gw_create is changed
|
||||
|
||||
- name: Re-run with same compressed IPv6 gateway - idempotency
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
net_config:
|
||||
- subnet: "fcb8:b02:5b0e:bc43:aa32:8d2c::/96"
|
||||
gateway: "fcb8:b02:5b0e:bc43:aa32:8d2c::1"
|
||||
- subnet: "10.89.55.0/24"
|
||||
gateway: "10.89.55.1"
|
||||
register: ipv6_gw_same
|
||||
diff: true
|
||||
|
||||
- name: Check idempotency with compressed IPv6 gateway
|
||||
assert:
|
||||
that:
|
||||
- ipv6_gw_same is not changed
|
||||
|
||||
- name: Re-run with expanded IPv6 gateway - must also be idempotent
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
net_config:
|
||||
- subnet: "fcb8:b02:5b0e:bc43:aa32:8d2c::/96"
|
||||
gateway: "fcb8:b02:5b0e:bc43:aa32:8d2c:0:1"
|
||||
- subnet: "10.89.55.0/24"
|
||||
gateway: "10.89.55.1"
|
||||
register: ipv6_gw_expanded
|
||||
diff: true
|
||||
|
||||
- name: Check idempotency with expanded IPv6 gateway
|
||||
assert:
|
||||
that:
|
||||
- ipv6_gw_expanded is not changed
|
||||
|
||||
- name: Re-run with fully expanded IPv6 gateway and subnet
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
net_config:
|
||||
- subnet: "fcb8:0b02:5b0e:bc43:aa32:8d2c:0000:0000/96"
|
||||
gateway: "fcb8:0b02:5b0e:bc43:aa32:8d2c:0000:0001"
|
||||
- subnet: "10.89.55.0/24"
|
||||
gateway: "10.89.55.1"
|
||||
register: ipv6_gw_full
|
||||
diff: true
|
||||
|
||||
- name: Check idempotency with fully expanded IPv6
|
||||
assert:
|
||||
that:
|
||||
- ipv6_gw_full is not changed
|
||||
|
||||
# Test 2: standalone subnet/gateway params with IPv6
|
||||
- name: Delete network
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
state: absent
|
||||
|
||||
- name: Create IPv6 network with standalone subnet/gateway
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
subnet: "fd00:dead:beef::/48"
|
||||
gateway: "fd00:dead:beef::1"
|
||||
register: ipv6_standalone_create
|
||||
|
||||
- name: Check network was created
|
||||
assert:
|
||||
that:
|
||||
- ipv6_standalone_create is changed
|
||||
|
||||
- name: Re-run with same compressed notation - idempotency
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
subnet: "fd00:dead:beef::/48"
|
||||
gateway: "fd00:dead:beef::1"
|
||||
register: ipv6_standalone_same
|
||||
diff: true
|
||||
|
||||
- name: Check standalone IPv6 idempotency
|
||||
assert:
|
||||
that:
|
||||
- ipv6_standalone_same is not changed
|
||||
|
||||
- name: Re-run with expanded notation - must be idempotent
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
subnet: "fd00:dead:beef:0000::/48"
|
||||
gateway: "fd00:dead:beef:0000:0000:0000:0000:0001"
|
||||
register: ipv6_standalone_expanded
|
||||
diff: true
|
||||
|
||||
- name: Check standalone IPv6 idempotency with expanded form
|
||||
assert:
|
||||
that:
|
||||
- ipv6_standalone_expanded is not changed
|
||||
|
||||
# Test 3: dual-stack net_config with mixed IPv4 and IPv6
|
||||
- name: Delete network
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
state: absent
|
||||
|
||||
- name: Create dual-stack network
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
ipv6: true
|
||||
net_config:
|
||||
- subnet: "172.31.55.0/24"
|
||||
gateway: "172.31.55.1"
|
||||
- subnet: "fd99:abcd:ef01::/64"
|
||||
gateway: "fd99:abcd:ef01::1"
|
||||
register: dual_create
|
||||
|
||||
- name: Check dual-stack was created
|
||||
assert:
|
||||
that:
|
||||
- dual_create is changed
|
||||
|
||||
- name: Re-run dual-stack with expanded IPv6 - idempotency
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
ipv6: true
|
||||
net_config:
|
||||
- subnet: "172.31.55.0/24"
|
||||
gateway: "172.31.55.1"
|
||||
- subnet: "fd99:abcd:ef01:0000::/64"
|
||||
gateway: "fd99:abcd:ef01:0:0:0:0:1"
|
||||
register: dual_expanded
|
||||
diff: true
|
||||
|
||||
- name: Check dual-stack idempotency with expanded IPv6
|
||||
assert:
|
||||
that:
|
||||
- dual_expanded is not changed
|
||||
|
||||
always:
|
||||
|
||||
- name: Cleanup network
|
||||
containers.podman.podman_network:
|
||||
executable: "{{ test_executable | default('podman') }}"
|
||||
name: "{{ network_name }}"
|
||||
state: absent
|
||||
ignore_errors: true
|
||||
Loading…
Add table
Add a link
Reference in a new issue