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
37 stars 33 forks source link

Verification of MessageId on log service #586

Open edtanous opened 1 month ago

edtanous commented 1 month ago

The intent of LogEntry.MessageId is that it: A. References an entry in a known registry. B. Matches the message string (with arguments populated), Severity, and Resolution string of that given entry.

Are these two things that Redfish-Service-Validator should verify the correctness of?

mraineri commented 1 month ago

Strictly for "Redfish events", A is correct, and B is sort-of correct. LogEntry can be used to cover more than just Redfish events defined in a message registry, but this is easily checkable with the "EntryType" property.

Both Severity and Resolution do not need to be used as-is from the message registry; services are allowed to replace these for an event where it can fill in something more appropriate. The same message in different contexts can have different severities and resolutions.

The message should match the registry, but keep in mind we might fix message strings in errata releases of registries, but the MessageId does not include errata version info, so it may be problematic to enforce.

Now... Should we be checking this... I think it's a reasonable thing to do, but I'd be cautious about how a registry is "known" in the first place. I would hope at this point everyone populates /redfish/v1/Registries with pointers to messages registries to download.

edtanous commented 1 month ago

Just to make sure I understand, LogEntry could be used for many things, but IF it has a MessageId field (and accompanying EntryType), it should match up with a registry. Sound right?

services are allowed to replace these for an event where it can fill in something more appropriate.

If this is the case, why are they in the Registry at all?

I would hope at this point everyone populates /redfish/v1/Registries with pointers to messages registries to download.

Right, the thinking would be that a validator implementation would, upon finding a LogEntry, go look up the Registry in /redfish/v1/Registries to make sure it's there, and that the MessageId matches an entry in the registry with the appropriate prefix.

edtanous commented 1 month ago

we might fix message strings in errata releases of registries

Thinking about this a little more, in the case this happens, the implementation under test should have the new errata registry in its RegistryCollection, and have coded against the new strings, right? I can't imagine a case where we would call it a "correct" implementation to have a message string that didn't match the registry it had published, regardless of the version.

mraineri commented 1 month ago

If this is the case, why are they in the Registry at all?

Default values in case the service has nothing better to use, or doesn't even provide those properties in responses.

Right, the thinking would be that a validator implementation would, upon finding a LogEntry, go look up the Registry in /redfish/v1/Registries to make sure it's there, and that the MessageId matches an entry in the registry with the appropriate prefix.

Caching the contents like it does with schema files might scale better, but generally yes.

Thinking about this a little more, in the case this happens, the implementation under test should have the new errata registry in its RegistryCollection, and have coded against the new strings, right?

I would hope so! That is the correct thing to do.

edtanous commented 1 month ago

Default values in case the service has nothing better to use, or doesn't even provide those properties in responses.

I went to go post a "We should document this in the schema" message, but it's already documented, I must've just missed it in the past.

Caching

Yep, I omitted caching just to keep it simple in principle, but you're right, and implementation would very likely need to cache the registries upfront.

So it sounds like no issues, in theory we could add something like this to the validator.