diff options
3 files changed, 52 insertions, 27 deletions
diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 3ea3b802..9eb07077 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -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(.*).*$') @@ -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 diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 64681b8d..5676f457 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -30,6 +30,9 @@ from ironic_python_agent.tests.unit.samples import hardware_samples 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') @@ -251,7 +254,7 @@ class TestImageExtension(base.IronicAgentTest): mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), - ('', ''), ('', ''), + (EFI_RESULT, ''), (EFI_RESULT, ''), ('', ''), ('', '')]) expected = [mock.call('efibootmgr', '--version'), @@ -260,7 +263,7 @@ class TestImageExtension(base.IronicAgentTest): 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', @@ -299,7 +302,7 @@ class TestImageExtension(base.IronicAgentTest): mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), - ('', ''), ('', ''), + (EFI_RESULT, ''), (EFI_RESULT, ''), ('', ''), ('', '')]) expected = [mock.call('efibootmgr', '--version'), @@ -308,7 +311,7 @@ class TestImageExtension(base.IronicAgentTest): 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', @@ -353,10 +356,11 @@ Boot0000 ironic1 HD(1,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BO 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'), @@ -365,7 +369,7 @@ Boot0002 VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) 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', @@ -416,6 +420,7 @@ Boot0002* Hard Disk VenHw(1fad3248-0000-7950-2166-a1e506fdb83a,01000000)..GO 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, ''), ('', ''), @@ -427,7 +432,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 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, @@ -470,8 +475,8 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), - ('', ''), ('', ''), - ('', ''), ('', ''), + (EFI_RESULT, ''), (EFI_RESULT, ''), + (EFI_RESULT, ''), ('', ''), ('', '')]) expected = [mock.call('efibootmgr', '--version'), @@ -480,7 +485,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 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', @@ -2379,7 +2384,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), - ('', ''), ('', ''), + ('', ''), (b'', ''), ('', ''), ('', ''), ('', '')]) @@ -2388,7 +2393,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 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', @@ -2432,18 +2437,19 @@ Boot0001 Vendor String HD(2,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\B 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, @@ -2475,7 +2481,7 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), - ('', ''), ('', ''), + ('', ''), (b'', ''), ('', ''), ('', ''), ('', '')]) @@ -2484,7 +2490,7 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) 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', @@ -2513,7 +2519,7 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), - ('', ''), ('', ''), + ('', ''), (b'', ''), ('', ''), ('', ''), ('', '')]) @@ -2522,7 +2528,7 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) 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', @@ -2625,12 +2631,14 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) 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', diff --git a/releasenotes/notes/fixes-efibootmgr-character-encoding-19e531ba694824c1.yaml b/releasenotes/notes/fixes-efibootmgr-character-encoding-19e531ba694824c1.yaml new file mode 100644 index 00000000..b4c3270c --- /dev/null +++ b/releasenotes/notes/fixes-efibootmgr-character-encoding-19e531ba694824c1.yaml @@ -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. |