summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2022-06-14kvm_websocket_test: delete the testNan Zhou1-114/+0
This test is not enabled and referenced anywhere. Per Discord discussion, this test is obsolete and can be deleted now. https://discord.com/channels/775381525260664832/855566794994221117/985996960840429568 """ bmcweb used to have the full RFB (VNC) server in it, and access the linux device directly, that's what that unit test was checking, but that code is long gone in lieu of what we have now (unix socket to the rfbserver) which is better. """ Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: If42801c15009f8b33ea5d15749a067dccda935e9
2022-06-14LogService: Fix setUpRedfishRoute() for dumpsClaire Weinan1-9/+7
There's an inconsistency between how setUpRedfishRoute() is called for BMC dump vs. System dump. In requestRoutesSystemDumpEntry(), setUpRedfishRoute() is called within getDumpEntryById, while in requestRoutesBMCDumpEntry() setUpRedfishRoute() is called before getDumpEntryById() as well as within getDumpEntryById(). The inconsistency was introduced in https://gerrit.openbmc.org/c/openbmc/bmcweb/+/52393/20/redfish-core/lib/log_services.hpp and seems to be accidental. This change removes setUpRedfishRoute() from getDumpEntryById() and makes requestRoutesSystemDumpEntry() call setUpRedfishRoute() before calling getDumpEntryById(). In addition to fixing the inconsistency, this change prevents setUpRedfishRoute() from being called twice for BMC dump. Tested: After creating System dump entry, retrieved it successfully with ./curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Entries/1 Saw “setup redfish route” message in journalctl: bmcweb[19717]: (2022-06-13 16:42:52) [DEBUG "routing.hpp":1294] Matched rule '/redfish/v1/Systems/system/LogServices/Dump/Entries/<str>/' 2 / 4 bmcweb[19717]: (2022-06-13 16:42:52) [DEBUG "query.hpp":19] setup redfish route Received the expected error when adding a query parameter for a non-collection resource, which indicates setUpRedfishRoute() had been called: ./curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Entries/1?\$skip=1 { "@odata.id": "/redfish/v1/Systems/system/LogServices/Dump/Entries/1", "@odata.type": "#LogEntry.v1_8_0.LogEntry", "AdditionalDataSizeBytes": 0, "AdditionalDataURI": "/redfish/v1/Systems/system/LogServices/Dump/Entries/1/attachment", "Created": "1970-01-01T00:27:35.135000+00:00", "DiagnosticDataType": "OEM", "EntryType": "Event", "Id": "1", "Name": "System Dump Entry", "OEMDiagnosticDataType": "System", "error": { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "Querying is not supported on the requested resource.", "MessageArgs": [], "MessageId": "Base.1.11.0.QueryNotSupportedOnResource", "MessageSeverity": "Warning", "Resolution": "Remove the query parameters and resubmit the request if the operation failed." } ], "code": "Base.1.11.0.QueryNotSupportedOnResource", "message": "Querying is not supported on the requested resource." } } Repeated the same testing for a BMC dump entry. Signed-off-by: Claire Weinan <cweinan@google.com> Change-Id: I41ea93bfc6971a775241a368491e4615295cc4db
2022-06-13storage: Refactor Drive and StorageController into separate methodWilly Tu1-169/+173
Cleanup dbus path with with object_path.filename() to determine to get the resource ID/Name. There will be no changes to the redfish tree with this change. This change help manage the Drive/Storage functions by diving it up into smaller function with no functional changes. TESTED: Redfish Validation passed for Storage Storage Get ``` wget -qO- http://localhost:80/redfish/v1/Systems/system/Storage/storage0 { "@odata.id": "/redfish/v1/Systems/system/Storage/storage0", "@odata.type": "#Storage.v1_7_1.Storage", "Drives": [ { "@odata.id": "/redfish/v1/Systems/system/Storage/storage0/Drives/drive0" } ], "Drives@odata.count": 1, "Id": "1", "Name": "Storage", "Status": { "Health": "OK", "HealthRollup": "OK", "State": "Enabled" }, "StorageControllers": [ { "@odata.id": "/redfish/v1/Systems/system/Storage/storage0#/StorageControllers/0", "@odata.type": "#Storage.v1_7_0.StorageController", "MemberId": "cpld", "Name": "cpld", "Status": { "Health": "OK", "HealthRollup": "OK", "State": "Enabled" } } ] } ``` Change-Id: I334e2233f42efc8e390a410493d7594d5d81bd4e Signed-off-by: Willy Tu <wltu@google.com>
2022-06-13Add PCIeSlots schemas to the BMCEd Tanous6-1/+656
We plan on using these schemas, add them to the allow list. Tested: Code generated. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I4d2cc4d580115bbaf2445469577940984e3045f0
2022-06-13Document common error of replacing the json objectEd Tanous1-0/+29
Prior to having query params and aggregation supported, this wasn't much of an issue, but now that we have these features, we need to code against doing multiple things in parallel. We have had cases in the past prior to these new features, but these were generally localized to a single handler, so the fixes tended to go under the radar. This commit documents this common pitfall, and propose a solution. The commit below has already done most of the work to make the codebase consistent in this regard. Documenting this will hopefully ensure that regressions don't happen again, or if they do, we have documentation that we can point to. 1476687d Remove brace initialization of json objects Tested: Documentation only. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id9e761987fd90955218fbb232b277a08b0227339
2022-06-12Add initial documentation about bmcweb clientsEd Tanous1-0/+82
bmcweb has a number of clients that it interacts with. Many times there have been questions about what clients are available to test with, and what the expected level of compatibility is. This commit attempts to start documenting it. It should not be considered complete, but aims to start documenting what the expected level of support one could expect when running the matrix of bmcweb vs particular clients, and, as we get more evolved, what the status is for the CI integration of said clients tests. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ifdb34b951d366a74d7255ce41a6df12532f22b2f
2022-06-11On logout set Session cookie with expired dateGunnar Mills1-0/+5
The Session cookie is an HttpOnly cookie. HttpOnly means the cookie cannot be accessed through client side script because of this the GUI can not delete this cookie on log out. Recommendation online was setting this cookie to an expired date. From https://tools.ietf.org/search/rfc6265 "Finally, to remove a cookie, the server returns a Set-Cookie header with an expiration date in the past. The server will be successful in removing the cookie only if the Path and the Domain attribute in the Set-Cookie header match the values used when the cookie was created." For more information see https://stackoverflow.com/questions/5285940/correct-way-to-delete-cookies-server-side Modern browsers delete expired cookies although based on reading it might not be right away but on the next request from that domain or when the browser is cleaning up cookies. When I tested the cookie is deleted right away. Also set the SESSION to an empty string. Discussed in discord here: https://discord.com/channels/775381525260664832/855566794994221117/982351098998321163 Webui-vue and phosphor-webui both use this /logout route: https://github.com/openbmc/webui-vue/blob/a5fefd0ad25753e5f7da03d77dfe7fe10255ebb6/src/store/modules/Authentication/AuthenticanStore.js#L50 https://github.com/openbmc/phosphor-webui/blob/339db9a4c8610c5ecb92993c0bbc2219933bc858/app/common/services/userModel.js#L46 It seemed unnecessary to add it to the SessionCollection Post. Tested: No longer have the cookie after log out on webui-vue. Tested on Firefox and Chrome. Signed-off-by: Gunnar Mills <gmills@us.ibm.com> Change-Id: Ic12b6f628293a80c93ffbbe1bf06c9b2d6a53af7
2022-06-10query_param: remove dead fileNan Zhou1-0/+0
This file is empty and is not referenced anywhere. So just delete it. Tested: code compiles. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Ib2c743cf6ff06b8a68b7a887da5eee28a6d481f0
2022-06-10memory: Precisely match DIMM IDSui Chen1-1/+11
Currently, getDimmData uses std::string::find to determine if a DBus path contains a dimmId. The result is if we have the following paths: /xyz/openbmc_project/inventory/system/chassis/motherboard/dimm1 /xyz/openbmc_project/inventory/system/chassis/motherboard/dimm10 /xyz/openbmc_project/inventory/system/chassis/motherboard/dimm11 ... /xyz/openbmc_project/inventory/system/chassis/motherboard/dimm19 They will all be considered to match dimmId "dimm1" with string::find. This change switches to sdbusplus::message::object_path::filename(), so partial matches may be prevented. This change also considers partition data and works for partitions. Tested: 1. No new Redfish Validator errors 2. In my mock environment 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": "Volatile", "OffsetMiB": 0, "PassphraseEnabled": false, "RegionId": "", "SizeMiB": 1024 } ], "Status": { "Health": "OK", "HealthRollup": "OK", "State": "Enabled" } } Signed-off-by: Sui Chen <suichen@google.com> Change-Id: I122774be1f81da6e5c3a833b83d2bd81b437c298
2022-06-10query params: avoid copyNan Zhou1-1/+1
|processAllParams| now can take reference of parsed |Query| given that it's read-only now. The only copy is kept by the lambda. Tested: 1. on my mock environment, query parameter works as expected. Tested $only, $expand, $top, and $skip. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I1edf53d3b0e030c7370eb7ba937644d5ced074dc
2022-06-10ComputerSystem: fix ResetActionInfoNan Zhou1-12/+20
There is a regression that "Parameters" in ActionInfo now become an object rather than an array, as defined in the Schema, https://redfish.dmtf.org/schemas/ActionInfo.v1_2_0.json Tested: 1. on my mock environment, ``` { "@odata.id": "/redfish/v1/Systems/system/ResetActionInfo", "@odata.type": "#ActionInfo.v1_1_2.ActionInfo", "Id": "ResetActionInfo", "Name": "Reset Action Info", "Parameters": [ { "AllowableValues": [ "On", "ForceOff", "ForceOn", "ForceRestart", "GracefulRestart", "GracefulShutdown", "PowerCycle", "Nmi" ], "DataType": "String", "Name": "ResetType", "Required": true } ] } ``` 2. Redfish Service Validator Passes *** /redfish/v1/Systems/system/ResetActionInfo Attempt 1 of /redfish/v1/Systems/system/ResetActionInfo Response Time for GET to /redfish/v1/Systems/system/ResetActionInfo: 0.001620268914848566 seconds. Type (ActionInfo.v1_1_2.ActionInfo), GET SUCCESS (time: 0:00:00.001832) PASS Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I36db7fddaf565a2464378e31b18ecea688254f0e Signed-off-by: Ed Tanous <edtanous@google.com>
2022-06-08Chassis: fix ResetActionInfoNan Zhou1-5/+8
There is a regression that "Parameters" in ActionInfo now become an object rather than an array, as defined in the Schema, https://redfish.dmtf.org/schemas/ActionInfo.v1_2_0.json Tested: 1. On my mock environment, { "@odata.id": "/redfish/v1/Chassis/fake_chassis/ResetActionInfo", "@odata.type": "#ActionInfo.v1_1_2.ActionInfo", "Id": "ResetActionInfo", "Name": "Reset Action Info", "Parameters": [ { "AllowableValues": [ "PowerCycle" ], "DataType": "String", "Name": "ResetType", "Required": true } ] } 2. Redfish Service Validator passes ``` *** /redfish/v1/Chassis/fake_chassis/ResetActionInfo Attempt 1 of /redfish/v1/Chassis/fake_chassis/ResetActionInfo Response Time for GET to /redfish/v1/Chassis/fake_chassis/ResetActionInfo: 0.0017544100992381573 seconds. Type (ActionInfo.v1_1_2.ActionInfo), GET SUCCESS (time: 0:00:00.001957) PASS ``` Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I85d1c14d6a37b6f360732ee107d0163c2d3ff15f Signed-off-by: Ed Tanous <edtanous@google.com>
2022-06-07Leave off firmware properties if EBADRGunnar Mills1-0/+8
Have seen where between the mapper call and the inner call here to phosphor-bmc-code-mgmt, phosphor-bmc-code-mgmt deleted the image. This was during code update and phosphor-bmc-code-mgmt was deleting the backup image. Redfish lists all associated images under the manager resource: "SoftwareImages": { "description": "The images that are associated with this manager.", ... See https://redfish.dmtf.org/schemas/Manager.v1_15_0.json. bmcweb needs to look at the image purpose hence the call to the backup image. EBADR is the resource not found error code. If EBADR is returned when populating the firmware properties just leave off the firmware properties. These properties aren't required. Discussed in discord here: https://discord.com/channels/775381525260664832/981260009256140852/981263933442785290 We do similar checks for an EBADR return code other places in bmcweb. Tested: Everything looked the same. To actually test this code path had to be creative. Made this call look at a bad path: *version; } }, - obj.second[0].first, obj.first, + obj.second[0].first, obj.first + "badid", "org.freedesktop.DBus.Properties", "GetAll", "xyz.openbmc_project.Software.Version"); When doing so I saw the following traces but no internal error: (2022-06-01 20:29:41) [ERROR "fw_utils.hpp":139] error_code = generic:53 (2022-06-01 20:29:41) [ERROR "fw_utils.hpp":140] error msg = Invalid request descriptor The firmware version and software links were left off. The GUI handled this missing information well. The validator passed. Signed-off-by: Gunnar Mills <gmills@us.ibm.com> Change-Id: I9d8cd8b04acadfdd10f660cf9b7ca5dc6f36b4d0
2022-06-06Fix www-authenticate behaviorEd Tanous2-7/+14
bmcweb is in a weird position where, on the one hand, we would like to support Redfish to the specification, while also supporting a secure webui. For better or worse, the webui can't currently use non-cookie auth because of the impacts to things outside of Redfish like websockets. This has lead to some odd code in bmcweb that tries to "detect" whether the browser is present, so we don't accidentally pop up the basic auth window if a user happens to get logged out on an xhr request. Basic auth in a browser actually causes CSRF vulnerabilities, as the browser caches the credentials, so we don't want to make that auth method available at all. Previously, this detection was based on the presence of the user-agent header, but in the years since this code was originally written, a majority of implementations have moved to sending a user-agent by default, which makes this check pretty much useless for its purpose. To work around that, this patchset relies on the X-Requested-With header, to determine if a json payload request was done by xhr. In theory, all browsers will set this header when doing xhr requests, so this should provide a "more correct" solution to this issue. Background: https://en.wikipedia.org/wiki/List_of_HTTP_header_fields "X-Requested-With Mainly used to identify Ajax requests (most JavaScript frameworks send this field with value of XMLHttpRequest)" Tested: curl -vvvv --insecure https://192.168.7.2/redfish/v1/SessionService/Sessions Now returns a WWW-Authenticate header Redfish-protocol-validator now passes 7 more tests from the RESP_HEADERS_WWW_AUTHENTICATE category. Launched webui-vue and logged in. Responses in network tab appear to work, and data populates the page as expected. Used curl to delete redfish session from store with DELETE /redfish/v1/SessionService/Sessions/<SessionId> Then clicked an element on the webui, page forwarded to login page as expected. Opened https://localhost:8000/redfish/v1/CertificateService in a browser, and observed that page forwarded to the login page as it should. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I60345caa41e520c23fe57792bf2e8c16ef144a7a
2022-06-06redfish: Add Chassis listing associated driveJohn Edward Broadbent4-0/+142
If chassis has drives a drive url is added to the chassis, of the form: redfish/v1/Chassis/<chassis>/Drives When queried, the drive URL will list all drives associated with the chassis. This is in accordance with the redfish schema. Samples for the following URLs are below wget -qO- http://localhost:80/redfish/v1/Chassis/DC_SCM/Drives { "@odata.id": "/redfish/v1/Chassis/DC_SCM/Drives", "@odata.type": "#DriveCollection.DriveCollection", "Members": [ { "@odata.id": "/redfish/v1/Chassis/DC_SCM/Drives/mmcblk0" } ], "Members@odata.count": "1", "Name": "Drive Collection" } Tested: With the redfish validator: No new errors Change-Id: Ibdbe7fee5014d6515a77683c8eaca9ca86b6b148 Signed-off-by: John Edward Broadbent <jebr@google.com>
2022-06-03Remove extraneous semicolonEd Tanous1-1/+1
This semicolon does nothing, and causes build errors on clang. Remove it. Tested: Code compiles on clang. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I8506261121114c5359eafeb143cae9f6500014e0
2022-06-02Enable redfish journal by defaultWilly Tu1-1/+1
The journal logs in redfish provide really useful information for debugging. Enabling it by default for all system to use it. Tested: Redfish Validator Passed Working example, ``` wget -qO- http://localhost:80/redfish/v1/Managers/bmc/LogServices/Journal { "@odata.id": "/redfish/v1/Managers/bmc/LogServices/Journal", "@odata.type": "#LogService.v1_1_0.LogService", "DateTime": "1970-01-02T22:46:40+00:00", "DateTimeLocalOffset": "+00:00", "Description": "BMC Journal Log Service", "Entries": { "@odata.id": "/redfish/v1/Managers/bmc/LogServices/Journal/Entries" }, "Id": "BMC Journal", "Name": "Open BMC Journal Log Service", "OverWritePolicy": "WrapsWhenFull" } ``` ``` { "@odata.id": "/redfish/v1/Managers/bmc/LogServices/Journal/Entries", "@odata.type": "#LogEntryCollection.LogEntryCollection", "Description": "Collection of BMC Journal Entries", "Members": [ { "@odata.id": "/redfish/v1/Managers/bmc/LogServices/Journal/Entries/91187366227", "@odata.type": "#LogEntry.v1_8_0.LogEntry", "Created": "1970-01-02T01:19:47+00:00", "EntryType": "Oem", "Id": "91187366227", "Message": "bmcweb: (1970-01-02 01:19:47) [DEBUG \"memory.hpp\":438] Get available system components.", "Name": "BMC Journal Entry", "OemRecordFormat": "BMC Journal Entry", "Severity": "OK" }, ... } ``` Change-Id: I4f22e82884b28f76d7b505cca8b690132bc357b9 Signed-off-by: Willy Tu <wltu@google.com>
2022-06-02Fix indentation of clang-format off blocksJiaqing Zhao2-32/+32
After applying the new clang-format rule in commit 002d39b ("Try to fix the lambda formatting issue"), some clang-format off blocks need to be reindented. Tested: Build pass. Just removed some whitespaces. Change-Id: Ib56ebecfaf774fe99230779adba49537c9b58b8d Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
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