| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
clang-format-17 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: I2f9540cf0d545a2da4d6289fc87b754f684bc9a7
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
|
|
C++20 brought us std::ranges for a lot of algorithms. Most of these
conversions were done using comby, similar to:
```
comby -verbose 'std::lower_bound(:[a].begin(),:[b].end(),:[c])' 'std::ranges::lower_bound(:[a], :[c])' $(git ls-files | grep "\.[hc]\(pp\)\?$") -in-place
```
Change-Id: I0c99c04e9368312555c08147d474ca93a5959e8d
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
std::format is a much more modern logging solution, and gives us a lot
more flexibility, and better compile times when doing logging.
Unfortunately, given its level of compile time checks, it needs to be a
method, instead of the stream style logging we had before. This
requires a pretty substantial change. Fortunately, this change can be
largely automated, via the script included in this commit under
scripts/replace_logs.py. This is to aid people in moving their
patchsets over to the new form in the short period where old patches
will be based on the old logging. The intention is that this script
eventually goes away.
The old style logging (stream based) looked like.
BMCWEB_LOG_DEBUG << "Foo " << foo;
The new equivalent of the above would be:
BMCWEB_LOG_DEBUG("Foo {}", foo);
In the course of doing this, this also cleans up several ignored linter
errors, including macro usage, and array to pointer deconstruction.
Note, This patchset does remove the timestamp from the log message. In
practice, this was duplicated between journald and bmcweb, and there's
no need for both to exist.
One design decision of note is the addition of logPtr. Because the
compiler can't disambiguate between const char* and const MyThing*, it's
necessary to add an explicit cast to void*. This is identical to how
fmt handled it.
Tested: compiled with logging meson_option enabled, and launched bmcweb
Saw the usual logging, similar to what was present before:
```
[Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled
[Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800
[Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist
[Info src/webserver_main.cpp:59] Starting webserver on port 18080
[Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file.
[Info src/webserver_main.cpp:137] Start Hostname Monitor Service...
```
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8
|
|
The Health populate calls GetManagedObjects at `/` which can take a lot
of time. Add the option to disable to improve performance if it is not
needed.
Tested:
```
$ meson build -Dhealth-populate=disabled
...
User defined options
backend : ninja
health-populate : disabled
```
Build passed.
Health Status removed. Some resource still create HealthPopulate, but
does not populate. It will require further refactoring to clean it out.
Testing on `/redfish/v1/Chassis?$expand=.($levels=1)`
On 14 chassis, from about 2.5 seconds to 400 ms. :)
Before:
```
Getting times for chassis
Getting good line count with wget -q -O- localhost:80/redfish/v1/Chassis?$expand=.($levels=1)
Line count: 980
17:05:56: real 0m2.908s user 0m0.000s sys 0m0.030s
17:05:59: real 0m2.414s user 0m0.010s sys 0m0.010s
17:05:03: real 0m3.410s user 0m0.000s sys 0m0.020s
17:05:09: real 0m2.372s user 0m0.000s sys 0m0.010s
17:05:13: real 0m3.407s user 0m0.010s sys 0m0.000s
17:05:19: real 0m2.420s user 0m0.010s sys 0m0.000s
17:05:23: real 0m3.463s user 0m0.010s sys 0m0.000s
17:05:29: real 0m2.414s user 0m0.000s sys 0m0.010s
17:05:33: real 0m2.843s user 0m0.010s sys 0m0.010s
17:05:38: real 0m2.512s user 0m0.000s sys 0m0.020s
17:05:42: real 0m2.474s user 0m0.000s sys 0m0.010s
17:05:47: real 0m2.557s user 0m0.010s sys 0m0.010s
17:05:52: real 0m2.439s user 0m0.020s sys 0m0.000s
17:05:56: real 0m3.127s user 0m0.010s sys 0m0.000s
17:05:01: real 0m2.563s user 0m0.020s sys 0m0.000s
17:05:06: real 0m2.392s user 0m0.020s sys 0m0.020s
17:05:10: real 0m2.405s user 0m0.020s sys 0m0.000s
17:05:15: real 0m2.514s user 0m0.010s sys 0m0.010s
17:05:19: real 0m2.809s user 0m0.020s sys 0m0.010s
17:05:24: real 0m2.944s user 0m0.010s sys 0m0.010s
17:05:29: real 0m2.537s user 0m0.010s sys 0m0.000s
17:05:34: real 0m3.290s user 0m0.000s sys 0m0.000s
17:05:39: real 0m2.601s user 0m0.040s sys 0m0.000s
17:05:43: real 0m2.398s user 0m0.010s sys 0m0.040s
17:05:48: real 0m2.664s user 0m0.000s sys 0m0.020s
17:05:53: real 0m2.323s user 0m0.010s sys 0m0.000s
17:05:57: real 0m3.033s user 0m0.000s sys 0m0.010s
17:05:02: real 0m3.243s user 0m0.000s sys 0m0.010s
17:05:07: real 0m2.604s user 0m0.010s sys 0m0.010s
17:05:12: real 0m2.813s user 0m0.010s sys 0m0.010s
17:05:17: real 0m2.325s user 0m0.020s sys 0m0.000s
17:05:21: real 0m2.577s user 0m0.010s sys 0m0.000s
17:05:26: real 0m2.882s user 0m0.030s sys 0m0.000s
17:05:31: real 0m2.572s user 0m0.000s sys 0m0.020s
17:05:35: real 0m2.678s user 0m0.010s sys 0m0.010s
17:05:40: real 0m2.656s user 0m0.010s sys 0m0.010s
17:05:45: real 0m2.921s user 0m0.020s sys 0m0.000s
17:05:49: real 0m2.723s user 0m0.000s sys 0m0.020s
17:05:54: real 0m2.910s user 0m0.010s sys 0m0.010s
17:05:59: real 0m2.601s user 0m0.020s sys 0m0.000s
17:05:04: real 0m2.615s user 0m0.000s sys 0m0.000s
```
After:
```
Getting times for chassis
Getting good line count with wget -q -O- localhost:80/redfish/v1/Chassis?$expand=.($levels=1)
Line count: 980
16:04:43: real 0m0.188s user 0m0.020s sys 0m0.000s
16:04:43: real 0m0.195s user 0m0.010s sys 0m0.000s
16:04:45: real 0m0.219s user 0m0.010s sys 0m0.000s
16:04:48: real 0m0.226s user 0m0.020s sys 0m0.000s
16:04:50: real 0m0.208s user 0m0.020s sys 0m0.010s
16:04:52: real 0m0.226s user 0m0.010s sys 0m0.010s
16:04:54: real 0m0.419s user 0m0.000s sys 0m0.010s
16:04:57: real 0m0.222s user 0m0.010s sys 0m0.020s
16:04:59: real 0m0.194s user 0m0.000s sys 0m0.010s
16:04:01: real 0m0.191s user 0m0.010s sys 0m0.010s
16:04:04: real 0m0.276s user 0m0.010s sys 0m0.020s
16:04:06: real 0m0.183s user 0m0.020s sys 0m0.000s
16:04:08: real 0m0.193s user 0m0.040s sys 0m0.000s
16:04:10: real 0m0.406s user 0m0.020s sys 0m0.010s
16:04:13: real 0m0.317s user 0m0.000s sys 0m0.000s
16:04:15: real 0m0.442s user 0m0.005s sys 0m0.005s
16:04:18: real 0m0.226s user 0m0.010s sys 0m0.000s
16:04:20: real 0m0.217s user 0m0.020s sys 0m0.000s
16:04:22: real 0m0.200s user 0m0.010s sys 0m0.030s
16:04:24: real 0m0.423s user 0m0.010s sys 0m0.010s
16:04:27: real 0m0.203s user 0m0.020s sys 0m0.010s
16:04:29: real 0m0.433s user 0m0.000s sys 0m0.000s
16:04:31: real 0m0.318s user 0m0.020s sys 0m0.000s
16:04:34: real 0m1.206s user 0m0.000s sys 0m0.010s
16:04:37: real 0m0.403s user 0m0.000s sys 0m0.020s
16:04:39: real 0m0.353s user 0m0.010s sys 0m0.000s
16:04:42: real 0m0.291s user 0m0.000s sys 0m0.030s
16:04:44: real 0m0.742s user 0m0.020s sys 0m0.010s
16:04:47: real 0m0.369s user 0m0.010s sys 0m0.000s
16:04:49: real 0m0.215s user 0m0.020s sys 0m0.000s
16:04:52: real 0m0.204s user 0m0.000s sys 0m0.010s
16:04:54: real 0m0.418s user 0m0.000s sys 0m0.000s
16:04:56: real 0m0.215s user 0m0.000s sys 0m0.010s
16:04:58: real 0m0.202s user 0m0.010s sys 0m0.010s
16:04:01: real 0m0.202s user 0m0.010s sys 0m0.010s
16:04:03: real 0m0.212s user 0m0.010s sys 0m0.000s
16:04:05: real 0m0.694s user 0m0.010s sys 0m0.010s
16:04:08: real 0m0.201s user 0m0.010s sys 0m0.010s
16:04:10: real 0m0.230s user 0m0.000s sys 0m0.020s
16:04:12: real 0m0.206s user 0m0.010s sys 0m0.010s
16:04:15: real 0m0.446s user 0m0.010s sys 0m0.010s
```
Change-Id: I90b242e2cd24973420de871fedf9793dd1e310f3
Signed-off-by: Willy Tu <wltu@google.com>
|
|
The task payload object can be null in certain cases. As per the schema
for Task, there is a standard property Hidepayload which can be set to
true at this case. Setting this property as true in the response body
when Payload is nil.
Tested: Created a task without creating the task payload object and able
to see Hidepayload as true in response body.
Change-Id: I370d1eb9b5b96adb56cff2216b467357b0b34b42
Signed-off-by: Arun Thomas Baby <arunx.thomas.baby.baby.mathew@intel.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
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
|
|
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>
|
|
There's some tough-to-track-down safety problems in http Request. This
commit is an attempt to make things more safe, even if it isn't clear
how the old code was wrong.
Previously, the old code took a url_view from the target() string for a
given URI. This was effectively a pointer, and needed to be updated in
custom move/copy constructors that were error prone to write.
This commit moves to taking the URI by non-view, which involves a copy,
but allows us to use the default move and copy constructors, as well as
have no internal references within Request, which should improve the
safety and reviewability.
There's already so many string copies in bmcweb, that this is unlikely
to show up as any sort of performance regression, and simple code is
much better in this case.
Note, because of a bug in boost::url, we have to explicitly construct a
url_view in any case where we want to use segments() or query() on a
const Request. This has been reported to the boost maintainers, and is
being worked for a long term solution.
https://github.com/boostorg/url/pull/704
Tested: Redfish service validator passed on last commit in series.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I49a7710e642dff624d578ec1dde088428f284627
|
|
Per cpp core guidelines, we should be returning this via a function
call, not a direct member variable. Doing this also improves the
safety, as we don't have to remember to move the references over in a
move.
Tested: Tested as part of top patch in series.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I837d6fd277ffa076ba5425003d6e6ee79204d014
|
|
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>
|
|
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>
|
|
clang-tidy warns on these when run directly in a header file. Fix them.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib3366699c36e85644107690c23467f2ed22e398d
|
|
Most of these missing includes were found by running clang-tidy on all
files, including headers. The existing scripts just run clang-tidy on
source files, which doesn't catch most of these.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic741fbb2cc9e5e92955fd5a1b778a482830e80e8
|
|
We don't follow this cpp core guidelines rule well. This is something
that we should aspire to cleaning up in the future, but for the moment,
lets turn the rule on in clang-tidy to stop the bleeding, add ignores
for the things that we know need some better abstractions, and work on
these over time.
Most of this commit is just adding NOLINTNEXTLINE exceptions for all of
our globals. There was one case in the sensor code where clang
correctly noted that those globals weren't actually const, which got
missed because of the use of auto.
Tested: CI should be good enough for this. Passes clang-tidy.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ieda08fee69a3b209d4b3e9771809a6c41524f066
|
|
There's a few last places (outside of tests) where we still use
nlohmann brace initialization. Per the transforms we've been doing,
move these to constructing the objects explicitly, using operator[],
nlohmann::object_t and nlohmann::array_t. Theses were found by manual
inspection grepping for all uses of nlohmann::json.
This is done to reduce binary size and reduce the number of intermediate
objects being constructed. This commit saves a trivial amount of size
(~4KB, Half a percent of total) and in addition but makes our
construction consistent.
Tested:
Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7478479a9fdc41b254eef325002d413c1fb411a0
|
|
According to Redfish Base Message Registry definition [1], the first
argument of ResourceNotFound and ResourceAlreadyExists is the schema
name of the resource. This patch changes the first argument to non-
versioned schema name treewide.
Tested:
Verified the error message matches the definition, and Redfish Service
Validator passed.
[1] https://redfish.dmtf.org/registries/Base.1.13.0.json
Change-Id: Ib5cd853578ef0bffda1184d10827241e94faaf68
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
|
We should do this because it means that variables are only initialized
once. In the simplest case:
struct MyClass{
std::string myString
MyClass(){
myString = "foo";
}
}
in the language, myString is constructed twice, once with empty string,
then a second time with "foo". If you do the construction in the
initializer list for the class the construction only happens once.
Now, the above case is contrived, the optimizer can see through it and
likely optimizes this case because std::string is relatively simple, but
for more complex structures, it's possible this generates less and
bettercompiled code, and this is worth having the check for, and making
our existing code correct.
Tested: cppcheck passing.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie5c7f293598408d437e7bf7b3fc93b0819e25f9f
|
|
We've accumulated several time utility functions in the http classes.
Time isn't a core HTTP primitive, so http is not where those functions
below.
This commit moves all the time functions from the crow::utility
namespace into the redfish::time_utils namespace, as well as moves the
unit tests.
No code changes where made to the individual functions, with the
exception of changing the namespace on the unit tests.
Tested: Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8493375f60aea31899c84ae703e0f71a17dbdb73
|
|
The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines. Possible replacements are for:
* bus_t
* exception_t
* manager_t
* match_t
* message_t
* object_t
* slot_t
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I46a5eec210002af84239af74a93c830b1d4a13f1
|
|
cppcheck correctly notes that a lot of variables in the new code can be
const. Make most of them const.
Tested: WIP
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8f37b6353fd707923f533e1d61c5b5419282bf23
|
|
We essentially follow this rule already, not relying on implicit
operators, although there are a number of cases where in theory we
could've implicitly constructed an object.
This commit enables the clang-tidy check.
Tested: Code compiles, passes clang-tidy.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia428463313b075c69614fdb326e8c5c094e7adde
|
|
For Redfish Aggregation, we need a common point to check the D-Bus
for satellite configs. If they are available then we perform the
aggregation operations. The functions in query.hpp are used by all
endpoints making them the logical location. The aggregation code
requires a shared_ptr to the AsyncResp so these functions need to be
able to supply that.
This patch is broken out of a future patch for routing Redfish
Aggregation requests
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310
The follow commands can be used to perform most of the replacements:
find . -type f | xargs sed -i 's/setUpRedfishRoute(app, req, asyncResp->res/setUpRedfishRoute(app, req, asyncResp/g'
find . -type f | xargs sed -i 's/setUpRedfishRouteWithDelegation(app, req, asyncResp->res/setUpRedfishRouteWithDelegation(app, req, asyncResp/g'
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I4f4f9f22cdcfb14a3bd94b9a8f3d64aae34e57bc
|
|
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
|
|
Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies. This patchset aims to remove a majority of them in
lieu of operator[]. Interestingly, this saves about 1% of the binary
size of bmcweb.
This also has an added benefit that as a design pattern, we're never
constructing a new object, then moving it into place, we're always
adding to the existing object, which in the future _could_ make things
like OEM schemas or properties easier, as there's no case where we're
completely replacing the response object.
Tested:
Ran redfish service validator. No new failures.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iae409b0a40ddd3ae6112cb2d52c6f6ab388595fe
|
|
For better or worse, the series ahead of this is making use of
setUpRedfishRoute to do the common "redfish specified" things that need
to be done for a connection, like header checking, filtering, and other
things. In the current model, where BMCWEB_ROUTE is a common function
for all HTTP routes, this means we need to propagate this injection call
into the whole tree ahead of the requests being handled.
In a perfect world, we would invent something like a REDFISH_ROUTE
macro, but because macros are discouraged, the routes take a variadic
template of parameters, and each call to the route has a .privileges()
call in the middle, there's no good way to effect this change in a less
costly manner. This was messaged both in the prior reviews, and on
discord sourcing improvements on this pattern, to which none arose.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id29cc799e214edad41e48fc7ce6eed0521f90ecb
|
|
This saves about 4k on the binary size
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9546227a19c691b1aecb80e80307889548c0293f
|
|
The only changes were to make some functions static, which is
essentially no-op.
Changes were done by the robot.
Tested: Unit tests pass, changes no-op
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id84ca2bee6f237877ba2900b2cbe4679b38a91dc
|
|
The existing codes cast uint64_t into time_t which is int32_t in
most 32-bit systems. It results overflow if the timestamp is larger
than INT_MAX.
time_t will be 64 bits in future releases of glibc. See
https://sourceware.org/bugzilla/show_bug.cgi?id=28182.
This change workarounds the year 2038 problem via boost's ptime.
std::chrono doesn't help since it is still 32 bits.
Tested on QEMU.
Example output for certificate:
{
"Name": "HTTPS Certificate",
"Subject": null,
"ValidNotAfter": "2106-01-28T20:40:31Z",
"ValidNotBefore": "2106-02-06T18:28:16Z"
}
Previously, the format is like "1969-12-31T12:00:00+00:00". Note
that the ending "+00:00" is the time zone, not ms.
Tested the schema on QEMU. No new Redfish Service Validator errors.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8ef0bee3d724184d96253c23f3919447828d3f82
|
|
This commit attempts to automate the creation of our privileges
structures from the redfish privilege registry. It accomplishes this by
updating parse_registries.py to also pull down the privilege registry
from DMTF.
The script then generates privilege_registry.hpp, which include const
defines for all the privilege registry entries in the same format that
the Privileges struct accepts. This allows new clients to simply
reference the variable to these privilege structures, instead of having
to manually (ie error pronely) put the privileges in themselves.
This commit updates all the routes.
For the moment, override and OEM schemas are not considered. Today we
don't have any OEM-specific Redfish routes, so the existing ones inherit
their parents schema. Overrides have other issues, and are already
incorrect as Redfish defines them.
Binary size remains unchanged after this patchset.
Tested:
Ran redfish service validator
Ran test case from f9a6708c4c6490257e2eb6a8c04458f500902476 to ensure
that the new privileges constructor didn't cause us to regress the brace
construction initializer.
Checked binary size with:
gzip -c
$BBPATH/tmp/work/s7106-openbmc-linux-gnueabi/obmc-phosphor-image/1.0-r0/rootfs/usr/bin/bmcweb
| wc -c
1244048
(tested on previous patchset)
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ideede3d5b39d50bffe7fe78a0848bdbc22ac387f
|
|
This commit adds the support for "DateTimeLocalOffset" property under
"/redfish/v1/Managers/bmc/" Redfish URI.
And it also adds the support for "DateTime" & "DateTimeLocalOffset"
properties under "/redfish/v1/Systems/system/LogServices/<id>/" &
"/redfish/v1/Managers/bmc/LogServices/<id>/" Redfish URI's.
These properties shows the current Date, Time & the UTC offset that the
current DateTime property value contains.
Tested:
- Redfish Validator Test passed.
curl -k -H "X-Auth-Token: $token" -H "Content-Type: application/json" -X GET https://${bmc}/redfish/v1/Managers/bmc/
{
"@odata.id": "/redfish/v1/Managers/bmc",
"@odata.type": "#Manager.v1_11_0.Manager",
"Actions": {
"#Manager.Reset": {
"@Redfish.ActionInfo": "/redfish/v1/Managers/bmc/ResetActionInfo",
"target": "/redfish/v1/Managers/bmc/Actions/Manager.Reset"
},
"#Manager.ResetToDefaults": {
"ResetType@Redfish.AllowableValues": [
"ResetAll"
],
"target": "/redfish/v1/Managers/bmc/Actions/Manager.ResetToDefaults"
}
},
"DateTime": "2021-06-04T12:18:28+00:00",
"DateTimeLocalOffset": "+00:00",
"Description": "Baseboard Management Controller",
"EthernetInterfaces": {
"@odata.id": "/redfish/v1/Managers/bmc/EthernetInterfaces"
},
"FirmwareVersion": "2.11.0-dev-114-gc1989599d",
"GraphicalConsole": {
"ConnectTypesSupported": [
"KVMIP"
],
"MaxConcurrentSessions": 4,
"ServiceEnabled": true
},
"Id": "bmc",
"LastResetTime": "2021-06-04T12:07:02+00:00",
"Links": {
"ActiveSoftwareImage": {
"@odata.id": "/redfish/v1/UpdateService/FirmwareInventory/419c86fb"
},
"ManagerForServers": [
{
"@odata.id": "/redfish/v1/Systems/system"
}
],
"ManagerForServers@odata.count": 1,
"SoftwareImages": [
{
"@odata.id": "/redfish/v1/UpdateService/FirmwareInventory/419c86fb"
}
],
"SoftwareImages@odata.count": 1
},
"LogServices": {
"@odata.id": "/redfish/v1/Managers/bmc/LogServices"
},
"ManagerType": "BMC",
"Model": "OpenBmc",
"Name": "OpenBmc Manager",
"NetworkProtocol": {
"@odata.id": "/redfish/v1/Managers/bmc/NetworkProtocol"
},
"Oem": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem",
"@odata.type": "#OemManager.Oem",
"OpenBmc": {
"@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc",
"@odata.type": "#OemManager.OpenBmc",
"Certificates": {
"@odata.id": "/redfish/v1/Managers/bmc/Truststore/Certificates"
}
}
},
"PowerState": "On",
"SerialConsole": {
"ConnectTypesSupported": [
"IPMI",
"SSH"
],
"MaxConcurrentSessions": 15,
"ServiceEnabled": true
},
"ServiceEntryPointUUID": "1832ebbb-0b54-44e9-90d7-b49108f6863c",
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Enabled"
},
"UUID": "7fe3d13d-4ae7-4a4f-add1-2d60308124b4"
}
curl -k -H "X-Auth-Token: $token" -H "Content-Type: application/json" -X GET https://${bmc}/redfish/v1/Systems/system/LogServices/EventLog/
{
"@odata.id": "/redfish/v1/Systems/system/LogServices/EventLog",
"@odata.type": "#LogService.v1_1_0.LogService",
"Actions": {
"#LogService.ClearLog": {
"target": "/redfish/v1/Systems/system/LogServices/EventLog/Actions/LogService.ClearLog"
}
},
"DateTime": "2021-06-04T12:11:10+00:00",
"DateTimeLocalOffset": "+00:00",
"Description": "System Event Log Service",
"Entries": {
"@odata.id": "/redfish/v1/Systems/system/LogServices/EventLog/Entries"
},
"Id": "EventLog",
"Name": "Event Log Service",
"OverWritePolicy": "WrapsWhenFull"
}
Signed-off-by: Tejas Patil <tejaspp@ami.com>
Change-Id: I416d13ae11e236cf4552f817a9bd69b48f9b5afb
|
|
There are a number of endpoints that assume that a given routes
privileges are governed by a single set of privileges, instead of
multiple sets ORed together. To handle this, there were two overloads
of the privileges() method, one that took a vector of Privileges, and
one that took an initializer_list of const char*. Unfortunately, this
leads some code in AccountService to pick the wrong overload when it's
called like this
.privileges( {{"ConfigureUsers"}, {"ConfigureManager"},
{"ConfigureSelf"}})
This is supposed to be "User must have ConfigureUsers, or
ConfigureManager, or ConfigureSelf". Currently, because it selects the
wrong overload, it computes to "User must have ConfigureUsers AND
ConfigureManager AND ConfigureSelf.
The double braces are supposed to cause this to form a vector of
Privileges, but it appears that the initializer list gets consumed, and
the single invocation of initializer list is called. Interestingly,
trying to put in a privileges overload of
intializer_list<initializer_list<const char*>> causes the compilation to
fail with an ambiguous call error, which is what I would've expected to
see previously in this case, but alas, I'm only a novice when it comes
to how the C++ standard works in these edge cases. This is likely due
in part to the fact that they were templates of an unused template param
(seemingly copied from the previous method) and SFINAE rules around
templates.
This commit functionally removes one of the privileges overloads, and
adds a second set of braces to every privileges call that previously had
a single set of braces. Previous code will not compile now, which is
IMO a good thing.
This likely popped up in the Node class removal, because the Node class
explicitly constructs a vector of Privilege objects, ensuing it can hit
the right overload
Tested:
Ran Redfish service validator
Tested the specific use case outlined on discord with:
Creating a new user with operator privilege:
```
redfishtool -S Always -u root -p 0penBmc -vvvvvvvvv -r 192.168.7.2
AccountService adduser foo mysuperPass1 Operator
```
Then attempting to list accounts:
```
curl -vvvv --insecure --user foo:mysuperPass1
https://192.168.7.2/redfish/v1/AccountService/Accounts/foo
```
Which succeeded and returned the account in question.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I83e62b70e97f56dc57d43b9081f333a02fe85495
|
|
Reduces the total number of lines and will allow for easier testing of
the redfish responses.
A main purpose of the node class was to set app.routeDynamic(). However
now app.routeDynamic can handle the complexity that was once in critical
to node. The macro app.routeDynamic() provides a shorter cleaner
interface to the unerlying app.routeDyanic call. The old pattern set
permissions for 6 interfaces (get, head, patch, put, delete_, and post)
even if only one interface is created. That pattern creates unneeded
code that can be safely removed with no effect.
Unit test for the responses would have to mock the node the class in
order to fully test responses.
see https://github.com/openbmc/bmcweb/issues/181
The following files still need node to be extracted.
virtual_media.hpp
account_service.hpp
redfish_sessions.hpp
ethernet.hpp
The files above use a pattern that is not trivial to address. Often their
responses call an async lambda capturing the inherited class. ie
(https://github.com/openbmc/bmcweb/blob/ffed87b5ad1797ca966d030e7f979770
28d258fa/redfish-core/lib/account_service.hpp#L1393)
At a later point I plan to remove node from the files above.
Tested:
I ran the docker unit test with the following command.
WORKSPACE=$(pwd) UNIT_TEST_PKG=bmcweb
./openbmc-build-scripts/run-unit-test-docker.sh
I ran the validator and this change did not create any issues.
python3 RedfishServiceValidator.py -c config.ini
Signed-off-by: John Edward Broadbent <jebr@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I147a0289c52cb4198345b1ad9bfe6fdddf57f3df
|
|
Get the core using AsyncResp everywhere, and not have each individual handler
creating its own object.We can call app.handle() without fear of the response
getting ended after the first tree is done populating.
Don't use res.end() anymore.
Tested:
1. Validator passed.
Signed-off-by: zhanghaicheng <zhanghch05@inspur.com>
Change-Id: I867367ce4a0caf8c4b3f4e07e06c11feed0782e8
|
|
The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases. http connection already does this
properly, but a bunch of code has snuck in (mostly in redfish) that
ignores this, and calls it incorrectly. This can potentially lead to a
crash if the wrong thing throws on invalid UTF8 characters.
This audits the whole codebase, and replaces every dump() call with the
correct dump(2, ' ', true, nlohmann::json::error_handler_t::replace)
call. For correct output, the callers should expect no change, and in
practice, this would require injecting non-utf8 characters into the
BMC.
Tested:
Ran several of the endpoints/error conditions in question, including
some of the error cases. Observed correct responses. I don't know of a
security issue that would allow injecting invalid utf8 into the BMC, but
in theory if it were possible, this would prevent a crash.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4a15b8e260e3db129bc20484ade4ed5449f75ad0
|
|
In order to implement full task support for code update,
the PercentComplete property is added to the task service.
Testd: Validator passes.
curl -k -H "X-Auth-Token: $token" -X GET https://${bmc}/redfish/v1/TaskService/Tasks/0
{
"@odata.id": "/redfish/v1/TaskService/Tasks/0",
"@odata.type": "#Task.v1_4_3.Task",
... ...
"PercentComplete": 100,
"TaskMonitor": "/redfish/v1/TaskService/Tasks/0/Monitor",
"TaskState": "Completed",
"TaskStatus": "OK"
}
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: I030bf0bbff098dec2f45158642f149711554285d
|
|
Several pieces of code seems to be using the adl_serializer from
nlohmann. This unfortunately has very undesirable behavior in some
cases, and makes a lot of things really difficult to track back to the
function that did the serialization, which has caused several bugs in
the past with incorrect types.
This patchset removes them, and opts for the inline version of the
nlohmann json serialization.
Tested:
Booted bmcweb, and logged in.
cat bmcweb_persistent_data.json showed persistent data written properly.
Logged into bmc through webui-vue
systemctl restart bmcweb
Then refreshed webui-vue, and didn't get logged out.
Change-Id: I92868629c54d08b37dd1d956f7c2e2a954f9b670
|
|
Lots of bad includes got put in recently, including big things, like
boost/http and beast/core. These are lots of code to parse, and leads
to files including things they didn't mean to.
Tested:
Code compiles
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I38de889fdfd9b23f66a2259bb30bf6584345e77f
|
|
It is not required. Also, move the deleted destructor to a public
member where it should be.
Tested:
Code builds. No functional changes.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I4b04ee0870e908fb8204d91e2cd8505518272c7a
|
|
The commit implements the Push style events when a Task
goes through its life cycle.
Tested by: (Used https://github.com/DMTF/Redfish-Event-Listener)
1. Create a subscription by passing "ResourceTypes" as ["Task"]
POST -D headers.txt https://${bmc}/redfish/v1/EventService/Subscriptions
-d '{"Destination" : "https://<host:port>, "ResourceTypes":["Task"], "Protocol":"Redfish"}'
2. Run a command which starts the Task
3. Verify if the task state changes are reported as events
Signed-off-by: Sunitha Harish <sunharis@in.ibm.com>
Change-Id: I4cea83f221e6f4f40cfbdc3da3e95cd920744316
|
|
This commit enables the "unused variables" warning in clang. Throughout
this, it did point out several issues that would've been functional
bugs, so I think it was worthwhile. It also cleaned up several unused
variable from old constructs that no longer exist.
Tested:
Built with clang. Code no longer emits warnings.
Downloaded bmcweb to system and pulled up the webui, observed webui
loads and logs in properly.
Change-Id: I51505f4222cc147d6f2b87b14d7e2ac4a74cafa8
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This commit enables clang warnings, and fixes all warnings that were
found. Most of these fall into a couple categories:
Variable shadow issues were fixed by renaming variables
unused parameter warnings were resolved by either checking error codes
that had been ignored, or removing the name of the variable from the
scope.
Other various warnings were fixed in the best way I was able to come up
with.
Note, the redfish Node class is especially insidious, as it causes all
imlementers to have variables for parameters, regardless of whether or
not they are used. Deprecating the Node class is on my list of things
to do, as it adds extra overhead, and in general isn't a useful
abstraction. For now, I have simply fixed all the handlers.
Tested:
Added the current meta-clang meta layer into bblayers.conf, and added
TOOLCHAIN_pn-bmcweb = "clang" to my local.conf
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Ia75b94010359170159c703e535d1c1af182fe700
|
|
Middlewares, while kinda cool from an academic standpoint, make our
build times even worse than they already are. Given that we only really
use 1 real middleware today (token auth) and it needs to move into the
parser mode anyway (for security limiting buffer sizes), we might as well
use this as an opportunity to delete some code.
Some other things that happen:
1. Persistent data now moves out of the crow namespace
2. App is no longer a template
3. All request_routes implementations no longer become templates. This
should be a decent (unmeasured) win on compile times.
This commit was part of a commit previously called "various cleanups".
This separates ONLY the middleware deletion part of that.
Note, this also deletes about 400 lines of hard to understand code.
Change-Id: I4c19e25491a153a2aa2e4ef46fc797bcb5b3581a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
These spelling errors were found using
https://github.com/codespell-project/codespell
Tested: Top commit (along with this) was built and ran against
validator.
Change-Id: Ic9dce27b1de8567eedf7753164ef564d3aedf8ca
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This is from openbmc/docs/style/cpp/.clang-format
Other OpenBMC repos are doing the same.
Tested: Built and validator passed.
Change-Id: Ief26c755c9ce012823e16a506342b0547a53517a
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This adds reporting of percent updated and changes
"staged" to paused to indicate some further action
must happen to change state to Completed.
Tested: validator passed
"Messages": [
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has started.",
"MessageArgs": [
"2"
],
"MessageId": "TaskEvent.1.0.1.TaskStarted",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 5 percent complete.",
"MessageArgs": [
"2",
5
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 10 percent complete.",
"MessageArgs": [
"2",
10
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 15 percent complete.",
"MessageArgs": [
"2",
15
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 20 percent complete.",
"MessageArgs": [
"2",
20
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 25 percent complete.",
"MessageArgs": [
"2",
25
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 30 percent complete.",
"MessageArgs": [
"2",
30
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 35 percent complete.",
"MessageArgs": [
"2",
35
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 40 percent complete.",
"MessageArgs": [
"2",
40
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 45 percent complete.",
"MessageArgs": [
"2",
45
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 50 percent complete.",
"MessageArgs": [
"2",
50
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 55 percent complete.",
"MessageArgs": [
"2",
55
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 60 percent complete.",
"MessageArgs": [
"2",
60
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 65 percent complete.",
"MessageArgs": [
"2",
65
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 70 percent complete.",
"MessageArgs": [
"2",
70
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 75 percent complete.",
"MessageArgs": [
"2",
75
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 80 percent complete.",
"MessageArgs": [
"2",
80
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 85 percent complete.",
"MessageArgs": [
"2",
85
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 90 percent complete.",
"MessageArgs": [
"2",
90
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 95 percent complete.",
"MessageArgs": [
"2",
95
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has changed to progress 100 percent complete.",
"MessageArgs": [
"2",
100
],
"MessageId": "TaskEvent.1.0.1.TaskProgressChanged",
"Resolution": "None.",
"Severity": "OK"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has been paused.",
"MessageArgs": [
"2"
],
"MessageId": "TaskEvent.1.0.1.TaskPaused",
"Resolution": "None.",
"Severity": "Warning"
},
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 2 has Completed.",
"MessageArgs": [
"2"
],
"MessageId": "TaskEvent.1.0.1.TaskCompletedOK",
"Resolution": "None.",
"Severity": "OK"
}
],
Change-Id: I32103e53486d459fe945a8b451d2092232c12e83
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
|
Task registry messages make more sense to use
for task events then standard registry entries
when applicable. Use them.
Tested:
"Messages": [
{
"@odata.type": "#Message.v1_0_0.Message",
"Message": "The task with id 0 has started.",
"MessageArgs": [
"0"
],
"MessageId": "TaskEvent.1.0.1.TaskStarted",
"Resolution": "None.",
"Severity": "OK"
}
],
Validator passed
Change-Id: I707492544e18def2833e8a2e2216ce803c42c775
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
|
The boost::asio::io_context::post() is deprecated and will not build
when boost library is version 1.71.0.
See CMakeLists.txt
271 if ("${Boost_VERSION}" STREQUAL "107100")
272 add_definitions (-DBOOST_ASIO_NO_DEPRECATED)
273 endif ()
Signed-off-by: Zhenfei Tai <ztai@google.com>
Change-Id: Iacd657af320ba6eebb1518cd86a955f355ed5068
|
|
This is defined in the Redfish protocol.
Easiest way to allow this is to end the Node URL with "/", which most
Nodes in bmcweb already had.
Before:
curl -k https://${bmc}/redfish/v1/TaskService/
Not Found
After both /redfish/v1/TaskService/ and /redfish/v1/TaskService
return the Task Service.
Tested:
Validator passed.
Change-Id: Ic806dc5c91f631b87642e49b486a6b6da7fdf955
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This adds the payload values to task responses.
Tested: passed validator
Change-Id: I50467e28ce8142d198f916ea0c63bd413edcd524
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
