Skip to content

fix(net): skip v1 physical entries with no name - #7017

Open
shAsh-cy wants to merge 1 commit into
canonical:mainfrom
shAsh-cy:fix/6982-physdevs-none-name
Open

fix(net): skip v1 physical entries with no name#7017
shAsh-cy wants to merge 1 commit into
canonical:mainfrom
shAsh-cy:fix/6982-physdevs-none-name

Conversation

@shAsh-cy

@shAsh-cy shAsh-cy commented Aug 20, 2026

Copy link
Copy Markdown
  • I have signed the CLA: https://ubuntu.com/legal/contributors
  • I have included a comprehensive commit message using the guide below
  • I have added unit tests to cover the new behavior under tests/unittests/
  • I have kept the change small, avoiding unnecessary whitespace or non-functional changes.
  • I have added a reference to issues that this PR relates to in the PR message
  • I have updated the documentation with the changed behavior. Not applicable, no user-facing change.

Proposed Commit Message

fix(net): skip v1 physical entries with no name

_version_1 skipped entries without a mac_address but not entries
without a name, so a None name reached device_driver() and raised
TypeError in sys_dev_path(). This crashed wait_for_physdevs while
applying network config. Skip nameless entries, matching the
existing behaviour of _version_2.

This uses the same `if not name` condition as _version_2, so an
entry with an empty name is skipped too. That case does not crash
today, but an empty name is equally unusable for renaming.

Fixes GH-6982

Additional Context

The crash. A v1 entry of type physical with a mac_address but no usable name reaches device_driver(name) with name=None, which raises TypeError in sys_dev_path(). That propagates out of wait_for_physdevs() in Init.apply_network_config(), which is not wrapped, so network config fails. DataSourceHetzner emits such an entry when a private NIC has not hotplugged yet. Note that device_devid(name) on the following line has the same exposure, so a guard around only device_driver would have moved the crash rather than fixed it.

Why skip rather than guard the lookups. I first considered guarding only the device_driver/device_devid calls and letting the row through with name=None, to keep the MAC in wait_for_physdevs's expected_macs. Tracing the other consumer showed that is worse. extract_physdevs also feeds _rename_interfaces() via apply_network_config_names(). With driver and device_id both None, entry_match() falls through to matching on MAC alone, so a nameless row matches a present device and reaches ("rename", mac, None, (cur_name, None)). If that device is up and downable it is taken down first, the rename then fails, and the bring-up fails too, leaving the interface down. _apply_netcfg_names() wraps all of this in except Exception and logs a warning, so it would fail silently. Today that path is unreachable because the TypeError fires first. Skipping in _version_1 keeps the row out of both consumers.

Why this cannot regress. Every v1 config reaching this path with a nameless physical entry crashes today, so there is no working behaviour for the skip to change.

Trade-off. The MAC is no longer waited for in wait_for_physdevs. If nameless-but-MAC'd v1 entries are considered legitimate input worth waiting for, the alternative is to keep the row and reject it in _rename_interfaces instead. Happy to do that as a follow-up if preferred.

Test Steps

Before this change:

>>> from cloudinit.net import extract_physdevs
>>> extract_physdevs({"version": 1, "config": [{"type": "physical",
...   "mac_address": "86:00:00:3c:5c:43", "name": None,
...   "subnets": [{"type": "dhcp", "ipv4": True}]}]})
TypeError: can only concatenate str (not "NoneType") to str

After this change the same call returns []. A well-formed v1 entry is unaffected and still resolves to [['86:00:00:3c:5c:43', 'eth0', 'virtio', '0x1000']].

New unit test: TestExtractPhysdevs::test_get_v1_type_physical_skips_if_no_name, placed alongside the existing v2 equivalent. It asserts the entry is skipped and that device_driver/device_devid are never called. The latter matters because the class's autouse fixture mocks both, so the return-value assertion alone would not pin the actual defect.

Rebased onto bb81cd0. Full suite: 5744 passed, 5 skipped, 13 xfailed (tox -e py3). tox -e check_format passes.

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

_version_1 skipped entries without a mac_address but not entries
without a name, so a None name reached device_driver() and raised
TypeError in sys_dev_path(). This crashed wait_for_physdevs while
applying network config. Skip nameless entries, matching the
existing behaviour of _version_2.

This uses the same `if not name` condition as _version_2, so an
entry with an empty name is skipped too. That case does not crash
today, but an empty name is equally unusable for renaming.

Fixes canonicalGH-6982
@shAsh-cy
shAsh-cy force-pushed the fix/6982-physdevs-none-name branch from d54296d to c7b7f9b Compare August 21, 2026 08:29
@holmanb holmanb self-assigned this Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants