| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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
|
|
In the way we store the message registry, we store both Severity and
MessageSeverity. Severity as a field is deprecated, and in every case
in every registry both fields have the same value. We shouldn't
duplicate data in that way. This commit changes the parse_registries.py
script to stop producing the Severity field into the struct. The few
uses we have left are moved over to use MessageRegistry.
Tested:
Redfish service validator shows no errors on the
/redfish/v1/Registries tree. Other errors present that were there
previously and are unchanged.
This saves a trivial amount: about 1kB on our compressed binary size.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ibbaf533dc59eb08365d6ed309aba16b54bc40ca1
|
|
The message_registries namespace is overly wordy, and results in very
long defines. Doing this one minor change reduces the code by 50 lines.
This seems worthwhile.
Tested: Unit tests pass. Namespace change only.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib1401580b3fa47596eb56cdc86e60eeeb1c2f952
|
|
We don't have too many violations here, probably because we don't have
many optional parameters. Fix the existing instances, and enable the
check.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4d512f0ec90b060fb60a42fe3cd6ba72fb6c6bcb
|
|
clang-tidy added cppcoreguidelines-init-variables as a check, which is
something we already enforce to some extent, but getting CI to enforce
it will help reviews move faster.
Tested: Code compiles. Noop changes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7e10950de617b1d3262265572b1703f2e60b69d0
|
|
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
|
|
Change the name of registry1 to a more descriptive variable name
registryMatch
Tested: none
Change-Id: I217d576e80822c81fae2cd7fdb22e2b8eebf5cd8
Signed-off-by: John Edward Broadbent <jebr@google.com>
|
|
This change will allow for unit testing of the free function.
There are no changes to logic in the response, only the organization.
Signed-off-by: John Edward Broadbent <jebr@google.com>
Change-Id: Ic485cea978b5b0ee06fe4011cbfc43333519bb50
|
|
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
|
|
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
|
|
The latest version of the registries causes a validator failure:
"ERROR - Messages.UndeterminedFault.ParamTypes: Value of Collection
property is null but Collections cannot be null, only their entries"
This appears to be because of a base registry bug where
UndeterminedFault has "NumberOfArgs": 1, but then is missing the
ParamTypes key. This causes bmcweb to produce
ParamTypes: null
in the json, which is definitely incorrect.
This commit throws some duct tape over the problem for the moment, and
forces the key to at least be the correct type. This changes the
response to:
ParamTypes: []
While this is still incorrect, bmcweb now passes the service validator,
which I think is ok for the moment. I will follow up with a DMTF bug
against the base registry shortly, but because it will likely take them
several months to release a new revision, this temporary fix will be
required in the meantime.
Tested:
Ran service validator before and after this patch, and observed the
aformentioned behavior, and the validator now passes on the base
Registry schema.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7d007a0614e833b0a254c78cad068d3eb5fe88e3
|
|
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
|
|
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
|
|
This commit enables the "unused variables" warning in clang. Throughout
this, it did point out several issues that would've been functional
bugs, so I think it was worthwhile. It also cleaned up several unused
variable from old constructs that no longer exist.
Tested:
Built with clang. Code no longer emits warnings.
Downloaded bmcweb to system and pulled up the webui, observed webui
loads and logs in properly.
Change-Id: I51505f4222cc147d6f2b87b14d7e2ac4a74cafa8
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Middlewares, while kinda cool from an academic standpoint, make our
build times even worse than they already are. Given that we only really
use 1 real middleware today (token auth) and it needs to move into the
parser mode anyway (for security limiting buffer sizes), we might as well
use this as an opportunity to delete some code.
Some other things that happen:
1. Persistent data now moves out of the crow namespace
2. App is no longer a template
3. All request_routes implementations no longer become templates. This
should be a decent (unmeasured) win on compile times.
This commit was part of a commit previously called "various cleanups".
This separates ONLY the middleware deletion part of that.
Note, this also deletes about 400 lines of hard to understand code.
Change-Id: I4c19e25491a153a2aa2e4ef46fc797bcb5b3581a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This commit adds new files for DMTF resource registry
and corresponding json message definitions.
Tested by:
1. GET https://${bmc}/redfish/v1/Registries
2. GET https://${bmc}/redfish/v1/Registries/ResourceEvent
3. GET https://${bmc}/redfish/v1/Registries/ResourceEvent/ResourceEvent
4. Redfish Validator passed ( giving out messages as == Severity:
The given property is deprecated by revision: This property has been deprecated
in favor of MessageSeverity, which ties the values to the enumerations defined
for the Health property within Status.)
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: Iacbedaeca85c0dcfc955bcf9b10973c0d47c98e0
|
|
This implements the MessageSeverity property which tools and users
should use instead of the deprecated Severity property.
Since the registries use common infrastructure, easiest if just
bumped together and now allows grabbing the latest when implementing
a new registry, e.g. ResourceEvent.
Implement this new required property, MessageSeverity, in the openbmc
registry. Follow Redfish registries in having both MessageSeverity
and Severity.
Modified parse_registries.py to look at latest Base and
TaskEvent registries and ran parse_registries.py.
Tested: Built and validator passes. See new registries:
curl -k https://$bmc/redfish/v1/Registries/Base
{
"@odata.id": "/redfish/v1/Registries/Base",
"@odata.type": "#MessageRegistryFile.v1_1_0.MessageRegistryFile",
"Description": "DMTF Base Message Registry File Location",
"Id": "Base",
"Languages": [
"en"
],
"Languages@odata.count": 1,
"Location": [
{
"Language": "en",
"PublicationUri": "https://redfish.dmtf.org/registries/Base.1.8.1.json",
"Uri": "/redfish/v1/Registries/Base/Base"
}
],
...
curl -k https://$bmc/redfish/v1/Registries/Base/Base
{
"@Redfish.Copyright": "Copyright 2014-2020 DMTF. All rights reserved.",
"@odata.type": "#MessageRegistry.v1_4_0.MessageRegistry",
"Description": "This registry defines the base messages for Redfish",
"Id": "Base.1.8.1",
"Language": "en",
"Messages": {
"AccessDenied": {
"Description": "Indicates that while attempting to access, connect to or transfer to/from another resource, the service denied access.",
"Message": "While attempting to establish a connection to %1, the service denied access.",
"MessageSeverity": "Critical",
"NumberOfArgs": 1,
"ParamTypes": [
"string"
],
"Resolution": "Attempt to ensure that the URI is correct and that the service has the appropriate credentials.",
"
...
Change-Id: I6495af0e02036ea527036d942d6b6b5f55178bb2
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This is from openbmc/docs/style/cpp/.clang-format
Other OpenBMC repos are doing the same.
Tested: Built and validator passed.
Change-Id: Ief26c755c9ce012823e16a506342b0547a53517a
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This updates the parse registries script and
adds the task registry to be used by task service.
This templates the original Base Registry so it
can be reused for all registries.
Tested: script works, validator passes
Change-Id: Id1cf3a41fb76ccaadace114725480f410bfba3e8
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
|
Redfish made odata.context optional (1.6.0 of DSP0266) and
has removed odata.context from example payloads in the
specification (1.7.0 of DSP0266), removed it from the mockups,
and Redfish recommended not using.
This removes the last odata.type.
Tested: Built this commit and the commits below and loaded on a
Witherspoon. No validator errors.
Change-Id: I4cff07b9196f7e9461ea49bb8bcc706e3945e033
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
using the list of warnings from here:
https://github.com/lefticus/cppbestpractices/blob/e73393f25a85f83fed7399d8b65cb117d00b2231/02-Use_the_Tools_Available.md#L100
Seems like a good place to start, and would improve things a bit
type-wise. This patchset attempts to correct all the issues in one
shot.
Tested:
It builds. Will test various subsystems that have been touched
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I588c26440e5a97f718a0f0ea74cc84107d53aa1e
|
|
A recent commit introduced Service Validator regressions in the
Registries that we use. This was largely because the commits were
structured in a way that was hard to review.
This commit Resolves that, and structures the data memebers in a more
readable way.
Tested:
ran service validator, observed no failures on Registries.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I7e15899187e333f843e5571ac9908b22624c16e9
|
|
This change removes the static OpenBMC Message Registry file and
replaces it with a compile-time structure.
Tested:
Verified the OpenBMC Message registry is correctly returned from
the existing endpoints without using the static files.
Change-Id: I60ab3ce0d23c9ac7e91ebb85f445fb9ca731983d
Signed-off-by: Jason M. Bills <jason.m.bills@linux.intel.com>
|
|
This extends the Message Registry parsing to include header
details that are part of the redfish resource.
Tested:
Verified that the MessageRegistry info all returns correctly
in the redfish response.
Change-Id: I6179c07f4067cd4520fce3e774d18530fede0a95
Signed-off-by: Jason M. Bills <jason.m.bills@linux.intel.com>
|
|
This change removes the static Base Message Registry file and
replaces it with a compile-time structure.
A script is used to pull the Base Message Registry file from
the DMTF and parse it into the .hpp structure.
Tested:
Verified that after running the script, I can get the same
Redfish data back from the existing endpoints without using
the static files.
Change-Id: Ide3c61ecff62801c06619d5c3edc2229c945d8e7
Signed-off-by: Jason M. Bills <jason.m.bills@linux.intel.com>
|
