summaryrefslogtreecommitdiff
path: root/http/http_client.hpp
AgeCommit message (Collapse)AuthorFilesLines
2023-10-31Move to file_body in boostEd Tanous1-20/+22
As is, it reads the whole file into memory before sending it. While fairly fast for the user, this wastes ram, and makes bmcweb less useful on less capable systems. This patch enables using the boost::beast::http::file_body type, which has more efficient serialization semantics than using a std::string. To do this, it adds a openFile() handler to http::Response, which can be used to properly open a file. Once the file is opened, the existing string body is ignored, and the file payload is sent instead. openFile() also returns success or failure, to allow users to properly handle 404s and other errors. To prove that it works, I moved over every instance of direct use of the body() method over to using this, including the webasset handler. The webasset handler specifically should help with system load when doing an initial page load of the webui. Tested: Redfish service validator passes. Change-Id: Ic7ea9ffefdbc81eb985de7edc0fac114822994ad Signed-off-by: Ed Tanous <ed@tanous.net>
2023-08-24Avoid setting SNI hostname for IP addressesRavi Teja1-0/+7
ssl_handshake fails while establishing connection to IPv6 destination address, as IPv6 addresses considered as invalid value for SNI hostname due to special characters. SNI allows valid HostName which allows characters are only {alphabetic characters (A-Z), numeric characters (0-9), the minus sign This commit adds check to avoid setting SNI hostname if its an IP address Tested By: Verified redfish events 1. Subscribing Destination with IPv6 address. 2. Subscribing Destination with IPv4 address. Change-Id: I32d30292bbc29c753f1c1815c66fcc93e8074eaa Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
2023-08-23Move http client to URLEd Tanous1-95/+66
Type safety is a good thing. In: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/65606 It was found that splitting out the URI into encoded pieces in the early phase removed some information we needed, namely whether or not a URI was ipv6. This commit changes http client such that it passes all the information through, with the correct type, rather than passing in hostname, port, path, and ssl separately. Opportunistically, because a number of log lines are changing, this uses the opportunity to remove a number of calls to std::to_string, and rely on std::format instead. Now that we no longer use custom URI splitting code, the ValidateAndSplitUrl() method can be removed, given that our validation now happens in the URI class. Tested: Aggregation works properly when satellite URIs are queried. Change-Id: I9f605863179af54c5af2719bc5ce9d29cbfffab7 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-08-14Reduce some Error log severitiesCarson Labrado1-1/+1
There are instances of ERROR logs that would work better as WARNING or DEBUG since they do not actually result in bailing early and returning an error response. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: I1e7bca0bb38487b26a4642ab72ce475170bb53c6
2023-08-07Fix bugprone-unchecked-optional-access findingsEd Tanous1-8/+13
Clang-tidy has the aforementioned check, which shows a few places in the core where we ignored the required optional checks. Fix all uses. Note, we cannot enable the check that this time because of some weird code in health.hpp that crashes tidy[1]. That will need to be a future improvement. There are tests that call something like ASSERT(optional) EXPECT(optional->foo()) While this isn't an actual violation, clang-tidy doesn't seem to be smart enough to deal with it, so add some explicit checks. [1] https://github.com/llvm/llvm-project/issues/55530 Tested: Redfish service validator passes. Change-Id: Ied579cd0b957efc81aff5d5d1091a740a7a2d7e3 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-07-20Replace logging with std::formatEd Tanous1-100/+91
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
2023-07-14AsyncResolve cleanups and error handlingEd Tanous1-1/+1
The Async DBus resolver really has nothing to do with crow, which is our core http library namespace and has some opportunistic cleanups that can be done. This commit moves it into the bmcweb namespace (unimportantly) and breaks out one of the larger functions such that it can be unit tested, and unit tests it. Tested: Unit tests pass. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie3cfbb0ef81a027a1ad42358c04967a517471117
2023-07-11HTTP Client: Improve error messageSunitha Harish1-6/+12
This commit adds the host and port data to the existing error messages that are hit during redfish event Tested by: Checked the traces on BMC when there is a bad subscriber Change-Id: I3f18bc3b999c136c42c4c0021c5fcadddb9e4bff Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
2023-06-05Add SSE style subscription support to eventserviceAppaRao Puli1-5/+5
This commit adds the SSE style eventservice subscription style event Using this, end user can subscribe for Redfish event logs using GET on SSE uris from browser. Tested: - From Browser did GET on above SSE URI and generated some Redfish event logs(power cycle) and saw redfish event logs streaming on browser. - After SSE registration, Check Subscription collections and GET on individual subscription and saw desired response. - Ran RedfishValidation and its passed. Change-Id: I7f4b7a34974080739c4ba968ed570489af0474de Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com> Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com> Signed-off-by: Ed Tanous <edtanous@google.com>
2023-05-30Allow async resolver to be optionalEd Tanous1-12/+20
This commit adds a meson option to allow selecting which dns resolver bmcweb uses. There are use cases, like Open Compute Project Inband Management Agent, that would require not using dbus, which would require us to fall back to the asio resolver. This commit makes the existing asio resolver constructor, and async_resolve methods match the equivalents in asio (which we intended to do anyway), then adds a macro and configure option for being able to select which resolver backend to rely on. Tested: Code can now compile without sdbusplus. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I3220214367179f131a60082bdfaf7e725d35c125
2023-05-19Capture all boost::system::error_codes by refEd Tanous1-3/+3
Capturing these possibly overloaded values by reference can avoid a copy in some cases, and it's good to be consistent. This change was made automatically by grep/sed. Tested: Code compiles. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iafeaca2a5dc52f39753b5a3880419d6bc943f81b
2023-05-11clang-format: copy latest and re-formatPatrick Williams1-1/+0
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>
2023-03-20Aggregation: Increase response read limit to 50MBCarson Labrado1-136/+57
With Redfish aggregation, responses from satellite BMCs can be on the order of MBs due to use cases like logging or binary payloads. Offloading $expand could similar result in responses that exceed the current read limit of 128 KB. Splits the connection pools used for aggregation and EventService so that the response read limit is 50MB for responses associated with aggregation. Pools used by EventService keep the current limit of 2^17 bytes or 128 KB. It also propogates a ConnectionPolicy object that gets instantiated within HttpClient, which allows per-client policies for retry/byte limits. This allows EventService and aggregation to have different policies. Tested: With aggregation enabled I was able to return a response from a satellite BMC which was than 2MB. Ran the Redfish Mockup Creator and it was able to successfully query all aggregated resources as part of walking the tree. Also verified that HTTP push events still work with EventListener. Change-Id: I91de6f82aadf8ad6f7bc3f58dfa0d14c0759dd47 Signed-off-by: Carson Labrado <clabrado@google.com> Signed-off-by: Ed Tanous <edtanous@google.com>
2023-02-24Take boost error_code by referenceEd Tanous1-1/+1
By convention, we should be following boost here, and passing error_code by reference, not by value. This makes our code consistent, and removes the need for a copy in some cases. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id42ea4a90b6685a84818b87d1506c11256b3b9ae
2023-02-23HTTP Client: Increase max conns and reqsCarson Labrado1-3/+4
With Redfish Aggregation there is a need for a large amount of requests to be sent to a satellite BMC. Our current throughput of 4 connections with a 50 request buffer can be easily overwhelmed due to the use of $expand. Also, BMCWeb itself can support 100 active connections so multiple clients could be attempting to query a satellite BMC that is being aggregated. Increase the maximum number of connections to a destination to 20 and increase the buffer request size to 500. These figures should be fine since the requests themselves have a very small memory footprint and the number of active connections is only a fifth of what the BMCWeb in the satellite BMC should be able to support. Note that these figures are an improvement over the current values. They allowed making multi-level $expand requests without dropping any subrequests due to the buffer becoming full. Further tuning will be done in a future patch if it is determined that optimal performance can be obtained by choosing different values. Tested: Debug logs after sending a multi-level $expand query showed the entire connection pool being filled as well as well over 100 Requests being queued. The additional connections provided enough throughput to handle repeated simultaneous requests. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: I96f165b5fbc76086e55b65faaaa49eb2753f8ef6
2023-02-03Aggregation: Better handle dropped requestsCarson Labrado1-0/+5
It's possible for HTTP client's request buffer to become full (especially when $expand is used). Instead of ignoring the requests we should provide a 429 Too Many Requests response for the provided callback to process. The aggregator's response handling also needs to account for this possibility so that it only completely overwrites the asyncResp object when it receives a response from a satellite. Also added more test cases for the response processing functions. Tested: Unit tests passed Flooded aggregator with requests for satellite resources. Requests began returning 429 Too Many Requests errors after the request buffer became full. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: Ib052dc0454d759de7fae761977ca26d6b8d208e5
2023-01-18Fix a boatload of #includesEd Tanous1-2/+2
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
2022-10-25HTTP Client: Improve handling operation timeoutsCarson Labrado1-22/+46
Now that we are using timer.async_wait() with the async http operations we need to account for the scenario where the timer fails before the operation fails. When that occurs we need to abort the operation once its callback gets called. Currently we proceed as if the timer doesn't exist. This causes a fault if one of the operations times out. This patch adds a check to the start of each async operation so that we do not continue with the normal message sending flow when an operation times out. Tested: In order to create a connection timeout I created a dummy interface and set the IP of my satellite BMC to route to the interface: ip link add dummy0 type dummy ip link set dev dummy0 up ip route add 120.60.30.15 dev dummy0 All packets sent to 120.60.30.15 will get dropped and thus connection attempts will timeout. This does not cause bmcweb to crash. To make the satellite reachable again I used this command to delete the routing: ip route del 120.60.31.15 dev dummy0 After doing so messages were once again able to be forwarded correctly to the satellite. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: Ie8d022c2195838e383eefcd0e12ae8cfab76e3e1
2022-10-12header cleanupsNan Zhou1-2/+4
This commit fixed several places (but not all) where wrong include directory is specified and prevent the clean up in the chidren changes. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Ibbba62e2c0cfe3583a65f1befa1b233bd3eebf19
2022-09-12ConnectionInfo move to bind_frontEd Tanous1-59/+59
There's a few last calls that are inline lambdas. As we've been moving away from inline lambdas due to the problems with debug logging and usability, this file is the next cleanup. Tested: Ran Redfish-Event-Listener, and set an event into /var/log/redfish with ``` echo "2022-08-04T21:37:49.156798+00:00 OpenBMC.0.1.ServiceFailure,com.intel.DomainMapper.service" > /var/log/redfish ``` Simulating a crash. Saw the event get sent to Redfish-Event-Listener. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I61361eef03d1301dc126a28c695fdd74e504f891
2022-08-24Remove tcp_streamEd Tanous1-22/+50
tcp_stream has several problems in its current implementation. First, it takes up a significant amount of binary size, for features that we don't use. Next, it has some implied guarantees about timeouts that we erronously rely on, namely that async_connect will timeout appropriately given the tcp_stream timeout (it doesn't). We already have a timer present in the ConnectionInfo class anyway, this commit just makes use of it for ALL timeout operations, rather than just the connect timeout operations. This flow is roughly analogous to what we do in http_connection.hpp already, so the pattern is well troden. This saves 2.8% of the binary size of bmcweb, and solves several race conditions. Tested: Relying on Carson: Aggregated a sub-bmc, and ensured that top level collections returned correctly under GET /redfish/v1/Chassis Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I72e8e97b667826f272bb4921afc2b16907f3b271
2022-08-24Add SSL support for http_client (EventService)AppaRao Puli1-123/+276
This commit adds the initial SSL support for http_client which can be used for sending asynchronous Events/MetricReports to subscribed Event Listener servers over secure channel. Current implementation of http client only works for http protocol. With current implementation, http client can be configured to work with secure http (HTTPS). As part of implementation it adds the SSL handshake mechanism and enforces the peer ceritificate verification. The http-client uses the cipher suites which are supported by mozilla browser and as recommended by OWASP. For better security enforcement its disables the SSLv2, SSLv3, TLSv1, TLSv1.1 as described in below OWASP cheetsheet. It is validated with RootCA certificate(PEM) for now. Adding support for different certificates can be looked in future as need arises. [1]: https://cheatsheetseries.owasp.org/cheatsheets/TLS_Cipher_String_Cheat_Sheet.html Tested: - Created new subscription with SSL destination(https) and confirmed that events are seen on EventListener side. URI: /redfish/v1/EventService/Subscriptions Method: POST Body: { "Context": "CustomText", "Destination": "https://<IP>:4000/service/collector/event_logs", "EventFormatType": "Event", "DeliveryRetryPolicy": "RetryForever", "Protocol": "Redfish" } - Unit tested the non-SSL connection by disabling the check in code (Note: EventService blocks all Non-SSL destinations). Verified that all events are properly shown on EventListener. URI: /redfish/v1/EventService/Subscriptions Method: POST Body: { "Context": "CustomText", "Destination": "http://<IP>:4001/service/collector/event_logs", "EventFormatType": "Event", "Protocol": "Redfish" } - Combined above two tests and verified both SSL & Non-SSL work fine in congention. - Created subscription with different URI paths on same IP, Port and protocol and verified that events sent as expected. Change-Id: I13b2fc942c9ce6c55cd7348aae1e088a3f3d7fd9 Signed-off-by: AppaRao Puli <apparao.puli@intel.com> Signed-off-by: Ed Tanous <edtanous@google.com>
2022-08-24Redfish Aggregation: Router to satellite resourcesCarson Labrado1-0/+6
Adds ability to route requests to either native resources or resources that belong to a satellite BMC as part of Redfish Aggregation. A prefix in the URI denotes if the resource is actually from a satellite BMC. Prefixes are only used to denote satellite resources. The URI of resources on the local/aggregating BMC will remain unchanged. Prefixes are separated from the resource ID by an underscore. This means that underscores cannot be used in the prefix name itself. The prefixes used by satellite BMCs are revealed via D-Bus as well as the config information needed to connect to that BMC. Requests for satellite resources will not be handled locally. Care should be taken to not name any local resources in a way that could cause a collision (e.g. having a Chassis object named "aggregated0_1U" on the aggregating BMC). The patch only covers routing requests. Requests to collection URIs like /redfish/v1/Chassis will only return resources on the local BMC. A future patch will cover adding satellite resources to collections. Also note that URIs returned in the responses will not have the proper prefix included. Fixing these URIs will be addressed in future patches. A number of TODO comments are included in the code to indicate that this functionality (collections and URI fixup) still needs to be implemented. Example URIs w/o Redfish Aggregation: /redfish/v1/Chassis/1U/ /redfish/v1/Systems/system/ /redfish/v1/Managers/bmc/ Example URIs after enabling Redfish Aggregation if the associated resources are located on the local/aggregating BMC: /redfish/v1/Chassis/1U/ /redfish/v1/Systems/system/ /redfish/v1/Managers/bmc/ Example URIs if resources are instead located on a satellite BMC named "aggregated0": /redfish/v1/Chassis/aggregated0_1U/ /redfish/v1/Systems/aggregated0_system/ /redfish/v1/Managers/aggregated0_bmc/ Tested: I was able to query supported resources located on the local BMC as well as on a satellite BMC. Requests with unknown prefixes return a 404. Requests to resource collections only return the resources that are located on the aggregating BMC. Signed-off-by: Carson Labrado <clabrado@google.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I87a3deb730bda95e72ecd3144ea40b0e5ee7d491
2022-08-17HTTP Client: Further increase httpReadBodyLimitCarson Labrado1-1/+1
A continuation of 4d94272fe54c147974f86788092bbbdd950406aa, increases the size of httpReadBodyLimit to 2^17 bytes (about 128 KB). This is because with Redfish Aggregation we need to be able to handle larger response sizes such as json schema files. The ComputerSystem schema in particular is over 120 KB. Going forward we will not be able to keep increasing the limit. We need a better solution for handling larger response that are larger than httpReadBodyLimit. Tested: Used https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310/68 to successfully retrieve /redfish/v1/JsonSchemas/ComputerSystem/ComputerSystem.json from a satellite BMC. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: Ida80b8fddbd1df1310d18a77ecfb44b7fdf292ef
2022-07-15HTTP Client: Fix handling on connection timeoutCarson Labrado1-11/+12
If a destination is not reachable, then the connection will timeout in doConnect(). This causes issues later when the client attempts to resend the message. The error check in doClose() should not exit early since that will result in the connection's status not being marked as closed and thus it will never get reused. Similarly, doCloseAndRetry() should not exit early since that will cause the retry flow to hang and the connection's callback function will not get deleted. Tested: Used Redfish Aggregation patches in the chain through https://gerrit.openbmc.org/c/openbmc/bmcweb/+/54896 to verify that requests to collections such as /redfish/v1/Chassis no longer hang when the specified Satellite BMC does not exist Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: Ic8369b0de8efb00ff168bc1ed43f1d7fd6c7366a
2022-07-12Fix const correctness issuesEd Tanous1-2/+2
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
2022-07-06Remove redfish message from http clientEd Tanous1-1/+1
HttpClient these days is intended to be a generic feature. It should not be relying directly on Redfish messages. In this case, we only ever rely on the return code internally, so replace messages with a simple bad_gateway. Tested: Code compiles. Unit tests pass. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie4bd65dc0b90b75f61ab0e31ca535eefbf0f4ebb
2022-07-05Make resHandler constEd Tanous1-2/+2
cppcheck correctly notes this parameter can be const. Unfortunately we've making a copy here, but looking at the ownership, that can't be avoided simply without a shared_ptr, and that's far more invasive. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib70c89a7a1bc7210219052ba2a44bc7a1c20c7f2
2022-07-02Remove unused variableEd Tanous1-1/+0
This variable was clearly for the print statement below, but then got abandoned at some point. Clean it up. cppcheck found this as well. Tested: Unused code Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I2d143f00dc6956c87ba77d5cbe1b96be631ca795
2022-07-02Fix unused branches in http_clientEd Tanous1-10/+6
cppcheck correctly finds that these branches will always be hit, so the branch is unneeded. Let's start by getting the code cleaned up. Tested: CI only, cpp check. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I7b89337a81e676915243d5bc9d26c24a89c74aef
2022-07-01Fix #includes on http_client.hppEd Tanous1-0/+10
Include what you use. Tested: Code compiles Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I23c89fee6f3e39d2f2a7dc1c3ba2db8819e8adc7
2022-06-28Fix shadowed variable issuesEd Tanous1-15/+17
This patchset is the conclusion of a multi-year effort to try to fix shadowed variable names. Variables seem to be shadowed all over, and in most places they exist, there's a "code smell" of things that aren't doing what the author intended. This commit attempts to clean up these in several ways by: 1. Renaming variables where appropriate. 2. Preferring to refer to member variables directly when operating within a class 3. Rearranging code so that pass through variables are handled in the calling scope, rather than passing them through. These patterns are applied throughout the codebase, to the point where -Wshadow can be enabled in meson.build. Tested: Code compiles, unit tests pass. Still need to run redfish service validator. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: If703398c2282f9e096ca2694fd94515de36a098b
2022-06-28HttpClient: Increase httpReadBodyLimitCarson Labrado1-2/+3
Testing of https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310 revealed that recvMessage() was failing because the body limit was being exceeded. httpReadBodyLimit was being used to set both the body limit as well as the buffer size. Add a different variable to set the buffer size so that the body limit can be doubled to 16K while the fixed buffer can be set as a smaller size of 4K. Tested: Queries using the mentioned patch stopped failing after applying this change. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: I33b62dfc12ff078fba9fb92b2f95a05f41acce3d
2022-06-15bmcweb: Set Retry Policy Valid Response CodesCarson Labrado1-15/+28
Allows individual retry policies to specify what HTTP response codes are considered valid. Sets functions for the EventService and Redfish Aggregation retry policies. Those functions expect a response code and return an error code based on what the response code is. This change is needed because EventService only considers 2XX codes to be valid. Any code outside of that range would trigger a retry attempt. Redfish Aggregation by design will need to return errors outside of that range such as 404. It should not retry to send a message when it receives a 404 from a satellite BMC. Right now 404 is the only error code that is handled differently between the services. Going forward, Redfish Aggregation will likely want to allow other error codes as its functionality is expanded. Tested: Used Redfish-Event-Listener with ssh port forwarding to create 3 subscriptions. I then closed the ssh connection and sent a test event. Bmcweb made 3 retry attempts for each subscription. At that point the max retry amount (as defined by EventService) was reached and bmcweb stop attempting to resend the messages. There were no errors when the Redfish-Event-Listener was correctly connected. Test events resulted in messages being sent for each subscription. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: Ifdfaf638d28982ed18998f3ca05280a288e0020a
2022-06-02Make code compile on clang againEd Tanous1-1/+1
The usual updates to make code compile on clang again. Extra semicolons that have snuck in, missing inline and static definitions. Tested: Code compiles on clang. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id7f889de98cafaa89471d75ed3e3bb97ab3855cd
2022-06-01Try to fix the lambda formatting issueEd Tanous1-103/+98
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
2022-05-23bmcweb: Remove hardcoded HTTP verbs and headersCarson Labrado1-42/+40
Modifies HttpClient so that the HTTP verb and headers can be set for each individual message sent. Right now those fields are set when a connection is first created and then reused by each message sent using that connection. Tested: Launched two Event Listener servers that created 6 and 2 subscriptions. Sending a test event resulted in the servers receiving 6 requests and 2 requests, respectively. Change-Id: I8d7e2d54385bc2c403498293820adb584bff8b57 Signed-off-by: Carson Labrado <clabrado@google.com> Signed-off-by: Ed Tanous <edtanous@google.com>
2022-05-17Add callback for response handling to HttpClientCarson Labrado1-11/+55
Adds sendDataWithCallback() which allows the caller to include a callback specifying how to handle the response to that request. This will be utilized for Redfish Aggregation including returning the responses received when forwarding requests to satellite BMCs. Change-Id: I93826c8b254a5f28a982295d4145453352a90fae Signed-off-by: Carson Labrado <clabrado@google.com>
2022-05-11Refactor HttpClient ClassCarson Labrado1-159/+431
Refactors HttpClient with the following changes: - Convert class to singleton - Replace circular buffers with devectors - Sending queued requests and closing connections handled within their own callback - Add connection pooling (max size 4) - HttpClient supports multiple connections to multiple clients - Retry policies can be set for specific use cases Also modifies its use in the Subscription class to be compatible with the refactored code. It is assumed that a BMC will be able to handle 4 parallel connections and thus the max pool size is set as 4. The max number of queued messages was left unchanged at 50. Eventually we may want to allow tuning of these limits to boost performance. That would come in a future patch. Tested: Launched two Event Listener servers that created 6 and 2 subscriptions. Sending a test event created a connection pool for each server. 4 and 2 connections were added to each pool, respectively and were used to send the test request. For the first pool the 2 extra requests were placed into a queue until connections became available. After a request completed, its associated connection was used to send the next request in the queue. Resending the test event caused those prior connections to be reused instead of new connections being added to the pools. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: Iba72b3e342cdc05d1fb972e2e9856763a0a1b3c5
2022-04-19Remove regex uses in event service and consolidateEd Tanous1-2/+2
As the patch at https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/50994 can attest, parsing urls with a regex is error prone. We should avoid it where possible, and we have boost::urls that implements a full, correct, and unit tested parser. Ideally, eventually this helper function would devolve into just the parse_uri, and setting defaults portion, and we could rely on the boost::urls::url class to pass into things like http_client. As a side note, because boost url implements port as a proper type-safe uint16, some interfaces that previously accepted port by std::string& needed to be modified, and is included in this patch. Also, once moved, the branch on the ifdef for HTTP push support was failing a clang-tidy validation. This is a known limitation of using ifdefs for our code, and something we've solved with the header file, so move the http push enabler to the header file. Also note that given this reorganization, two EXPECT statements are added to the unit tests for user input behaviors that the old code previously did not handle properly. Tested: Unit tests passing Ran Redfish-Event-Listener, saw subscription create properly: Subcription is successful for https://192.168.7.2, /redfish/v1/EventService/Subscriptions/2197426973 Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ia4127c6cbcde6002fe8a50348792024d1d615e8f
2022-03-07Don't rely on operator << for object loggingEd Tanous1-3/+5
In the upcoming fmt patch, we remove the use of streams, and a number of our logging statements are relying on them. This commit changes them to no longer rely on operator>> or operator+ to build their strings. This alone isn't very useful, but in the context of the next patch makes the automation able to do a complete conversion of all log statements automatically. Tested: enabled logging on local and saw log statements print to console Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I0e5dc2cf015c6924037e38d547535eda8175a6a1
2022-02-07Enable readability-redundant-control-flow checksEd Tanous1-3/+0
These checks are a nice addition to our static analysis, as they simplify code quite a bit, as can be seen by this diff being negative lines. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I60ede4ad23d7e5337e811d70ddcab24bf8986891
2022-01-28Enable readability-container-size-empty testsEd Tanous1-1/+1
This one is a little trivial, but it does help in readability. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5366d4eec8af2f781b3bad804131ae2eb806e3aa
2021-12-15Make timer system use boostEd Tanous1-0/+1
The original crow timeout system had a timer queue setup for handling many thousands of connections at a time efficiently. The most common use cases for the bmc involve a handful of connections, so this code doesn't help us much. These days, boost asio also implements a very similar timer queue https://www.boost.org/doc/libs/1_72_0/boost/asio/detail/timer_queue.hpp internally, so the only thing we're loosing here is the "fuzzy" coalescing of timeout actions, for which it's tough to say if anyone will even notice. This commit implements a timer system that's self contained within each connection, using steady_timer. This is much more "normal" and how most of the beast examples implement timers. Tested: Minimal touch testing to ensure that things work, but more testing is required, probably using sloworis to ensure that our timeouts are no longer issues. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I19156411ce46adff6c88ad97ee8f6af8c858fe3c
2021-12-07fixed errors in EventServiceKrzysztof Grobelny1-4/+2
- Initialized boost::circular_buffer_space_optimized<std::string> which was not initialized. It prevented any event from being send. - Removed line 'parser->skip(true)' which cause all received responses to be interpreted as errors. It was triggering retry which resulted in sensing same event multiple times. Tested: POST redfish/v1/EventService/Subscriptions, body: { "Destination": "https://127.0.0.1:4042/", "Protocol": "Redfish", "EventFormatType": "MetricReport" } { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "The resource has been created successfully", "MessageArgs": [], "MessageId": "Base.1.8.1.Created", "MessageSeverity": "OK", "Resolution": "None" } ] } POST redfish/v1/TelemetryService/MetricReportDefinitions, body: { "Id": "TestReport", "Metrics": [ { "MetricId": "TestMetric", "MetricProperties": [ "/redfish/v1/Chassis/chassis/Thermal#/Temperatures/7/ReadingCelsius" ] } ], "MetricReportDefinitionType": "OnRequest", "ReportActions": [ "RedfishEvent", "LogToMetricReportsCollection" ] } { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "The resource has been created successfully", "MessageArgs": [], "MessageId": "Base.1.8.1.Created", "MessageSeverity": "OK", "Resolution": "None" } ] } GET redfish/v1/TelemetryService/MetricReports/TestReport { "@odata.id": "/redfish/v1/TelemetryService/MetricReports/TestReport", "@odata.type": "#MetricReport.v1_3_0.MetricReport", "Id": "TestReport", "MetricReportDefinition": { "@odata.id": "/redfish/v1/TelemetryService/MetricReportDefinitions/TestReport" }, "MetricValues": [ { "MetricId": "TestMetric", "MetricProperty": "/redfish/v1/Chassis/chassis/Thermal#/Temperatures/7/ReadingCelsius", "MetricValue": "-8388608000.000000", "Timestamp": "1970-04-12T02:28:53+00:00" } ], "Name": "TestReport", "Timestamp": "1970-04-12T06:08:17+00:00" } EVENT RECEIVED { "@odata.id": "/redfish/v1/TelemetryService/MetricReports/TestReport", "@odata.type": "#MetricReport.v1_3_0.MetricReport", "Id": "TestReport", "MetricReportDefinition": { "@odata.id": "/redfish/v1/TelemetryService/MetricReportDefinitions/TestReport" }, "MetricValues": [ { "MetricId": "TestMetric", "MetricProperty": "/redfish/v1/Chassis/chassis/Thermal#/Temperatures/7/ReadingCelsius", "MetricValue": "-8388608000.000000", "Timestamp": "1970-04-12T02:28:53+00:00" } ], "Name": "TestReport", "Timestamp": "1970-04-12T06:08:17+00:00" } Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com> Change-Id: I4912853c3b59593e7032424d0b48aca7a36889b3
2021-11-18EventService: Pass httpHeaders to subscriberSunitha Harish1-2/+4
Custom http headers provided by the subscriber was not passed on to the http client request header This commit passes the http headers to the Subscriber constructor Tested by: Subscribe to events with custom headers Verify the event request packet sending the custom header to the subscriber Verified persistency of the subscription Signed-off-by: Sunitha Harish <sunharis@in.ibm.com> Change-Id: I9a4f59b6e9477fae96b380565885f4ac51e2a628
2021-10-27Make code build in clangEd Tanous1-2/+0
The newest version of clang correctly recognizes that all values of this enumeration are already covered by distinct cases, and that this is unneeded. Considering the default case does nothing, it can just be removed entirely. Tested: Code compiles in clang. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I15333be16affda91d1a733e5d97aaa028a424efd
2021-10-19Cleanup HttpClient to use inline initializationEd Tanous1-10/+8
For variables that aren't sent through the constructor, it's less code and cleaner to initialize them inline as part of the struct. Tested: Per Appu: "Did basic check and its initialized to default as before." Per Sunitha: "Tested the basic event notification scenario with this commit. Works fine" Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I60e096686eb328edba51b26f27c3c879dae25a84
2021-10-19Only generate headers once in EventServiceEd Tanous1-18/+8
This patchset builds on https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/45761 and moves the header generation into the constructor, rather than attempting to repurpose addHeaders(). Tested: Per Appu: "Did basic check and its initialized to default as before." Per Sunitha: "Tested the basic event notification scenario with this commit. Works fine" https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/46703/2 Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ia9e8b89574c7e0137f0d93f08378e45c2fcf5376
2021-10-17Improve HttpHeaders in EventServiceEd Tanous1-9/+2
This commit moves the internal data structures to use boost::beast::http::fields as its internal data structure. fields is a hyper-optimized map implementation for http headers, and has a lot of nice escaping properties. It is what boost::beast::http::request uses under the covers, so this has some niceties in reducing the amount of code, and means we can completely remove the headers structure, and simply rely on req. When this conversion was done, now the type safety of the incoming data needs to have better checking, as loading into the keys has new requirements (like values must be strings), so that type conversion code for to and from json was added, and the POST and PATCH handler updated to put into the new structure. Tested: curl -vvvv --insecure -u root:0penBmc "https://192.168.7.2:443/redfish/v1/EventService/Subscriptions" -X POST -d "{\"Destination\":\"http://192.168.7.2:443/\",\"Context\":\"Public\",\"Protocol\":\"Redfish\",\"HttpHeaders\":[{\"Foo\":\"Bar\"}]}" returned 200. Tested various "bad" headers, and observed the correct type errors. Issued: systemctl restart bmcweb. Subscription restored properly verified with. GET https://localhost:8001/redfish/v1/EventService/Subscriptions/183211400 Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I331f65e1a3960f1812c9baac27dbdcb1d54f112c
generated by cgit v1.2.3 (git 2.39.0) at 2024-09-29 15:52:00 +0300