Age | Commit message (Collapse) | Author | Files | Lines |
|
This commit adds a utility function |sortJsonArrayByKey|. It can sort an
json array by value of a given key of each element.
Use cases includes:
1. sort the MemberCollection by @odata.id
Tested:
1. unit test passed;
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Idc175fab3af5c6102a5a3439b712b659ecb76468
|
|
System includes should be included with <>, in-tree includes should be
included with "". This was found manually, with the help of the
following grep statement[1].
git grep -o -h "#include .*" | sort | uniq
Tested:
Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I1a6b2a5ba35ccbbb61c67b7c4b036a2d7b3a36a3
|
|
The new pre-defined usergroup named "hostconsole" is added to
differentiate access between host console and manager console.
The only users allowed to interact with host console are part of the
"hostconsole" group and they are in an administrator role.
Note: The changes are spread across multiple repositories listed under
"Related commits:"
The bmcweb changes to incorporate new group are as follows:
- The new user is added in the hostconsole group only if it has an
administrative role.
- The ssh usergroup is only translated to ManagerConsole redfish group
and hostconsole usergroup is translated to HostConsole redfish group.
- The following changes are made to check the privileges for host console
access
- The new OEM privilege "OpenBMCHostConsole" added for host console
access. This privilege is not shared externally hence it is not
documented.
- Updated obmc_console BMCWEB_ROUTE to use the new privilege.
- Router functions now save user role and user groups in the session
- getUserPrivileges() function now takes session reference instead
of user role. This function now also checks for the user group
"hostconsole" and add the new privilege if user is member of this
group.
- Updated all callers of the getUserPrivileges to pass session
reference.
- Added test to validate that new privilege is set correctly.
Tested:
Loaded code on the system and validated that;
- New user gets added in hostconsole group. NOTE: Prior to this commit
all groups are assigned to new user. This drop does not change that
behavior.
- Access from the web gui is only available for users in hostconsole
group. Used IBM internal simulator called simics to test this. This
simulator allows accessing openbmc from GUI.
- Checked the role collection and there is no change.
$ curl -k -H "X-Auth-Token: $TOKEN" -X GET \
https://${bmc}/redfish/v1/AccountService/Roles
$ curl -k -H "X-Auth-Token: $TOKEN" -X GET \
https://${bmc}/redfish/v1/AccountService/Roles/Administrator
$ curl -k -H "X-Auth-Token: $TOKEN" -X GET \
https://${bmc}/redfish/v1/AccountService/Roles/ReadOnly
$ curl -k -H "X-Auth-Token: $TOKEN" -X GET \
https://${bmc}/redfish/v1/AccountService/Roles/Operator
- HostConsole is in AccountType when hostconsole group is present in
UserGroups D-Bus property
$ id user99
uid=1006(user99) gid=100(users) groups=1000(priv-admin),1005(web),\
1006(redfish),1013(hostconsole),100(users)
$ curl -k https://${bmc}/redfish/v1/AccountService/Accounts/user99
{
"@odata.id": "/redfish/v1/AccountService/Accounts/user99",
"@odata.type": "#ManagerAccount.v1_4_0.ManagerAccount",
"AccountTypes": [
"HostConsole",
"Redfish",
"WebUI",
"ManagerConsole"
],
"Description": "User Account",
"Enabled": true,
"Id": "user99",
"Links": {
"Role": {
"@odata.id": "/redfish/v1/AccountService/Roles/Administrator"
}
},
"Locked": false,
"Locked@Redfish.AllowableValues": [
"false"
],
"Name": "User Account",
"Password": null,
"PasswordChangeRequired": false,
"RoleId": "Administrator",
"UserName": "user99"
- The hostconsole group is not present for readonly or operator users
and also made sure that console access is not provided. This testing
is done one the system and console access was tried by modifying the
https://github.com/openbmc/bmcweb/blob/master/scripts/websocket_test.py
+ curl -k https://${bmc}/redfish/v1/AccountService/Accounts/user99
{
"@odata.id": "/redfish/v1/AccountService/Accounts/user99",
"@odata.type": "#ManagerAccount.v1_4_0.ManagerAccount",
"AccountTypes": [
"IPMI",
"Redfish",
"WebUI",
"ManagerConsole"
],
"Description": "User Account",
"Enabled": true,
"Id": "user99",
"Links": {
"Role": {
"@odata.id": "/redfish/v1/AccountService/Roles/ReadOnly"
}
},
"Locked": false,
"Locked@Redfish.AllowableValues": [
"false"
],
"Name": "User Account",
"Password": null,
"PasswordChangeRequired": false,
"RoleId": "ReadOnly",
"UserName": "user99"
[INFO "http_connection.hpp":209] Request: 0x150ac38 HTTP/1.1 GET /console0 ::ffff:x.x.xx.xxx
[DEBUG "routing.hpp":1265] Matched rule (upgrade) '/console0' 1 / 2
[DEBUG "routing.hpp":1084] userName = user99 userRole = priv-user
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=ipmi
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=redfish
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=ssh
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=web
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: Login
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: ConfigureSelf
[DEBUG "routing.hpp":113] checkPrivileges: OEM REQUIRED: OpenBMCHostConsole
[ERROR "routing.hpp":1192] Insufficient Privilege
+ curl -k https://${bmc}/redfish/v1/AccountService/Accounts/user99
{
"@odata.id": "/redfish/v1/AccountService/Accounts/user99",
"@odata.type": "#ManagerAccount.v1_4_0.ManagerAccount",
"AccountTypes": [
"IPMI",
"Redfish",
"WebUI",
"ManagerConsole"
],
"Description": "User Account",
"Enabled": true,
"Id": "user99",
"Links": {
"Role": {
"@odata.id": "/redfish/v1/AccountService/Roles/Operator"
}
},
"Locked": false,
"Locked@Redfish.AllowableValues": [
"false"
],
"Name": "User Account",
"Password": null,
"PasswordChangeRequired": false,
"RoleId": "Operator",
"UserName": "user99"
[INFO "http_connection.hpp":209] Request: 0x21c7c38 HTTP/1.1 GET /console0 ::ffff:x.x.xx.xxx
[DEBUG "routing.hpp":1265] Matched rule (upgrade) '/console0' 1 / 2
[DEBUG "routing.hpp":1084] userName = user99 userRole = priv-operator
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=ipmi
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=redfish
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=ssh
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=web
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: Login
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: ConfigureComponents
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: ConfigureSelf
[DEBUG "routing.hpp":113] checkPrivileges: OEM REQUIRED: OpenBMCHostConsole
[ERROR "routing.hpp":1192] Insufficient Privilege
Related commits:
NOTE: docs, openbmc, obmc-console changes are already merged. bmcweb
and phosphor-user-manager will be merged together.
docs: https://gerrit.openbmc.org/c/openbmc/docs/+/60968
phosphor-user-manager: https://gerrit.openbmc.org/c/openbmc/phosphor-user-manager/+/61583
openbmc: https://gerrit.openbmc.org/c/openbmc/openbmc/+/61582
obmc-console: https://gerrit.openbmc.org/c/openbmc/obmc-console/+/61581
bmcweb: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/61580
Change-Id: Ia5a33dafc9a76444e6a8e74e752f0f90cb0a31c8
Signed-off-by: Ninad Palsule <ninadpalsule@us.ibm.com>
|
|
This is a pretty simple test, but should be able to catch the regression
injected in the previous commit.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I67de097059a6e0dd8d2c02c1aa6c69954a6d7be3
|
|
We have no unit tests for this. This isn't very extensive, but we
should have at least one.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I212ee528b354f2ed47f88076be009fd6e16fb760
|
|
Boost 1.82 dropped a lovely new toy, boost::urls::format, which is a lot
like our urlFromPieces method, but better in that it makes the resulting
uris more readable, and allows doing things like fragments in a single
line instead of multiple. We should prefer it in some cases.
Tested:
Redfish service validator passes.
Spot checks of URLs work as expected.
Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia7b38f0a95771c862507e7d5b4aa68aa1c98403c
|
|
The latest version of the specification is 1.17.0, and arguably, we
should be updating this every time we pull in any new feature, but that
hasn't happened.
So far as I'm aware, there are no tools that actually look at this
parameter to make branching decisions in the client about supported
features, so the likelihood this has impact is basically nil.
Tested:
GET /redfish/v1 returns RedfishVersion of 1.17.0
Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I055b6010329599b7b39c587fa85faf51a38c9b57
|
|
There is code in ibm/locks that has had clang-tidy warnings disabled
for a while due to multiple safety and endianness issues. The code
has not been fixed in a while and with clang-16 it is unable to be
exempted further. Disable it until someone who cares can fix this
in the proper way.
```
../include/ibm/locks.hpp:522:14: error: 'p' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
uint8_t* p = reinterpret_cast<uint8_t*>(&resourceId1);
~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/ibm/locks.hpp:527:25: note: used in buffer access here
uint8_t pPosition = p[position];
^
../include/ibm/locks.hpp:524:14: error: 'q' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
uint8_t* q = reinterpret_cast<uint8_t*>(&resourceId2);
~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/ibm/locks.hpp:529:25: note: used in buffer access here
uint8_t qPosition = q[position];
```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I8a7fcbed1099419ad1715c86ffcbfef20820251e
|
|
The clang-tidy warning 'modernize-use-emplace' correctly flags a
few places where emplace should be used over push.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I6ca79285a87d6927e718345dc8dce0387e6b1eda
|
|
Json has a bunch of implicit overloaded operators for comparing
vectors/arrays/maps with their json counterparts. Unfortunately, when
used in unit tests, these cause warnings in clang, so update the code to
use the ElementsAre check from gmock.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I658557cb59d568fd50cf6f3bef73d6f90b5c56cf
|
|
clang-format-16 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest .clang-format from the docs repository and reformat the
repository.
Change-Id: I75f89d2959b0f1338c20d72ad669fbdc1d720835
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
|
|
Adds a function to process responses from URIs that are uptree from
a top level collection. A follow-up patch will hook this into the
aggregation code to allow adding links to top level collections which
are only supported by satellite BMCs.
Adds test cases to validate this function is working correctly.
Tested:
New test cases pass
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I7f0fd6c3955398e2fde136c1d3b37a6bf4bf06b9
|
|
The router historically came from crow. Crow supported wildcards of
<int>, <float>, and <double>. bmcweb doesn't use them, nor should it in
basically any case, as we now have explicit 404 handling.
This commit removes them. This amounts to about -450 lines of code, but
it's some of the scarier code we have, some of it existing in the
namespace "black_magic". Reducing the brain debt for people working in
this subsystem seems worthwhile. There is no case in the future where
we would use integer based url parameters.
Tested: Redfish service validator passes. Should be good enough
coverage for a code removal.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I34add8df7d3486952474ca7ec3dc6be990c50ed0
|
|
Adds a search function which is able to determine if a passed URI is a
top level collection, is uptree from a top level collection, or both.
The type being searched for depends on a second argument passed to the
function.
Each of these searches are used to add links to top level collections
which are only supported by a satellite BMC. They all use similar steps
so rolling them into a single function cuts down on redundant code.
Adds test cases to verify the implementation is correct.
Tested:
New test cases pass
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I72ae7442d5f314656b57a73aee544bca516fa7c2
|
|
The "HttpHeaders" property in a response is an array of HTTP headers. We
perform prefix fixing on the "Location" header from responses so we
should also fix any "Location" headers which are contained by
"HttpHeaders" in an aggregated response. This requires special handling
since each header is represented as a single string in the response.
Added testcase for HttpHeaders property
Tested:
All unit tests pass
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I3040c4ea52b2bebcb6e206bb50585c6a75538f0a
|
|
There are cases in aggregation where an expand parameter might get
forwarded to a client. Because our previous expand algorithm assumed
that any endpoint within bmcweb would only produce "depth=1" responses,
it was reasonable to assume that the pre-response could not contain
expanded content. Aggregated resources can't make that assumption.
This commit attempts to pass through depth through the request, to
ensure that we only expand the level that the user requested, and not
any level returned by the request. This is done by using the existence
of the resource identifer "@odata.id" to indicate each level in an
expanded response. This should be fine since the Redfish spec requires
that property to exist.
Added unit tests to cover aggregation scenarios. Modified existing
$expand tests to comply with the resource identifier dependency.
Tested:
New unit tests pass
Queried '/redfish/v1/Systems?$expand=.($levels=2)' on an aggregated
system whose satellite BMC supported $expand. The overall response was
correctly expanded for both resources on the aggregating BMC as well as
on the satellite BMC. Expanding the satellite resources did not require
sending multiple queries to the satellite.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I20ba60ee39bac11ffb3fe1768cec6299cf9ee13e
Signed-off-by: Carson Labrado <clabrado@google.com>
|
|
Per cpp core guidelines, these should be methods.
Tested: on last patchset of the series.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib16479db9d2b68da68e7ad6e825c7e205c64f1de
|
|
This property was added in Redfish 2022.3 to allow clients to determine
which manager is hosting the ServiceRoot, such that they can find uptime
statistics, and other metrics from that resource, without needing to
attach them directly to serviceroot.
Tested:
Redfish service validator passes.
GET /redfish/v1/Managers/bmc returns the expected response.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If2b78528d1499fbdae46120e1a1792ecf7ceb1d3
|
|
This change moves getMessage and getMessageFromRegistry to a .cpp file
so that they can be easily tested.
Tested: Unit test passes
Signed-off-by: Sui Chen <suichen@google.com>
Change-Id: Ia9fc91e5a47036198bf013ff3ea21ea9f6d5259a
|
|
string_view should always be passed by value; This commit is a sed
replace of the code to make all string_views pass by value, per general
coding guidelines[1].
[1] https://quuxplusone.github.io/blog/2021/11/09/pass-string-view-by-value/
Tested: Code compiles.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I55b342a29a0fbfce0a4ed9ea63db6014d03b134c
|
|
Only id in event_service and account_service have not been updated due
to the risk of it breaking the username/id. It will require further
testing to verify.
Use urlFromPieces wherever that is needed to insert a variable in the
URI. Don't use urlFromPieces when it is hardcoded values. This allow us
to control all resource URIs that is dynamically added and to sync with
the current recommanded method for `@odata.id`. The goal is to have a
common place to manage the url created from dbus-paths in order to
manage/update it easily when needed.
Tested:
RedfishValidtor Passed for all resource including the sensors with the
fragments.
Change-Id: I95cdfaaee58fc7f21c95f5944e1e5c813b3215f2
Signed-off-by: Willy Tu <wltu@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
Corrections style complaints in the aggregator code.
Tested:
Jenkins output did not show any style complaints
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I87426fcf2a48448a62152e0ad4a6c3aa54a7fc45
|
|
There are certain cases where we use this split function, and we expect
tokens to be read out. For example:
/xyz/openbmc_project/sensors/unit/name
Should split into a "" in the first position. This use case is not
common, and a quick grep shows only two places in the code expect this
behavior. Boost::split has this behavior already, which is what this
function is emulating. While we could fix these, in the end they should
be following the rules outlined in COMMON_ERRORS.md, which disallow
this kind of parsing completely.
Tested: New unit tests passing.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iec3dcbf2b495b2b3b4ed419172c4133b16f7c65d
|
|
For systems implementing to the OWASP security guidelines[1] (of which all
should ideally) we should be checking the content-type header all times
that we parse a request as JSON.
This commit adds an option for parsing content-type, and sets a default
of "must get content-type". Ideally this would not be a breaking
change, but given the number of guides and scripts that omit the content
type, it seems worthwhile to add a trapdoor, such that people can opt
into their own model on how they would like to see this checking work.
Tested:
```
curl --insecure -H "Content-Type: application/json" -X POST -D headers.txt https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName":"root", "Password":"0penBmc"}'
```
Succeeds.
Removing Content-Type argument causes bmc to return
Base.1.13.0.UnrecognizedRequestBody.
[1] cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html
Change-Id: Iaa47dd563b40036ff2fc2cacb70d941fd8853038
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
boost::split has a documented false-positive in clang-tidy. While
normally we'd handle this with NOLINTNEXTLINE, this doesn't appear to
work in all cases. Unclear why, but seems to be due to some of our
lambda callback complexity.
Each of these uses is a case where we should be using a more specific
check, rather than split, but for the moment, this is the best we have.
Tested: clang-tidy passes.
[1] https://github.com/llvm/llvm-project/issues/40486
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I144c6610cb740287b7225e2be03b4142a64f9563
|
|
In the continual quest to get tidy passing when run in isolation, fix
some more includes.
This includes removing a circular #include to app.hpp. We don't use
app.hpp in these files, which is why our code compiles but having this
include it here causes a few circular dependencies
app.hpp -> http_server.hpp -> persistent_data.hpp -> app.hpp.
app.hpp -> http_server.hpp -> authentication.hpp -> app.hpp.
This confuses clang when run on header files directly.
Fix a couple more includes at the same time.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib62d78b49c7e38ef7061c9fbbf6b3d463f11917d
|
|
The aggregator did not propagate header's fields from aggregated
responses. This change will take into account of response code other
than 200, which will modify a field called "Location". The Location
field in the response's header will point to where the response data
can be read from. This "Location" field in response Header will now
contain the correct URI with the prefix appended.
We will also copy over other Header Values to aggregated response. These
header values include "Content-Type", "Allow", "Retry-After", and also
the response's body
Added some test cases for the above fixes.
Tested:
Unit Tests pass.
Queries reponse that returns other result than 200 that has Location
field and the response received is as expected.
Signed-off-by: Khang Kieu <khangk@google.com>
Change-Id: I77c7dae32a103fbec3015fe14b51a3ed0022143e
|
|
It's possible for HTTP client's request buffer to become full
(especially when $expand is used). Instead of ignoring the requests
we should provide a 429 Too Many Requests response for the provided
callback to process.
The aggregator's response handling also needs to account for this
possibility so that it only completely overwrites the asyncResp
object when it receives a response from a satellite.
Also added more test cases for the response processing functions.
Tested:
Unit tests passed
Flooded aggregator with requests for satellite resources. Requests
began returning 429 Too Many Requests errors after the request buffer
became full.
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ib052dc0454d759de7fae761977ca26d6b8d208e5
|
|
This function is something that's easily unit tested. Do it.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8d664c77ec4b3a9886128597449c5f9c041b86b3
|
|
We only attempt prefix matching when we receive a 200 response. For
the retry policy we consider 2XX and 404 to be valid codes. Instead we
should forward all responses to the client and let them decide what
action they want to take. As part of that we should always attempt to
do prefix fixing on the response.
Also fixes an oversight where we attempt to do prefix fixing on
"OriginOfCondition" properties. That property is only a URI when it is
an Action parameter in a SubmitTestEvent request. It is an object when
it appears as a response property.
Adds test cases for the above fixes.
Tested:
Tests pass. Queries to top level collections and aggregated URIs still
return expected results with added prefixes.
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ic76324ceab618160061be5f3c687af20a857fa25
|
|
Per the title, add unit tests for this function.
Tested: Unit tests pass
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ifdd9c314d2fa62ae3fa3b8f8150fcdd224a7eb03
|
|
Update the clearDump() implementation to call the DeleteAll D-Bus
method instead of iterating through D-Bus objects representing
individual log entries and calling the Delete D-Bus method on each one.
(It's more efficient for phosphor-debug-collector to iterate through
entries in its DeleteAll method handler than for bmcweb to iterate
through them.)
It seems like clearDump() wasn't originally implemented using DeleteAll
because dumps of various types were under the same D-Bus path namespace
at the time and there wasn't a way to selectively clear dumps of only a
specific type. The commit at [1] put different dump types under
different path namespaces (enabling us to now use DeleteAll).
Now clients should see a bit of performance improvement when running
the ClearLog action on dump LogServices, due to the reduced number of
D-Bus method calls needed to execute ClearLog.
Also updated getDumpServiceInfo() to populate the ClearLog action for
dump LogServices based on whether their dump manager object implements
xyz.openbmc_project.Collection.DeleteAll.
Tested:
Cleared the fault log containing 100 entries.
Ran with the time command several times before and after the change:
```
time curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Managers/bmc/LogServices/FaultLog/Actions/LogService.ClearLog
```
Before the change, "real" time reported was ~1.2s.
After the change, "real" time reported was ~0.4s.
Forced creation of dump entries and then ran Redfish ClearLog action on
each dump type:
```
curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Managers/bmc/LogServices/Dump/Actions/LogService.ClearLog
curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Managers/bmc/LogServices/FaultLog/Actions/LogService.ClearLog
curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Actions/LogService.ClearLog
```
Then verified that there were no dump LogService entries afterwards:
```
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Managers/bmc/LogServices/Dump/Entries
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Managers/bmc/LogServices/FaultLog/Entries
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Entries
```
Also verified that the corresponding D-Bus objects were gone from the
D-Bus tree after running ClearLog on each dump type:
Before ClearLog:
busctl tree xyz.openbmc_project.Dump.Manager
`-/xyz
`-/xyz/openbmc_project
`-/xyz/openbmc_project/dump
|-/xyz/openbmc_project/dump/bmc
| `-/xyz/openbmc_project/dump/bmc/entry
| `-/xyz/openbmc_project/dump/bmc/entry/101
|-/xyz/openbmc_project/dump/faultlog
| `-/xyz/openbmc_project/dump/faultlog/entry
| |-/xyz/openbmc_project/dump/faultlog/entry/11
| |-/xyz/openbmc_project/dump/faultlog/entry/12
| |-/xyz/openbmc_project/dump/faultlog/entry/13
| |-/xyz/openbmc_project/dump/faultlog/entry/14
| |-/xyz/openbmc_project/dump/faultlog/entry/15
| |-/xyz/openbmc_project/dump/faultlog/entry/16
| |-/xyz/openbmc_project/dump/faultlog/entry/17
| |-/xyz/openbmc_project/dump/faultlog/entry/18
| |-/xyz/openbmc_project/dump/faultlog/entry/19
| `-/xyz/openbmc_project/dump/faultlog/entry/20
|-/xyz/openbmc_project/dump/internal
| `-/xyz/openbmc_project/dump/internal/manager
`-/xyz/openbmc_project/dump/system
`-/xyz/openbmc_project/dump/system/entry
|-/xyz/openbmc_project/dump/system/entry/3
`-/xyz/openbmc_project/dump/system/entry/4
After ClearLog:
busctl tree xyz.openbmc_project.Dump.Manager
`-/xyz
`-/xyz/openbmc_project
`-/xyz/openbmc_project/dump
|-/xyz/openbmc_project/dump/bmc
|-/xyz/openbmc_project/dump/faultlog
|-/xyz/openbmc_project/dump/internal
| `-/xyz/openbmc_project/dump/internal/manager
`-/xyz/openbmc_project/dump/system
Confirmed that ClearLog action is listed for the following
LogServices:
/redfish/v1/Managers/bmc/LogServices/Dump
/redfish/v1/Managers/bmc/LogServices/FaultLog
/redfish/v1/Systems/system/LogServices/Dump
Then ran "systemctl stop xyz.openbmc_project.Dump.Manager" (which
removes dump manager objects including their
xyz.openbmc_project.Collection.DeleteAll interface) and saw that the
ClearLog action was no longer listed. Also locally built a version of
phosphor-debug-collecor with the interface
xyz.openbmc_project.Collection.DeleteAll removed from dump managers and
ran it and saw that the ClearLog action wasn't listed.
Redfish Service Validator passed on the following URIs
(with service xyz.openbmc_project.Dump.Manager running):
/redfish/v1/Managers/bmc/LogServices/Dump
/redfish/v1/Managers/bmc/LogServices/FaultLog
/redfish/v1/Systems/system/LogServices/Dump
Note: Most dump LogService unit tests were removed in this patchset
since this patchset adds a D-Bus call to getDumpServiceInfo(), and
we haven't decided how to mock D-Bus calls for unit testing yet.
[1] https://github.com/openbmc/phosphor-debug-collector/commit/fef66a951fe6fe283515480b2c493dfdc2275a95
Signed-off-by: Claire Weinan <cweinan@google.com>
Change-Id: Ic5f8f9e3528f521887766d8710bd77f969d8236a
|
|
Utilize the new array of top level collection URIs to determine if a
given URI in the response needs to have the aggregation prefix added.
This removes the need to check for specific collections like
/redfish/v1/UpdateService/FirmwareInventory which do not fit the
generic format of /redfish/v1/<collection>.
Future patches will use this same approach to improve the logic for
initially determining if and how a request should be aggregated.
This patch also adds a series of unit tests for the function
responsible for adding a prefix to a given URI. Cases covered include
valid URIs that involve a selection of aggregated resources, top level
collection URIs, other invalid URIs, and URIs with a trailing "/".
Tested:
Unit tests pass.
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I676983d3c77ae3126c04e9f57ad8698c51df2675
|
|
c1d019a6056a2a0ef50e577b3139ab5a8dc49355 Sensor Optimization
Recently changed the way Ids were calculated in the sensor subsystem.
Unfortunately, it wasn't clear to the author that this would effect the
sensor override system, which relies on matching up a member ID with a
dbus path, and was broken by this change.
This commit breaks out the code to calculate the type and name from a
given URI segment into a helper method.
Tested: Inspection only. Very few systems support this feature. Code appears more correct than previously, which is known broken, so the lack of testing here seems reasonable.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9aa8099a947a36b5ce914bc07ae60f1ebf0d209b
|
|
The new boost URL now interops properly with std::string_view, which is
great, and cleans up a bunch of mediocre code to convert one to another.
It has also been pulled into boost-proper, so we no longer need a
boost-url dependency that's separate.
Unfortunately, boost url makes these improvements by changing
boost::string_view for boost::urls::const_string, which causes us to
have some compile errors on the missing type.
The bulk of these changes fall into a couple categories, and have to be
executed in one commit.
string() is replaced with buffer() on the url and url_view types
boost::string_view is replaced by std::string_view for many times, in
many cases removing a temporary that we had in the code previously.
Tested: Code compiles with boost 1.81.0 beta.
Redfish service validator passes.
Pretty good unit test coverage for URL-specific use cases.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8d3dc89b53d1cc390887fe53605d4867f75f76fd
|
|
There are a number of properties of Type "string (uri)" for which we
do not currently support adding prefixes. This patch adds support
for all existing URI properties which are missed by the existing
implementation.
This change will be needed by future patches which will expand
aggregation support to all top level collections defined by the
schema. Those collections that are not currently supported include
properties whose URIs should be fixed, but would be missed by the
existing implementation.
Tested:
New unit test passes.
URI properties are still handled correctly.
```shell
curl localhost/redfish/v1/Chassis/5B247A_<chassisID>
{
"@odata.id": "/redfish/v1/Chassis/5B247A_<chassisID>",
"@odata.type": "#Chassis.v1_16_0.Chassis",
"Actions": {
"#Chassis.Reset": {
"@Redfish.ActionInfo": "/redfish/v1/Chassis/5B247A_<chassisID>/ResetActionInfo",
"target": "/redfish/v1/Chassis/5B247A_<chassisID>/Actions/Chassis.Reset"
}
},
...
}
```
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I3b3e06ee3191564d266598f7bc9f1641e6fcb333
|
|
As is, the router designates routes for every possible boost verb, of
which there are 31. In bmcweb, we only make use of 6 of those verbs, so
that ends up being quite a bit of wasted space and cache non-locality.
This commit invents a new enum class for declaring a subset of boost
verbs that we support, and a mapping between bmcweb verbs and boost
verbs.
Then it walks through and updates the router to support converting one
to another.
Tested:
Unit Tested
Redfish Service Validator performed on future commit
Signed-off-by: Ed Tanous <edtanous@google.com>
Signed-off-by: Edward Lee <edwarddl@google.com>
Change-Id: I3c89e896c632a5d4134dbd08a30b313c12a60de6
|
|
Testd: unit test only change
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I0a8d1e97d8f0be8a79b9c40a75eeb0659bba638b
|
|
This is a dummy commit to test owner plugin. It can be merged in as well
given that it adds a little bit readability.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ia4cc1866ebeb19e6d0f1d1ceece0ecc73fb4b468
|
|
Add helper function to append pieces to existing url to allow more
flexible control over the url. This allows us to avoid have each
resource append the pieces outside of the utility functions and help
maintain all url modifications in a central place for easy management.
Tested: Does not affect Redfish Tree. Unit Test passed.
Change-Id: I751f3c120cbadb465915b12aa253edd53ef32123
Signed-off-by: Willy Tu <wltu@google.com>
|
|
This commit implements the Redfish PowerSubsystem schema and collects
default property values.
PowerSupplies will be implemented in the next commit.
ref:
https://www.dmtf.org/sites/default/files/standards/documents/
DSP0268_2022.2.pdf (6.86 PowerSubsystem 1.1.0)
https://redfish.dmtf.org/schemas/v1/PowerSupply.v1_1_0.json
Tested: Validator and UT passes
1. curl -k -H "X-Auth-Token: $token" -X GET
https://${bmc}/redfish/v1/Chassis/chassis/PowerSubsystem
{
"@odata.id": "/redfish/v1/Chassis/chassis/PowerSubsystem",
"@odata.type": "#PowerSubsystem.v1_1_0.PowerSubsystem",
"Id": "PowerSubsystem",
"Name": "Power Subsystem",
"Status": {
"Health": "OK",
"State": "Enabled"
}
}
2. bad chassisID
curl -k -H "X-Auth-Token: $token" -X GET https://${bmc}
/redfish/v1/Chassis/badchassisID/PowerSubsystem/
PowerSupplies/powersupply0
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The requested resource of type Chassis named
badchassisID was not found.",
"MessageArgs": [
"Chassis",
"badchassisID"
],
"MessageId": "Base.1.13.1.ResourceNotFound",
"MessageSeverity": "Critical",
"Resolution": "Provide a valid resource identifier
and resubmit the request."
}
],
"code": "Base.1.13.1.ResourceNotFound",
"message": "The requested resource of type Chassis named
badchassisID was not found."
}
}
Signed-off-by: Chicago Duan <duanzhijia01@inspur.com>
Change-Id: I6885b1777082538eceaf7ea85a8f69966459ee43
|
|
This commit fixed several places (but not all) where wrong include
directory is specified and prevent the clean up in the chidren changes.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ibbba62e2c0cfe3583a65f1befa1b233bd3eebf19
|
|
- Index was not checked against size before dereference. Which cased to
override memory.
- Header without colon could put parser into invalid state. Now it will
return with error.
- Content after boundary was not correctly discarded.
- Parser did not check body for final boudary. Now missing final
boundary will return with error.
Tested:
- Tested that payload with header without colon doesn't cause memory
corruption anymore.
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Change-Id: I12f496ab5f53e6c088cdfdf2e96be636d66f7c7f
|
|
An HTTP header of Accepts: */* throws a big wrench into our
implementation for a couple reasons. First, because it's the default in
a lot of commonly-used libraries, and second, because clients use it
when they certainly don't mean what the specification says it should
mean "ie, I accept ANY type".
This commit tries to address some of that, by making an explicit option
for content-type="ANY" and pushes it to the individual callers to handle
explicitly as if it were yet another type. In most protocols, there's a
"most common" representation, so protocols are free to use that, or to
explicitly handle it, and require that the user be explicit.
Tested:
Redfish Protocol Validator no longer locks up. (TBD, getting bugs filed
with protocol validator for this missing Accepts header).
For ServiceRoot
GET /redfish/v1 Accepts: application/json - returns json
GET /redfish/v1 Accepts: */* - returns json
GET /redfish/v1 Accepts: text/html - returns html
GET /redfish/v1 no-accepts header - returns json
Redfish-service-validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iae6711ae587115d3e159a48a6fc46a903ed6c403
|
|
Like other C++ projects, unit tests normally are in a separate repo and
respect the folder structure of the file under test.
This commit deleted all "ut" folder and move tests to a "test" folder.
The test folder also has similar structure as the main folder.
This commit also made neccessary include changes to make codes compile.
Unused tests are untouched.
Tested: unit test passed.
Reference:
[1] https://github.com/grpc/grpc/tree/master/test
[2] https://github.com/boostorg/core/tree/414dfb466878af427d33b36e6ccf84d21c0e081b/test
[3] Many other OpenBMC repos: https://github.com/openbmc/entity-manager/tree/master/test
[4] https://stackoverflow.com/questions/2360734/whats-a-good-directory-structure-for-larger-c-projects-using-makefile
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4521c7ef5fa03c47cca5c146d322bbb51365ee96
|