| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Add SystemType to hypervisor page. Set Systemtype to 'OS'
Redfish Schema definition of 'OS' [1]
"OS": "A SystemType of OS typically represents an OS or hypervisor
view of the system.",
[1] https://redfish.dmtf.org/schemas/v1/ComputerSystem.v1_15_0.json
Testing:
1) Passed Redfish validator
2) Curl testing
curl -k -H "X-Auth-Token: $token" https://$bmc/redfish/v1/Systems/hypervisor
{
"@odata.id": "/redfish/v1/Systems/hypervisor",
"@odata.type": "#ComputerSystem.v1_6_0.ComputerSystem",
...
"SystemType": "OS"
}
Signed-off-by: Ali Ahmed <ama213000@gmail.com>
Change-Id: If2a68e592303f1d862864fc06b6d33c146cb3450
|
|
Currently Patch on ipv4addresses overrides values because of
missing return statement.
Tested By:
PATCH -d '{"IPv4Addresses": [null]}' https://${bmc_ip}/redfish/v1/Systems/hypervisor/EthernetInterfaces/eth0
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
Change-Id: I1839572e635f137a93c777468a6cc6cc18059f4e
|
|
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
|
|
Fixes #181
Lots of specific details around why the node class have been removed are
in the previous patchsets. This commit actually does the deed and makes
it go away entirely.
Now that this is finally done, we can compare binary size. Surprisingly
enough, this series saves a full 72KB of compressed binary size, which
amounts to about 6.4% of the total code size.
Before: 1197632 bytes
After: 1124688 bytes
This IMO makes it worth it, considering we've significantly reduced the
amount of code at the same time.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I3c8688715f933b381cad0be75a079ccfd72c3130
|
|
Pursuant to the other patchsets further up in this chain, continue to
move things to the simpler BMCWEB_ROUTE mechanism. This moves the
Ethernet modules up the chain.
Tested:
Ed Tested on redfish service validator, no new errors (unrelated UUID
error present)
Gunnar Tested Series on validator and GUI.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id1937e0d3f525d58744e6ff6360ef23f66350c66
|
|
The /redfish/v1/systems/hypervisor endpoints seem to have copy/pasted a
lot of code from ethernet.hpp, including all the function naming. This
is causing naming conflicts as part of removing the Node class. For the
moment, just put these methods into the redfish::hypervisor namespace,
along with a comment, so we can at least get code that builds. At some
point in the future we can deduplicate the duplicated code, and give the
unique things unique method names so we don't have collisions and this
can be undone.
Tested:
Ran redfish service validator on 42a9e0ee96ef1928732ffd8d567ad656a4f41887
No change in behavior, one failure on Manager UUID that appears
unrelated.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I0d715ed3bf04a86a93eb7842804319568083f86d
|
|
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 fixes bmcweb crash while patching
"IPv4StaticAddresses" property with below values
'{"IPv4StaticAddresses": null}'
'{"IPv4StaticAddresses": []}'
Tested By:
GET
PATCH
'{"IPv4StaticAddresses": null}' returned a 400
'{"IPv4StaticAddresses": []}' returned a 400
'{"IPv4StaticAddresses": [null]}' returned a 200 and deleted the entry
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
Change-Id: Ia310818c87fc1a425d32dd3648c2cbdd6fe5f526
|
|
If the customer has requested the hypervisor stop at its Standby state
vs. booting all the way to Running, then a mechanism is needed to
request the hypervisor boot to Running. A common use case for IBM is
the system user has requested the hypervisor stop at Standby so some
manual debug can be performed and then they want to move the hypervisor
into the Running state so they can boot their operating system.
Asking the hypervisor to stop at Standby is done via the
BootSourceOverrideEnabled and BootSourceOverrideTarget properties under
the redfish/v1/Systems/system.
Utilize the ComputerSystem.Reset action for this. Similar to how
redfish/v1/Systems/system/Actions/ComputerSystem.Reset is utilized for
the overall system, implement a limited subset of it for the hypervisor
object.
Tested:
- Verified when phosphor-hypervisor-state-manager package is not
installed that Redfish API returns same info it does currently
- Verified when phosphor-hypervisor-state-manager was installed that the
hypervisor state was returned correctly, the Actions field was filled
in, and a post to the Action with ResetType set to "On" was correctly
propagated to RequestedHostTransition
- Verified that an invalid ResetType (i.e. "Off") returned the
appropriate "invalid parameter" error message to the Redfish API
- Verified no new errors logged by Redfish validator on system with this
hypervisor package installed
- Verify resource not found when hypervisor not enabled
curl -k -H "X-Auth-Token: $TOKEN" -X POST https://${BMC_IP}/redfish/v1/Systems/hypervisor/Actions/ComputerSystem.Reset -d '{"ResetType": "On"}'
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The requested resource of type Actions named Reset was not found.",
"MessageArgs": [
"Actions",
"Reset"
],
"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 Actions named Reset was not found."
}
}
- Verify ResourceNotFound returned when hypervisor not enabled
curl -k -H "X-Auth-Token: $TOKEN" -X GET https://${BMC_IP}/redfish/v1/Systems/hypervisor/ResetActionInfo
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The requested resource of type hypervisor named ResetActionInfo was not found.",
"MessageArgs": [
"hypervisor",
"ResetActionInfo"
],
"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 hypervisor named ResetActionInfo was not found."
}
}
- Verify input parameters validated
curl -k -H "X-Auth-Token: $TOKEN" -X POST https://${BMC_IP}/redfish/v1/Systems/hypervisor/Actions/ComputerSystem.Reset -d '{"ResetTypeInvalid": "On"}'
{
"ResetTypeInvalid@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The property ResetTypeInvalid is not in the list of valid properties for the resource.",
"MessageArgs": [
"ResetTypeInvalid"
],
"MessageId": "Base.1.8.1.PropertyUnknown",
"MessageSeverity": "Warning",
"Resolution": "Remove the unknown property from the request body and resubmit the request if the operation failed."
}
]
}
curl -k -H "X-Auth-Token: $TOKEN" -X POST https://${BMC_IP}/redfish/v1/Systems/hypervisor/Actions/ComputerSystem.Reset -d '{"ResetType": "OnInvalid"}'
{
"ResetType@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The value OnInvalid for the property ResetType is not in the list of acceptable values.",
"MessageArgs": [
"OnInvalid",
"ResetType"
],
"MessageId": "Base.1.8.1.PropertyValueNotInList",
"MessageSeverity": "Warning",
"Resolution": "Choose a value from the enumeration list that the implementation can support and resubmit the request if the operation failed."
}
]
}
Change-Id: Ia7b4e78b7b0d907cc06eb3f20d51ff87b7dde564
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
|
|
phosphor-state-manager support a new optional package,
phosphor-state-manager-hypervisor. IBM plans to include this package on
their system to monitor and control the hypervisor firmware running on
the system.
Since this package is optional, this patch set is written to just ignore
any errors associated with the package and not report hypervior state
in these cases.
Tested:
- Verified when phosphor-hypervisor-state-manager package is not
installed that Redfish API returns same info it does currently
- Verified when phosphor-hypervisor-state-manager was installed that the
hypervisor state was returned correctly.
- The redfish validator was run on the final patch in this series
Change-Id: I3843914894ded9494f92b96714c1f88a5deb5ec3
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
|
|
The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases. http connection already does this
properly, but a bunch of code has snuck in (mostly in redfish) that
ignores this, and calls it incorrectly. This can potentially lead to a
crash if the wrong thing throws on invalid UTF8 characters.
This audits the whole codebase, and replaces every dump() call with the
correct dump(2, ' ', true, nlohmann::json::error_handler_t::replace)
call. For correct output, the callers should expect no change, and in
practice, this would require injecting non-utf8 characters into the
BMC.
Tested:
Ran several of the endpoints/error conditions in question, including
some of the error cases. Observed correct responses. I don't know of a
security issue that would allow injecting invalid utf8 into the BMC, but
in theory if it were possible, this would prevent a crash.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4a15b8e260e3db129bc20484ade4ed5449f75ad0
|
|
Lots of code gets checked in that does this path checking incorrectly.
So much so, that we have it documented in COMMON_ERRORS.md, yet, we
persist. This patchset starts using the new object_path::filename()
method that was added recently to sdbusplus. Overall, it deletes code,
and makes for a much better developer experience.
Tested:
Pulled down several endpoints and verified that filename() method works
properly, and the collections are returned as expected.
curl -vvvv --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/AccountService/Accounts
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ief1e0584394fb139678d3453265f7011bc931f3c
|
|
Some new features are needed within the hypervisor system schema. Rename
source file to reflect this.
Change-Id: I7fa09089a4f52610b47bbb4496064556ab93f985
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
|
