<div dir="ltr"><div>On Mon, Jun 8, 2020 at 10:04 AM Richard Hanley <<a href="mailto:rhanley@google.com" target="_blank">rhanley@google.com</a>> wrote:<br>
> I recently started looking into using OpenAPI for Redfish as part of an internal project, and I want to try using some of that work in bmcweb.<br>><br>
> Right now the OpenAPI support for C++ doesn't mesh very well with nlohmann and boost::beast. Assuming that we can handle the integration, what concerns or objectections should I keep in mind?<br>
><br>
> When I brought the topic up a number of months ago, the biggest issue was increasing the flash size usage. If a solution is made that doesn't bloat the image, how do people feel about OpenAPI?<br>
><br>
> Any other thoughts or ideas on the topic?<br></div><div><br></div><div>
Way late on the reply to this one;  Meant to send it a while back and lost track of it.  I'm frankly surprised more people haven't replied, as this was a pretty popular topic a year or so ago, with several implementations of generated bindings being proposed.<br>
<br>
You might want to be more specific about what you're hoping to get working.  bmcweb exposes an openapi introspection endpoint today under /redfish/v1/JsonSchemas<br>
Source: <a href="https://github.com/openbmc/bmcweb/tree/master/static/redfish/v1/JsonSchemas" rel="noreferrer" target="_blank">https://github.com/openbmc/bmcweb/tree/master/static/redfish/v1/JsonSchemas</a><br>
<br>
I'm guessing you're wanting to generate c++ structure definitions for use within the server to ease serialization and deserialization of endpoint structures.  (If I'm incorrect in my assumption, disregard what follows)<br>
<br>
This has been tried by many, many people over the short span of time that bmcweb has existed;  Prior to the bmcweb-on-master times, there was also a competing web stack, written in go, that revolved around generating the server entirely from schema.   So far as I'm aware, nobody got past the "Look at this example" phase.  With that said, if you think it's needed, and would be better than what's there, by all means attempt it;  In the efforts of saving your time, I'd recommend talking to one of the people that tried in the past.  First off, one thing that would've helped the past attempts would be to be clear about what present problem you're trying to fix, and include some examples of code that you don't like, and what the equivalent code (once you're done) would look like.  I think that would help a lot of people provide input.  Full disclosure, several years ago I spent a day trying to do exactly this, and determined it was a non-starter, because it added a lot of complexity for some seemingly simple calls.<br>
<br>
I'm going to try to summarize the roadblocks that the various efforts hit (this is by no means a complete list)<br><div dir="auto">
1. The redfish OpenAPI is a complete definition of all properties and endpoints that _could_ exist.   The majority of the openapi model properties are specified as optional in the definition; Therefore, because bmcweb/OpenBMC implements a subset of these, there needs to be some kind of compile time binding spec that determines which of the optional properties bmcweb _will_ implement.  This also needs to be extended into "which enums will bmcweb support" and "what validation will we use on certain strings", as redfish is fairly loose in what it allows for a lot of things, like resource names, whereas dbus is more stringent.  Adding this auxiliary definition defeated most of the perceived complexity that OpenAPI would've improved upon.</div><br>
2. The generated code was far too verbose to inspect for security problems line by line.    In one case (I'm not picking on the person that did this if you're listening) python code read in the spec, and used Jinja to generate templated c++ structures that were then converted into serialize/deserialize functions for every endpoint.  Tracing code from a user given json parameter, to a type-safe c++ parameter that could be operated on was a mess, and it was difficult to inspect what kind of safety could be expected for a given variable/structure (example, could this string contain a unicode character that's disallowed on DBus?).  This could certainly be optimized and improved in newer attempts, but it's a lot of effort.</div><div><br>
3. So far as I'm aware, nobody even made it to being able to handle the PATCH case sanely, where most properties are essentially optional, and there's some binding changes between properties that might have an implied order.<br>
There have been cases in the past where we've misunderstood the intent behind the CSDL (XML) definition (before the openapi stuff existed), and implemented the wrong input parsing code.   The one that comes to mind was a case where we were accepting an object as a property directly, when it needed to be an array of properties (usually with size 1) to be compatible.  In the efforts to not break the people that had already coded against the "wrong" implementation, we now accept both, but warn on the former.  Implementing these rare corner backward compatibility cases in generic code tends to be very difficult.</div><div><br>
4. None of the efforts properly implemented the correct error message handling for missing/bad properties.  This is non-trivial, and about half of the readJson code (that you would likely be replacing) is simply mapping the schema violations to the correct Redfish error message.<br>
Binary sizes weren't great (as you've already noted) because they required redfish to have different parsing code than the rest of the system (dbus-rest, ect).  This is probably the least of my problems with the effort, and is something that's certainly solvable from a technical standpoint.<br>
Because OpenAPI would be run on endpoints that are not yet trusted (like PATCH /Sessions/) it adds all of the openAPI code to the high risk security envelope of unauthenticated requests, and increases the effort of those that manually have to audit the authentication code to meet security standards.</div><div><br>
5. Most of the efforts failed the Redfish schema validator for the endpoints they produced.  This could certainly be debugged and improved.</div><div><br></div><div>6. (opinion) As is, bmcweb already has a problem with submitters not testing their code.  This is why it now requires running the Redfish service validator on every change.  Having pre-generated bindings is likely to make developers assume that their schemas are correct, and are less likely to run a test beforehand.  With that said, most of the problems <br>
<br>
<br>
With all of that said, if you're looking to generate something, I think some low hanging fruit could be found in the permissions definitions.  As-is, there is an implicit expectation that the committer has checked the Privilege Registry file, and updated their permission definitions accordingly.  In practice, most commiters don't realize the privilege registry even exists, and simply copy a parameter from another schema.  This seems like something that's ripe for generated code to generate the privilege mappings that people could simply #include against.  While we're in the realm of the beach boys (wouldn't it be nice).  A tool that used the openapi definitions to "fuzz" redfish would be another great opportunity for generated code.</div><div><br></div><div><br></div><div>
-Ed<br><br>
</div>
</div>