Skip to content

Synchronise wallaby with upstream #54

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

Merged
merged 1 commit into from
Jul 17, 2023
Merged

Synchronise wallaby with upstream #54

Show file tree
Hide file tree
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
12 changes: 10 additions & 2 deletions ironic_python_agent/extensions/image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,15 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,

# Before updating let's get information about the bootorder
LOG.debug("Getting information about boot order.")
original_efi_output = utils.execute('efibootmgr', '-v')
# Invokes binary=True so we get a bytestream back.
original_efi_output = utils.execute('efibootmgr', '-v', binary=True)
# Bytes must be decoded before regex can be run and
# matching to work as intended.
# Also ignore errors on decoding, as we can basically get
# garbage out of the nvram record, this way we don't fail
# hard on unrelated records.
original_efi_output = original_efi_output[0].decode(
'utf-16', errors='ignore')
# NOTE(TheJulia): regex used to identify entries in the efibootmgr
# output on stdout.
entry_label = re.compile(r'Boot([0-9a-f-A-F]+)\*?\s(.*).*$')
Expand Down Expand Up @@ -304,7 +312,7 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
label = 'ironic' + str(label_id)

# Iterate through standard out, and look for duplicates
for line in original_efi_output[0].split('\n'):
for line in original_efi_output.split('\n'):
match = entry_label.match(line)
# Look for the base label in the string if a line match
# occurs, so we can identify if we need to eliminate the
Expand Down
58 changes: 33 additions & 25 deletions ironic_python_agent/tests/unit/extensions/test_image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
from ironic_python_agent import utils


EFI_RESULT = ''.encode('utf-16')


@mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
@mock.patch.object(tempfile, 'mkdtemp', lambda *_: '/tmp/fake-dir')
Expand Down Expand Up @@ -251,7 +254,7 @@ def test__uefi_bootloader_given_partition(

mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
(EFI_RESULT, ''), (EFI_RESULT, ''),
('', ''), ('', '')])

expected = [mock.call('efibootmgr', '--version'),
Expand All @@ -260,7 +263,7 @@ def test__uefi_bootloader_given_partition(
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
'-p', '1', '-w',
'-L', 'ironic1', '-l',
Expand Down Expand Up @@ -299,7 +302,7 @@ def test__uefi_bootloader_find_partition(
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
(EFI_RESULT, ''), (EFI_RESULT, ''),
('', ''), ('', '')])

expected = [mock.call('efibootmgr', '--version'),
Expand All @@ -308,7 +311,7 @@ def test__uefi_bootloader_find_partition(
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
'-p', '1', '-w',
'-L', 'ironic1', '-l',
Expand Down Expand Up @@ -353,10 +356,11 @@ def test__uefi_bootloader_with_entry_removal(
Boot0001 ironic2 HD(1,GPT,4f3c6294-bf9b-4208-9808-111111111112)File(\EFI\Boot\BOOTX64.EFI)
Boot0002 VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
""" # noqa This is a giant literal string for testing.
stdout_msg = stdout_msg.encode('utf-16')
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
(stdout_msg, ''), ('', ''),
('', ''), ('', ''),
(stdout_msg, ''), (EFI_RESULT, ''),
(EFI_RESULT, ''), (EFI_RESULT, ''),
('', ''), ('', '')])

expected = [mock.call('efibootmgr', '--version'),
Expand All @@ -365,7 +369,7 @@ def test__uefi_bootloader_with_entry_removal(
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-b', '0000', '-B'),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
'-p', '1', '-w',
Expand Down Expand Up @@ -416,6 +420,7 @@ def test__uefi_bootloader_with_entry_removal_lenovo(
Boot0003* Network VenHw(1fad3248-0000-7950-2166-a1e506fdb83a,05000000)..GO..NO............U.E.F.I.:. . . .S.L.O.T.2. .(.2.F./.0./.0.). .P.X.E. .I.P.4. . .Q.L.o.g.i.c. .Q.L.4.1.2.6.2. .P.C.I.e. .2.5.G.b. .2.-.P.o.r.t. .S.F.P.2.8. .E.t.h.e.r.n.e.t. .A.d.a.p.t.e.r. .-. .P.X.E........A....................%.4..Z...............................................................Gd-.;.A..MQ..L.P.X.E. .I.P.4. .Q.L.o.g.i.c. .Q.L.4.1.2.6.2. .P.C.I.e. .2.5.G.b. .2.-.P.o.r.t. .S.F.P.2.8. .E.t.h.e.r.n.e.t. .A.d.a.p.t.e.r. .-. .P.X.E.......BO..NO............U.E.F.I.:. . . .S.L.O.T.1. .(.3.0./.0./.0.). .P.X.E. .I.P.4. . .Q.L.o.g.i.c. .Q.L.4.1.2.6.2. .P.C.I.e. .2.5.G.b. .2.-.P.o.r.t. .S.F.P.2.8. .E.t.h.e.r.n.e.t. .A.d.a.p.t.e.r. .-.
Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x64000)/File(\EFI\boot\shimx64.efi)
""" # noqa This is a giant literal string for testing.
stdout_msg = stdout_msg.encode('utf-16')
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
(stdout_msg, ''), ('', ''),
Expand All @@ -427,7 +432,7 @@ def test__uefi_bootloader_with_entry_removal_lenovo(
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-b', '0000', '-B'),
mock.call('efibootmgr', '-b', '0004', '-B'),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
Expand Down Expand Up @@ -470,8 +475,8 @@ def test__add_multi_bootloaders(

mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
(EFI_RESULT, ''), (EFI_RESULT, ''),
(EFI_RESULT, ''), ('', ''),
('', '')])

expected = [mock.call('efibootmgr', '--version'),
Expand All @@ -480,7 +485,7 @@ def test__add_multi_bootloaders(
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
'-p', '1', '-w',
'-L', 'ironic1', '-l',
Expand Down Expand Up @@ -2379,7 +2384,7 @@ def test__manage_uefi(self, mkdir_mock, mock_utils_efi_part,
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']

mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), (b'', ''),
('', ''), ('', ''),
('', '')])

Expand All @@ -2388,7 +2393,7 @@ def test__manage_uefi(self, mkdir_mock, mock_utils_efi_part,
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
'-p', '1', '-w',
'-L', 'ironic1', '-l',
Expand Down Expand Up @@ -2432,18 +2437,19 @@ def test__manage_uefi_found_csv(self, mkdir_mock, mock_utils_efi_part,
Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
""" # noqa This is a giant literal string for testing.

mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), (dupe_entry, ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', '')])
mock_execute.side_effect = iter([
('', ''), ('', ''),
('', ''), (dupe_entry.encode('utf-16'), ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', '')])

expected = [mock.call('partx', '-a', '/dev/fake', attempts=3,
delay_on_retry=True),
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-b', '0000', '-B'),
mock.call('efibootmgr', '-b', '0001', '-B'),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
Expand Down Expand Up @@ -2475,7 +2481,7 @@ def test__manage_uefi_nvme_device(self, mkdir_mock, mock_utils_efi_part,
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']

mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), (b'', ''),
('', ''), ('', ''),
('', '')])

Expand All @@ -2484,7 +2490,7 @@ def test__manage_uefi_nvme_device(self, mkdir_mock, mock_utils_efi_part,
mock.call('udevadm', 'settle'),
mock.call('mount', '/dev/fakenvme0p1',
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-v', '-c', '-d', '/dev/fakenvme0',
'-p', '1', '-w',
'-L', 'ironic1', '-l',
Expand Down Expand Up @@ -2513,7 +2519,7 @@ def test__manage_uefi_wholedisk(
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']

mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), (b'', ''),
('', ''), ('', ''),
('', '')])

Expand All @@ -2522,7 +2528,7 @@ def test__manage_uefi_wholedisk(
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
'-p', '1', '-w',
'-L', 'ironic1', '-l',
Expand Down Expand Up @@ -2625,12 +2631,14 @@ def test__run_efibootmgr_no_bootloaders(self, mock_execute, mock_dispatch):
mock_execute.assert_has_calls(expected)

def test__run_efibootmgr(self, mock_execute, mock_dispatch):
mock_execute.return_value = ('', '')
mock_execute.side_effect = [
(b'', ''),
('', '')]
result = image._run_efibootmgr(['EFI/BOOT/BOOTX64.EFI'],
self.fake_dev,
self.fake_efi_system_part,
self.fake_dir)
expected = [mock.call('efibootmgr', '-v'),
expected = [mock.call('efibootmgr', '-v', binary=True),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
'-p', self.fake_efi_system_part, '-w',
'-L', 'ironic1', '-l',
Expand Down
9 changes: 9 additions & 0 deletions releasenotes/notes/fixes-efibootmgr-character-encoding-19e531ba694824c1.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
fixes:
- |
Fixes UEFI NVRAM record handling with efibootmgr so we can accept and
handle UTF-16 encoded data which is to be expected in UEFI NVRAM as
the records are UTF-16 encoded.
- |
Fixes handling of UEFI NVRAM records to allow for unexpected characters
in the response, so it is non-fatal to Ironic.