Aggregation: Improve handling of certain requests
This patch cleans up a few edge cases that aren't handled properly.
We need to break out of the aggregation code earlier when there are
no satellite configs. The logs are showing mixed messages of
Aggregation not being enabled due to no found satellite configs
followed by processing the request anyway until we fail to actually
find a satellite BMC to forward the request to.
When we don't have any satellite configs, but a request is sent to
what should be a valid satellite URI such as
/redfish/v1/Chassis/5B247A_ChassisID then we need to make sure we
return a 404 within the aggregation code since we won't locally
handle the request. We don't have to worry about collection
requests since by design we will also locally handle the request.
This patch is also prep to allow forwarding non-GET requests to
resources that are not supported by BMCWeb. The aggregation code
will get to handle all such requests and we need to make sure that
we do not forward non-GET requests to top level collections.
Tested:
Without any satellite configs the aggregation code exited before
it began trying to send a request to all satellites for
/redfish/v1/Chassis. The same occurred for a request for a satellite
resource. In the latter case the aggregation code also returned a
404.
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Idd1a71ebb485a77795ba47b873624c8e53c36a4c
diff --git a/redfish-core/include/redfish_aggregator.hpp b/redfish-core/include/redfish_aggregator.hpp
index 622bb5b..4d05c96 100644
--- a/redfish-core/include/redfish_aggregator.hpp
+++ b/redfish-core/include/redfish_aggregator.hpp
@@ -362,6 +362,14 @@
const crow::Request& thisReq,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp)
{
+ if ((isCollection == AggregationType::Collection) &&
+ (thisReq.method() != boost::beast::http::verb::get))
+ {
+ BMCWEB_LOG_DEBUG
+ << "Only aggregate GET requests to top level collections";
+ return;
+ }
+
// Create a copy of thisReq so we we can still locally process the req
std::error_code ec;
auto localReq = std::make_shared<crow::Request>(thisReq.req, ec);
@@ -379,7 +387,7 @@
localReq, asyncResp));
}
- static void findSatelite(
+ static void findSatellite(
const crow::Request& req,
const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
const std::unordered_map<std::string, boost::urls::url>& satelliteInfo,
@@ -402,6 +410,11 @@
return;
}
}
+
+ // We didn't recognize the prefix and need to return a 404
+ boost::urls::string_value name = req.urlView.segments().back();
+ std::string_view nameStr(name.data(), name.size());
+ messages::resourceNotFound(asyncResp->res, "", nameStr);
}
// Intended to handle an incoming request based on if Redfish Aggregation
@@ -416,6 +429,23 @@
{
return;
}
+
+ // No satellite configs means we don't need to keep attempting to
+ // aggregate
+ if (satelliteInfo.empty())
+ {
+ // For collections we'll also handle the request locally so we
+ // don't need to write an error code
+ if (isCollection == AggregationType::Resource)
+ {
+ boost::urls::string_value name =
+ sharedReq->urlView.segments().back();
+ std::string_view nameStr(name.data(), name.size());
+ messages::resourceNotFound(asyncResp->res, "", nameStr);
+ }
+ return;
+ }
+
const crow::Request& thisReq = *sharedReq;
BMCWEB_LOG_DEBUG << "Aggregation is enabled, begin processing of "
<< thisReq.target();
@@ -440,7 +470,7 @@
crow::utility::OrMorePaths()))
{
// Must be FirmwareInventory or SoftwareInventory
- findSatelite(thisReq, asyncResp, satelliteInfo, memberName);
+ findSatellite(thisReq, asyncResp, satelliteInfo, memberName);
return;
}
@@ -449,8 +479,13 @@
thisReq.urlView, "redfish", "v1", std::ref(collectionName),
std::ref(memberName), crow::utility::OrMorePaths()))
{
- findSatelite(thisReq, asyncResp, satelliteInfo, memberName);
+ findSatellite(thisReq, asyncResp, satelliteInfo, memberName);
+ return;
}
+
+ // We shouldn't reach this point since we should've hit one of the
+ // previous exits
+ messages::internalError(asyncResp->res);
}
// Attempt to forward a request to the satellite BMC associated with the