From db18fc98241b4fc9a4274f45c1d6b07d7bd1b93d Mon Sep 17 00:00:00 2001
From: Carson Labrado <clabrado@google.com>
Date: Tue, 23 Aug 2022 23:48:44 +0000
Subject: 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
---
 redfish-core/include/redfish_aggregator.hpp | 41 ++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/redfish-core/include/redfish_aggregator.hpp b/redfish-core/include/redfish_aggregator.hpp
index 622bb5b459..4d05c96fac 100644
--- a/redfish-core/include/redfish_aggregator.hpp
+++ b/redfish-core/include/redfish_aggregator.hpp
@@ -362,6 +362,14 @@ class RedfishAggregator
                          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 @@ class RedfishAggregator
                                             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 @@ class RedfishAggregator
                 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 @@ class RedfishAggregator
         {
             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 @@ class RedfishAggregator
                 crow::utility::OrMorePaths()))
         {
             // Must be FirmwareInventory or SoftwareInventory
-            findSatelite(thisReq, asyncResp, satelliteInfo, memberName);
+            findSatellite(thisReq, asyncResp, satelliteInfo, memberName);
             return;
         }
 
@@ -449,8 +479,13 @@ class RedfishAggregator
                 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
-- 
cgit v1.2.3