Age | Commit message (Collapse) | Author | Files | Lines |
|
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
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
|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
|
|
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>
|
|
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>
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
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>
|
|
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>
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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
|
|
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>
|
|
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>
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
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
|