summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2022-06-02Make code compile on clang againEd Tanous4-6/+6
The usual updates to make code compile on clang again. Extra semicolons that have snuck in, missing inline and static definitions. Tested: Code compiles on clang. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id7f889de98cafaa89471d75ed3e3bb97ab3855cd
2022-06-02Allow boost url and url_view to be added to jsonEd Tanous2-0/+34
The latest version of nlohmann seems to have support for adding any arbitrary iterable object as an array in json. Unfortunately, because boost::urls::url produces at iterable of unsigned char, this means that trying to encode urls leads to something like: "@odata.id": [ 47, 114, 101, 100, 102 ] Which is super unhelpful in that it does this implicitly. Given this behavior, there are two options here, make it so that code doesn't compile, or rely on the adl_serializer to just do the expected thing. This patchset opts for the later, to simply to the reasonable behavior, and call string() on the url before loading it into the json. Tested: Unit tests passing. Fixes bug in subsequent patchset. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id2f49bc8bd7153a0ad0c0fa8be2e13ce7c538e7f
2022-06-01Expand query: reimplement the way to do subqueriesNan Zhou2-79/+114
For any expand query, the current implementation does all queries in a single MultiAsyncResp, where the code sends a bunch of requests without Query parameters. This makes it impossible to invoke efficient expand handlers, since efficent handlers will only be invoked when a query has $expand in its parameters. (Delegation only happens when the query contains query parameters) To solve it, in this commit, we proposed to send a bunch of requests **WITH** Query parameters in MultiAsyncResp. This makes "/redfish/v1/Chassis/chassis?expand=.($levels=2)" be able to invoke efficient expand handlers that we developed for sensors, which existing implementation can't do. This decreases latency by nearly 100 times (the improvement that efficient sensor expand handler provides) on real hardware which contains 5+ chassis and totally 220+ sensors. This commit aligns with future $select support well, since the recursive queries can add $select as part of the query parameters. With this commit, though we create multiple MultiAsyncResp objects memory doesn't increase significantly; part of the reason is that we are not copying Query anymore in MultiAsyncResp. No out-of-memory issues are found when 4 threads are querying expand=levels=6 at the service root on a real large hardware which contains 2+ sockets, 5+ chassis, 220+ sensors, 30+ DIMMs, etc. Tested: 1. On real hardware, /redfish/v1/Chassis?$expand=.(level=3) is giving the correct result and invokes efficient sensor Expand handler 2. stress test ``` for i in {1..4}; do echo "thread $i" wget -qO- 'http://localhost:18080/redfish/v1?$expand=*($levels=6)' > "/tmp/$i.log" & done for i in {1..1000}; do top -b -n 1 | grep bmcweb >> /tmp/bmcweb_ori.log sleep 1 done ``` Results ``` 25878 2856 root R 194m 20% 1 38% /tmp/bmcweb_after 19005 2856 root R 215m 22% 1 36% /tmp/bmcweb_ori ``` Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I0e661db0263f56dd0cab66047a0a5d4fff31b69a
2022-06-01SensorCollection: use inline functions+bind_frontNan Zhou1-39/+42
This commit changes the `/redfish/v1/Chassis/<str>/Sensors/` route to take std::bind_front instead of lambdas. We can clearly see the indent levels decrease. It increases the readability. Tested: 1. trivial change; code compiles. 2. tested on my local mock environment; URL:/redfish/v1/Chassis/fake_chassis/Sensors/ Response: { "@odata.id": "/redfish/v1/Chassis/fake_chassis/Sensors", "@odata.type": "#SensorCollection.SensorCollection", "Description": "Collection of Sensors for this Chassis", "Members": [ { "@odata.id": "/redfish/v1/Chassis/fake_chassis/Sensors/sensor0" }, { "@odata.id": "/redfish/v1/Chassis/fake_chassis/Sensors/sensor1" }, { "@odata.id": "/redfish/v1/Chassis/fake_chassis/Sensors/sensor5" }, { "@odata.id": "/redfish/v1/Chassis/fake_chassis/Sensors/sensor6" } ], "Members@odata.count": 4, "Name": "Sensors" } 3. Service Validator Passes *** /redfish/v1/Chassis/fake_chassis/Sensors Type (SensorCollection.SensorCollection), GET SUCCESS (time: 0:00:00.002345) Attempt 1 of /redfish/v1/Chassis/fake_chassis/Sensors/sensor0 Response Time for GET to /redfish/v1/Chassis/fake_chassis/Sensors/sensor0: 0.006815780187025666 seconds. Attempt 1 of /redfish/v1/Chassis/fake_chassis/Sensors/sensor1 Response Time for GET to /redfish/v1/Chassis/fake_chassis/Sensors/sensor1: 0.004200570052489638 seconds. Attempt 1 of /redfish/v1/Chassis/fake_chassis/Sensors/sensor5 Response Time for GET to /redfish/v1/Chassis/fake_chassis/Sensors/sensor5: 0.004602659028023481 seconds. Attempt 1 of /redfish/v1/Chassis/fake_chassis/Sensors/sensor6 Response Time for GET to /redfish/v1/Chassis/fake_chassis/Sensors/sensor6: 0.00432420102879405 seconds. PASS Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Ibdebd9b5427db5b42d5047367ae8548fa981ddea
2022-06-01sensors: use inline functions + bind_frontNan Zhou1-77/+80
This commit changes the `/redfish/v1/Chassis/<str>/Sensors/<str>/` route to take std::bind_front instead of lambdas. We can clearly see the indent levels decrease. It increases the readability. Tested: 1. trivial change; code compiles. 2. tested on my local mock environment; URL: /redfish/v1/Chassis/fake_chassis/Sensors/sensor0 Response: { "@odata.id": "/redfish/v1/Chassis/fake_chassis/Sensors/sensor0", "@odata.type": "#Sensor.v1_0_0.Sensor", "Id": "sensor0", "Name": "sensor0", "Reading": 0.0, "ReadingRangeMax": null, "ReadingRangeMin": null, "ReadingType": "Current", "ReadingUnits": "A", "Status": { "Health": "OK", "State": "Enabled" } } 3. Service Validator Pass *** /redfish/v1/Chassis/fake_chassis/Sensors/sensor0 Type (Sensor.v1_0_0.Sensor), GET SUCCESS (time: 0:00:00.007105) PASS Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Ic60521a937a8b18d317390fc75d792c58f56e3e6
2022-06-01Move redfish/v1 instantiationEd Tanous3-3/+5
Make /redfish/v1 get instantiated in the same place as the other redfish routes, and not in main(). Tested: curl -vvvv --insecure --user root:0penBmc https://192.168.7.2/redfish returns the same value as previously. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Icb93954c00a4cf41708f1b323ddbd83e61146e5d
2022-06-01Try to fix the lambda formatting issueEd Tanous60-14690/+13905
clang-tidy has a setting, LambdaBodyIndentation, which it says: "For callback-heavy code, it may improve readability to have the signature indented two levels and to use OuterScope." bmcweb is very callback heavy code. Try to enable it and see if that improves things. There are many cases where the length of a lambda call will change, and reindent the entire lambda function. This is really bad for code reviews, as it's difficult to see the lines changed. This commit should resolve it. This does have the downside of reindenting a lot of functions, which is unfortunate, but probably worth it in the long run. All changes except for the .clang-format file were made by the robot. Tested: Code compiles, whitespace changes only. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43
2022-05-31Document prior experiences with headersEd Tanous1-0/+50
Several developers have spent time on the problem of "bmcweb makes too many uses of headers". This document is to attempt to document those cases, such that others don't duplicate time. If this document is successful, it will eventually be deleted when we solve this issue. Tested: Documentation only. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I64c84100abbf542d68925060c2f4fe6f6bff1402
2022-05-31meson option: make the insecure-disable-auth macro more accurateNan Zhou4-13/+13
The "auth" term is overloaded in meson option and macros. This commit changes the macro from BMCWEB_INSECURE_DISABLE_AUTHENTICATION to BMCWEB_INSECURE_DISABLE_AUTHX, given that if "insecure-disable-auth" is enabled, both authentication and authorization are disabled. Tested: 1. set 'insecure-disable-auth=enabled', no authz nor authn is performed, no crash on AccountService as well. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Iddca1f866d16346bcc2017338fa6f077cb89cef9
2022-05-27memory: set @odata attributes only if the object is foundNan Zhou1-5/+5
The existing code returns a JSON payload with @odata attributes even if it is a 404 not found. This commit corrects that by moving @odata after the object is found. Tested: 1. before ``` { "@odata.id": "/redfish/v1/Systems/system/Memory/dimm5", "@odata.type": "#Memory.v1_11_0.Memory", "error": { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "The requested resource of type Memory named 'dimm5' was not found.", "MessageArgs": [ "Memory", "dimm5" ], "MessageId": "Base.1.11.0.ResourceNotFound", "MessageSeverity": "Critical", "Resolution": "Provide a valid resource identifier and resubmit the request." } ], "code": "Base.1.11.0.ResourceNotFound", "message": "The requested resource of type Memory named 'dimm5' was not found." } } ``` after ``` { "error": { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "The requested resource of type Memory named 'dimm5' was not found.", "MessageArgs": [ "Memory", "dimm5" ], "MessageId": "Base.1.11.0.ResourceNotFound", "MessageSeverity": "Critical", "Resolution": "Provide a valid resource identifier and resubmit the request." } ], "code": "Base.1.11.0.ResourceNotFound", "message": "The requested resource of type Memory named 'dimm5' was not found." } } ``` 2. Service Validator on MemoryResource passes. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Id0f912015b0ecf25cacb22e919ebe88708187677
2022-05-27Enable -Wno-psabi for bmcweb buildsEd Tanous1-0/+1
This gcc warning is just a warning, and not a problem in bmc usages as we don't rely on abi. Having it in clogs the gcc logs when triaging other things. Tested: Compiled with another compiler error present. Didn't see "This behavior changed in gcc 7.1" warnings. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I2bd1ec8a774fdce15557d6344a03f4321df6d95a
2022-05-27Include-what-you-use in http connectionEd Tanous1-0/+4
Lots of #includes were missing in this file that we tangentially got through boost/beast/websocket.hpp. Tested: Code builds. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iac5198f2f65eabaecf47d0fb6bb05bfa5a261f32
2022-05-26auth: change authorization.hpp to authentication.hppNan Zhou2-6/+6
The existing authorization header is actually doing "authentication" work. The authorization is happening in routing.hpp where we fetch the role of the authenticated user and get their privilege set. This commits changes the name of the file, as well as the namespace, to be more precise on what the file actually does. Tested: 1. Trivial change, it builds Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Ib91ed70507a7308522c7e5363ed2f4dc279a19d9
2022-05-26health: take json_ptr instead of reference when filling statusNan Zhou3-7/+18
The existing codes populates the health status on the |AsyncResponse| or a given JSON reference. This doesn't work if we want to populates status on an array of objects, since the array can be resized which changes the address of each object. This commit changed the contructor to take a JSON pointer instead. |HealthPopulate| will populates status on |AsyncResponse->res.jsonValue|[json_ptr]. If the point can't be resolved in the |jsonValue|, |HealthPopulate| populates nothing. Fixed all places where the old reference based constructor is used. This commit is extremely useful when implementing efficient level-1 expand handler on ResourceCollections. It also prevents issues on reference lifecycles. Tested: 1. It builds 2. Tested DIMM/System/Storage health on real hardware, works as expected 3. Tested on Redfish Service Validator, no new failures on health properties. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I305515522af50b48be92a3f4689d8166f3bc0cc0
2022-05-26memory: move Partition codes from callbacks into separate functionsNan Zhou1-61/+65
Another change to move codes from callbacks to functions in the memory resource. It is a bit cleaner to have separate functions rather than keep codes in the callback, as callback normally have deeper indent. The main reason is that this helps code review of later changes that make Expand at MemoryCollection efficient. Tested: 1. on my mockup environment; added partition data into the fake dimm; URL /redfish/v1/Systems/system/Memory/dimm0 ``` { "@odata.id": "/redfish/v1/Systems/system/Memory/dimm0", "@odata.type": "#Memory.v1_11_0.Memory", "AllowedSpeedsMHz": [], "BaseModuleType": "RDIMM", "BusWidthBits": 0, "CapacityMiB": 1024, "DataWidthBits": 0, "ErrorCorrection": "NoECC", "FirmwareRevision": "0", "Id": "dimm0", "Name": "DIMM Slot", "OperatingSpeedMhz": 0, "RankCount": 0, "Regions": [ { "MemoryClassification": "", "OffsetMiB": 0, "PassphraseEnabled": false, "RegionId": "", "SizeMiB": 1024 } ], "Status": { "Health": "OK", "HealthRollup": "OK", "State": "Enabled" } } ``` 2. No new Redfish Validator failures on MemoryCollection on real hardware. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I27b251ff32bab026d6fa919abf7b6dcf2905e4a3
2022-05-25Remove messages::operationFailed from processorEd Tanous1-7/+0
A file write error being returned from dbus is by definition, an internal error that the user can do nothing about, so it should be returning internal error (ie 500) to the user, rather than OperationFailed. OperationFailed refers to aggregation proxy use cases, so its use here, while an understandable mistake, because dbus is arguably a "proxy", is incorrect, and should be fixed. Tested: Code compiles, no good way to test error cases. Inspection only. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I01aae6bcf377d019ff6e868309f87959281a2156
2022-05-25Change operationFailed to return 502Ed Tanous1-1/+1
Redfish base registry for operation failed says: "Indicates that one of the internal operations necessary to complete the request failed. Examples of this are when an internal service provider is unable to complete the request, such as in aggregation or RDE." In terms of return codes, this translates to 502, Bad Gateway, given that we're talking about proxy behavior here. There is currently one usage of messages::operationFailed, which arguably should've used internalErrror, and will be fixed in the next patchset. Tested: Code compiles. No (correct) users. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iff20e04d4f297b9f6595039f76321d0927f86c4d
2022-05-25ethernet: Remove PATCH VLANId supportJiaqing Zhao1-14/+11
phosphor-networkd does not support changing VLAN ID of an existing VLAN interface. Though the DBus property can be updated, the change never takes effect. This patch disallows PATCH VLANId requests. Tested: PATCH {"VLANId": 3} /redfish/v1/Managers/bmc/EthernetInterfaces/eth0 /VLANs/eth0_1 returns PropertyNotWritable error. Change-Id: Ice43064de761d63aa3cfde8019e5d4db138fcf02 Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
2022-05-24ethernet: Use std::optional<uint32> for VLAN IDJiaqing Zhao1-7/+7
According to Redfish EthernetInterface and VLanNetworkInterface schema, VLANId is "The ID for this VLAN", meaning that each interface can only have at most one VLAN ID. (Though EthernetInterface schema says "If this interface supports more than one VLAN, the VLAN collection link shall be present", the collection link is depracated in 1.7.0 and the spec suggests "using individual EthernetInterface resources to show VLAN information".) OpenBMC network stack implementation uses linux's virtual interface to represent a VLAN (named as <interface-name>.<vlan-id>, e.g. eth0.100). In both design and implementation, an interface can have either zero or one VLAN ID. This patch replaces the std::vector for VLAN ID with std::optional to match the design. It has no impact on the Redfish response. Tested: Verified GET /redfish/v1/Managers/bmc/EthernetInterfaces/eth0 can list all VLANs on eth0, and GET, PATCH and DELETE /redfish/v1/Managers/bmc /EthernetInterfaces/eth0/VLANs/eth0_1 works. Change-Id: Iab05e859d76639b2e60546cd5549efd34effafb7 Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
2022-05-24bmcweb: fixes virtual media buffer overflowTroy Lee1-2/+3
The bmcweb is implementated as async i/o access, sometimes the input buffer still has unprocessed data, and the next websocket message comes in. The input buffer originally reserved only 1 nbd request packet size, so it will cause buffer overflow exception. Extend the buffer size and correctly check the remaining buffer size. v8: fix coding style v7: remove debug log and proxy.wait() change to keep this change simple v4: fix coding style v3: fix coding style v2: fix coding style Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> Change-Id: I8df2445503393f63401678d9f2486a80d31aee16
2022-05-24Fix segmentation fault when deleting the sessionswukaihua-fii-na1-1/+2
Fix the segmentation fault caused by deleting the sessions via Redfish. Do not compare the username when deleting the sessions with no-auth. Tested: Delete the session via Redfish and bmcweb not crashed Signed-off-by: wukaihua-fii-na <eason.kh.wu@fii-na.com> Change-Id: I7f5268e7243a22ba5010ba5b8b4c82f19b8b4f20
2022-05-23bmcweb: Remove hardcoded HTTP verbs and headersCarson Labrado2-44/+43
Modifies HttpClient so that the HTTP verb and headers can be set for each individual message sent. Right now those fields are set when a connection is first created and then reused by each message sent using that connection. Tested: Launched two Event Listener servers that created 6 and 2 subscriptions. Sending a test event resulted in the servers receiving 6 requests and 2 requests, respectively. Change-Id: I8d7e2d54385bc2c403498293820adb584bff8b57 Signed-off-by: Carson Labrado <clabrado@google.com> Signed-off-by: Ed Tanous <edtanous@google.com>
2022-05-23bmcweb: Fetch Satellite Config from D-BusCarson Labrado4-1/+247
Adds a RedfishAggregator class which is able to pull configuration information from D-Bus for Satellite BMCs. These BMCs will be aggregated by Redfish Aggregation. Also added is a new compiler option which will be used to enable Redfish Aggregation. This patch only allows configurations with unencrypted and unauthenticated satellite BMC communication. Support for encryption and authentication willneed to be added in future patches. Note that this patch does not actually use the config information after it has been fetched. That functionality will be added in future patches. Tested: I made this example config information available on D-Bus busctl introspect xyz.openbmc_project.EntityManager \ /xyz/openbmc_project/inventory/system/board/SatelliteBMC/aggregated0 \ xyz.openbmc_project.Configuration.SatelliteController NAME TYPE SIGNATURE RESULT/VALUE FLAGS .AuthType property s "None" emits-change .Hostname property s "127.0.0.1" emits-change .Name property s "aggregated0" emits-change .Port property t 443 emits-change .Type property s "SatelliteController" emits-change That information was picked up by the changes in this CL: [DEBUG "redfish_aggregator.hpp":80] Found Satellite Controller at /xyz/openbmc_project/inventory/system/board/SatelliteBMC/aggregated0 [DEBUG "redfish_aggregator.hpp":209] Added satellite config aggregated0 at http://127.0.0.1:443 [DEBUG "redfish_aggregator.hpp":52] Redfish Aggregation enabled with 1 satellite BMCs [DEBUG "redfish_aggregator.hpp":21] There were 1 satellite configs found at startup Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: Ib5eee2c93aeb209157191055975c127759d73627
2022-05-23Include tinyxml2 dependency for all buildsCody Smith1-7/+5
Tinyxml2 needs to be included in all cases now. The current Meson build will fail unless tinyxml2 is installed locally. given the inclusion of tinyxml2.xml in dbus_monitor.hpp. In the past, the 'rest' option was enabled by default, so this bug wasn't hit very often in practice. Now that rest is disabled, this bug is much more apparent. Tested: Code compiles Signed-off-by: Cody Smith <scody@google.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I258356151ae0226c7b08a80ef78b043bc90731bc
2022-05-20google_api: Fix build issueJiaqing Zhao1-1/+2
Commit 4cee35e ("Add RootOfTrustCollection and RootOfTrust under Google service root.") still uses the crow::openbmc_mapper::GetSubTreeType removed in b9d36b4 ("Consitently use dbus::utility types"), causing build failure. This patch fixes the build issue by using the dbus::utility::MapperGetSubTreeResponse instead. Tested: Build pass. Change-Id: Ia2ca965f320ef18f431bfcb6d62c9c44eb935d9d Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
2022-05-20Add RootOfTrustCollection and RootOfTrust under Google service root.Vidya Satyamsetti8-14/+426
These are Google only resources powered by Hoth DBus interface. The ComponentsProtected links is hardcoded for now. But it will be queried from DBus and interpreted accordingly in the future. TEST: $curl -u root:0penBmc -X GET http://[::1]:$PORT/google/v1/RootOfTrustCollection { "@odata.id": "/google/v1/RootOfTrustCollection", "@odata.type": "#RootOfTrustCollection.RootOfTrustCollection", "Members": [ { "@odata.id": "/google/v1/RootOfTrustCollection/Hoth" } ], "Members@odata.count": 1 } $ curl -u root:0penBmc -X GET http://[::1]:$PORT/google/v1/RootOfTrustCollection/Hoth { "@odata.id": "/google/v1/RootOfTrustCollection/Hoth", "@odata.type": "#RootOfTrust.v1_0_0.RootOfTrust", "Actions": { "#RootOfTrust.SendCommand": { "target": "/google/v1/RootOfTrustCollection/Hoth/Actions/RootOfTrust.SendCommand" } }, "Id": "Hoth", "Location": { "PartLocation": { "ServiceLabel": "Hoth", "Locationtype": "Embedded" } }, "Name": "RootOfTrust-Hoth", "Status": { "State": "Enabled" } $ curl -u root:0penBmc -X POST -d @req.json -H "Content-Type: application/json" http://[::1]:$PORT/google/v1/RootOfTrustCollection/Hoth/Actions/RootOfTrust.SendCommand { "CommandResponse": "033B0000" } Signed-off-by: Vidya Satyamsetti <satyamsetti@google.com> Change-Id: If64612468bb89e6d9251d848697608b7daf37339
2022-05-19memory: move long code blocks in callbacks into separate functionsNan Zhou1-267/+257
It is a bit cleaner to have separate functions rather than keep codes in the callback, as callback normally have deeper indent. The main reason is that this helps code review of later changes that make Expand at MemoryCollection efficient. Tested: 1. Tested on my mock environment, ``` URI: /redfish/v1/Systems/system/Memory/dimm0 { "@odata.id": "/redfish/v1/Systems/system/Memory/dimm0", "@odata.type": "#Memory.v1_11_0.Memory", "AllowedSpeedsMHz": [], "BaseModuleType": "RDIMM", "BusWidthBits": 0, "CapacityMiB": 1024, "DataWidthBits": 0, "ErrorCorrection": "NoECC", "FirmwareRevision": "0", "Id": "dimm0", "Name": "DIMM Slot", "OperatingSpeedMhz": 0, "RankCount": 0, "Status": { "Health": "OK", "HealthRollup": "OK", "State": "Enabled" } } ``` 2. No new Redfish Validator failures on MemoryCollection on real hardware. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I7693388049aeffa6ebd285b958e5ca6622e5d3b6
2022-05-19Change query param errors to be 400 instead of 403Ed Tanous1-2/+2
Any query param errors from the base registry being sent imply that the user-provided parameters were bad, which should return bad request, not forbidden. This is in line with the spec. Luckily, the only usage of these parameters as of now is from within the query params support model, so changing it shouldn't cause any backward compatibility issues, and because these are meant for "bad request" type messages, it's unlikely it would even be able to effect clients. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I13a4601d1236d9eaac6bbf5fb25e0f1b28c04a21
2022-05-17Add callback for response handling to HttpClientCarson Labrado2-12/+56
Adds sendDataWithCallback() which allows the caller to include a callback specifying how to handle the response to that request. This will be utilized for Redfish Aggregation including returning the responses received when forwarding requests to satellite BMCs. Change-Id: I93826c8b254a5f28a982295d4145453352a90fae Signed-off-by: Carson Labrado <clabrado@google.com>
2022-05-17Handle HEAD and Allow headers per the specEd Tanous3-11/+80
The Redfish specification calls out that the Allow header should be returned for all resources to give a client an indication of what actions are allowed on that resource. The router internally has all this data, so this patchset allows the router to construct an allow header value, as well as return early on a HEAD request. This was reverted once here: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/53637 Due to a redfish validator failure. With the previous patches workaround, this error has now been resolved. Tested: Called curl with various parameters and observed the Allow header curl -vvvv --insecure -X <VERB> --user root:0penBmc https://<bmc>/url HEAD /redfish/v1/SessionService/Sessions returned Allow: GET, POST HEAD /redfish/v1 returned Allow: GET HEAD /redfish/v1/SessionService returned Allow: GET, PATCH POST /redfish/v1 returned Allow: GET (method not allowed) GET /redfish/v1 returned Allow: GET GET /redfish/v1/SessionService returned Allow: GET, PATCH Redfish-Protocol-Validator now reports more tests passing. Prior to this patch: Pass: 255, Warning: 0, Fail: 27, Not tested: 45 After this patch: Pass: 262, Warning: 0, Fail: 21, Not tested: 43 Diff: 7 more tests passing All tests under RESP_HEADERS_ALLOW_METHOD_NOT_ALLOWED and RESP_HEADERS_ALLOW_GET_OR_HEAD are now passing Included unit tests passing. Redfish service validator is now passing. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ibd52a7c2babe19020a0e27fa1ac79a9d33463f25
2022-05-17Fake out allow header in UpdateServiceEd Tanous1-0/+9
In an attempt to smooth out the transition on changing the URI for /redfish/v1/UpdateService:HttpPushUri, this patch modifies the allow header on the /redfish/v1/UpdateService to return the "wrong" value, and remove POST from the list of allowed verbs. While this is technically incorrect, this field is new, so there can't be any users relying on it, and if they were to use it, they would ideally code to HttpPushUri as the spec would suggest. Tested: Redfish-service-validator passes. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ida3bdb772646253af5376bc8e0e13db285048b93
2022-05-17Change UpdateService POST URIEd Tanous3-1/+38
As d01e32c3786f2fbbb70c9724a87cf979b4a06232 found, the Redfish specification doesn't allow a direct POST handler on UpdateService. Ideally clients would be following the specification, and relying on the HttpPushUri as the spec requires, so we could simply make this change. Unfortunately, a quick polling of the community shows that a significant number of instances, including the Redfish cheat sheet, and the robot tests, have hardcoded the non-spec behavior. This commit is present to give a trap door to allow easier porting of this behavior to the specification. The old uri is left, and now returns a WARNING http field, indicating that the uri is deprecated, in case clients have ignored the Redfish specification. Tested: Ran firmware update instructions from https://gerrit.openbmc-project.xyz/c/openbmc/docs/+/53664 Test gave the same result as previously. /redfish/v1/UpdateService returns an HttpPushUri that matches the above. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I7427f461d151c9460160b0b9b366dca5aefc49d5
2022-05-17Move update service post to free methodEd Tanous1-23/+26
Refactor the update service post method in a similar way to how we've done elsewhere. This is done to enable a refactor later. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iaa16b5f07fbdbbd1ffc244d5f3e94aa5efa39ad0
2022-05-16Fix regression in ComputerSystemEd Tanous1-2/+5
Despite the pseudo non-plural name "Links.Chassis", the Redfish schema states that Chassis is an array of chassis, not a singular chassis. While odd, we should obey the specification. This resolves the regression introduced by 1476687de Tested: Code compiles, code inspection looks correct. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I4d6d0554865e412137c44562d173a6efe3f88f4a
2022-05-16log_services: fix wrong AdditionalDataURIPotin Lai1-2/+2
fix wrong AdditionalDataURI of EventLog entries Tested: [Before] $ curl -s -u root:0penBmc -k https://10.10.11.203/redfish/v1/Systems/system/LogServices/EventLog/Entries/60 |\ > python -c 'import sys, json; print(json.load(sys.stdin)["AdditionalDataURI"])' /redfish/v1/Systems/system/LogServices/EventLog/attachment/60 $ curl -s -u root:0penBmc -k https://10.10.11.203/redfish/v1/Systems/system/LogServices/EventLog/attachment/60 Not Found [After] $ curl -s -u root:0penBmc -k https://10.10.11.203/redfish/v1/Systems/system/LogServices/EventLog/Entries/60 |\ > python -c 'import sys, json; print(json.load(sys.stdin)["AdditionalDataURI"])' /redfish/v1/Systems/system/LogServices/EventLog/Entries/60/attachment $ curl -s -u root:0penBmc -k https://10.10.11.203/redfish/v1/Systems/system/LogServices/EventLog/Entries/60/attachment BQAAADwAAAAGAAAAEeFXk4ABAAAtAAAAAAAAAHh5ei5vcGVuYm1jX3Byb2plY3QuTG9nZ2luZy5TRUwuRXJyb3IuQ3JlYXRlZAYAAAAAAAAACwAAAAAAAABFVkVOVF9ESVI9MQ8AAAAAAAAAR0VORVJBVE9SX0lEPTMyDQAAAAAAAABSRUNPUkRfVFlQRT0yEgAAAAAAAABTRU5TT1JfREFUQT01MjAyQzE6AAAAAAAAAFNFTlNPUl9QQVRIPS94eXovb3BlbmJtY19wcm9qZWN0L3NlbnNvcnMvdm9sdGFnZS9QMTJWX0ZBTjEIAAAAAAAAAF9QSUQ9MzAxAAAAAAAAAAAAIAAAAAAAAAAyLjEyLjAtZGV2LTEwOTQtZ2MzNDk4NzlmNi1kaXJ0eRHhV5OAAQAAAAAAAAAAAAAAAAAAAAAAAA== Signed-off-by: Potin Lai <potin.lai@quantatw.com> Change-Id: I859638a942a0afcb57f68d6a6613d5c3498ab3be
2022-05-16Fix regression in brace initializationEd Tanous1-1/+1
1476687de introduced a regression because of a simple copy/paste transcription error. Unfortunately, dump logs aren't enabled on a majority of systems, so this typo wasn't caught in the tested statement for that commit, but was only caught in later CI. Tested: Code compiles, code inspection (regression resolution) Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I98297546be3ed624e21461edbe8c5781287787db
2022-05-16Return OData.Version with responsesEd Tanous1-0/+2
The Redfish specification section 8.1 lists header values that should be returned from responses. It lists OData.Version: 4.0 as required in this same section. Implement this to the specification. Tested: Redfish protocol validator RESP_HEADERS_ODATA_VERSION test now passes. curl --vvv https://$bmc/redfish/v1 shows Odata.Version header set in response. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id02ec218299524905fbd91cb161bfe727a51fc65
2022-05-13Move /redfish to free methodsEd Tanous1-8/+11
Similar to the refactors done elsewhere, move the /redfish route to a free method. Tested: curl /redfish/v1 returns the same payload as previously. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Icd7668e8e3779926508aa901959b2ca6d079c8f0
2022-05-13Add setUpRedfishRoute to /redfish/ routeEd Tanous1-2/+6
Because this file originally wasn't in redfish-core when we handled this site-wide change, it didn't get the additional redfish decorators added to it. This commit adds it. Tested: curl /redfish returns as before. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I89a01a8fd75ab081d031de34ced6c9ae9c0ca75a
2022-05-13LogService: Sort dump entries collection by IdClaire Weinan1-0/+6
Ordering by ID (represented internally as the last part of the D-Bus object path, after the rightmost slash) is done for human readability, but please note that Redfish clients should not be written in a way that assumes a particular ordering of entries in a collection. Without this change, entries are presented in the collection in whatever order entries are returned by the D-Bus method GetManagedObjects(), called in getDumpEntryCollection(). The effect of this change is that entries are presented in chronological order (by ID) with the earliest entry appearing first. Testing: 1. Prerequisite: Fixed createDump() locally, similar to https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/38954 2. Created 12 BMC dump entries by repeatedly calling CollectDiagnosticData: curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Managers/bmc/LogServices/Dump/Actions/LogService.CollectDiagnosticData -d '{"DiagnosticDataType":"Manager", "OEMDiagnosticDataType":"BMC"}' 3. Retrieved BMC dump entries collection and verified that entries were sorted by ID (1,2,3,4,5,6,7,8,9,10,11,12): curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Managers/bmc/LogServices/Dump/Entries Signed-off-by: Claire Weinan <cweinan@google.com> Change-Id: I99f96dd6679163cea443353ad0e4c8c750cd4330
2022-05-13Separate validFilename into a separate functionJosh Lehan3-2/+44
This will generalize it and make it callable from other places Tested: Added test cases, they pass Signed-off-by: Josh Lehan <krellan@google.com> Change-Id: I8df30d6fe6753a2454d7051cc2d8813ddbf14bad
2022-05-13Remove brace initialization of json objectsEd Tanous32-986/+1140
Brace initialization of json objects, while quite interesting from an academic sense, are very difficult for people to grok, and lead to inconsistencies. This patchset aims to remove a majority of them in lieu of operator[]. Interestingly, this saves about 1% of the binary size of bmcweb. This also has an added benefit that as a design pattern, we're never constructing a new object, then moving it into place, we're always adding to the existing object, which in the future _could_ make things like OEM schemas or properties easier, as there's no case where we're completely replacing the response object. Tested: Ran redfish service validator. No new failures. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iae409b0a40ddd3ae6112cb2d52c6f6ab388595fe
2022-05-13Move /bus/system/<str>/<path> POST to methodEd Tanous1-345/+317
Per the reorganization we've done elsewhere, move this large lambda function to simplify it. Tested: Code move only. Code compiles. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib0586b34809167120bdc127868706ac517db4474
2022-05-12Move redfish v1 into redfish-coreEd Tanous1-0/+0
This file has existed for a long time, and predates redfish-core, so it was put in an inopportune place. Move the code to where it should be, in lib. Tested: Code compiles. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I422c27563a5e0f2a5debb2b1d4713aa8db0fe331
2022-05-11Refactor HttpClient ClassCarson Labrado2-188/+457
Refactors HttpClient with the following changes: - Convert class to singleton - Replace circular buffers with devectors - Sending queued requests and closing connections handled within their own callback - Add connection pooling (max size 4) - HttpClient supports multiple connections to multiple clients - Retry policies can be set for specific use cases Also modifies its use in the Subscription class to be compatible with the refactored code. It is assumed that a BMC will be able to handle 4 parallel connections and thus the max pool size is set as 4. The max number of queued messages was left unchanged at 50. Eventually we may want to allow tuning of these limits to boost performance. That would come in a future patch. Tested: Launched two Event Listener servers that created 6 and 2 subscriptions. Sending a test event created a connection pool for each server. 4 and 2 connections were added to each pool, respectively and were used to send the test request. For the first pool the 2 extra requests were placed into a queue until connections became available. After a request completed, its associated connection was used to send the next request in the queue. Resending the test event caused those prior connections to be reused instead of new connections being added to the pools. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: Iba72b3e342cdc05d1fb972e2e9856763a0a1b3c5
2022-05-11Revert "Handle HEAD and Allow headers per the spec"Ed Tanous3-80/+11
This reverts commit 867b2056d44300db9769e0d0b8883435a179834c. Apparently we have broken the Redfish spec in a way that adding this feature now allows the service validator to find. @odata.id /redfish/v1/UpdateService ERROR - Allow header should NOT contain POST for UpdateService.v1_5_0.UpdateService Need to figure out what to do, but for now, revert to get the build passing again. Change-Id: Ieef20573b9caa03aba6fd2bbc999e517e4b7de3d Signed-off-by: Ed Tanous <edtanous@google.com>
2022-05-10Handle HEAD and Allow headers per the specEd Tanous3-11/+80
The Redfish specification calls out that the Allow header should be returned for all resources to give a client an indication of what actions are allowed on that resource. The router internally has all this data, so this patchset allows the router to construct an allow header value, as well as return early on a HEAD request. Tested: Called curl with various parameters and observed the Allow header curl -vvvv --insecure -X <VERB> --user root:0penBmc https://<bmc>/url HEAD /redfish/v1/SessionService/Sessions returned Allow: GET, POST HEAD /redfish/v1 returned Allow: GET HEAD /redfish/v1/SessionService returned Allow: GET, PATCH POST /redfish/v1 returned Allow: GET (method not allowed) GET /redfish/v1 returned Allow: GET GET /redfish/v1/SessionService returned Allow: GET, PATCH Redfish-Protocol-Validator now reports more tests passing. Prior to this patch: Pass: 255, Warning: 0, Fail: 27, Not tested: 45 After this patch: Pass: 262, Warning: 0, Fail: 21, Not tested: 43 Diff: 7 more tests passing All tests under RESP_HEADERS_ALLOW_METHOD_NOT_ALLOWED and RESP_HEADERS_ALLOW_GET_OR_HEAD are now passing Included unit tests passing. Change-Id: Ib99835050b15eb4f419bfd21375b26e4db74fa2c Signed-off-by: Ed Tanous <edtanous@google.com>
2022-05-10Implement $top and $skipEd Tanous2-1/+114
$top and $skip are parameters for controlling the responses of collections, to limit their size, per the Redfish specification section 7.4. $skip=integer "Applies to resource collections. Returns a subset of the members in a resource collection, or an empty set of members if the $skip value is greater than or equal to the member count. This paging query parameter defines the number of members in the resource collection to skip." $top=<integer> "Applies to resource collections. Defines the number of members to show in the response. Minimum value is 0 , though a value of 0 returns an empty set of members." This commit implements them within the resource query. Tested: curl --insecure --user root:0penBmc https://localhost:18080/redfish/v1/Registries\?\$top\=1 Returns 1 value. Walking through values of 1-5 (there are 4 registries currently) returns the appropriate sizes of collection (with 5 returning 4 entries). curl --insecure --user root:0penBmc https://localhost:18080/redfish/v1/Registries\?\$skip\=0 Returns the collection. $skip values of 0-5 return descending number of results. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ied8a8f8338f119173509fb4b7ba2bd4a6c49cae8
2022-05-06system: Handle PATCH request with multi-depth readJsonJiaqing Zhao1-67/+48
This reduces readJson calls and makes the code more readable, and saves ~164 bytes of compressed image size. Tested: Verified PATCH /redfish/v1/Systems/system is handled as expected. Change-Id: I90dd5e0d4b0b055ee370288ad159d26e5bb40281 Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
2022-05-06query: expand: fix a bug in inLinksNan Zhou2-5/+14
Old codes handle links incorrectly; when links appear before some keys, old codes don't expand these keys. Tested: 1. On real hardware, Expand with links are working correctly. 2. Unit tests. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I028b55579d833f23120987a24cef4442fdd5800d