e93abac6 | 14-Jun-2024 |
Ginu George <ginugeorge@ami.com> |
Pass redfishPropertyName earlier argument
It was pointed out that the setDbusProperty method should have an end that approximately matches dbus-send and busctl set-property in its arguments, to aid
Pass redfishPropertyName earlier argument
It was pointed out that the setDbusProperty method should have an end that approximately matches dbus-send and busctl set-property in its arguments, to aid with debug. This seems reasonable.
Tested: Redfish service validator passes.
Change-Id: Ic20295d93c71c957e3e76704e1eda9da187861b1 Signed-off-by: Ginu George <ginugeorge@ami.com> Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
0bdda665 | 03-Aug-2023 |
Ed Tanous <edtanous@google.com> |
Remove nlohmann::json::items()
nlohmann::json::items() throws an exception if the object in question is not a json object. This has the potential to cause problems, and isn't in line with the stand
Remove nlohmann::json::items()
nlohmann::json::items() throws an exception if the object in question is not a json object. This has the potential to cause problems, and isn't in line with the standard that we code against.
Replace all uses of items with iterating the nlohmann::json::object_t.
This adds a new error check for pulling the object_t out of the nlohmann::json object before each iteration, to ensure that we're handling errors.
Tested: Redfish service validator passes.
Change-Id: I2934c9450ec296c76544c2a7c5855c9b519eae7f Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
f653730d | 19-Jun-2024 |
Ed Tanous <ed@tanous.net> |
Clean up time utils
We have std::format and c++20 now, this code can be simplified.
Tested: Unit tests pass. Good coverage.
Change-Id: Idc466d0ace9128a72bcd098290010a6a19da8536 Signed-off-by: Ed
Clean up time utils
We have std::format and c++20 now, this code can be simplified.
Tested: Unit tests pass. Good coverage.
Change-Id: Idc466d0ace9128a72bcd098290010a6a19da8536 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
253f11b8 | 16-May-2024 |
Ed Tanous <ed@tanous.net> |
Allow configuring "bmc" and "system"
In the early days of bmcweb, we made two pretty critical assumptions; First, is that a given platform would only have a single BMC instance (represented as "bmc"
Allow configuring "bmc" and "system"
In the early days of bmcweb, we made two pretty critical assumptions; First, is that a given platform would only have a single BMC instance (represented as "bmc") and a single host instance (represented as "system"). Second we assumed that, given that Redfish suggests against hardcoding URIs in client implementation and leaves them freeform, clients would code to the standard.
Our own webui-vue hardcodes Redfish URIs [1], and the documentation is littered with examples of hardcoded curl examples of hardcoding these URIs. That bug was filed in 2020, and the issue has only gotten worse over time.
This patchset is an attempt to give a target that we can start solving these issues, without trying to boil the ocean and fix all clients in parallel.
This commit adds the meson options redfish-manager-uri-name and redfish-system-uri-name
These are used to control the "name" that bmcweb places in the fixed locations in the ManagerCollection and ComputerSystemCollection schemas.
Note, managers is added, but is not currently testable. It will be iterated on over time.
Tested: Changed the URL options to "edsbmc" and "edssystem" in meson options.
Redfish service validator passes. URLs appear changed when walking the tree.
[1] https://github.com/openbmc/webui-vue/issues/43
Change-Id: I4b44685067051512bd065da8c2e3db68ae5ce23a Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
102a4cda | 15-Apr-2024 |
Jonathan Doman <jonathan.doman@intel.com> |
Manage Request with shared_ptr
This is an attempt to solve a class of use-after-move bugs on the Request objects which have popped up several times. This more clearly identifies code which owns the
Manage Request with shared_ptr
This is an attempt to solve a class of use-after-move bugs on the Request objects which have popped up several times. This more clearly identifies code which owns the Request objects and has a need to keep it alive. Currently it's just the `Connection` (or `HTTP2Connection`) (which needs to access Request headers while sending the response), and the `validatePrivilege()` function (which needs to temporarily own the Request while doing an asynchronous D-Bus call). Route handlers are provided a non-owning `Request&` for immediate use and required to not hold the `Request&` for future use.
Tested: Redfish validator passes (with a few unrelated fails). Redfish URLs are sent to a browser as HTML instead of raw JSON.
Change-Id: Id581fda90b6bceddd08a5dc7ff0a04b91e7394bf Signed-off-by: Jonathan Doman <jonathan.doman@intel.com> Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
1aa375b8 | 13-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Implement client certificate schemas
The Redfish standard seems to have caught up with some of the OEM schemas and features we already have, namely MutualTLS and Basic Auth disablement.
This commit
Implement client certificate schemas
The Redfish standard seems to have caught up with some of the OEM schemas and features we already have, namely MutualTLS and Basic Auth disablement.
This commit implements most of the GET parameters for which we already have backends. ClientCertificate is pointed to the same resources as TrustStore.
Tested: generate_auth_certificates.py succeeds, and shows a certificate in ClientCertificate collection
Get AccountService, and ClientAuthentication/Certificates returns expected values.
Redfish service validator passes.
Change-Id: If18e34e9dfa8f38293fceff288596811afd16d4a Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
25b54dba | 17-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Bring consistency to config options
The configuration options that exist in bmcweb are an amalgimation of CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms and meson options usi
Bring consistency to config options
The configuration options that exist in bmcweb are an amalgimation of CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms and meson options using a config file. This history has led to a lot of different ways to configure code in the codebase itself, which has led to problems, and issues in consistency.
ifdef options do no compile time checking of code not within the branch. This is good when you have optional dependencies, but not great when you're trying to ensure both options compile.
This commit moves all internal configuration options to: 1. A namespace called bmcweb 2. A naming scheme matching the meson option. hyphens are replaced with underscores, and the option is uppercased. This consistent transform allows matching up option keys with their code counterparts, without naming changes. 3. All options are bool true = enabled, and any options with _ENABLED or _DISABLED postfixes have those postfixes removed. (note, there are still some options with disable in the name, those are left as-is) 4. All options are now constexpr booleans, without an explicit compare.
To accomplish this, unfortunately an option list in config/meson.build is required, given that meson doesn't provide a way to dump all options, as is a manual entry in bmcweb_config.h.in, in addition to the meson_options. This obsoletes the map in the main meson.build, which helps some of the complexity.
Now that we've done this, we have some rules that will be documented. 1. Runtime behavior changes should be added as a constexpr bool to bmcweb_config.h 2. Options that require optionally pulling in a dependency shall use an ifdef, defined in the primary meson.build. (note, there are no options that currently meet this class, but it's included for completeness.)
Note, that this consolidation means that at configure time, all options are printed. This is a good thing and allows direct comparison of configs in log files.
Tested: Code compiles Server boots, and shows options configured in the default build. (HTTPS, log level, etc)
Change-Id: I94e79a56bcdc01755036e4e7278c7e69e25809ce Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
95c6307a | 26-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Break out formatters
In the change made to move to std::format, we defined some custom type formatters in logging.hpp. This had the unintended effect of making all compile units pull in the majorit
Break out formatters
In the change made to move to std::format, we defined some custom type formatters in logging.hpp. This had the unintended effect of making all compile units pull in the majority of boost::url, and nlohmann::json as includes.
This commit breaks out boost and json formatters into their own separate includes.
Tested: Code compiles. Logging changes only.
Change-Id: I6a788533169f10e19130a1910cd3be0cc729b020 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
ed4de7a8 | 26-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Add type safety for NTP server objects
NTPServers is our last usage of nlohmann::json in a readJson unpack. The capability and unit tests are left in place for that type in case we need them in the
Add type safety for NTP server objects
NTPServers is our last usage of nlohmann::json in a readJson unpack. The capability and unit tests are left in place for that type in case we need them in the future, but for now, document them as deprecated.
Tested: Redfish service validator passes. Redfish protocol validator passes most tests (1 known failure in SSE is unrelated to this change).
Change-Id: If4b2ea061a941cc23d47189af7ff453094dc7dca Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
c98dbc64 | 07-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Remove logically dead code
Stage::T is never set, so it can never happen. Remove it entirely.
Found using static analysis.
Tested: Unit tests pass, good coverage here
Change-Id: I0dfb1aad5bef3ab
Remove logically dead code
Stage::T is never set, so it can never happen. Remove it entirely.
Found using static analysis.
Tested: Unit tests pass, good coverage here
Change-Id: I0dfb1aad5bef3ab4451df5e81794e56074f6e939 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
c09966bd | 07-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Give static analysis some hints
Static analysis tools complain that for certain template types, these comparisons do nothing. Doing nothing is what they're intended to do, as we have other checks.
Give static analysis some hints
Static analysis tools complain that for certain template types, these comparisons do nothing. Doing nothing is what they're intended to do, as we have other checks.
Add some constexpr if hints so static analysis tools don't complain.
Change-Id: I60297d094626936d021382ccdc11e4c8b698e3d8 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
76b038f2 | 06-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Fix object type in json utils
This code accidentally makes a copy, given that getJsonObject returns a std::optional<nlohmann::json::object_t> which is then loaded into a std::optional<nlohmann::json
Fix object type in json utils
This code accidentally makes a copy, given that getJsonObject returns a std::optional<nlohmann::json::object_t> which is then loaded into a std::optional<nlohmann::json>. Because nlohmann::json is implicitly constructible from an object_t, this code works and compiles, but we shouldn't need the intermediate object at all.
Change the code to simply load the value as object_t.
Tested: Unit tests pass. Good coverage.
Change-Id: Ic57953e66958e69a1233e18a5bbd980405cac58e Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
f1a1e3dc | 06-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Simplify query_param
Static analysis notes that the values in these functions are never initialized, and that a small section of the branch is no longer possible to hit, now that a default case has
Simplify query_param
Static analysis notes that the values in these functions are never initialized, and that a small section of the branch is no longer possible to hit, now that a default case has been added in 4da0490bc07a458ad3fc7d586c7eabf6053c572f
Remove the dead code and initialize variables where appropriate.
Tested: Unit tests pass. Decent coverage here.
Change-Id: I42ec4678672fea5b21f98aaae05dfca0629652e7 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
1827b4f1 | 03-Apr-2024 |
Asmitha Karunanithi <asmitk01@in.ibm.com> |
Move to Redfish Action specific setProperty call
This commit will migrate all the setProperty calls initiated by a redfish"Action" to "setDbusProperty" method in Redfish namespace that handles all D
Move to Redfish Action specific setProperty call
This commit will migrate all the setProperty calls initiated by a redfish"Action" to "setDbusProperty" method in Redfish namespace that handles all DBuserrors in a consistent manner.
This method will determine if a setProperty is called during redfish "Action" or just setting of a dbus property and internally call appropriate methods that handles different set of errors.
All the Redfish action specific errors are defined in error_messages.hpp file. This specific change moves setProperty call in hypervisor_system.hpp and covers errors in the mentioned file only.
Tested-By: <Yet to test this usecase>
Change-Id: I3da48fbeabcdcf088c4481021232f08a44797c86 Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com> Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
8cb2c024 | 27-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Fix moves/forward
Clang has new checks for std::move/std::forward correctness, which catches quite a few "wrong" things where we were making copies of callback handlers.
Unfortunately, the lambda s
Fix moves/forward
Clang has new checks for std::move/std::forward correctness, which catches quite a few "wrong" things where we were making copies of callback handlers.
Unfortunately, the lambda syntax of
callback{std::forward<Callback>(callback)}
in a capture confuses it, so change usages to callback = std::forward<Callback>(callback)
to be consistent.
Tested: Redfish service validator passes.
Change-Id: I7a111ec00cf78ecb7d5f5b102c786c1c14d74384 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
8099c517 | 01-Nov-2023 |
Ed Tanous <edtanous@google.com> |
Allow parsing null or value classes
In Redfish schema, just about all values can be a type (string, EDM.Numeric, etc) or null. Most APIs don't allow explicitly setting null, but there are a few cas
Allow parsing null or value classes
In Redfish schema, just about all values can be a type (string, EDM.Numeric, etc) or null. Most APIs don't allow explicitly setting null, but there are a few cases where it is useful, namely in lists, where an an empty object {} keeps the value the same, and null deletes the value from the list.
Previously we handled this by unpacking as nlohmann::json, but this allowed things like
[1.0, {}] to pass the check for an array of string values. We'd ideally like to reject the 1.0 at the first stage, as well as reduce the number of tiered readJson calls that we make.
This commit introducess support for unpacking std::variant types, that allows unpacking a known type, or explicitly allowing null, by unpacking std::nullptr_t.
Tested: Unit tests pass.
Change-Id: Ic7451877c824ac743faf1951cc2b5d9f8df8019c Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
5be2b14a | 27-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Move where appropriate
Clang-tidy-18 has new checks that can find more cases where we've missed an opportunity to std::move.
Fix them.
Tested: Logging works, unit tests pass.
Change-Id: I0cf58204
Move where appropriate
Clang-tidy-18 has new checks that can find more cases where we've missed an opportunity to std::move.
Fix them.
Tested: Logging works, unit tests pass.
Change-Id: I0cf58204ce7265828693b787a7b3a16484c3d5e5 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
9de65b34 | 27-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Fix redundant inline operators
inline is not required on member methods. Clang-tidy has a check for this. Enable the check and fix the two bad usages.
Tested: Code compiles.
Change-Id: I3115b7c0
Fix redundant inline operators
inline is not required on member methods. Clang-tidy has a check for this. Enable the check and fix the two bad usages.
Tested: Code compiles.
Change-Id: I3115b7c0c4005e1082e0005b818fbe6569511f49 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
4da0490b | 19-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Use no-switch-default on clang
clang-18 improves this check so that we can actually use it. Enable it and fix all violations.
Change-Id: Ibe4ce19c423d447a4cbe593d1abba948362426af Signed-off-by: Ed
Use no-switch-default on clang
clang-18 improves this check so that we can actually use it. Enable it and fix all violations.
Change-Id: Ibe4ce19c423d447a4cbe593d1abba948362426af Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
d02aad39 | 13-Feb-2024 |
Ed Tanous <ed@tanous.net> |
Create Redfish specific setProperty call
There are currently 78 sdbusplus::asio::setProperty calls in redfish-core. The error handler for nearly all of them looks something like:
``` if (ec) {
Create Redfish specific setProperty call
There are currently 78 sdbusplus::asio::setProperty calls in redfish-core. The error handler for nearly all of them looks something like:
``` if (ec) { const sd_bus_error* dbusError = msg.get_error(); if ((dbusError != nullptr) && (dbusError->name == std::string_view( "xyz.openbmc_project.Common.Error.InvalidArgument"))) { BMCWEB_LOG_WARNING("DBUS response error: {}", ec); messages::propertyValueIncorrect(asyncResp->res, "<PropertyName>", <PropertyValue>); return; } messages::internalError(asyncResp->res); return; } messages::success(asyncResp->res);
```
In some cases there are more errors handled that translate to more error messages, but the vast majority only handle InvalidArgument. Many of these, like the ones in account_service.hpp, do the error handling in a lambda, which causes readability problems. This commit starts to make things more consistent, and easier for trivial property sets.
This commit invents a setDbusProperty method in the redfish namespace that tries to handle all DBus errors in a consistent manner. Looking for input on whether this will work before changing over the other 73 calls. Overall this is less code, fewer inline lambdas, and defaults that should work for MOST use cases of calling an OpenBMC daemon, and fall back to more generic errors when calling a "normal" dbus daemon.
As part of this, I've ported over several examples. Some things that might be up in the air: 1. Do we always return 204 no_content on property sets? Today there's a mix of 200, with a Base::Success message, and 204, with an empty body. 2. Do all DBus response codes map to the same error? A majority are covered by xyz.openbmc_project.Common.Error.InvalidArgument, but there are likely differences. If we allow any daemon to return any return code, does that cause compatibility problems later?
Tested: ``` curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"HostName":"openbmc@#"}' https://192.168.7.2/redfish/v1/Managers/bmc/EthernetInterfaces/eth0 ```
Returns the appropriate error in the response Base.1.16.0.PropertyValueIncorrect
Change-Id: If033a1112ba516792c9386c997d090c8f9094f3a Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
47f2934c | 19-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Fix redundant init issues
clang-tidy-18 must've fixed their checking for these in headers. Resolve as the robot commands.
Tested: Noop changes made by tidy. Code compiles.
Change-Id: I1de7686c597
Fix redundant init issues
clang-tidy-18 must've fixed their checking for these in headers. Resolve as the robot commands.
Tested: Noop changes made by tidy. Code compiles.
Change-Id: I1de7686c597deffb0df91c30dae1a29f9ba7900e Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
b6164cbe | 06-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Make readJson accept object_t
Redfish supports several type systems for json. This makes parsing into proper types a challenge. Nlohmann supports 3 core data types, nlohmann::json, which supports
Make readJson accept object_t
Redfish supports several type systems for json. This makes parsing into proper types a challenge. Nlohmann supports 3 core data types, nlohmann::json, which supports all json types (float, int, array, object). Nlohmann::json::object_t, which is a specific typedef of std::map, and nlohmann::json::array_t, which is a specific typedef of std::map.
Redfish allows reading our arrays of complex objects, similar to
NtpServers: [null, {}, "string"]
Which makes it a challenge to support. This commit allows parsing out objects as a nlohmann::object_t, which gives the ability to later use it in a type safe manner, without having to call get_ptr<nlohmann::json::object_t later>.
Tested: Unit tests pass.
Change-Id: I4134338951ce27c2f56841a45b56bc64ad1753db Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
c51afd54 | 07-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Call systemd SetTime directly
Internally inside phosphor-time-manager, the elapsed(uint64) dbus call just forwards the request directly to systemd after static casting to int64_t (signed).
bmcweb s
Call systemd SetTime directly
Internally inside phosphor-time-manager, the elapsed(uint64) dbus call just forwards the request directly to systemd after static casting to int64_t (signed).
bmcweb should just call systemd directly, for several reasons.
phosphor-timesyncd might block on other calls, given it's a single threaded blocking design, due to bugs like #264. Calling systemd directly means that calls that don't require phosphor networkd won't be blocked.
Calling systemd directly allows bmcweb to drop some code that parses a date as int64_t, then converts it to uint64_t to fulfill the phosphor datetime interface. We can now keep int64_t all the way through.
Calling systemd directly allows bmcweb to give a more specific error code in the case there NTP is enabled, registering a PropertyValueConflict error, instead of a 500 InternalError.
Tested: Patching DateTime property with NTP enabled returns 400, PropertyValueConflict ``` curl -vvvv -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"DateTime":"2020-12-15T15:40:52+00:00"}' https://192.168.7.2/redfish/v1/Managers/bmc ```
Disabling NTP using the following command: ``` curl -vvvv -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"NTP":{"ProtocolEnabled":false}}' https://192.168.7.2/redfish/v1/Managers/bmc/NetworkProtocol ```
Allows the prior command to succeed.
[1] https://github.com/openbmc/phosphor-time-manager/blob/5ce9ac0e56440312997b25771507585905e8b360/bmc_epoch.cpp#L126
Change-Id: I6fbb6f63e17de8ab847ca5ed4eadc2bd313586d2 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
52e31629 | 23-Jan-2024 |
Ed Tanous <ed@tanous.net> |
Simplify body
Now that we have a custom boost http body class, we can use it in more cases. There's some significant overhead and code when switching to a file body, namely removing all the headers
Simplify body
Now that we have a custom boost http body class, we can use it in more cases. There's some significant overhead and code when switching to a file body, namely removing all the headers. Making the body class support strings would allow us to completely avoid that inefficiency. At the same time, it would mean that we can now use that class for all cases, including HttpClient, and http::Request. This leads to some code reduction overall, and means we're reliant on fewer beast structures.
As an added benefit, we no longer have to take a dependency on boost::variant2.
Tested: Redfish service validator passes, with the exception of badNamespaceInclude, which is showing warnings prior to this commit.
Change-Id: I061883a73230d6085d951c15891465c2c8445969 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
18f8f608 | 18-Jul-2023 |
Ed Tanous <edtanous@google.com> |
Remove some boost includes
The less we rely on boost, and more on std algorithms, the less people have to look up, and the more likely that our code will deduplicate.
Replace all uses of boost::alg
Remove some boost includes
The less we rely on boost, and more on std algorithms, the less people have to look up, and the more likely that our code will deduplicate.
Replace all uses of boost::algorithms with std alternatives.
Tested: Redfish Service Validator passes.
Change-Id: I8a26f39b5709adc444b4178e92f5f3c7b988b05b Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|