#
45ca1b86 |
| 25-Mar-2022 |
Ed Tanous <edtanous@google.com> |
Add setUpRedfishRoute to all nodes in redfish
For better or worse, the series ahead of this is making use of setUpRedfishRoute to do the common "redfish specified" things that need to be done for a
Add setUpRedfishRoute to all nodes in redfish
For better or worse, the series ahead of this is making use of setUpRedfishRoute to do the common "redfish specified" things that need to be done for a connection, like header checking, filtering, and other things. In the current model, where BMCWEB_ROUTE is a common function for all HTTP routes, this means we need to propagate this injection call into the whole tree ahead of the requests being handled.
In a perfect world, we would invent something like a REDFISH_ROUTE macro, but because macros are discouraged, the routes take a variadic template of parameters, and each call to the route has a .privileges() call in the middle, there's no good way to effect this change in a less costly manner. This was messaged both in the prior reviews, and on discord sourcing improvements on this pattern, to which none arose.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id29cc799e214edad41e48fc7ce6eed0521f90ecb
show more ...
|
#
b9d36b47 |
| 26-Feb-2022 |
Ed Tanous <edtanous@google.com> |
Consitently use dbus::utility types
This saves about 4k on the binary size
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I9546227a19c691b1aecb
Consitently use dbus::utility types
This saves about 4k on the binary size
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I9546227a19c691b1aecb80e80307889548c0293f
show more ...
|
#
8cc8edec |
| 28-Feb-2022 |
Ed Tanous <edtanous@google.com> |
Don't rely on operator << for object logging
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 long
Don't rely on operator << for object logging
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
show more ...
|
#
15ed6780 |
| 14-Dec-2021 |
Willy Tu <wltu@google.com> |
json_utils: Add support jsonRead Patch/Action
Added support for readJson for Patch and Action. The only difference is that Patch does not allow empty json input while Action does. Action with empty
json_utils: Add support jsonRead Patch/Action
Added support for readJson for Patch and Action. The only difference is that Patch does not allow empty json input while Action does. Action with empty input will use the default value based on the implementation and return 200 OK response code.
readJsonPatch will replace the existing readJson and be used for path requests. It will not allow empty json input and all requested keys are required in the json input.
readJsonAction will be used for Action requests where it is possible for all of the properties to be optional and allow empty request. The optional properties are determined by the requested values type.
All current Action readJson are replaced with readJsonAction. It does not change the existing behavior since it needs `std::optional`. This will have to be updated later as we define the default behavior.
Tested: Added unit tests and readJsonAction allows empty empty json object.
No Change to Redfish Tree.
Change-Id: Ia5e1f81695c528a20f1dc985aee19c920d8adaea Signed-off-by: Willy Tu <wltu@google.com>
show more ...
|
#
e662eae8 |
| 25-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enable readability-implicit-bool-conversion checks
These checks ensure that we're not implicitly converting ints or pointers into bools, which makes the code easier to read.
Tested: Ran series thro
Enable readability-implicit-bool-conversion checks
These checks ensure that we're not implicitly converting ints or pointers into bools, which makes the code easier to read.
Tested: Ran series through redfish service validator. No changes observed. UUID failing in Qemu both before and after.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I1ca0be980d136bd4e5474341f4fd62f2f6bbdbae
show more ...
|
#
9eb808c1 |
| 25-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enable readability-avoid-const-params-in-decls
This check involves explicitly declaring variables const when they're declared auto, which helps in readability, and makes it more clear that the varia
Enable readability-avoid-const-params-in-decls
This check involves explicitly declaring variables const when they're declared auto, which helps in readability, and makes it more clear that the variables are const.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I71198ea03850384a389a56ad26f2c4a48c75b148
show more ...
|
#
26f6976f |
| 25-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enable readability-container-size-empty tests
This one is a little trivial, but it does help in readability.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5366d4eec8af2f781b3bad804131a
Enable readability-container-size-empty tests
This one is a little trivial, but it does help in readability.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5366d4eec8af2f781b3bad804131ae2eb806e3aa
show more ...
|
#
914e2d5d |
| 07-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enforce const correctness
For all async calls, we should be consistently capturing non trivial objects by const reference. This corrects bmcweb to be consistent and capture errors by const value, a
Enforce const correctness
For all async calls, we should be consistently capturing non trivial objects by const reference. This corrects bmcweb to be consistent and capture errors by const value, and objects by const reference.
Tested: Code compiles. Trivial changes.
This saves about 300 bytes on our compressed binary size.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib3e0b6edef9803a1c480701556949488406305d4
show more ...
|
#
711ac7a9 |
| 20-Dec-2021 |
Ed Tanous <edtanous@google.com> |
Consistently use ManagedObjectType
Some subsystems seem to have invented their own typedefs for this stuff, move to using the one typedef in dbus::utility so we're consistent, and we reduce our temp
Consistently use ManagedObjectType
Some subsystems seem to have invented their own typedefs for this stuff, move to using the one typedef in dbus::utility so we're consistent, and we reduce our templates.
Tested: code compiles
This saves a negligible amount (104 bytes compressed) on our binary size.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I952ea1f960aa703808d0ac80f35dc24cdd8d5027
show more ...
|
#
168e20c1 |
| 13-Dec-2021 |
Ed Tanous <edtanous@google.com> |
Move to common variant This saves approximately 34kB in the compressed binary size of bmcweb due to reduced template instantiations. This amounts to a 2.5% reduction in the overall
Move to common variant This saves approximately 34kB in the compressed binary size of bmcweb due to reduced template instantiations. This amounts to a 2.5% reduction in the overall size. Note, there were a few places where we broke const-correctness in the form of pulling a non-const reference out of a const variant. This new variant now requires const correctness, so some consts are added where required. Tested: Code compiles. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I6a60c8881c1268627eedb4ffddf16689dc5f6ed2
show more ...
|
#
1d8782e7 |
| 30-Nov-2021 |
Nan Zhou <nanzhoumails@gmail.com> |
fix the year 2038 problem in getDateTime The existing codes cast uint64_t into time_t which is int32_t in most 32-bit systems. It results overflow if the timestamp is larger than INT
fix the year 2038 problem in getDateTime The existing codes cast uint64_t into time_t which is int32_t in most 32-bit systems. It results overflow if the timestamp is larger than INT_MAX. time_t will be 64 bits in future releases of glibc. See https://sourceware.org/bugzilla/show_bug.cgi?id=28182. This change workarounds the year 2038 problem via boost's ptime. std::chrono doesn't help since it is still 32 bits. Tested on QEMU. Example output for certificate: { "Name": "HTTPS Certificate", "Subject": null, "ValidNotAfter": "2106-01-28T20:40:31Z", "ValidNotBefore": "2106-02-06T18:28:16Z" } Previously, the format is like "1969-12-31T12:00:00+00:00". Note that the ending "+00:00" is the time zone, not ms. Tested the schema on QEMU. No new Redfish Service Validator errors. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I8ef0bee3d724184d96253c23f3919447828d3f82
show more ...
|
#
0fda0f12 |
| 15-Nov-2021 |
George Liu <liuxiwei@inspur.com> |
Update clang-format refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format `Don't break long string literals` Tested: built bmcweb successfully and RedfishVa
Update clang-format refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format `Don't break long string literals` Tested: built bmcweb successfully and RedfishValidator Passed. Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: Ib58f7c942fd3838592e043c57e0b6ffcdc3d963b
show more ...
|
#
abb93cdd |
| 02-Sep-2021 |
Ed Tanous <edtanous@google.com> |
Make code pass clang-tidy-13 clang-tidy-13 appears to have improved its "use brace initialization" checker to handle more cases, and now correctly fails a couple cases that we posses
Make code pass clang-tidy-13 clang-tidy-13 appears to have improved its "use brace initialization" checker to handle more cases, and now correctly fails a couple cases that we posses. This commit simply makes the very mechanical changes to make this function. Tested: Ran previous commits checks to run ninja clang-tidy, and observed that clang-tidy now passes. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I665a7f3e71d25af93703e6b31af0f1eb12a44527
show more ...
|
#
4f48d5f6 |
| 21-Jun-2021 |
Ed Tanous <edtanous@google.com> |
Make code compile with clang-13 Clang-13 rightfully warns that the hasWebuiRoute variable isn't declared as static. This commit resolves that, and adds the static keyword so it can
Make code compile with clang-13 Clang-13 rightfully warns that the hasWebuiRoute variable isn't declared as static. This commit resolves that, and adds the static keyword so it can be used in multiple compile units. It also adds the static keyword to the privilege registry, and the inline keyword to many methods that now need it. clang-format is also updated to version 12 in parse_registies.py, as that's what CI uses, and what most people have installed. Tested: Followed clang-tidy instructions in README.md "bitbake bmcweb" step now succeeds. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id43b13606754cb37a404799fce155599ac3a3240
show more ...
|
#
ed398213 |
| 09-Jun-2021 |
Ed Tanous <edtanous@google.com> |
Automate PrivilegeRegistry to code This commit attempts to automate the creation of our privileges structures from the redfish privilege registry. It accomplishes this by updating p
Automate PrivilegeRegistry to code This commit attempts to automate the creation of our privileges structures from the redfish privilege registry. It accomplishes this by updating parse_registries.py to also pull down the privilege registry from DMTF. The script then generates privilege_registry.hpp, which include const defines for all the privilege registry entries in the same format that the Privileges struct accepts. This allows new clients to simply reference the variable to these privilege structures, instead of having to manually (ie error pronely) put the privileges in themselves. This commit updates all the routes. For the moment, override and OEM schemas are not considered. Today we don't have any OEM-specific Redfish routes, so the existing ones inherit their parents schema. Overrides have other issues, and are already incorrect as Redfish defines them. Binary size remains unchanged after this patchset. Tested: Ran redfish service validator Ran test case from f9a6708c4c6490257e2eb6a8c04458f500902476 to ensure that the new privileges constructor didn't cause us to regress the brace construction initializer. Checked binary size with: gzip -c $BBPATH/tmp/work/s7106-openbmc-linux-gnueabi/obmc-phosphor-image/1.0-r0/rootfs/usr/bin/bmcweb | wc -c 1244048 (tested on previous patchset) Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ideede3d5b39d50bffe7fe78a0848bdbc22ac387f
show more ...
|
#
72048780 |
| 02-Jun-2021 |
Abhishek Patel <Abhishek.Patel@ibm.com> |
Modify entityPrivileges for certificate service DMTF published new entity privileges for certificate service classes which modify entity privilege Certificate, CertificateCollection,
Modify entityPrivileges for certificate service DMTF published new entity privileges for certificate service classes which modify entity privilege Certificate, CertificateCollection, CertificateLocations, and CertificateService on bmcweb. Modification restricts a user without "ConfigureManager" from accessing the CertificateCollection and Certificate scehamas Redfish is a hypermedia API where the parent URI describes sub-URI. Thus, restricting sub-URI in a parent-URI data helps to forbidden user access, stricken the rule. So sub-URI only gets display if a user has access to that URI. Restricting the link allows the Redfish Validator to pass. These impact roles without ConfigureManager, which include operator and read-only. No access is not impacted since it already did not have access. The following are bmcweb user consequences: 1. ReadOnly and Operator role users are no longer able to view certificates or the certificate collection (LDAP, HTTPS, TrustStore) 2. Operator role users are no longer able to replace the certificates (LDAP, HTTPS, TrustStore), Install certificates (LDAP, HTTPS, TrustStore) or delete the Truststore Certificate. HTTPS and LDAP certificates do not have delete methods. Resolves openbmc/bmcweb#61 Tested: manually tested on Witherspoon system and run Redfish-Service- Validator with all roles root, operator, read-only, and No access. Test pass for root, operator, and read-only roles, And new errors get introduced for no access role. Signed-off-by: Abhishek Patel <Abhishek.Patel@ibm.com> Change-Id: Ibc5eed7db7e224e46f8572df8bcfba2a1ff47644
show more ...
|
#
432a890c |
| 14-Jun-2021 |
Ed Tanous <edtanous@google.com> |
Remove ambiguous privileges constructor There are a number of endpoints that assume that a given routes privileges are governed by a single set of privileges, instead of multiple set
Remove ambiguous privileges constructor There are a number of endpoints that assume that a given routes privileges are governed by a single set of privileges, instead of multiple sets ORed together. To handle this, there were two overloads of the privileges() method, one that took a vector of Privileges, and one that took an initializer_list of const char*. Unfortunately, this leads some code in AccountService to pick the wrong overload when it's called like this .privileges( {{"ConfigureUsers"}, {"ConfigureManager"}, {"ConfigureSelf"}}) This is supposed to be "User must have ConfigureUsers, or ConfigureManager, or ConfigureSelf". Currently, because it selects the wrong overload, it computes to "User must have ConfigureUsers AND ConfigureManager AND ConfigureSelf. The double braces are supposed to cause this to form a vector of Privileges, but it appears that the initializer list gets consumed, and the single invocation of initializer list is called. Interestingly, trying to put in a privileges overload of intializer_list<initializer_list<const char*>> causes the compilation to fail with an ambiguous call error, which is what I would've expected to see previously in this case, but alas, I'm only a novice when it comes to how the C++ standard works in these edge cases. This is likely due in part to the fact that they were templates of an unused template param (seemingly copied from the previous method) and SFINAE rules around templates. This commit functionally removes one of the privileges overloads, and adds a second set of braces to every privileges call that previously had a single set of braces. Previous code will not compile now, which is IMO a good thing. This likely popped up in the Node class removal, because the Node class explicitly constructs a vector of Privilege objects, ensuing it can hit the right overload Tested: Ran Redfish service validator Tested the specific use case outlined on discord with: Creating a new user with operator privilege: ``` redfishtool -S Always -u root -p 0penBmc -vvvvvvvvv -r 192.168.7.2 AccountService adduser foo mysuperPass1 Operator ``` Then attempting to list accounts: ``` curl -vvvv --insecure --user foo:mysuperPass1 https://192.168.7.2/redfish/v1/AccountService/Accounts/foo ``` Which succeeded and returned the account in question. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I83e62b70e97f56dc57d43b9081f333a02fe85495
show more ...
|
#
7e860f15 |
| 08-Apr-2021 |
John Edward Broadbent <jebr@google.com> |
Remove Redfish Node class Reduces the total number of lines and will allow for easier testing of the redfish responses. A main purpose of the node class was to set app.routeDyna
Remove Redfish Node class Reduces the total number of lines and will allow for easier testing of the redfish responses. A main purpose of the node class was to set app.routeDynamic(). However now app.routeDynamic can handle the complexity that was once in critical to node. The macro app.routeDynamic() provides a shorter cleaner interface to the unerlying app.routeDyanic call. The old pattern set permissions for 6 interfaces (get, head, patch, put, delete_, and post) even if only one interface is created. That pattern creates unneeded code that can be safely removed with no effect. Unit test for the responses would have to mock the node the class in order to fully test responses. see https://github.com/openbmc/bmcweb/issues/181 The following files still need node to be extracted. virtual_media.hpp account_service.hpp redfish_sessions.hpp ethernet.hpp The files above use a pattern that is not trivial to address. Often their responses call an async lambda capturing the inherited class. ie (https://github.com/openbmc/bmcweb/blob/ffed87b5ad1797ca966d030e7f979770 28d258fa/redfish-core/lib/account_service.hpp#L1393) At a later point I plan to remove node from the files above. Tested: I ran the docker unit test with the following command. WORKSPACE=$(pwd) UNIT_TEST_PKG=bmcweb ./openbmc-build-scripts/run-unit-test-docker.sh I ran the validator and this change did not create any issues. python3 RedfishServiceValidator.py -c config.ini Signed-off-by: John Edward Broadbent <jebr@google.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I147a0289c52cb4198345b1ad9bfe6fdddf57f3df
show more ...
|
#
8d1b46d7 |
| 31-Mar-2021 |
zhanghch05 <zhanghch05@inspur.com> |
Using AsyncResp everywhere Get the core using AsyncResp everywhere, and not have each individual handler creating its own object.We can call app.handle() without fear of the response
Using AsyncResp everywhere Get the core using AsyncResp everywhere, and not have each individual handler creating its own object.We can call app.handle() without fear of the response getting ended after the first tree is done populating. Don't use res.end() anymore. Tested: 1. Validator passed. Signed-off-by: zhanghaicheng <zhanghch05@inspur.com> Change-Id: I867367ce4a0caf8c4b3f4e07e06c11feed0782e8
show more ...
|
#
0daf14e0 |
| 08-Mar-2021 |
Gunnar Mills <gmills@us.ibm.com> |
Remove IBM copyright These aren't needed and are not in all files. These aren't being updated. Would perfer these go away. Tested: Not Tested. Change-Id: I050874d58f86c
Remove IBM copyright These aren't needed and are not in all files. These aren't being updated. Would perfer these go away. Tested: Not Tested. Change-Id: I050874d58f86cae138ce2ab8c0c53831aeba5b21 Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
show more ...
|
#
9c8e039e |
| 01-Feb-2021 |
Jonathan Doman <jonathan.doman@intel.com> |
Remove unnecessary error responses for LDAP certs Currently, /v1/CertificateService/CertificateLocations and /v1/AccountService/LDAP/Certificates endpoints assume the presence of xyz
Remove unnecessary error responses for LDAP certs Currently, /v1/CertificateService/CertificateLocations and /v1/AccountService/LDAP/Certificates endpoints assume the presence of xyz.openbmc_project.Certs.Manager.Client.Ldap service, and return an error on D-Bus failures. But this service can be missing if LDAP support is removed from the build, so we should just return empty responses instead of errors. Tested: Passed Redfish service validator. Signed-off-by: Jonathan Doman <jonathan.doman@intel.com> Change-Id: Ib8416e850b52e8ce0f8947017d863cee19f7b2c8
show more ...
|
#
17a897df |
| 12-Sep-2020 |
Manojkiran Eda <manojkiran.eda@gmail.com> |
Improve loops & fix cpp check warning - This commit improves certain while loops to range based for loops. - This commit also fixes the cppcheck warning that mentions about pe
Improve loops & fix cpp check warning - This commit improves certain while loops to range based for loops. - This commit also fixes the cppcheck warning that mentions about performance issues when using postfix operators on non-primitive types. Tested By: - A function is unittested. - GET on both EthernetInterfaces & certificate service looks good without any issues. Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com> Change-Id: I85420f7bf9af45a97e1a93b916f292c2516f5802
show more ...
|
#
f23b7296 |
| 15-Oct-2020 |
Ed Tanous <ed@tanous.net> |
Turn on ALL perf checks 1st, alphabetize the tidy-list for good housekeeping. Next, enable all the clang-tidy performance checks, and resolve all the issues. most of the issues
Turn on ALL perf checks 1st, alphabetize the tidy-list for good housekeeping. Next, enable all the clang-tidy performance checks, and resolve all the issues. most of the issues boil down to: 1. Using std::move on const variables. This does nothing. 2. Passing big variables (like std::string) by value. 3. Using double quotes on a find call, which constructs an intermediate string, rather than using the character overload. Tested Loaded on system, logged in successfully and pulled down webui-vue. No new errors. Walked the Redfish tree a bit, and observed no new problems. Ran redfish service validator. Got no new failures (although there are a lot of log service deprecation warnings that we should look at). Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I2238958c4b22c1e554e09a0a1787c744bdbca43e
show more ...
|
#
72d52d25 |
| 12-Oct-2020 |
Ed Tanous <ed@tanous.net> |
Fix integer constant from 20 to 18 A bad click merged a commit before it was ready (it was +2ed previously). So far as I'm aware, this was the only change needed. Happy to reve
Fix integer constant from 20 to 18 A bad click merged a commit before it was ready (it was +2ed previously). So far as I'm aware, this was the only change needed. Happy to revert it if someone sees need to. Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I4bb842edd78a4d580bb4842e4a708632761fa86d
show more ...
|
#
5207438c |
| 28-Sep-2020 |
Ed Tanous <ed@tanous.net> |
Use std::array instead of char array Char arrays are outdated, and not needed in this case. No functional changes Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Ib
Use std::array instead of char array Char arrays are outdated, and not needed in this case. No functional changes Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Ibdcebf05880ac2697c9a30f5a86155a09ff7b3d8
show more ...
|