summaryrefslogtreecommitdiff
path: root/redfish-core
AgeCommit message (Collapse)AuthorFilesLines
6 daysupdateservice: refactor task creationJagpal Singh Gill1-119/+117
Refactor task creation into a separate function so it can be used from different places in code. The new usage of this function will be from start update interface based flow. More details refer to - https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/65738 https://gerrit.openbmc.org/c/openbmc/docs/+/65739 Tested: Firmware update using curl. Change-Id: I5e8a0ab98f49657178ee733fa4d34fbf40a7b1f3 Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
6 daysupdateservice: refactor parse multi-part formsJagpal Singh Gill1-33/+72
Refactor parsing multi-part forms into a different function for using it in legacy vs start update flows. More more details refer to - https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/65738 https://gerrit.openbmc.org/c/openbmc/docs/+/65739 Tested: Used curl to test firmware update. Change-Id: I2ec1ec9f4ac04349a1fbd588664f2d51bae827ea Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
7 daysFix lesser used optionsEd Tanous1-15/+9
25b54dba775b31021a3a4677eb79e9771bcb97f7 missed several cases where we had ifndef instead of ifdef. because these weren't the defaults, these don't show up as failures when testing. Tested: Redfish service validator passes. Inspection primarily. Mechanical change. Change-Id: I3f6915a97eb44d071795aed76476c6bee7e8ed27 Signed-off-by: Ed Tanous <ed@tanous.net>
8 daysFix TFTP merge conflictEd Tanous1-5/+6
Static analysis now shows this code as "dead", even though it's not. This is a merge conflict that was handled wrong. Tested: inspection only. Suspect TFTP will now work. Change-Id: I51e52d62c51b251baf4c6ae74b100c1eda95603d Signed-off-by: Ed Tanous <ed@tanous.net>
9 daysAdd https parsingEd Tanous1-1/+25
This is yet another step in parsing HTTP requests. Tested: ''' curl -vvvv -k --user "root:0penBmc" -H "Content-Type: application/json" \ -X POST https://192.168.7.2/redfish/v1/UpdateService/Actions/UpdateService.SimpleUpdate \ -d '{ \ "TransferProtocol":"TFTP", \ "ImageURI":"https://192.168.7.1/myfile.bin" \ }' ''' Returns ActionParameterNotSupported TransferProtocol: Omitted ImageURI: https://192.168.7.1/myfile.bin Returns ActionParameterNotSupported TransferProtocol: Omitted ImageURI: 192.168.7.1/myfile.bin Returns ActionParameterValueTypeError TransferProtocol: Bad ImageURI: https:/192.168.7.1/myfile.bin Returns: ActionParameterNotSupported No changes to GET requests, so Redfish Service Validator not necessary. Change-Id: Ibf4b69877031f3b8617412c06d40f2d0d0827ac3 Signed-off-by: Ed Tanous <ed@tanous.net>
10 daysMove logging argsEd Tanous1-1/+0
Args captured by logging functions should be captured by rvalue, and use std::forward to get perfect forwarding. In addition, separate out the various std::out lines. While we're here, also try to optimize a little. We should ideally be writing each log line to the output once, and ideally not use iostreams, which induce a lot of overhead. Similar to spdlog[1] (which at one point this codebase used), construct the string, then call fwrite and fflush once, rather than calling std::cout repeatedly. Now that we don't have a dependency on iostreams anymore, we can remove it from the places where it has snuck in. Tested: Logging still functions as before. Logs present. [1] https://github.com/gabime/spdlog/blob/27cb4c76708608465c413f6d0e6b8d99a4d84302/include/spdlog/sinks/stdout_sinks-inl.h#L70C7-L70C13 Change-Id: I1dd4739e06eb506d68989a066d122109b71b92cd Signed-off-by: Ed Tanous <ed@tanous.net>
14 daysDelete IPv4 default gateway when deleting an IPv4 static addressJohnathan Mantey1-49/+117
The Redfish schema for creating static IPv4 addresses requires the IP address, the netmask, and a gateway IP address. There's an issue inherent with this method. A network interface is only permitted a single IPv4 default gateway. If more than one IPv4 static address is assigned to the NIC each entry is processed, and potentially conflicting default gateways may be assigned. The last entry processed assigns the IPv4 default gateway. This behavior will cause unexpected results. It is necessary to prevent assigning mismatched default gateway values. The IPv4 address removal process requires additional work also. The default gateway value is left in place even after the final static IPv4 address is removed. It is necessary to perform an additional action to clear the gateway address. Without explicit removal the network is left in a condition that may prevent IP traffic from being able to be sent from the BMC. This even in the event that the NIC is actively being managed via DHCPv4. Tested: Disabled DHCPv4 on a secondary NIC (eth1) Assigned a static IPv4 address. Inspected the systemd-networkd config file in order to confirm the Gateway entry is added. This is done to be explicitly sure the network.config file has the Gateway entry. Sent a Redfish PATCH command to delete the static IPv4 address. Confirmed that the systemd-networkd config file no longer contained a Gateway entry. This is done to be explicitly sure the network.config file no longer contains the Gateway entry. Created a PATCH containing multiple IPv4 static addresses all with different Gateway values. Confirmed an error is returned when a mismatch occurs in the Gateway values. Assigned a new static address, and then restored DHCPv4. Confirmed that the default gateway entry in the config file is removed. Submitted a delete request for the remaining static IPv4 address that is now orphaned by re-enabling DHCPv4. This removed the static IPv4 address. Change-Id: Ia12cf2a38ba86266ce71dc28475b0d07b7e09ebc Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
14 daysManage Request with shared_ptrJonathan Doman2-15/+22
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>
2024-05-03Implement client certificate schemasEd Tanous2-1/+131
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>
2024-05-02Fix regression in CrashDumpGunnar Mills1-1/+1
25b54db introduced a bug where CrashDump was not looking at the correct option. Was using BMCWEB_REDFISH_DUMP_LOG instead of the correct BMCWEB_REDFISH_CPU_LOG. This was caught in CI by a system that doesn't have CrashDump enabled but was hitting: 1 failGet errors in /redfish/v1/Systems/system/LogServices/Crashdump Tested: None. Visually inspected and this matches redfish-core/src/redfish.cpp. Change-Id: Ia6e72e5bbeaaa246fbbc5bcb2a525062e63d7d29 Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2024-05-02Fix regression in LogServiceEd Tanous1-1/+1
25b54db introduced an inadvertent inversion in options. Admittedly, this option (redfish-dbus-entries) chose to override URLs instead of creating new ones, which makes it incompatible. Normally we'd require unique URIs for each entry to make this error not possible, and I believe that this is the only instance of us registering two handlers for one url/verb. Tested: Romulus now boots and functions in qemu. GET /redfish/v1 returns results. Change-Id: I6125a5a0242b6cfc54ff19866665227c97f45d0a Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-01Bring consistency to config optionsEd Tanous17-340/+393
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>
2024-04-30Remove One chassis optionEd Tanous1-2/+0
The One chassis option has been gone for a long time, but this ifdef looks like it got missed. Remove it. Tested: code compiles. Change-Id: I013e824806e72bc608ae4383ce4ba707641aeec6 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-30Consolidate Vm implementationsEd Tanous1-4/+5
As much as the two vm implementations SEEM different, the differences largely lie in how we're getting the nbd proxy socket. One is relying on launching a process (nbd-proxy), the other is getting the fd from dbus. Given [1] exists and is in process, we need to have a plan for getting these two VM implementations into one, once that patchset is complete. This commit: Splits the vm-websocket option into vm-websocket-provider, providing two options, nbd-proxy, and virtual-media (the names of the respective apps). To accomplish this, it moves the contents of nbd-proxy into include/vm-websocket, so we can compare the similarities and start consolidating. The longer term intent is that the nbd-proxy option will be completely removed, and the code deleted. This has the additional advantage that we will no longer require the boost::process dependency, as all info will be available on dbus. As part of this, the nbd proxy websocket is also registered at /vm/0/0, to be backward compatible with the old interfaces. Tested: Code compiles. Need some help here. [1] https://gerrit.openbmc.org/c/openbmc/jsnbd/+/49944 Change-Id: Iedbca169ea40d45a8775f843792b874a248bb594 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-29Break out formattersEd Tanous2-0/+2
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>
2024-04-29Use actual language for registries listAlexander Paramonov1-2/+2
Use language from header for registries list instead of hardcoded 'en'. Language from header is already being used in some parts of code[1], but is hardcoded sometimes. This commit fixes inconsistency. TESTED: current language in header with Redfish definition is now consistently taken into account. [1] https://gerrit.openbmc.org/c/openbmc/bmcweb/+/70741/1/redfish-core/lib/message_registries.hpp#b214 Change-Id: Ic5e8e5e76d171b1cb18953e5602f09132b222f3b Signed-off-by: Alexander Paramonov <Sasha110397@gmail.com>
2024-04-27Move to process v2Ed Tanous1-2/+1
Boost process v2 brings some significant benefits to our launching of processes[1]. In bmcweb terms: 1. The code is radically simpler, which decreaeses compile times, and reduces the scope for code scanning tools. 2. The code now uses standard asio pipes instead of inventing its own. 3. Separate compilation. Tested: We don't have a lot of unit tests for the virtual media stuff that I can run, but we do have unit tests for credentials pipe, which in this change have been ported over, so the feature works. Unit tests are passing. [1] https://www.boost.org/doc/libs/1_80_0/doc/html/boost_process/v2.html#boost_process.v2.introduction Change-Id: Ia20226819d75ff6e492f8852185f0b73e8f5cf83 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-25AccountService: Add HTTPBasicAuth supportRavi Teja1-3/+30
This commit adds HTTPBasicAuth Get/Patch support Tested By: Redfish service validator passes. ``` curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"HTTPBasicAuth":"Enabled"}' https://192.168.7.2/redfish/v1/AccountService ``` Succeeds with various values. Enabled: Basic auth succeeds. Disabled: Basic auth no longer works. AccountService reports "Disabled" For HTTPBasicAuth status. Change-Id: Ic417bf3cd4135f05ab34c8613c7fbce953157b03 Signed-off-by: Ravi Teja <raviteja28031990@gmail.com> Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-24Handle error code properlyEd Tanous1-5/+6
As part of the previous patch tests, UpdateService shows this bug, where on a multipart parser failure, the dbus match object gets instantiated, and eventually fails. This leads to mediocre logging, and possibly could leave update service in an undesirable state. Fix the error by moving the conditional up. Tested: Filling a 16MB file with all zeros and sending it now no longer logs that a monitor has been set up, and returns immediately instead of waiting for timeout. ``` dd if=/dev/zero of=zeros-file bs=1048576 count=16 of=16mb.txt curl -k --location POST https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":[]} ;type=application/json' -H "Expect:" --user "root:0penBmc" -F UpdateFile=@16mb.txt -v ``` Change-Id: I0962d15c624936b4fa199a675123702003dd697b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-19Add missing headersEd Tanous12-3/+25
Most of these were found by breaking every redfish class handler into its own compile unit: When that's done, these missing headers become compile errors. We should just fix them. In addition, this allows us to enable automatic header checking in clang-tidy using misc-header-cleaner. Because the compiler can now "see" all the defines, it no longer tries to remove headers that it thinks are unused. [1] https://github.com/openbmc/bmcweb/commit/4fdee9e39e9f03122ee16a6fb251a380681f56ac Tested: Code compiles. Change-Id: Ifa27ac4a512362b7ded7cc3068648dc4aea6ad7b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-18Initialize schemas array with explicit sizeMyung Bae1-1/+2
Currently `update_schemas.py` generates a schema list definition like redfish-core/include/schemas.hpp: ``` constexpr std::array schemas { "AccountService", "ActionInfo", ... "OpenBMCAccountService", }; ``` However, if the number of schemas is more than the clang's default max size, CI may fail. The default is `-fbracket-depth=256`. ``` In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/functional:65: [1m/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/array:288:52: [0m [0;1;31mfatal error: instantiating fold expression with 276 arguments exceeded expression nesting limit of 256 288 | -> array<enable_if_t<(is_same_v<_Tp, _Up> && ...), _Tp>, [0m | [0;1;32m ~~~~~~~~~~~~~~~~~~~~~~~~^~~~ [0m [1m../redfish-core/include/schemas.hpp:17:26: [0m [0;1;30mnote: while substituting deduced template arguments into function template '<deduction guide for array>' [with _Tp = const char *, _Up = <const char *, const char *, const char *, const char *, const char *, ... const char *>] [0m 17 | constexpr std::array schemas { [0m | [0;1;32m ^ [0m1 error generated. ``` To avoid the failure, we can set the size explicitly like ``` constexpr std::array<std::string_view,277> schemas { "AccountService", ... ``` Tested: 1) Remove `include_list` so that all possible schemas are to be used 2) Run with the fixed `scripts/update_schemas.py` 3) Compiles successfully Change-Id: Ib68db9fd3be1b6dbe0c4b5cc0e9a4324966d759e Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2024-04-17Refactor tftp parserEd Tanous1-32/+63
This function in the next patch will be used for more than just TFTP, so rename it to match intent, and refactor to use non-TFTP specific types. Tested: Rename only. Need help on TFTP setups if we need it. Change-Id: Ifc7485aa60ec53407c38b3d1bec530bdacf50075 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-17Break out DoTftpUpdateEd Tanous1-34/+44
This refactor of code is in preparation for adding new SimpleUpdate types. Separating out TFTP helps to keep code organized. Tested: Need help here. TFTP isn't enabled a lot. Change-Id: Ifbdd4b73bb0f9c31092d729d1ec3d3f395f680b8 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-17Refactor UpdateServiceEd Tanous2-247/+251
Similar to other refactorings we've been doing, make UpdateService call into methods instead of inline lambdas. Tested: Redfish service validator passes. Structural changes only. Change-Id: I96b6db5e14fa0f7d357fb0faf63d0457b7963581 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-17Fix NTP set race conditionEd Tanous1-32/+28
There's currently a problem with phosphor-timesyncd, where enabling NTP doesn't immediately reflect in the system status on return[1]. To say it another way, NTP is not enabled/disabled atomically, which leads to the following problem. // Disable NTP PATCH /redfish/v1/Managers/bmc/NetworkProtocol {"NTP":{"ProtocolEnabled": false}} // Set the time manually PATCH /redfish/v1/Managers/bmc {"DateTime": "<timestring"} Doing this in rapid succession leads to a 500 error, which is obviously a bug. In the prior commit, this error was changed to a PropertyValueConflict error, which is still incorrect, but at least informative of what's going on. REST APIs are intended to have CRUD compliance. The response should not be returned until the value has been accepted, and not doing that can lead to problems. This commit changes the backend to use systemd directly, rather than routing through phosphor-settings, to avoid this race. Quite possibly resolves #264 but haven't tested that. Tested: The above procedure succeeds. [1] https://github.com/systemd/systemd/pull/11424 Change-Id: I19241e7677d9b6415aff79ac65c474ae71984417 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-16Stage 2 refactor LDAP parametersEd Tanous1-80/+83
ReadJsonObject isn't required for cases where we don't have a list of structures, and ideally we should consolidate all fixed readJson calls in one place (and not have multi-depth readJson calls). This commit moves all the calls up, and consolidates all the LDAP patch params into a single struct that can be moved between the layers, rather than having the parameters individually. Tested: Does LDAP work anymore? Could use some help if anyone has test scripts, otherwise code compiles and this is inspection only, but similar to other mechanical changes we've made recently Change-Id: I77c0a8b97d4783fdca875c86d7dace122a0a55d7 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-16Add type safety for NTP server objectsEd Tanous2-22/+48
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>
2024-04-16Update Privilege Registry from 1.3.0 to 1.5.0Gunnar Mills1-2/+395
Change 1 line in scripts/parse_registries.py and rerun the script. Long term OpenBMC/bmcweb need more direction here on the Privilege Registry, but for now continue with the current direction of using the Privilege Registry and taking it from the DMTF. There is new entries in 1.4/1.5 that are needed for future development. Tested: It builds. Change-Id: I4337dc44e794c58f00f7307ea0508b84f14e8c8f Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2024-04-15Fix Privileges warningsEd Tanous1-3/+3
After splitting up the compile units, warnings start showing up on the console about privileges. This is due to them being a static global, and not shared between the compile units. Move the array to a constexpr structure so that all compile units see the same init. Change-Id: I6e035342a5b229a0601c02092ca3ce665b14bbdb Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-15Move to Redfish setProperty callAsmitha Karunanithi6-392/+134
This commit changes sdbusplus setProperty calls (in various files) to setDbusProperty method in Redfish namespace that handles all DBus errors in a consistent manner. It also handles and translates additional DBus errors to Redfish specific errors in dbus_utils file. Tested By: Not tested yet Change-Id: If440774879413754f4c24f9b6572c3c9fa1fd033 Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
2024-04-13Fix nullptr failures for image uploadEd Tanous2-0/+12
Several places that call *req.ioService were missing nullptr checks. Add them, and fix the one case where it might not be filled in. Tested: With HTTP2 enabled, the following command succeeds. ``` curl -k https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":["/redfish/v1/Managers/bmc"]} ;type=application/json' --user "root:0penBmc" -F UpdateFile=@/home/ed/bmcweb/16mb.txt -v -H "Expect:" ``` Change-Id: I81e7944c22f5922d461bf5d231086c7468a16e62 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Fix post code parsingEd Tanous1-18/+24
If we use string_view with std::from_chars, we can use begin() and end() directly (because they return pointers) and not have to do silly things like dereference end(), which, while works in practice, is technically undefined behavior, and some static analyzers complain about it. Tested: Unit tests pass against both old parsePostCode and new. Change-Id: Icfdec3b81f4a9c9bed3599571a8bc8779f9bfb98 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Use rvalue referenceEd Tanous1-1/+1
Change-Id: Ie1a03bac54183b206bf27e37f1fed804601c8643 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Remove a copyEd Tanous1-1/+1
Capturing by auto here causes a copy. Found using static analysis. Change-Id: Ifbb08f9af0cd6eeec1e611c610e7adf53e17665c Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Add nullptr checkEd Tanous2-4/+13
Change-Id: If511f1210cca7bd1da3a8c5152688487d3036e2f Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Fix file removalEd Tanous1-2/+11
This code used std::remove, which is a mechanism for removing characters from strings. Clearly it meant std::filesystem::remove(), which removes files from the filesystem. Correct it. Change-Id: I030966203c1682a11c723c596accdf34637dd1ba Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Remove logically dead codeEd Tanous1-6/+0
This code path is subtle, but given that slotPresent is only set to true if totalCores is incremented, there's no way to actually hit this section of code. Looking for input on if this is the right behavior. Change-Id: Ie6dadd2c7a0ca6b8402148ddd9b8a369a4a38b2e Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Remove logically dead codeEd Tanous1-6/+1
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>
2024-04-11Give static analysis some hintsEd Tanous1-12/+21
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>
2024-04-11Fix large copies with url_view and segments_viewEd Tanous6-63/+82
Despite these objects being called "view" they are still relatively large, as clang-tidy correctly flags, and we ignore. Change all function uses to capture by: const boost::urls::url_view_base& Which is the base class of all boost URL types, and any class (url, url_view, etc) is convertible to that base. Change-Id: I8ee2ea3f4cfba38331303a7e4eb520a2b6f8ba92 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Fix object type in json utilsEd Tanous1-6/+4
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>
2024-04-11Pull in all registriesGunnar Mills15-0/+4317
Before Redfish put all messages in Base, but as the Messages became more specific Redfish started creating new registries. Redfish might have went a little registry happy. HeartbeatEvent just has 1 message and all these new ones registries each just have a handful of messages. Add the remaining 15 registries: composition, environmental, ethernetfabric, fabric, heartbeat_event, job_event, license, logservice, networkdevice, platform, power, sensor_event, storage_device, telemetry, update. Some of these are wanted for both current development and future development but it is hard to decide which ones so just added them all. power, fabric, telemetry, update are all things we support today. Having a UpdateInProgress or UpdateSuccessful makes a lot of sense and this enables that. Put these alphabetically. Use a new for loop to do this. Make changes to scripts/parse_registries.py and run the tool. No difference in size. Before: 66928640 Apr 10 13:32 obmc-phosphor-image-p10bmc-20240410183051.ext4.mmc.tar After: 66928640 Apr 10 13:18 obmc-phosphor-image-p10bmc-20240410181439.ext4.mmc.tar Tested: bmcweb builds. "./scripts/parse_registries.py --registries license,update" works. Change-Id: I43b4d041531cf338e9e7e621714ca7d95f6b01a5 Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2024-04-10Simplify query_paramEd Tanous1-12/+4
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>
2024-04-10Fix account serviceEd Tanous1-2/+2
Session might not be initialized, and might be nullptr. This line was accessing the session BEFORE the nullptr check. Move it to after. Found using static analysis. Change-Id: I966c642aee7c76a29c7d0d57d3b78f5f7bef7d62 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-10Move run and redfish to compile unitsEd Tanous2-233/+229
Meson supports unity builds[1] natively. There's no reason to continue with the pseudo unity build we've been using by putting implementations in header files. This commit is the first in a long series of starting to break this up into smaller compile units, in the hopes of dropping incremental compile times for developers, and reduce the total per-core memory usage that gcc requires. This commit breaks out the run() function from main() and the constructor of RedfishService from redfish.hpp into their own compile units. According to tracing, even after broken out, these are still by far the two longest to compile units in the build. Tested: Code compiles. Debug build on a 24 core build server results in a decrease in compile time for compiling just bmcweb from 1m38s to 1m22s. [1] https://mesonbuild.com/Unity-builds.html Change-Id: Ibf352e8aba61d64c9a41a7a76e94ab3b5a0dde4b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-09Move to Redfish Action specific setProperty callAsmitha Karunanithi5-28/+127
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>
2024-04-07Fix moves/forwardEd Tanous10-30/+31
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>
2024-04-07Remove redundant static modifierEd Tanous2-17/+15
"inline static void func()" Doesn't make sense when put in a header file. Find all instances, and make them inline like we do everywhere else. Tested: Code compiles. Change-Id: I7da5821b1e372941680f82939627af39fdc2a4eb Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-07Fix undefined behavior in getUniqueEntryIDEd Tanous1-33/+30
Changing static bool getUniqueEntryID to inline bool getUniqueEntryID Exposes the fact that there's some undefined behavior here, and unit tests start failing, likely due to stack being reused where it previously wasn't. This commit cleans up the code to simplify it, and remove the problem. Tested: Unit tests pass. Good coverage. Change-Id: I5b9b8e8bb83c656560193e680d246c8513ed6c02 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-04Remove redfish-health-populateGunnar Mills9-375/+3
The redfish-health-populate option was scheduled to be removed in 1Q 2024. It is now 2Q, so remove the option. No upstream layers enabled it and did not find a downstream layer that did either. This was always limited to a few resources. Overall this design was only half done. A future "HealthRollup" can be proposed. Some discord discussion: [1]: https://discord.com/channels/775381525260664832/855566794994221117/1110728560819327069 Commit disabling this (merged 10 months ago): [2]: https://github.com/openbmc/bmcweb/commit/6f8273e49cffdd347c223b9538558edfb05e818a Tested: Code compiles Change-Id: I4d33c1e674ecdb0fd256df62f3795073454ae7a1 Signed-off-by: Gunnar Mills <gmills@us.ibm.com>