Age | Commit message (Collapse) | Author | Files | Lines |
|
It is now Q2 2023, and this option has been deprecated. Maintainers
have not had any reports of this behavior breaking people.
Tested: Code delete only. Code compiles.
Change-Id: I9c1fe26e497806c6bc602fb019bafe0fc80d7619
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
We have no unit tests for this. This isn't very extensive, but we
should have at least one.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I212ee528b354f2ed47f88076be009fd6e16fb760
|
|
In order to be able to more easily debug bmcweb related issue,
a new meson option is added to set a specific logging level
Which generates the targeted logging traces rather than all of
debug traces.
The current option -Dbmcweb-logging which can be either
disabled or enabled is changed to allow to set the log level
for the specific level traces (e.g. error or critical traces)
to be written to the journal.
-Dbmcweb-logging=<log-level>
where <log-level> can be disabled, enabled, debug, info,
warning, error, or critical.
- `disabled`: Turns off all bmcweb log traces.
- `enabled` : treated as `debug`
- Other option can be described in
[Logging Levels](DEVELOPING.md).
For an example, to enable only 'error', 'critical' log
entries, bmcweb can be built with
-Dbmcweb-logging=error
Testing:
- Verified that only the specific logs (e.g. error and
critical logs) were displayed by compiling bmcweb with the
specific bmcweb-logging level.
Change-Id: I522ca26700ea420fee1a5cf688d3e8c6661f2f55
Signed-off-by: Myung Bae <myungbae@us.ibm.com>
|
|
Since meson 0.56, the `get_pkgconfig_variable` has been deprecated. In
meson 0.58 the `get_variable` was enhanced to no longer require the
`pkgconfig` keyword argument. Ensure meson 0.58 is required and update
the usage of all `get_pkgconfig_variable` and `get_variable` to be the
modern variant.
Change-Id: I61193ae9fb34cec80af60acef68ff643c392e29d
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
|
|
This change moves getMessage and getMessageFromRegistry to a .cpp file
so that they can be easily tested.
Tested: Unit test passes
Signed-off-by: Sui Chen <suichen@google.com>
Change-Id: Ia9fc91e5a47036198bf013ff3ea21ea9f6d5259a
|
|
For systems implementing to the OWASP security guidelines[1] (of which all
should ideally) we should be checking the content-type header all times
that we parse a request as JSON.
This commit adds an option for parsing content-type, and sets a default
of "must get content-type". Ideally this would not be a breaking
change, but given the number of guides and scripts that omit the content
type, it seems worthwhile to add a trapdoor, such that people can opt
into their own model on how they would like to see this checking work.
Tested:
```
curl --insecure -H "Content-Type: application/json" -X POST -D headers.txt https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName":"root", "Password":"0penBmc"}'
```
Succeeds.
Removing Content-Type argument causes bmc to return
Base.1.13.0.UnrecognizedRequestBody.
[1] cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html
Change-Id: Iaa47dd563b40036ff2fc2cacb70d941fd8853038
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
boost::split has a documented false-positive in clang-tidy. While
normally we'd handle this with NOLINTNEXTLINE, this doesn't appear to
work in all cases. Unclear why, but seems to be due to some of our
lambda callback complexity.
Each of these uses is a case where we should be using a more specific
check, rather than split, but for the moment, this is the best we have.
Tested: clang-tidy passes.
[1] https://github.com/llvm/llvm-project/issues/40486
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I144c6610cb740287b7225e2be03b4142a64f9563
|
|
If you look at an OpenBMC build, every source file is compiled for every
source unit. This is pretty wasteful, but meson documents how to reuse
object files between link units. This has a couple benefits:
- Source files are only compiled once
- There are less likely to be unit-test specific binary differences (if,
for example, the unit tests used different compiler flags).
- Errors in a source file only appear once in the output, instead of 10
times.
On my 36 core Xeon, a normal build went from 2m38s down to 2m34s, which
is quite underwhelming, and less than you'd expect from a change this
large, but it turns out that if you have more parallelism than you have
compile units, building things twice doesn't appear to matter much.
With that said, if I simulate a smaller machine, using -J 8, the build
time with this commit remains at 2m38s, whereas the commit previous to
this takes 4m48s, so clearly there's some benefit here.
This regression appears to have been caused by
0ad63df8aa8ab60f74395794d8ffce64c82ee031 which caused us to build
multiple executables (one per unit test). This fixes the regression.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib77e8679b69e0f2242f65c20b129f2fb8e370edb
|
|
Clang warns on this enum no matter what we do. Rather than adding all
27 entries of the boost::beast::verb to this, lets just ignore the
warning.
Tested: Code compiles on clang without warnings
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iba5809d8f855abe7f040d81d6052eb385be8f5a1
|
|
c1d019a6056a2a0ef50e577b3139ab5a8dc49355 Sensor Optimization
Recently changed the way Ids were calculated in the sensor subsystem.
Unfortunately, it wasn't clear to the author that this would effect the
sensor override system, which relies on matching up a member ID with a
dbus path, and was broken by this change.
This commit breaks out the code to calculate the type and name from a
given URI segment into a helper method.
Tested: Inspection only. Very few systems support this feature. Code appears more correct than previously, which is known broken, so the lack of testing here seems reasonable.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9aa8099a947a36b5ce914bc07ae60f1ebf0d209b
|
|
The new boost URL now interops properly with std::string_view, which is
great, and cleans up a bunch of mediocre code to convert one to another.
It has also been pulled into boost-proper, so we no longer need a
boost-url dependency that's separate.
Unfortunately, boost url makes these improvements by changing
boost::string_view for boost::urls::const_string, which causes us to
have some compile errors on the missing type.
The bulk of these changes fall into a couple categories, and have to be
executed in one commit.
string() is replaced with buffer() on the url and url_view types
boost::string_view is replaced by std::string_view for many times, in
many cases removing a temporary that we had in the code previously.
Tested: Code compiles with boost 1.81.0 beta.
Redfish service validator passes.
Pretty good unit test coverage for URL-specific use cases.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8d3dc89b53d1cc390887fe53605d4867f75f76fd
|
|
There are a number of properties of Type "string (uri)" for which we
do not currently support adding prefixes. This patch adds support
for all existing URI properties which are missed by the existing
implementation.
This change will be needed by future patches which will expand
aggregation support to all top level collections defined by the
schema. Those collections that are not currently supported include
properties whose URIs should be fixed, but would be missed by the
existing implementation.
Tested:
New unit test passes.
URI properties are still handled correctly.
```shell
curl localhost/redfish/v1/Chassis/5B247A_<chassisID>
{
"@odata.id": "/redfish/v1/Chassis/5B247A_<chassisID>",
"@odata.type": "#Chassis.v1_16_0.Chassis",
"Actions": {
"#Chassis.Reset": {
"@Redfish.ActionInfo": "/redfish/v1/Chassis/5B247A_<chassisID>/ResetActionInfo",
"target": "/redfish/v1/Chassis/5B247A_<chassisID>/Actions/Chassis.Reset"
}
},
...
}
```
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I3b3e06ee3191564d266598f7bc9f1641e6fcb333
|
|
As is, the router designates routes for every possible boost verb, of
which there are 31. In bmcweb, we only make use of 6 of those verbs, so
that ends up being quite a bit of wasted space and cache non-locality.
This commit invents a new enum class for declaring a subset of boost
verbs that we support, and a mapping between bmcweb verbs and boost
verbs.
Then it walks through and updates the router to support converting one
to another.
Tested:
Unit Tested
Redfish Service Validator performed on future commit
Signed-off-by: Ed Tanous <edtanous@google.com>
Signed-off-by: Edward Lee <edwarddl@google.com>
Change-Id: I3c89e896c632a5d4134dbd08a30b313c12a60de6
|
|
This commit fixes a minor regression induced when we moved to using
submodule projects in meson. When including boost and boost-url
headers, some compilers take issue with them, so they need to be
included as -isystem, instead of -I. This also helps with running
clang-tidy within meson.
Tested:
meson buildlocal
ninja -C buildlocal
Compiles on a fresh clone.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic92ed1a1cc2c661b715c63a974e0db35c793f5f4
|
|
'''
WARNING: Project targets '>=0.57.0' but uses feature introduced in
0.59.0': fs.stem_file. meson.build:386:
'''
Warning is present on master, because we use features from 0.59, and
0.60. Update to a minimu meson version of 0.63, to match both yocto and
openbmc CI.
Tested:
meson build
no longer throws the above warning.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ifac119aa8c8e45859e7374e1c468ea0c91664c2d
|
|
This is to match yocto. subproject is already pointed at 1.80, but for
a while CI still had 1.79. But OpenBMC CI has long since been moved [1]
to 1.80, so upgrade the dependency requirements in turn.
[1] https://github.com/openbmc/openbmc-build-scripts/blob/05fb2a0ab2ff772679272047b0bf0608adaf41fa/scripts/build-unit-test-docker#L80
Tested: Code compiles. No functional changes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ife6d163660b734893f0793da0e642cd61730acec
|
|
As written, when we pull in the boost and boost-url subprojects, we hit
problems in the meson files.
```
../meson.build:291: WARNING: include_directories sandbox violation!
```
This commit resolves this issue, by adding explicit meson.build files
for both boost and boost-url.
Tested:
meson buildlocal
No longer returns the above error, and shows
Subprojects
boost : YES
boost-url : YES
Whereas previously those two dependencies showed up as NO.
Code compiles.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib25340723c8cb7d6139e3e51db023e9d90e30aab
|
|
This commit implements the Redfish PowerSubsystem schema and collects
default property values.
PowerSupplies will be implemented in the next commit.
ref:
https://www.dmtf.org/sites/default/files/standards/documents/
DSP0268_2022.2.pdf (6.86 PowerSubsystem 1.1.0)
https://redfish.dmtf.org/schemas/v1/PowerSupply.v1_1_0.json
Tested: Validator and UT passes
1. curl -k -H "X-Auth-Token: $token" -X GET
https://${bmc}/redfish/v1/Chassis/chassis/PowerSubsystem
{
"@odata.id": "/redfish/v1/Chassis/chassis/PowerSubsystem",
"@odata.type": "#PowerSubsystem.v1_1_0.PowerSubsystem",
"Id": "PowerSubsystem",
"Name": "Power Subsystem",
"Status": {
"Health": "OK",
"State": "Enabled"
}
}
2. bad chassisID
curl -k -H "X-Auth-Token: $token" -X GET https://${bmc}
/redfish/v1/Chassis/badchassisID/PowerSubsystem/
PowerSupplies/powersupply0
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The requested resource of type Chassis named
badchassisID was not found.",
"MessageArgs": [
"Chassis",
"badchassisID"
],
"MessageId": "Base.1.13.1.ResourceNotFound",
"MessageSeverity": "Critical",
"Resolution": "Provide a valid resource identifier
and resubmit the request."
}
],
"code": "Base.1.13.1.ResourceNotFound",
"message": "The requested resource of type Chassis named
badchassisID was not found."
}
}
Signed-off-by: Chicago Duan <duanzhijia01@inspur.com>
Change-Id: I6885b1777082538eceaf7ea85a8f69966459ee43
|
|
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/57648/8 is unfortunately
rejected by some CI issues where # lines covered by unit test is not
consistent.
Thus, this commit is written to bypass subdir, but still addes the
ability to run a test for a specific component. This speeds up iteration
when developers are working on a subset of the project.
For example, we can compile and run the query_param_test separately. The
speed up will be more obvious when we have better solution to deal with
the current headers and inline functions in the future.
```
meson test query_param_test -C b
ninja: Entering directory `/usr/local/google/home/nanzhou/Desktop/bmcweb/b'
ninja: no work to do.
1/1 query_param_test OK 0.01s
Ok: 1
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
```
The compile time increases a little bit. This doesn't matter too much
given tests are disabled in Yocto builds.
```
[hi on] nanzhou@nanzhou:~/Desktop/bmcweb$ time ninja test -C b
ninja: Entering directory `b'
[49/50] Running all tests.
1/1 bmcweb_unit_test OK 0.07s
Ok: 1
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
Full log written to /usr/local/google/home/nanzhou/Desktop/bmcweb/b/meson-logs/testlog.txt
real 1m56.361s
user 12m11.587s
sys 1m15.924s
[hi on] nanzhou@nanzhou:~/Desktop/bmcweb$ time ninja test -C b
ninja: Entering directory `b'
[247/248] Running all tests.
1/23 crow_getroutes_test OK 0.34s
2/23 router_test OK 0.31s
3/23 utility_test OK 0.29s
4/23 dbus_utility_test OK 0.28s
5/23 google_service_root_test OK 0.27s
6/23 http_utility_test OK 0.26s
7/23 human_sort_test OK 0.24s
8/23 multipart_test OK 0.21s
9/23 openbmc_dbus_rest_test OK 0.20s
10/23 privileges_test OK 0.18s
11/23 registries_test OK 0.17s
12/23 hex_utils_test OK 0.16s
13/23 ip_utils_test OK 0.15s
14/23 json_utils_test OK 0.15s
15/23 query_param_test OK 0.13s
16/23 stl_utils_test OK 0.12s
17/23 chassis_test OK 0.10s
18/23 service_root_test OK 0.04s
19/23 thermal_subsystem_test OK 0.03s
20/23 configfile_test OK 0.23s
21/23 lock_test OK 0.22s
22/23 time_utils_test OK 0.11s
23/23 log_services_dump_test OK 0.07s
Ok: 23
Expected Fail: 0
Fail: 0
Unexpected Pass: 0
Skipped: 0
Timeout: 0
Full log written to /usr/local/google/home/nanzhou/Desktop/bmcweb/b/meson-logs/testlog.txt
real 2m8.792s
user 29m15.844s
sys 3m10.264s
```
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I6f763173c1e7de96ab757673fb5ed0a73e4532f5
|
|
This pattern is used in other places, e.g.,
https://github.com/openbmc/phosphor-logging
In this way, we can cleanly add the bmcweb_config.h into include
directory. Otherwise, any subdir we have in this project will need to
include the root.
Tested: it builds.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I882e51f3acb256a881c9474f6e4d4e19fea4a413
|
|
Like other C++ projects, unit tests normally are in a separate repo and
respect the folder structure of the file under test.
This commit deleted all "ut" folder and move tests to a "test" folder.
The test folder also has similar structure as the main folder.
This commit also made neccessary include changes to make codes compile.
Unused tests are untouched.
Tested: unit test passed.
Reference:
[1] https://github.com/grpc/grpc/tree/master/test
[2] https://github.com/boostorg/core/tree/414dfb466878af427d33b36e6ccf84d21c0e081b/test
[3] Many other OpenBMC repos: https://github.com/openbmc/entity-manager/tree/master/test
[4] https://stackoverflow.com/questions/2360734/whats-a-good-directory-structure-for-larger-c-projects-using-makefile
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4521c7ef5fa03c47cca5c146d322bbb51365ee96
|
|
This patchset transitions System dump route handlers to the new style
of calling common dump handlers via std::bind_front() instead of
defining a lambda. BMC dump and FaultLog dump were previously
transitioned to the new style (see "LogService: Add support for Fault
Log" at https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53175).
Note that System dump wasn't using a common dump handler for outputting
its LogService info (/redfish/v1/Systems/system/LogServices/Dump/)
before, so calling the common handler here is new.
No expected client impact.
Tested:
Get System dump LogService info:
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/
(Diffed the ouput before and after applying this patchset and confirmed
they were equivalent except for the "DateTime", as expected. Also added
unit tests for getting dump LogService info.)
Create System dump entry. (Existing code for creating an entry via the
Redfish LogService action CollectDiagnosticData isn't currently working
for System dump, so instead directly call the corresponding D-Bus
method by running the following from the BMC console):
busctl call xyz.openbmc_project.Dump.Manager /xyz/openbmc_project/dump/system xyz.openbmc_project.Dump.Create CreateDump a{sv} 0
Get Entry:
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Entries/1
Get Entry Collection:
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Entries
Delete Entry:
curl -k -H "X-Auth-Token: $token" -X DELETE http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Entries/1
Clear Log:
curl -k -H "X-Auth-Token: $token" -X POST http://${bmc}/redfish/v1/Systems/system/LogServices/Dump/Actions/LogService.ClearLog
Redfish Service Validator succeeded on the following URI tree:
/redfish/v1/Systems/system/LogServices/Dump
Signed-off-by: Claire Weinan <cweinan@google.com>
Change-Id: I5d66da17794c29672be8713481018bf3ce397ddf
|
|
CI only has 1.79 present currently, and can't upgrade to 1.80 until some
sdbusplus issues (which i'm looking at) are sorted out.
Temporarily only require 1.79 to unblock CI.
Tested: CI only
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib81c1c9a6fb23f62a39fca6ce64edff75ceb050c
|
|
Pretty trivial move. No breaking changes between these two versions.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Icdd0c47cab42f1f6c420856d2dc3f685bce6bd8f
|
|
The newest yocto now gives warnings about TMPDIR being located as a
string in your binary. There is one OpenSSL_free call that seems to
print our source location.
Setting OPENSSL_NO_FILENAMES disables this.
Tested:
bitbake bmcweb no longer prints warning about TMPDIR
cat bmcweb | grep -a host_name_verification.ipp
No longer shows the debug string present.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I53bccdfdcd3846952c13863227728182d93dc50c
|
|
Yocto complains that some logging information is getting included in the
bmcweb binary that points to the yocto TMPDIR location. One of these
things is boost::url, which has a flag for disabling this behavior.
Enable the flag. The only downside to this is that we lose the per-file
information in our error messages, which in my obvservation, we don't
actually log, so the behavior doesn't change. To keep a reproducible
build, this seems reasonable, and in line with the behavior we want.
Note, there is still one file remaining, host_name_verification.ipp, so
the error is still present in builds, but this gets us closer.
Tested:
strings bmcweb | grep tmp
no longer contains references to the boost::url
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If5789613b0de2a55684d686aaf5857b73245e4bd
|
|
IBM doesn't use the Redfish OEM fan data in OemManager.
IBM does not use phosphor-pid-control instead using
phosphor-fan-presence and such.
This is data such as PidControllers, StepwiseControllers, FanZones,
FanControllers, and Profile.
This has been in bmcweb since Oct 2018 so defaulting this flag to
enabled to not break anyone.
Why we want a flag:
1) Have observed 500 errors with getting the thermalMode.
"Jan 24 16:34:57 rain534 bmcweb[435]: (2022-01-24 16:34:57) [ERROR
"managers.hpp":1196] GetPIDValues: Can't get thermalModeIface
/xyz/openbmc_project/control/thermal/0"
2) This Redfish OEM fan data includes PATCHing.
Commit turning this off in meta-ibm:
https://gerrit.openbmc.org/c/openbmc/openbmc/+/56327
Tested: With this flag enabled and disabled.
Manager resource looks as expected.
Before on a dummy PATCH to this:
curl -k -X PATCH https://$bmc/redfish/v1/Managers/bmc -d \
'{"Oem":{"OpenBmc":{"Fan":{"Profile":"Acoustic"}}}}'
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The request failed due to an internal service...
With this change and the meta-ibm change (instead see a PropertyUnknown)
curl -k -X PATCH https://$bmc/redfish/v1/Managers/bmc -d \
'{"Oem":{"OpenBmc":{"Fan":{"Profile" : "Acoustic"} }}}'
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The property %1 is not in the list of valid...
"MessageArgs": [
"Oem"
],
"MessageId": "Base.1.13.0.PropertyUnknown",
PATCHed the DateTime with this enabled.
Change-Id: I374292ca2798e096b18d49df5bbc7a93c7f1c400
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
The ThermalSubsystem is a new resource in Redfish version 2020.4.
It is a root for fans and temperatures. Fans are a new schema.
Temperature sensors will be part of the new ThermalMetrics schema.
ThermalSubsystem can co-exist with the current Thermal resource.
You can also control compilation through flags.
ThermalSubsystem is an improvement on the existing Thermal schema
because
1. It includes the latest properties like LocationIndicatorActive
2. Fans and Temperatures were arrays in the old Thermal schema and
this was cumbersome and could hit limits of JSON arrays
3. Large amount of static data mixed with sensor readings, which
hurt performance
4. Inconsistent definitions of properties vs like Processor and
Memory schemas
In a future commits Fans and ThermalMetrics will be added soon.
Reference:
https://www.dmtf.org/sites/default/files/standards/documents/DSP0268_2020.4.pdf
https://redfish.dmtf.org/schemas/v1/ThermalSubsystem.v1_0_0.json
Test:
1. Validator passed.
2. doGet method:
~$ curl -k -H "X-Auth-Token: $token" -X GET https://${bmc}/redfish/v1/Chassis/chassis/ThermalSubsystem
{
"@odata.id": "/redfish/v1/Chassis/chassis/ThermalSubsystem",
"@odata.type": "#ThermalSubsystem.v1_0_0.ThermalSubsystem",
"Id": "chassis",
"Name": "Thermal Subsystem for Chassis",
"Status": {
"Health": "OK",
"State": "Enabled"
}
}
3. A bad chassis ID:
~$ curl -k -H "X-Auth-Token: $token" -X GET https://${bmc}/redfish/v1/Chassis/chassisSSBAD/ThermalSubsystem
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The requested resource of type Chassis named chassisSSBAD was not found.",
"MessageArgs": [
"Chassis",
"chassisSSBAD"
],
"MessageId": "Base.1.8.1.ResourceNotFound",
"MessageSeverity": "Critical",
"Resolution": "Provide a valid resource identifier and resubmit the request."
}
],
"code": "Base.1.8.1.ResourceNotFound",
"message": "The requested resource of type Chassis named chassisSSBAD was not found."
}
}
Signed-off-by: Xiaochao Ma <maxiaochao@inspur.com>
Change-Id: Ib19879f584304e5303f1a83d88bdd18c78a61633
Signed-off-by: Zhenwei Chen <zhenweichen0207@gmail.com>
|
|
Today `basic-auth` (and other options) can be enabled even if
`insecure-disable-auth` is enabled, which doesn't make sense.
With this block this commit added in meson, If we disable authx with
`insecure-disable-auth`, then all these auth options will be ignored.
Tested:
1. code compiles with and without 'insecure-disable-auth'.
2. No new service validator errors when 'insecure-disable-auth' is
turned on.
3. No new service validator errors when 'insecure-disable-auth' is
turned off.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I2c634851f7aa7b9e57158770c5d40c12954c93a7
|
|
It has been a convention that request route functions take inline
functions instead of lambdas. The benifets include less indents,
beging more readable + unit test-able (take a look at the unit test that
this commit adds for example).
This commit also fixed neccessary headers to make the test compile. The
headers of the unit test source is a complete list. But headers of the
core codes are not complete. These header clean up will be done in a
separate effort once https://gerrit.openbmc.org/c/openbmc/bmcweb/+/55138
is submitted.
Tested:
1. no service validator errors on real hardware.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4b23ba54707cea947b5db771c72aa64899041511
|
|
Tested: unit test worked.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4feb6c9cdf52930617a7011732a5837f06c1adda
|
|
Currently, the |systemBus| connection is a static variable declared in
headers. This has a problem that every translation unit will keep its
own copy. It's not a problem today because there's only one translation
unit "webserver_main.cpp.o". This issue was brounght up in
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/54758
Actually, the |systemBus| doesn't need to be a singleton. It can just be
a stack variable, which is normally more efficient than heap variables.
To keep minimum changes treeside, this commits keeps the existing
|systemBus| variable as an external variable. It is defined in its own
translation unit. It is initialized in the main translation unit.
Reference:
1. Extern
https://stackoverflow.com/questions/1433204/how-do-i-use-extern-to-share-variables-between-source-files
Tested:
1. Romulus QEMU robot Redfish test passed;
2. Start and restart service on real hardware, no issues;
3. No new validator failures
4. Code compies
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I03b387bd5f218a86c9d1765415a46e3c2ad83ff9
|
|
Tested: unit test passed.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I6ec2036c09eb73c2ff34779ed055dff7769c2f57
|
|
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
|
|
The test today exists but it isn't enabled. This commit revives the
test, and fixed obsolete interfaces.
Note that the current codes don't return the "/" route correctly. This
commit doesn't fix it but left a TODO.
Tested: unit test passed
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ie5be7f545f1930ddb2c01b829d8de2e312e936dc
|
|
This commit does nothing but moving test codes from
openbmc_jtag_rest_test.cc, a very old test file whose name is obsolote
now, to a more recent and well maintained unit test file
(openbmc_dbus_rest_test.cc).
Tested: unit test passed.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I3709d18c8ef5cbba5b3f6490a1e9d1798dfc8b52
|
|
The test today exists but it isn't enabled. This commit revives the
test, and fixed obsolete interfaces.
This commit also fixes the test case "i{si}b", which should be split
into {"i", "{si}", "b"}. Existing values might be typos.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I962c349c237d59be89337af5df88d3ee6f625f13
|
|
The "auth" term is overloaded in meson option and macros. This commit
changes the macro from BMCWEB_INSECURE_DISABLE_AUTHENTICATION to
BMCWEB_INSECURE_DISABLE_AUTHX, given that if "insecure-disable-auth"
is enabled, both authentication and authorization are disabled.
Tested:
1. set 'insecure-disable-auth=enabled', no authz nor authn is performed,
no crash on AccountService as well.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Iddca1f866d16346bcc2017338fa6f077cb89cef9
|
|
This gcc warning is just a warning, and not a problem in bmc usages as
we don't rely on abi. Having it in clogs the gcc logs when triaging
other things.
Tested:
Compiled with another compiler error present. Didn't see "This behavior
changed in gcc 7.1" warnings.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I2bd1ec8a774fdce15557d6344a03f4321df6d95a
|
|
Adds a RedfishAggregator class which is able to pull configuration
information from D-Bus for Satellite BMCs. These BMCs will be
aggregated by Redfish Aggregation. Also added is a new compiler
option which will be used to enable Redfish Aggregation.
This patch only allows configurations with unencrypted and
unauthenticated satellite BMC communication. Support for encryption
and authentication willneed to be added in future patches.
Note that this patch does not actually use the config information
after it has been fetched. That functionality will be added in
future patches.
Tested:
I made this example config information available on D-Bus
busctl introspect xyz.openbmc_project.EntityManager \
/xyz/openbmc_project/inventory/system/board/SatelliteBMC/aggregated0 \
xyz.openbmc_project.Configuration.SatelliteController
NAME TYPE SIGNATURE RESULT/VALUE FLAGS
.AuthType property s "None" emits-change
.Hostname property s "127.0.0.1" emits-change
.Name property s "aggregated0" emits-change
.Port property t 443 emits-change
.Type property s "SatelliteController" emits-change
That information was picked up by the changes in this CL:
[DEBUG "redfish_aggregator.hpp":80] Found Satellite Controller at /xyz/openbmc_project/inventory/system/board/SatelliteBMC/aggregated0
[DEBUG "redfish_aggregator.hpp":209] Added satellite config aggregated0 at http://127.0.0.1:443
[DEBUG "redfish_aggregator.hpp":52] Redfish Aggregation enabled with 1 satellite BMCs
[DEBUG "redfish_aggregator.hpp":21] There were 1 satellite configs found at startup
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ib5eee2c93aeb209157191055975c127759d73627
|
|
Tinyxml2 needs to be included in all cases now. The current Meson build
will fail unless tinyxml2 is installed locally. given the inclusion of
tinyxml2.xml in dbus_monitor.hpp. In the past, the 'rest' option was
enabled by default, so this bug wasn't hit very often in practice. Now
that rest is disabled, this bug is much more apparent.
Tested: Code compiles
Signed-off-by: Cody Smith <scody@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I258356151ae0226c7b08a80ef78b043bc90731bc
|
|
The Redfish specification calls out that the Allow header should be
returned for all resources to give a client an indication of what
actions are allowed on that resource. The router internally has all
this data, so this patchset allows the router to construct an allow
header value, as well as return early on a HEAD request.
This was reverted once here:
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/53637
Due to a redfish validator failure. With the previous patches
workaround, this error has now been resolved.
Tested:
Called curl with various parameters and observed the Allow header
curl -vvvv --insecure -X <VERB> --user root:0penBmc https://<bmc>/url
HEAD /redfish/v1/SessionService/Sessions returned Allow: GET, POST
HEAD /redfish/v1 returned Allow: GET
HEAD /redfish/v1/SessionService returned Allow: GET, PATCH
POST /redfish/v1 returned Allow: GET (method not allowed)
GET /redfish/v1 returned Allow: GET
GET /redfish/v1/SessionService returned Allow: GET, PATCH
Redfish-Protocol-Validator now reports more tests passing.
Prior to this patch:
Pass: 255, Warning: 0, Fail: 27, Not tested: 45
After this patch:
Pass: 262, Warning: 0, Fail: 21, Not tested: 43
Diff: 7 more tests passing
All tests under RESP_HEADERS_ALLOW_METHOD_NOT_ALLOWED and
RESP_HEADERS_ALLOW_GET_OR_HEAD are now passing
Included unit tests passing.
Redfish service validator is now passing.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ibd52a7c2babe19020a0e27fa1ac79a9d33463f25
|
|
As d01e32c3786f2fbbb70c9724a87cf979b4a06232 found, the Redfish
specification doesn't allow a direct POST handler on UpdateService.
Ideally clients would be following the specification, and relying on
the HttpPushUri as the spec requires, so we could simply make this
change. Unfortunately, a quick polling of the community shows that a
significant number of instances, including the Redfish cheat sheet, and
the robot tests, have hardcoded the non-spec behavior. This commit is
present to give a trap door to allow easier porting of this behavior to
the specification.
The old uri is left, and now returns a WARNING http field, indicating
that the uri is deprecated, in case clients have ignored the Redfish
specification.
Tested:
Ran firmware update instructions from
https://gerrit.openbmc-project.xyz/c/openbmc/docs/+/53664
Test gave the same result as previously.
/redfish/v1/UpdateService returns an HttpPushUri that matches the above.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7427f461d151c9460160b0b9b366dca5aefc49d5
|
|
This will generalize it and make it callable from other places
Tested: Added test cases, they pass
Signed-off-by: Josh Lehan <krellan@google.com>
Change-Id: I8df30d6fe6753a2454d7051cc2d8813ddbf14bad
|
|
This reverts commit 867b2056d44300db9769e0d0b8883435a179834c.
Apparently we have broken the Redfish spec in a way that adding this
feature now allows the service validator to find.
@odata.id /redfish/v1/UpdateService
ERROR - Allow header should NOT contain POST for UpdateService.v1_5_0.UpdateService
Need to figure out what to do, but for now, revert to get the build
passing again.
Change-Id: Ieef20573b9caa03aba6fd2bbc999e517e4b7de3d
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
The Redfish specification calls out that the Allow header should be
returned for all resources to give a client an indication of what
actions are allowed on that resource. The router internally has all
this data, so this patchset allows the router to construct an allow
header value, as well as return early on a HEAD request.
Tested:
Called curl with various parameters and observed the Allow header
curl -vvvv --insecure -X <VERB> --user root:0penBmc https://<bmc>/url
HEAD /redfish/v1/SessionService/Sessions returned Allow: GET, POST
HEAD /redfish/v1 returned Allow: GET
HEAD /redfish/v1/SessionService returned Allow: GET, PATCH
POST /redfish/v1 returned Allow: GET (method not allowed)
GET /redfish/v1 returned Allow: GET
GET /redfish/v1/SessionService returned Allow: GET, PATCH
Redfish-Protocol-Validator now reports more tests passing.
Prior to this patch:
Pass: 255, Warning: 0, Fail: 27, Not tested: 45
After this patch:
Pass: 262, Warning: 0, Fail: 21, Not tested: 43
Diff: 7 more tests passing
All tests under RESP_HEADERS_ALLOW_METHOD_NOT_ALLOWED and
RESP_HEADERS_ALLOW_GET_OR_HEAD are now passing
Included unit tests passing.
Change-Id: Ib99835050b15eb4f419bfd21375b26e4db74fa2c
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
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
|
|
Section 7.3 of the Redfish specification lays out a feature called
"expand" that allows users to expand portions of the Redfish tree
automatically on the server side. This commit implements them to the
specification.
To accomplish this, a new class, MultiAsyncResp is created, that allows
RAII objects to handle lifetime properly. When an expand query is
generated, a MultiAsyncResp object is instantiated, which allows "new"
requests to attach themselves to the multi object, and keep the request
alive until they all complete. This also allows requests to be created,
while requests are in flight, which is required for queries above
depth=1.
Negatives:
Similar to the previous $only commit, this requires that all nodes
redfish nodes now capture App by reference. This is common, but does
interfere with some of our other patterns, and attempts to improve the
syntactic sugar for this proved unworkable.
This commit only adds the above to service root and Computer systems, in
hopes that we find a better syntax before this merges.
Left to future patches in series:
Merging the error json structures in responses.
The Redfish spec isn't very clear on how errors propagate for expanded
queries, and in a conforming we shouldn't ever hit them, but
nonetheless, I suspect the behavior we have is sub-optimal (attaching an
error node to every place in the tree that had an issue) and we should
attempt to do better in the future.
Tested (on previous patch):
curl --insecure --user root:0penBmc https://localhost:18080/redfish/v1\?\$expand\=.\(\$levels\=255\)
Returns the full tree
Setting $levels=1 query returns only a depth of 1 tree being returned.
Unit tests passing
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I874aabfaa9df5dbf832a80ec62ae65369284791d
|
|
Query parameters in their initial incarnation will likely have security
consequences. For example, requesting ServiceRoot with expand depth 999
would likely run most BMCs out of memory. This isn't a good reason to
keep those features out of master, as there are a number of services
(webui-vue for example) that would like to test against them, and
identify the weaknesses.
The goal with this option is to allow users to test, so we can determine
things like the max depth we should support, which query params have
security consequences and how to mitigate them, and other testing. The
end goal would be for this option to be enabled by default. If it's
removed entirely would depend on the impacts of supporting query params
and is something we will have to discuss at a later date.
Tested:
Code compiles. Use of this option is added in next patchset in series.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I93ff31c938e4be2d92eb07b59a3288f8bacde2ac
|
|
There should be no case where we're throwing an exception through an
asio descriptor or through the io_context, so having these options
enabled don't do us much good.
Tested:
Code compiles, removes almost 7kB from the bmcweb binary size.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I91df03fd0dcb942bdfaa420ad8c0c1f8b0f634cc
|