vyos: Fix handling of IPv6 netmasks on OpenStack#90
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes VyOS network configuration on OpenStack when DHCP is disabled and IPv6 route netmasks are provided in address-form (e.g. ::) instead of a prefix length, which can cause invalid ipaddress.ip_network parsing.
Changes:
- Added an IPv6 netmask-to-prefix-length conversion helper.
- Updated v1 subnet route configuration to convert IPv6 netmasks before calling
ipaddress.ip_network.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cloudinit/config/cc_vyos.py
Outdated
| netmask = ipaddress.ip_address(item['netmask']) | ||
| if netmask.version == 6: | ||
| netmask = ipaddress_to_prefixlen(netmask) | ||
| ip_network = ipaddress.ip_network('{}/{}'.format( | ||
| item['network'], item['netmask'])) | ||
| item['network'], netmask)) |
There was a problem hiding this comment.
In route handling, netmask = ipaddress.ip_address(item['netmask']) will break when item['netmask'] is an integer prefix length (e.g. 64) or a prefix string (e.g. '64'): ip_address(64) becomes an IPv4 address and '64' raises ValueError, leading to invalid ip_network('{}/{}'.format(item['network'], netmask)). Consider passing through prefix lengths unchanged and only converting when the netmask is an IPv6 address-form string (like '::'/'ffff:...'), or use the existing cloudinit.net.network_state.ipv6_mask_to_net_prefix helper which already supports both prefix and IPv6-netmask inputs.
cloudinit/config/cc_vyos.py
Outdated
| # Python's ipaddress.ip_network does not support something like ::/:: as | ||
| # input. So we have to convert the IPv6 address notation of the netmask | ||
| # to the integer prefix length. | ||
| def ipaddress_to_prefixlen(addr): | ||
| prefixlen = addr.max_prefixlen | ||
| register = int(addr) | ||
| if register == 0: | ||
| return 0 | ||
| while not register & 1: | ||
| prefixlen -= 1 | ||
| register = register >> 1 | ||
| if register != 2**prefixlen - 1: | ||
| raise ValueError("{} does not appear to be a netmask".format(addr)) | ||
| return prefixlen |
There was a problem hiding this comment.
This PR adds ipaddress_to_prefixlen, but cloud-init already provides ipv6_mask_to_net_prefix in cloudinit.net.network_state, which handles both prefix-length strings (e.g. '64') and IPv6 netmask notation (e.g. 'ffff:ffff:ffff::') and validates contiguity. Reusing the shared helper would reduce duplicated logic and keep netmask parsing consistent across the codebase.
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
zdc
left a comment
zdc
left a comment
There was a problem hiding this comment.
Thanks for the PR!
- The Cloud-init specification is clear about requiring a netmask to be noted in CIDR for IPv6. So, the original issue is in OpenStack.
- However, even inside Cloud-init sources, we see usage of
ipv6_mask_to_net_prefix()specifically to cover this issue with OpenStack. - Therefore, it makes sense to adopt the same workaround for our module.
The PR is good and will be approved, but please squash all commits into a single one before we do this.
I think it makes sense to file the bug report in OpenStack as well. This is not the first time we have seen non-following specs on various platforms, and trying to cover everything with workarounds will be hard in the long run.
On OpenStack with a network with DHCP disabled, IPv6 routes are passed
like this:
'routes': [{'network': '::', 'netmask': '::', 'gateway': '2001:db8::1'}]
This results in a call to ipaddress.ip_network('::/::'), which Python
does not regard as valid network notation.
So this commit uses ipv6_mask_to_net_prefix from the package
cloudinit.net.network_state to convert the netmask for IPv6 networks.
sever-sever
left a comment
sever-sever
left a comment
There was a problem hiding this comment.
Fix Openstack IPv6 netmasks
vyosbot
added
mirror-initiated
Proposed Commit Message
Additional Context
Test Steps
Checklist: