#
08fad5d9
|
| 31-Jul-2025 |
Corey Ethington <cethington@coreweave.com> |
Add check to omit `DateTime` from etag calculation
Ignores any json property named `DateTime` when calculating the etag value of an HTTP response as per the updated Redfish Spec (section 6.5: ETags)
Add check to omit `DateTime` from etag calculation
Ignores any json property named `DateTime` when calculating the etag value of an HTTP response as per the updated Redfish Spec (section 6.5: ETags)
Tested: - Redfish Service Validator passes - Tested on romulus: 1. GET resource with a "DateTime" field ``` curl -k -H "X-Auth-Token: $XAUTH_TOKEN" "https://$BMC/redfish/v1/TaskService" \ --etag-save ./etag.txt -v ... < etag: "6A4CE897" ... { "@odata.id": "/redfish/v1/TaskService", "@odata.type": "#TaskService.v1_1_4.TaskService", "CompletedTaskOverWritePolicy": "Oldest", "DateTime": "2025-07-23T17:08:20+00:00", "Id": "TaskService", "LifeCycleEventOnTaskStateChange": true, "Name": "Task Service", "ServiceEnabled": true, "Status": { "State": "Enabled" }, "Tasks": { "@odata.id": "/redfish/v1/TaskService/Tasks" } ```
2. GET same resource again later, etag is same as before ``` curl -k -H "X-Auth-Token: $XAUTH_TOKEN" "https://$BMC/redfish/v1/TaskService" \ --etag-save ./etag.txt -v ... < etag: "6A4CE897" ... { "@odata.id": "/redfish/v1/TaskService", "@odata.type": "#TaskService.v1_1_4.TaskService", "CompletedTaskOverWritePolicy": "Oldest", "DateTime": "2025-07-23T17:10:48+00:00", "Id": "TaskService", "LifeCycleEventOnTaskStateChange": true, "Name": "Task Service", "ServiceEnabled": true, "Status": { "State": "Enabled" }, "Tasks": { "@odata.id": "/redfish/v1/TaskService/Tasks" } ``` "DateTime" is the only value to have changed, but since it is ignored the etag did not change
3. GET with if-none-match returns 304 ``` curl -k -H "X-Auth-Token: $XAUTH_TOKEN" "https://$BMC/redfish/v1/TaskService" \ --etag-save ./etag.txt --etag-compare ./etag.txt -v ... > if-none-match: "6A4CE897" ... < HTTP/2 304 < allow: GET < odata-version: 4.0 < strict-transport-security: max-age=31536000; includeSubdomains < pragma: no-cache < cache-control: no-store, max-age=0 < x-content-type-options: nosniff < etag: "6A4CE897" < date: Wed, 23 Jul 2025 17:14:39 GMT < content-length: 0 < ... ```
Change-Id: I51f7668e75719c69c55535e4a1e48c8bae7c9488 Signed-off-by: Corey Ethington <cethington@coreweave.com>
show more ...
|
#
66620686
|
| 05-Jun-2024 |
Ed Tanous <ed@tanous.net> |
Allow POST on aggregation sources
It would be useful for both testing and production if one could create aggregation sources dynamically, rather than using detected hardware. Redfish sources might e
Allow POST on aggregation sources
It would be useful for both testing and production if one could create aggregation sources dynamically, rather than using detected hardware. Redfish sources might exist outside of the physical chassis in a way that's detectable, so giving DC software a way to set up a non-trivial aggregation topology make this more extensible.
Note, that as documented in AGGREGATION.md, only a single satellite is supported by this feature. This patch does not change that behavior, and implementing an entity-manager satellite will override a POST created AggregationSource.
Tested:
Example commands succeed, and return the expected results. ``` curl -vvvv -k --http1.1 --user "root:0penBmc" --request DELETE https://192.168.7.2/redfish/v1/AggregationService/AggregationSources/ curl -vvvv -k --http1.1 --user "root:0penBmc" https://192.168.7.2/redfish/v1/AggregationService/AggregationSources -H "Content-Type: application/json" --request POST --data '{"HostName":"https://localhost:8000"}' ```
Redfish service validator passes.
Change-Id: I15711c3075645291b5f2555eefb80bb80418e7e5 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
19fab295
|
| 12-Feb-2025 |
Chandramohan Harkude <chandramohan.harkude@gmail.com> |
Authenticate expand query parameter
Addressed the below requirements for security GAP
1) PrivilegeRegistry is now enforced for all users and all expanded resources. If a user lacks access, the n
Authenticate expand query parameter
Addressed the below requirements for security GAP
1) PrivilegeRegistry is now enforced for all users and all expanded resources. If a user lacks access, the node will not be expanded.
2) If a user with Operator privilege calls '/redfish/v1?$expand', the response includes only resources the user has access to.
3) 'ReadOnly' and 'Operator' roles can expand, but not into nodes they lack privileges for.
4) Expand is completely disallowed without authentication. Requests without valid credentials receive 401 Unauthorized.
Testing: Tested and verified that users can access only authorized resources. Unauthorized access and unauthenticated expand requests are blocked.
Change-Id: Ic1c2761f30db73a2884cdecdb8eb08168446523b Signed-off-by: Chandramohan Harkude <chandramohan.harkude@gmail.com>
show more ...
|
#
608ad2cc
|
| 20-May-2024 |
Ed Tanous <ed@tanous.net> |
Make Request copy explicit
It is currently too easy to accidentally make copies of the Request object. Ideally code would parse out the Request in the first handler, then no longer require an async
Make Request copy explicit
It is currently too easy to accidentally make copies of the Request object. Ideally code would parse out the Request in the first handler, then no longer require an async copy. There is one case in the redfish query things where we actually need a copy of the request object, so we need these constructors, but we should make them explicit.
This commit moves the Request constructor to be private, and adds a new method called copy() for explicitly making a copy. Ironcially, this finds one place where we were actually making a copy of the request object unintentionally, so fix that to only capture the value required,the user session.
Tested: - Compiles - Run GET/PATCH related curl or If-Match like PATCH Account - Redfish Service Validator runs and passes
Change-Id: I19255981f42757ed736112c003201e3f758735ac Signed-off-by: Ed Tanous <ed@tanous.net> Signed-off-by: Myung Bae <myungbae@us.ibm.com>
show more ...
|
#
504af5a0
|
| 03-Feb-2025 |
Patrick Williams <patrick@stwcx.xyz> |
clang-format: update latest spec and reformat
Copy the latest format file from the docs repository and apply.
Change-Id: I2f0b9d0fb6e01ed36a2f34c750ba52de3b6d15d1 Signed-off-by: Patrick Williams <p
clang-format: update latest spec and reformat
Copy the latest format file from the docs repository and apply.
Change-Id: I2f0b9d0fb6e01ed36a2f34c750ba52de3b6d15d1 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
show more ...
|
#
d7857201
|
| 28-Jan-2025 |
Ed Tanous <etanous@nvidia.com> |
Fix includes
Clang-tidy misc-include-cleaner appears to now be enforcing significantly more headers than previously. That is overall a good thing, but forces us to fix some issues. This commit is
Fix includes
Clang-tidy misc-include-cleaner appears to now be enforcing significantly more headers than previously. That is overall a good thing, but forces us to fix some issues. This commit is largely just taking the clang-recommended fixes and checking them in. Subsequent patches will fix the more unique issues.
Note, that a number of new ignores are added into the .clang-tidy file. These can be cleaned up over time as they're understood. The majority are places where boost includes a impl/x.hpp and x.hpp, but expects you to use the later. include-cleaner opts for the impl, but it isn't clear why.
Change-Id: Id3fdd7ee6df6c33b2fd35626898523048dd51bfb Signed-off-by: Ed Tanous <etanous@nvidia.com> Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
show more ...
|
#
40e9b92e
|
| 10-Sep-2024 |
Ed Tanous <etanous@nvidia.com> |
Use SPDX identifiers
SPDX identifiers are simpler, and reduce the amount of cruft we have in code files. They are recommended by linux foundation, and therefore we should do as they allow.
This pa
Use SPDX identifiers
SPDX identifiers are simpler, and reduce the amount of cruft we have in code files. They are recommended by linux foundation, and therefore we should do as they allow.
This patchset does not intend to modify any intent on any existing copyrights or licenses, only to standardize their inclusion.
[1] https://www.linuxfoundation.org/blog/blog/copyright-notices-in-open-source-software-projects
Change-Id: I935c7c0156caa78fc368c929cebd0f068031e830 Signed-off-by: Ed Tanous <etanous@nvidia.com>
show more ...
|
#
478b7adf
|
| 15-Jul-2024 |
Ed Tanous <etanous@nvidia.com> |
Remove IWYU pragmas
These were added as part of d5c80ad9c07b94465d8ea62d2b6f87c30cac765e: test treewide: iwyu
Since then, Nan hasn't been very active on the project, and to my knowledge, since the
Remove IWYU pragmas
These were added as part of d5c80ad9c07b94465d8ea62d2b6f87c30cac765e: test treewide: iwyu
Since then, Nan hasn't been very active on the project, and to my knowledge, since the initial run, we've never used IWYU again.
clang-include-cleaner seems to work well without needing these pragmas, and is what we're using, even if it's less useful than IWYU.
Remove all mention of IWYU.
Tested: Code compiles.
Change-Id: I06feedeeac9a114f5bdec81d59ca83223efd8aa7 Signed-off-by: Ed Tanous <etanous@nvidia.com>
show more ...
|
#
bd79bce8
|
| 16-Aug-2024 |
Patrick Williams <patrick@stwcx.xyz> |
clang-format: re-format for clang-18
clang-format-18 isn't compatible with the clang-format-17 output, so we need to reformat the code with the latest version. The way clang-18 handles lambda forma
clang-format: re-format for clang-18
clang-format-18 isn't compatible with the clang-format-17 output, so we need to reformat the code with the latest version. The way clang-18 handles lambda formatting also changed, so we have made changes to the organization default style format to better handle lambda formatting.
See I5e08687e696dd240402a2780158664b7113def0e for updated style. See Iea0776aaa7edd483fa395e23de25ebf5a6288f71 for clang-18 enablement.
Change-Id: Iceec1dc95b6c908ec6c21fb40093de9dd18bf11a Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
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 ...
|
#
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 ...
|
#
1873a04f
|
| 01-Apr-2024 |
Myung Bae <myungbae@us.ibm.com> |
Reduce multi-level calls of req.req members
Several places access the members of `req` indirectly like `req.req.method()`. This can be simplified as `req.method()` .
This would also make the code
Reduce multi-level calls of req.req members
Several places access the members of `req` indirectly like `req.req.method()`. This can be simplified as `req.method()` .
This would also make the code clearer.
Tested: - Compiles - Redfish service validator passes
Change-Id: Ie129564ff907cdea7ac224b1e3d80cc0dedfbd7b Signed-off-by: Myung Bae <myungbae@us.ibm.com>
show more ...
|
#
e01d0c36
|
| 30-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Fix bugprone-unchecked-optional-access findings
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 c
Fix bugprone-unchecked-optional-access findings
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>
show more ...
|
#
62598e31
|
| 17-Jul-2023 |
Ed Tanous <ed@tanous.net> |
Replace logging with std::format
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
Replace logging with std::format
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
show more ...
|
#
32cdb4a7
|
| 23-May-2023 |
Willy Tu <wltu@google.com> |
query: Fix default expand level with delegated
With delegate expand, the default expand level is -= `queryCapabilities.canDelegateExpandLevel`. This creates an overlap of expand process between dele
query: Fix default expand level with delegated
With delegate expand, the default expand level is -= `queryCapabilities.canDelegateExpandLevel`. This creates an overlap of expand process between delegate expand vs. default expand.
With query.expandLevel = 2 -> query.expandLevel = 1 and delegated.expandLevel = 1.
Both delegated and default expand will try to only expand of level one instead of level 2 for the default.
The code in https://github.com/openbmc/bmcweb/blob/479e899d5f57a67647f83b7f615d2c8565290bcf/redfish-core/include/utils/query_param.hpp#L583-L597 stated that the level with "@odata.id" + other property is treated as a seperate level. So with `query.expandLevel = 1` it just loop through the id that was already expanded and is noop.
Tested:
Before: /redfish/v1/Chassis/BMC/Sensors?$expand=.($levels=2) returns the same result as level=1. Needs level=3 to expand to the next level.
The RelatedItem in here doesn't get expanded with level=2. ``` wget -qO- 'http://localhost:80/redfish/v1/Chassis/BMC/Sensors?$expand=.($levels=1)' ... { "@odata.id": "/redfish/v1/Chassis/BMC/Sensors/temperature_DIMMXX", "@odata.type": "#Sensor.v1_2_0.Sensor", "Id": "temperature_DIMMXX", "Name": "DIMMXX", "Reading": 30.0, "ReadingRangeMax": 127.0, "ReadingRangeMin": -128.0, "ReadingType": "Temperature", "ReadingUnits": "Cel", "RelatedItem": [ { "@odata.id": "/redfish/v1/Systems/system/Memory/dimmXX" } ], "Status": { "Health": "OK", "State": "Enabled" }, "Thresholds": { "LowerCaution": { "Reading": null }, "LowerCritical": { "Reading": null }, "UpperCaution": { "Reading": 93.0 }, "UpperCritical": { "Reading": 95.0 } } } ], "Members@odata.count": 242, "Name": "Sensors" } ```
After: level=2 was able to expand to the next level.
Change-Id: I542177a94a33f8df7afbb68837f3a53b86140c86 Signed-off-by: Willy Tu <wltu@google.com>
show more ...
|
#
30806a2b
|
| 06-Apr-2023 |
Ed Tanous <edtanous@google.com> |
Fix clang-tidy issue in aggregation
clang-tidy flags the following error, which is correct.
``` ../redfish-core/include/query.hpp:145:26: error: static member accessed through instance [readability
Fix clang-tidy issue in aggregation
clang-tidy flags the following error, which is correct.
``` ../redfish-core/include/query.hpp:145:26: error: static member accessed through instance [readability-static-accessed-through-instance,-warnings-as-errors] needToCallHandlers = RedfishAggregator::getInstance().beginAggregation( ```
Tested (Carson): Verified that queries to top level collections as well as aggregated resources still return as expected.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I440fb29d2e0b3da52bfd564676d33b72f69f2fbc Signed-off-by: Carson Labrado <clabrado@google.com>
show more ...
|
#
a1cbc192
|
| 29-Mar-2023 |
Hieu Huynh <hieuh@os.amperecomputing.com> |
Fix If Match header in Http layer
Commit [1] prevents the clients performing methods if missing ETag from the If-Match header. For the "If-Match: *" [2] that representing any resource, it should be
Fix If Match header in Http layer
Commit [1] prevents the clients performing methods if missing ETag from the If-Match header. For the "If-Match: *" [2] that representing any resource, it should be the valid command.
[1] https://github.com/openbmc/bmcweb/commit/2d6cb56b6b47c3fbb0d234ade5c1208edb69ef1f [2] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Match
Tested: Can performing methods GET/POST/PATCH/DELETE with "If-Match: *" header.
Signed-off-by: Hieu Huynh <hieuh@os.amperecomputing.com> Change-Id: I2e5a81ed33336a939b01bd6b64d3ff99501341d0
show more ...
|
#
39662a3b
|
| 06-Feb-2023 |
Ed Tanous <edtanous@google.com> |
Make url by value in Request
There's some tough-to-track-down safety problems in http Request. This commit is an attempt to make things more safe, even if it isn't clear how the old code was wrong.
Make url by value in Request
There's some tough-to-track-down safety problems in http Request. This commit is an attempt to make things more safe, even if it isn't clear how the old code was wrong.
Previously, the old code took a url_view from the target() string for a given URI. This was effectively a pointer, and needed to be updated in custom move/copy constructors that were error prone to write.
This commit moves to taking the URI by non-view, which involves a copy, but allows us to use the default move and copy constructors, as well as have no internal references within Request, which should improve the safety and reviewability.
There's already so many string copies in bmcweb, that this is unlikely to show up as any sort of performance regression, and simple code is much better in this case.
Note, because of a bug in boost::url, we have to explicitly construct a url_view in any case where we want to use segments() or query() on a const Request. This has been reported to the boost maintainers, and is being worked for a long term solution.
https://github.com/boostorg/url/pull/704
Tested: Redfish service validator passed on last commit in series.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I49a7710e642dff624d578ec1dde088428f284627
show more ...
|
#
3ccb3adb
|
| 13-Jan-2023 |
Ed Tanous <edtanous@google.com> |
Fix a boatload of #includes
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
Fix a boatload of #includes
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
show more ...
|
#
2d6cb56b
|
| 07-Jul-2022 |
Ed Tanous <edtanous@google.com> |
Implement If-Match header in Http layer
If-Match is a header in the HTTP specification[1] designed for handling atomic operations within a given HTTP tree. It allows a mechanism for an implementati
Implement If-Match header in Http layer
If-Match is a header in the HTTP specification[1] designed for handling atomic operations within a given HTTP tree. It allows a mechanism for an implementation to explicitly declare "only take this action if the resource has not been changed". While most things within the Redfish tree don't require this level of interlocking, it continues to round out our redfish support for the specific use cases that might require it.
Redfish specification 6.5 states: If a service supports the return of the ETag header on a resource, the service may respond with HTTP 428 status code if the If-Match or If-None-Match header is missing from the PUT or PATCH request for the same resource, as specified in RFC6585
This commit implements that behavior for all handlers to follow the following flow. If If-Match is present Repeat the same request as a GET Compare the ETag produced by the GET, to the one provided by If-Match If they don't match, return 428 if they do match, re-run the query.
[1] https://www.rfc-editor.org/rfc/rfc2616#section-14.24
As a consequence, this requires declaring copy and move constructors onto the Request object, so the request object can have its lifetime extended through a request, which is very uncommon.
Tested: Tests run on /redfish/v1/AccountService/Accounts/root PATCH with correct If-Match returns 200 success PATCH with an incorrect If-Match returns 419 precondition required GET returns the resource as expected
Redfish service validator passes Redfish protocol validator passes! ! ! ! !
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I530ab255259c32fe4402eb8e5104bd091925c77b
show more ...
|
#
05916cef
|
| 01-Aug-2022 |
Carson Labrado <clabrado@google.com> |
Aggregation: Prepare for routing requests
We do not want to allow a HW config to set its own prefix since that results in HW choosing and hardcoding resource URIs. Removes using "Name" from the sat
Aggregation: Prepare for routing requests
We do not want to allow a HW config to set its own prefix since that results in HW choosing and hardcoding resource URIs. Removes using "Name" from the satellite config as the config's prefix.
For now assume there will be no more than one satellite bmc. We will always assign that config to be "aggregated0". If more than one config is present then we will not attempt to forward any requests. In a future patch we will add support for aggregating multiple satellite BMCs. The aggregator will be responsible for assigning the prefixes to each satellite.
When we receive a request we parse the resource ID to see if it begins with "aggregated" and thus should be forwarded to a satellite BMC. In those cases we should not locally handle the request. We return a 500 error, but in a future patch that will be replaced by the actual code to forward the request to the appropriate satellite.
Requests for resource collections need to be both handled locally and forwarded. Place holders are added for where the forwarding will occur. A future patch will add that functionality.
Tested: Exposed two configs in an entity-manager json: "Exposes": [ { "Hostname": "127.0.0.1", "Port": "443", "Name": "Sat1", "Type": "SatelliteController", "AuthType": "None" }, { "Hostname": "127.0.0.1", "Port": "444", "Name": "Sat2", "Type": "SatelliteController", "AuthType": "None" },
It produced an error that only one satellite is supported and as a result both configs were ignored. I removed the second config and that resulted in the first (and only) config being added as "aggregated0".
Requests for local resources were ignored by the aggregation code. Requests for collections hit the forward collection endpoints and return local results.
500 returned for satellite resources such as: /redfish/v1/Chassis/aggregated0_Fake /redfish/v1/UpdateService/FirmwareInventory/aggregated0_Fake /redfish/v1/UpdateService/SoftwareInventory/aggregated0_Fake
Change-Id: I5c860c01534e7d5b1a37c95f75be5b3c1f695816 Signed-off-by: Carson Labrado <clabrado@google.com> Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
827c4902
|
| 02-Aug-2022 |
Nan Zhou <nanzhoumails@gmail.com> |
query: $select : use trie
The previous implementation has a downside: it stores all intermediate paths in a hashset. This increases the space complexity. This commit implements a more efficient (bot
query: $select : use trie
The previous implementation has a downside: it stores all intermediate paths in a hashset. This increases the space complexity. This commit implements a more efficient (both time and space) algorithm: Trie.
The commit contructs a trie from the values of $select. Upon delegation, it delegates the whole trie for now. When doing recursive select, now we only need to keep the current JSON node and corresponding TrieNode.
Given the following assumption: 1. size of the JSON tree (#nodes) is N 2. length of the $select properties is M 3. average length of a single $select property is K
The previous implementation has the following complexity: 1. time: worst case O(N + M * K), when a property is like /a/a/a/a/a 2. space: O(N + M * K^2), when a property is like /a/a/a/a/a
The current implementation improves complexity to: 1. time: O(N + M * K) 2. space: O(N + M * K)
The total image (squashfs) decreases 4096 bytes. The binaray size also decreases 4096 bytes.
Tested: 1. $select works on real hardware 2. No new service validator failures 3. Added more unit tests
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: If9f09db8c76e4e1abb6fa9b760be3816d9eb9f96
show more ...
|
#
e796c262
|
| 02-Aug-2022 |
Nan Zhou <nanzhoumails@gmail.com> |
query: iwyu
An incremental improvement on headers.
Tested: no tests. Code compiles.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Iec4d89c46af4381d1a56c3db3571780016b244d9
|
#
8a592810
|
| 04-Jun-2022 |
Ed Tanous <edtanous@google.com> |
Fix shadowed variable issues
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, the
Fix shadowed variable issues
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
show more ...
|
#
3ba00073
|
| 06-Jun-2022 |
Carson Labrado <clabrado@google.com> |
Expose AsyncResp shared_ptr when handling response
For Redfish Aggregation, we need a common point to check the D-Bus for satellite configs. If they are available then we perform the aggregation op
Expose AsyncResp shared_ptr when handling response
For Redfish Aggregation, we need a common point to check the D-Bus for satellite configs. If they are available then we perform the aggregation operations. The functions in query.hpp are used by all endpoints making them the logical location. The aggregation code requires a shared_ptr to the AsyncResp so these functions need to be able to supply that.
This patch is broken out of a future patch for routing Redfish Aggregation requests https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310
The follow commands can be used to perform most of the replacements: find . -type f | xargs sed -i 's/setUpRedfishRoute(app, req, asyncResp->res/setUpRedfishRoute(app, req, asyncResp/g' find . -type f | xargs sed -i 's/setUpRedfishRouteWithDelegation(app, req, asyncResp->res/setUpRedfishRouteWithDelegation(app, req, asyncResp/g'
Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: I4f4f9f22cdcfb14a3bd94b9a8f3d64aae34e57bc
show more ...
|