From 4ff512a6eed704447c9ecf5f463e9b0850f77237 Mon Sep 17 00:00:00 2001 From: Vanou Ishii Date: Tue, 18 Jul 2023 19:08:11 -0400 Subject: [PATCH] Fix missing ETag when patching Redfish resource Some Redfish implementations require current ETag of Redfish resource against which resource update is requested to be included in HTTP PATCH operation header. This commit adds missing ETag in PATCH operation header. Change-Id: I3f88d8a7032fd9071aa033c706768127da1d12af --- .../fix-missing-etags-ded8c0bb31fafef7.yaml | 6 +++++ sushy/resources/chassis/chassis.py | 3 ++- sushy/resources/manager/virtual_media.py | 4 ++- sushy/resources/settings.py | 2 +- sushy/resources/system/bios.py | 4 +++ sushy/resources/system/secure_boot.py | 4 ++- sushy/resources/system/storage/controller.py | 4 +++ sushy/resources/system/storage/drive.py | 3 ++- sushy/resources/system/system.py | 3 ++- .../unit/resources/chassis/test_chassis.py | 4 ++- .../resources/manager/test_virtual_media.py | 5 +++- .../system/storage/test_controller.py | 5 +++- .../resources/system/storage/test_drive.py | 4 ++- .../tests/unit/resources/system/test_bios.py | 23 +++++++++++++++-- .../unit/resources/system/test_secure_boot.py | 4 ++- .../unit/resources/system/test_system.py | 25 +++++++++++-------- sushy/tests/unit/resources/test_settings.py | 3 ++- 17 files changed, 81 insertions(+), 25 deletions(-) create mode 100644 releasenotes/notes/fix-missing-etags-ded8c0bb31fafef7.yaml diff --git a/releasenotes/notes/fix-missing-etags-ded8c0bb31fafef7.yaml b/releasenotes/notes/fix-missing-etags-ded8c0bb31fafef7.yaml new file mode 100644 index 00000000..565b7b5a --- /dev/null +++ b/releasenotes/notes/fix-missing-etags-ded8c0bb31fafef7.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes missing ETag in PATCH operation against Redfish resources with + backward compatibility for Redfish implementation which doesn't work + with ETag in header. diff --git a/sushy/resources/chassis/chassis.py b/sushy/resources/chassis/chassis.py index ca7b1857..a2930b74 100644 --- a/sushy/resources/chassis/chassis.py +++ b/sushy/resources/chassis/chassis.py @@ -214,9 +214,10 @@ class Chassis(base.ResourceBase): parameter='state', value=state, valid_values=' ,'.join(i.value for i in res_cons.IndicatorLED)) + etag = self._get_etag() data = {'IndicatorLED': state} - self._conn.patch(self.path, data=data) + self._conn.patch(self.path, data=data, etag=etag) self.invalidate() @property diff --git a/sushy/resources/manager/virtual_media.py b/sushy/resources/manager/virtual_media.py index c4aa7255..b4a5dc9f 100644 --- a/sushy/resources/manager/virtual_media.py +++ b/sushy/resources/manager/virtual_media.py @@ -248,8 +248,10 @@ class VirtualMedia(base.ResourceBase): parameter='verify_certificate', value=verify_certificate, valid_values='boolean (True, False)') + etag = self._get_etag() self._conn.patch(self.path, - data={'VerifyCertificate': verify_certificate}) + data={'VerifyCertificate': verify_certificate}, + etag=etag) self.invalidate() @property diff --git a/sushy/resources/settings.py b/sushy/resources/settings.py index ce6a23a0..88ffbf98 100644 --- a/sushy/resources/settings.py +++ b/sushy/resources/settings.py @@ -170,7 +170,7 @@ class SettingsField(base.CompositeField): :returns: Response object """ - return connector.patch(self.resource_uri, data=value) + return connector.patch(self.resource_uri, data=value, etag=self._etag) @property def resource_uri(self): diff --git a/sushy/resources/system/bios.py b/sushy/resources/system/bios.py index 9672e45d..2f26662d 100644 --- a/sushy/resources/system/bios.py +++ b/sushy/resources/system/bios.py @@ -149,6 +149,10 @@ class Bios(base.ResourceBase): payload = utils.process_apply_time_input( payload, apply_time, maint_window_start_time, maint_window_duration) + # NOTE(vanou): To retrieve current ETag value of @Redfish.Settings + # but not update cached _pending_settings_resource, because cached + # property is only this one and re-cache is not required + self.refresh(force=False) self._settings.commit(self._conn, payload) utils.cache_clear(self, force_refresh=False, diff --git a/sushy/resources/system/secure_boot.py b/sushy/resources/system/secure_boot.py index 4e01d863..e1961d9a 100644 --- a/sushy/resources/system/secure_boot.py +++ b/sushy/resources/system/secure_boot.py @@ -144,4 +144,6 @@ class SecureBoot(base.ResourceBase): raise exceptions.InvalidParameterValueError( "Expected a boolean for 'enabled', got %r" % enabled) - self._conn.patch(self.path, data={'SecureBootEnable': enabled}) + etag = self._get_etag() + self._conn.patch(self.path, data={'SecureBootEnable': enabled}, + etag=etag) diff --git a/sushy/resources/system/storage/controller.py b/sushy/resources/system/storage/controller.py index 7a1f0cce..4535f618 100644 --- a/sushy/resources/system/storage/controller.py +++ b/sushy/resources/system/storage/controller.py @@ -97,6 +97,10 @@ class StorageController(base.ResourceBase): payload = utils.process_apply_time_input( payload, apply_time, maint_window_start_time, maint_window_duration) + # NOTE(vanou): To retrieve current ETag value of @Redfish.Settings + # but not update cached pending_settings, because cached property is + # only this one and re-cache this is not required + self.refresh(force=False) r = self._settings.commit(self._conn, payload) utils.cache_clear(self, force_refresh=False, only_these=['pending_settings']) diff --git a/sushy/resources/system/storage/drive.py b/sushy/resources/system/storage/drive.py index 9abed6f1..4a8d2a6e 100644 --- a/sushy/resources/system/storage/drive.py +++ b/sushy/resources/system/storage/drive.py @@ -102,7 +102,8 @@ class Drive(base.ResourceBase): parameter='state', value=state, valid_values=' ,'.join(i.value for i in res_cons.IndicatorLED)) + etag = self._get_etag() data = {'IndicatorLED': state} - self._conn.patch(self.path, data=data) + self._conn.patch(self.path, data=data, etag=etag) self.invalidate() diff --git a/sushy/resources/system/system.py b/sushy/resources/system/system.py index a797bdf9..cfba3348 100644 --- a/sushy/resources/system/system.py +++ b/sushy/resources/system/system.py @@ -342,9 +342,10 @@ class System(base.ResourceBase): parameter='state', value=state, valid_values=' ,'.join(i.value for i in res_cons.IndicatorLED)) + etag = self._get_etag() data = {'IndicatorLED': state} - self._conn.patch(self.path, data=data) + self._conn.patch(self.path, data=data, etag=etag) self.invalidate() def _get_processor_collection_path(self): diff --git a/sushy/tests/unit/resources/chassis/test_chassis.py b/sushy/tests/unit/resources/chassis/test_chassis.py index a852e7ad..243129a8 100644 --- a/sushy/tests/unit/resources/chassis/test_chassis.py +++ b/sushy/tests/unit/resources/chassis/test_chassis.py @@ -34,6 +34,7 @@ class ChassisTestCase(base.TestCase): self.json_doc = json.load(f) self.conn.get.return_value.json.return_value = self.json_doc + self.conn.get.return_value.headers = {'ETag': 'd37f7bcd528e4d59'} self.chassis = chassis.Chassis(self.conn, '/redfish/v1/Chassis/Blade1', redfish_version='1.8.0') @@ -144,7 +145,8 @@ class ChassisTestCase(base.TestCase): self.chassis.set_indicator_led(sushy.IndicatorLED.BLINKING) self.chassis._conn.patch.assert_called_once_with( '/redfish/v1/Chassis/Blade1', - data={'IndicatorLED': 'Blinking'}) + data={'IndicatorLED': 'Blinking'}, + etag='d37f7bcd528e4d59') invalidate_mock.assert_called_once_with() diff --git a/sushy/tests/unit/resources/manager/test_virtual_media.py b/sushy/tests/unit/resources/manager/test_virtual_media.py index 18629ac1..e816115a 100644 --- a/sushy/tests/unit/resources/manager/test_virtual_media.py +++ b/sushy/tests/unit/resources/manager/test_virtual_media.py @@ -294,13 +294,16 @@ class VirtualMediaTestCase(base.TestCase): self.assertTrue(self.sys_virtual_media._is_stale) def test_set_verify_certificate(self): + self.conn.get.return_value.headers = {'Allow': 'GET,HEAD', + 'ETag': '3d7b8a7360bf2941d'} with mock.patch.object( self.sys_virtual_media, 'invalidate', autospec=True) as invalidate_mock: self.sys_virtual_media.set_verify_certificate(True) self.sys_virtual_media._conn.patch.assert_called_once_with( "/redfish/v1/Managers/BMC/VirtualMedia/Floppy1", - data={'VerifyCertificate': True}) + data={'VerifyCertificate': True}, + etag='3d7b8a7360bf2941d') invalidate_mock.assert_called_once_with() diff --git a/sushy/tests/unit/resources/system/storage/test_controller.py b/sushy/tests/unit/resources/system/storage/test_controller.py index 35ddfe28..a51fe2f5 100644 --- a/sushy/tests/unit/resources/system/storage/test_controller.py +++ b/sushy/tests/unit/resources/system/storage/test_controller.py @@ -65,6 +65,8 @@ class ControllerTestCase(base.TestCase): self.controller.supported_apply_times) def test_update(self): + self.conn.get.return_value.json.side_effect = [ + self.json_doc, self.json_doc] mock_response = mock.Mock() mock_response.status_code = http_client.ACCEPTED mock_response.headers = {'Content-Length': 42, @@ -81,7 +83,8 @@ class ControllerTestCase(base.TestCase): data={'ControllerRates': {'ConsistencyCheckRatePercent': 30}, '@Redfish.SettingsApplyTime': { '@odata.type': '#Settings.v1_0_0.PreferredApplyTime', - 'ApplyTime': 'OnReset'}}) + 'ApplyTime': 'OnReset'}}, + etag=None) self.assertIsInstance(tm, taskmonitor.TaskMonitor) self.assertEqual('/Task/545', tm.task_monitor_uri) diff --git a/sushy/tests/unit/resources/system/storage/test_drive.py b/sushy/tests/unit/resources/system/storage/test_drive.py index 72c7e91d..03c3de5d 100644 --- a/sushy/tests/unit/resources/system/storage/test_drive.py +++ b/sushy/tests/unit/resources/system/storage/test_drive.py @@ -83,13 +83,15 @@ class DriveTestCase(base.TestCase): self.assertEqual('3', volumes[1].identity) def test_set_indicator_led(self): + self.conn.get.return_value.headers = {'ETag': 'a3b01b63f80a4913'} with mock.patch.object( self.stor_drive, 'invalidate', autospec=True) as invalidate_mock: self.stor_drive.set_indicator_led(sushy.IndicatorLED.BLINKING) self.stor_drive._conn.patch.assert_called_once_with( '/redfish/v1/Systems/437XR1138/Storage/1/Drives/' - '32ADF365C6C1B7BD', data={'IndicatorLED': 'Blinking'}) + '32ADF365C6C1B7BD', data={'IndicatorLED': 'Blinking'}, + etag='a3b01b63f80a4913') invalidate_mock.assert_called_once_with() diff --git a/sushy/tests/unit/resources/system/test_bios.py b/sushy/tests/unit/resources/system/test_bios.py index 4686becd..ef429a9a 100644 --- a/sushy/tests/unit/resources/system/test_bios.py +++ b/sushy/tests/unit/resources/system/test_bios.py @@ -119,6 +119,10 @@ class BiosTestCase(base.TestCase): attributes.get('maintenance_window')) def test_set_attribute_apply_time(self): + self.conn.get.return_value.json.side_effect = [ + self.bios_json, + self.bios_json] + self.sys_bios.set_attribute( 'ProcTurboMode', 'Disabled', res_cons.ApplyTime.IN_MAINTENANCE_WINDOW_ON_RESET, @@ -131,9 +135,15 @@ class BiosTestCase(base.TestCase): '@odata.type': '#Settings.v1_0_0.PreferredApplyTime', 'ApplyTime': 'InMaintenanceWindowOnReset', 'MaintenanceWindowStartTime': '2020-09-01T04:30:00', - 'MaintenanceWindowDurationInSeconds': 600}}) + 'MaintenanceWindowDurationInSeconds': 600}}, + etag='9234ac83b9700123cc32') def test_set_attribute_on_refresh(self): + self.conn.get.return_value.json.side_effect = [ + self.bios_settings_json, + self.bios_json, + self.bios_settings_json] + self.conn.get.reset_mock() # make it to instantiate pending attributes self.sys_bios.pending_attributes @@ -150,6 +160,9 @@ class BiosTestCase(base.TestCase): self.assertTrue(self.conn.get.called) def test_set_attributes(self): + self.conn.get.return_value.json.side_effect = [ + self.bios_json] + self.sys_bios.set_attributes( {'ProcTurboMode': 'Disabled', 'UsbControl': 'UsbDisabled'}, res_cons.ApplyTime.AT_MAINTENANCE_WINDOW_START, @@ -163,9 +176,15 @@ class BiosTestCase(base.TestCase): '@odata.type': '#Settings.v1_0_0.PreferredApplyTime', 'ApplyTime': 'AtMaintenanceWindowStart', 'MaintenanceWindowStartTime': '2020-09-01T04:30:00', - 'MaintenanceWindowDurationInSeconds': 600}}) + 'MaintenanceWindowDurationInSeconds': 600}}, + etag='9234ac83b9700123cc32') def test_set_attributes_on_refresh(self): + self.conn.get.return_value.json.side_effect = [ + self.bios_settings_json, + self.bios_json, + self.bios_settings_json] + self.conn.get.reset_mock() # make it to instantiate pending attributes self.sys_bios.pending_attributes diff --git a/sushy/tests/unit/resources/system/test_secure_boot.py b/sushy/tests/unit/resources/system/test_secure_boot.py index 0a5a1111..978f74ed 100644 --- a/sushy/tests/unit/resources/system/test_secure_boot.py +++ b/sushy/tests/unit/resources/system/test_secure_boot.py @@ -29,6 +29,7 @@ class SecureBootTestCase(base.TestCase): self.secure_boot_json = json.load(f) self.conn.get.return_value.json.return_value = self.secure_boot_json + self.conn.get.return_value.headers = {'ETag': 'b26ae716a2c1f39f'} self.secure_boot = secure_boot.SecureBoot( self.conn, '/redfish/v1/Systems/437XR1138R2/SecureBoot', registries={}, redfish_version='1.1.0') @@ -79,7 +80,8 @@ class SecureBootTestCase(base.TestCase): self.secure_boot.set_enabled(True) self.conn.patch.assert_called_once_with( '/redfish/v1/Systems/437XR1138R2/SecureBoot', - data={'SecureBootEnable': True}) + data={'SecureBootEnable': True}, + etag='b26ae716a2c1f39f') def test_set_enabled_wrong_type(self): self.assertRaises(exceptions.InvalidParameterValueError, diff --git a/sushy/tests/unit/resources/system/test_system.py b/sushy/tests/unit/resources/system/test_system.py index c01ecfe4..570202ce 100644 --- a/sushy/tests/unit/resources/system/test_system.py +++ b/sushy/tests/unit/resources/system/test_system.py @@ -46,6 +46,8 @@ class SystemTestCase(base.TestCase): self.sys_inst = system.System( self.conn, '/redfish/v1/Systems/437XR1138R2', redfish_version='1.0.2') + self.sys_inst._get_etag = mock.Mock() + self.sys_inst._get_etag.return_value = '81802dbf61beb0bd' def test__parse_attributes(self): self.sys_inst._parse_attributes(self.json_doc) @@ -283,7 +285,7 @@ class SystemTestCase(base.TestCase): data={'Boot': {'BootSourceOverrideEnabled': 'Continuous', 'BootSourceOverrideTarget': 'Pxe', 'BootSourceOverrideMode': 'UEFI'}}, - etag=None) + etag='81802dbf61beb0bd') def test_set_system_boot_options_no_mode_specified(self): self.sys_inst.set_system_boot_options( @@ -293,7 +295,7 @@ class SystemTestCase(base.TestCase): '/redfish/v1/Systems/437XR1138R2', data={'Boot': {'BootSourceOverrideEnabled': 'Once', 'BootSourceOverrideTarget': 'Hdd'}}, - etag=None) + etag='81802dbf61beb0bd') def test_set_system_boot_options_no_target_specified(self): self.sys_inst.set_system_boot_options( @@ -303,7 +305,7 @@ class SystemTestCase(base.TestCase): '/redfish/v1/Systems/437XR1138R2', data={'Boot': {'BootSourceOverrideEnabled': 'Continuous', 'BootSourceOverrideMode': 'UEFI'}}, - etag=None) + etag='81802dbf61beb0bd') def test_set_system_boot_options_no_freq_specified(self): self.sys_inst.set_system_boot_options( @@ -313,7 +315,7 @@ class SystemTestCase(base.TestCase): '/redfish/v1/Systems/437XR1138R2', data={'Boot': {'BootSourceOverrideTarget': 'Pxe', 'BootSourceOverrideMode': 'UEFI'}}, - etag=None) + etag='81802dbf61beb0bd') def test_set_system_boot_options_nothing_specified(self): self.sys_inst.set_system_boot_options() @@ -348,7 +350,7 @@ class SystemTestCase(base.TestCase): '/redfish/v1/Systems/437XR1138R2', data={'Boot': {'BootSourceOverrideEnabled': 'Once', 'BootSourceOverrideTarget': 'UsbCd'}}, - etag=None) + etag='81802dbf61beb0bd') def test_set_system_boot_options_supermicro_no_usb_cd_boot(self): @@ -361,7 +363,7 @@ class SystemTestCase(base.TestCase): '/redfish/v1/Systems/437XR1138R2', data={'Boot': {'BootSourceOverrideEnabled': 'Once', 'BootSourceOverrideTarget': 'Cd'}}, - etag=None) + etag='81802dbf61beb0bd') def test_set_system_boot_options_settings_resource_nokia(self): with open('sushy/tests/unit/json_samples/settings-nokia.json') as f: @@ -487,10 +489,10 @@ class SystemTestCase(base.TestCase): data={'Boot': {'BootSourceOverrideEnabled': 'Continuous', 'BootSourceOverrideTarget': 'Pxe', 'BootSourceOverrideMode': 'UEFI'}}, - etag=None) + etag='81802dbf61beb0bd') def test_set_system_boot_source_with_etag(self): - self.conn.get.return_value.headers = {'ETag': '"3d7b838291941d"'} + self.conn.get.return_value.headers = {'ETag': '"81802dbf61beb0bd"'} self.sys_inst.set_system_boot_source( sushy.BOOT_SOURCE_TARGET_PXE, enabled=sushy.BOOT_SOURCE_ENABLED_CONTINUOUS, @@ -500,7 +502,7 @@ class SystemTestCase(base.TestCase): data={'Boot': {'BootSourceOverrideEnabled': 'Continuous', 'BootSourceOverrideTarget': 'Pxe', 'BootSourceOverrideMode': 'UEFI'}}, - etag='"3d7b838291941d"') + etag="81802dbf61beb0bd") def test_set_system_boot_source_no_mode_specified(self): self.sys_inst.set_system_boot_source( @@ -510,7 +512,7 @@ class SystemTestCase(base.TestCase): '/redfish/v1/Systems/437XR1138R2', data={'Boot': {'BootSourceOverrideEnabled': 'Once', 'BootSourceOverrideTarget': 'Hdd'}}, - etag=None) + etag='81802dbf61beb0bd') def test_set_system_boot_source_invalid_target(self): self.assertRaises(exceptions.InvalidParameterValueError, @@ -533,7 +535,8 @@ class SystemTestCase(base.TestCase): self.sys_inst.set_indicator_led(sushy.IndicatorLED.BLINKING) self.sys_inst._conn.patch.assert_called_once_with( '/redfish/v1/Systems/437XR1138R2', - data={'IndicatorLED': 'Blinking'}) + data={'IndicatorLED': 'Blinking'}, + etag='81802dbf61beb0bd') invalidate_mock.assert_called_once_with() diff --git a/sushy/tests/unit/resources/test_settings.py b/sushy/tests/unit/resources/test_settings.py index 84392e6b..32628ad5 100644 --- a/sushy/tests/unit/resources/test_settings.py +++ b/sushy/tests/unit/resources/test_settings.py @@ -78,7 +78,8 @@ class SettingsFieldTestCase(base.TestCase): instance.commit(conn, {'Attributes': {'key': 'value'}}) conn.patch.assert_called_once_with( '/redfish/v1/Systems/437XR1138R2/BIOS/Settings', - data={'Attributes': {'key': 'value'}}) + data={'Attributes': {'key': 'value'}}, + etag='9234ac83b9700123cc32') def test_get_status_failure(self): instance = self.settings._load(self.json, mock.Mock())