DMTF / Redfish-Service-Validator

The Redfish Service Validator is a Python3 tool for checking conformance of any "device" with a Redfish service interface against Redfish CSDL schema
Other
38 stars 33 forks source link

Remove uri_check toggling for inAnnotation variable #539

Closed tomasg2012 closed 1 year ago

tomasg2012 commented 1 year ago

Removes portion of code that removes URI checking on specific instances of "inAnnotation" variable being set to True, which occurs when URIs exist in particular contexts (this would toggle ignore_uri_checks). This should not ignore URIChecks, and correctly set variables for standard URIs.

However, the reason for this code to exist is not explained, nor is it practical as it applies on a first-come basis, which labels some URIs as based on Annotations when they are standard URIs.

Also adds a line which makes StrictURI flag true outside of the UriCheck block, which only affects the program if ignore_uri_checks was True.

Fixes #538

mraineri commented 1 year ago

@plmanik please check out this branch too for the false positives you're seeing on your system

mraineri commented 1 year ago

Testing on my end looks good so far.

plmanik commented 1 year ago

@plmanik please check out this branch too for the false positives you're seeing on your system

Hi @mraineri , Thanks for immediate reply and making changes in code Is there any issue in response i shared earlier? In commit 5c29a4a244f17da7b07c742199d22b750c8e2d1d I'm able to see changes in error message only. If I use uri-remove-toggle branch, whether issue reported earlier(URI /redfish/v1/Systems/Capabilities does not match object ID of resource) will not come. I would like to confirm whether below response is correct or not. I hope it's false positive and it will be fixed in tool not to show as error URI /redfish/v1/Systems/Capabilities does not match object ID of resource { "@odata.context": "/redfish/v1/$metadata#ComputerSystem.ComputerSystem", "@odata.etag": ""1525957699"", "@odata.id": "/redfish/v1/Systems/Capabilities", "@odata.type": "#ComputerSystem.v1_16_0.ComputerSystem", "Boot": { "BootSourceOverrideEnabled@Redfish.AllowableValues": [ "Disabled", "Once", "Continuous" ], "BootSourceOverrideEnabled@Redfish.OptionalOnCreate": true, "BootSourceOverrideEnabled@Redfish.UpdatableAfterCreate": false, "BootSourceOverrideTarget@Redfish.AllowableValues": [ "None", "Pxe", "Floppy", "Cd", "Usb", "Hdd", "BiosSetup", "Utilities", "Diags", "UefiShell", "UefiTarget", "SDCard", "UefiHttp", "RemoteDrive", "UefiBootNext" ], "BootSourceOverrideTarget@Redfish.OptionalOnCreate": true, "BootSourceOverrideTarget@Redfish.UpdatableAfterCreate": false }, "Boot@Redfish.OptionalOnCreate": true, "Description": "Zone Capabilities", "Description@Redfish.OptionalOnCreate": true, "Description@Redfish.SetOnlyOnCreate": true, "HostName@Redfish.OptionalOnCreate": true, "HostName@Redfish.UpdatableAfterCreate": false, "Id": "Capabilities", "Links": { "ResourceBlocks@Redfish.RequiredOnCreate": true, "ResourceBlocks@Redfish.UpdatableAfterCreate": true }, "Links@Redfish.RequiredOnCreate": true, "Name": "Capabilities for the Zone", "Name@Redfish.RequiredOnCreate": true, "Name@Redfish.SetOnlyOnCreate": true }

ERROR - URI /redfish/v1/Systems/Self/Processors/DevType1_CPU0/AccelerationFunctions/Compression does not match object ID of resource { "@odata.context": "/redfish/v1/$metadata#AccelerationFunction.AccelerationFunction", "@odata.etag": ""1676287999"", "@odata.id": "/redfish/v1/Systems/Self/Processors/DevType1_CPU0/AccelerationFunctions/Compression", "@odata.type": "#AccelerationFunction.v1_0_2.AccelerationFunction", "FpgaReconfigurationSlots": [ "AFU0" ], "Id": "Compression", "Links": { "Endpoints@odata.count": 0, "PCIeFunctions@odata.count": 0 }, "Name": "Compression Accelerator", "PowerWatts": 15, "Status": { "Health": "OK", "State": "Enabled" }, "UUID": "38947555-7742-3448-3784-823347823834", "Version": "Green Compression Type 1 v.1.00.86" }

Thanks, Mani

mraineri commented 1 year ago

There is an additional commit in this PR that is needed (https://github.com/DMTF/Redfish-Service-Validator/pull/539/commits/2aa9849b39fa7f2bfc74d41d19a3cefb666f8f3f); this branch contains more than just the error message change you highlighted.

If the response you provided is based on the 2.2.5 release (or what's in the master branch), then you should run the tool from this branch to see if it fixes the issue. Your payloads looks correct to me.

mraineri commented 1 year ago

Merging out of cycle since this does eliminate more false positives.

plmanik commented 1 year ago

Hi @mraineri With the changes, error for /redfish/v1/Systems/Capabilities , /redfish/v1/Systems/Self/Processors/DevType1_CPU0/AccelerationFunctions/Compression not coming. Thanks for making changes.

Two new issues coming now and details below Error 1 When ID is different from URI. I hope ID can be different from URI segement ERROR - The Id property does not match the last segment of the URI /redfish/v1/AccountService/ExternalAccountProviders/RADIUS { "@odata.context": "/redfish/v1/$metadata#ExternalAccountProvider.ExternalAccountProvider", "@odata.etag": "\"1676381115\"", "@odata.id": "/redfish/v1/AccountService/ExternalAccountProviders/RADIUS", "@odata.type": "#ExternalAccountProvider.v1_1_2.ExternalAccountProvider", "AccountProviderType": "OEM", "Description": "RADIUS server settings", "Id": "RADIUS Server", "Name": "RADIUS Settings", { oem . .}

Error 2

ERROR - URI /redfish/v1/Systems/Self/Bios/SD does not match the following required URIs in Schema of Bios.v1_2_0.Bios

{ "@odata.context": "/redfish/v1/$metadata#Bios.Bios", "@odata.etag": "\"1676381115\"", "@odata.id": "/redfish/v1/Systems/Self/Bios/SD", "@odata.type": "#Bios.v1_2_0.Bios", "AttributeRegistry": "BiosAttributeRegistry0ACOR.0.58.0", "Attributes": { "ACPI002": true, "ACPI004": false, . . }, "Description": "Future BIOS Settings", "Id": "SD", "Name": "Future BIOS Settings" } Please let us know if fix required in tool or response data needs correction. Above errors were not coming earlier and coming only with Tool Version: 2.2.6

Thanks, Mani

mraineri commented 1 year ago

@plmanik the first error is legitimate. The Id property contains "RADIUS Server", but the URI segment is RADIUS. This violates the Redfish Spec (and is the intent of the test).

Please file an issue for the second error; "Settings" resources are intended to be exempt from this sort of checking.

mraineri commented 1 year ago

@plmanik issue #542 was opened already for the BIOS settings error you have.

plmanik commented 1 year ago

@plmanik the first error is legitimate. The Id property contains "RADIUS Server", but the URI segment is RADIUS. This violates the Redfish Spec (and is the intent of the test).

Please file an issue for the second error; "Settings" resources are intended to be exempt from this sort of checking. Thanks for immediate reply and support. Based on this link https://github.com/DMTF/Redfish-Service-Validator/pull/536#issuecomment-1426363731, i thought ID, URI can be different, but you mentioned error in next comment. So we need to make sure ID matches with URI. But assume SD(settingsdata) is applicable for many URI's , URI pattern may be different but ID will be always SD, is that ok?

/redfish/v1/Systems/Self/Bios/SD -> ID is SD /redfish/v1/Systems/Self/BootOptions/0000/SD -> ID is SD I hope ID value is same for many URI's since we need to use last part of URI as ID

Thanks, Mani

mraineri commented 1 year ago

@plmanik those fall into the second case where resources from the settings, capabilities, and action info annotations do not need to follow URI rules (issue #542 is already open for tracking it). However, other than that exception, it is a "shall" statement in the spec that the last segment of the URI matches "Id". This applies to resources within collections where the last segment of the URI pattern is a variable token denoted with { }.

The identifier term in the URI pattern shall match the Id string property for the corresponding resource, or the MemberId string property for the corresponding object within a resource.

plmanik commented 1 year ago

@plmanik those fall into the second case where resources from the settings, capabilities, and action info annotations do not need to follow URI rules (issue #542 is already open for tracking it). However, other than that exception, it is a "shall" statement in the spec that the last segment of the URI matches "Id". This applies to resources within collections where the last segment of the URI pattern is a variable token denoted with { }.

The identifier term in the URI pattern shall match the Id string property for the corresponding resource, or the MemberId string property for the corresponding object within a resource.

Ok, got it. We can use same ID for different URI resources and thanks for explaining well

Thanks, Mani