Skip to content

Fix IP address collection with IPv6 disabled #3190

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.

Sign up for GitHub

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

Closed
m-horky wants to merge 1 commit into from
Closed

Fix IP address collection with IPv6 disabled #3190

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 23 additions & 13 deletions src/rhsmlib/facts/hwprobe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from rhsmlib.facts import cpuinfo
from rhsmlib.facts import collector

from typing import Callable, Dict, Optional, List, TextIO, Tuple, Union
from typing import Callable, Dict, Optional, List, Set, TextIO, Tuple, Union

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -771,6 +771,7 @@ def get_network_info(self) -> Dict[str, str]:
)
except Exception:
net_info["network.fqdn"] = hostname
infolist = []
else:
# getaddrinfo has to return at least one item
# and canonical name can't be empty string.
Expand All @@ -782,20 +783,29 @@ def get_network_info(self) -> Dict[str, str]:
else:
net_info["network.fqdn"] = hostname

try:
info: List[tuple] = socket.getaddrinfo(hostname, None, socket.AF_INET, socket.SOCK_STREAM)
ip_list = set([x[4][0] for x in info])
net_info["network.ipv4_address"] = ", ".join(ip_list)
except Exception as err:
log.debug("Error during resolving IPv4 address of hostname: %s, %s" % (hostname, err))
ipv4_addresses: Set[str] = set()
ipv6_addresses: Set[str] = set()
for address_tuple in infolist:
address: str = address_tuple[4][0]
if address_tuple[0] is socket.AF_INET:
ipv4_addresses.add(address)
if address_tuple[0] is socket.AF_INET6:
ipv6_addresses.add(address)

# FIXME In 2017-09 (616ec72), we added `_get_ipvX_addr_list()` functions, but the original code
# using sockets stayed; the new one was used just in the `except:` block. Why?
# TODO python-ethtool is deprecated and does not receive any updates, a new implementation should
# be created to replace both of these.
if ipv4_addresses:
net_info["network.ipv4_address"] = ", ".join(ipv4_addresses)
else:
log.debug("Could not obtain IPv4 addresses using socket inspection. Using ethtool instead.")
net_info["network.ipv4_address"] = ", ".join(self._get_ipv4_addr_list())

try:
info: List[tuple] = socket.getaddrinfo(hostname, None, socket.AF_INET6, socket.SOCK_STREAM)
ip_list = set([x[4][0] for x in info])
net_info["network.ipv6_address"] = ", ".join(ip_list)
except Exception as err:
log.debug("Error during resolving IPv6 address of hostname: %s, %s" % (hostname, err))
if ipv6_addresses:
net_info["network.ipv6_address"] = ", ".join(ipv6_addresses)
else:
log.debug("Could not obtain IPv6 addresses using socket inspection. Using ethtool instead.")
net_info["network.ipv6_address"] = ", ".join(self._get_ipv6_addr_list())
Copy link
Copy Markdown
Contributor

@jirihnidek jirihnidek Feb 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When IPv6 is completely disabled, then the list of addresses still contains ::1 address, which is not correct. When IPv6 is disabled, then there should not be any IPv6 address in the list.

Copy link
Copy Markdown
Contributor Author

@m-horky m-horky Feb 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't reproduce this. When I disable IPv6, no v6 addresses are listed, on both bare metal and in container.

Copy link
Copy Markdown
Contributor

@jirihnidek jirihnidek Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is strange. I can see: network.ipv6_address: ::1 in system facts, because of this:

https://github.com/candlepin/subscription-manager/blob/main/src/rhsmlib/facts/hwprobe.py#L747

It is strange, because loopback interface contains only IPv4 addresses:

net.interface.lo.ipv4_address: 127.0.0.1
net.interface.lo.ipv4_address_list: 127.0.0.1
net.interface.lo.ipv4_broadcast: Unknown
net.interface.lo.ipv4_broadcast_list: Unknown
net.interface.lo.ipv4_netmask: 8
net.interface.lo.ipv4_netmask_list: 8

It was probably me, who has implemented that, but I think that it is wrong now, because we should not fabricate any non-existent facts in the code.

Copy link
Copy Markdown
Contributor

@jirihnidek jirihnidek Feb 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could fix it in other PR. It is only little bit related to this case. When I was talking about unit testing, then I had this network.ipv6_address: ::1 in my mind.

Copy link
Copy Markdown
Contributor Author

@m-horky m-horky Feb 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I discovered it as well, the two paths could produce different facts (one with loopback, one without). I'd like to fix this later, either in following PR or in the upcoming re-implementation using ip.


except Exception as err:
Expand Down