| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The most outstanding concerns for $select query have been resolved. We
added a set of restrictions: character set, property length, # of
properties, which makes this feature safe to use.
This commit takes $select out of the insecure flag, so every system can
start to use it. This decision has been made in Discord, available at
[1] https://discord.com/channels/775381525260664832/994314752102760559/1006650821569675355
Tested:
1. unit test passed
2. no new service validator failure on hardware
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I1f669cd35afcc1a65473a3ed665768e172a423bc
|
|
According to Redfish Base Message Registry definition [1], the first
argument of ResourceNotFound and ResourceAlreadyExists is the schema
name of the resource. This patch changes the first argument to non-
versioned schema name treewide.
Tested:
Verified the error message matches the definition, and Redfish Service
Validator passed.
[1] https://redfish.dmtf.org/registries/Base.1.13.0.json
Change-Id: Ib5cd853578ef0bffda1184d10827241e94faaf68
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
|
This commit optimizes the release lock code and adds some traces to give
more data for lock conflict scenarios
Tested by:
1. With dual client connected, verified the conflicts are returned
2. Tested releaseLock usecase
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: I3cf99aaa5cc7c2967ae8dbc9c76c9f7378ecebdd
|
|
This commit fixes the below issues
1. Bump up the ConfigFile directory max limit
For large configurations on the system, the current directory size
upper limit of 10MB was exceeding and BMC was sending the error back
to the client. This fails the entire large config support.
This commit Increases this upper limit of the configFile dir to 25MB
2. Return 409 Error for a lock conflict
Tested by:
1. ConfigFile read
2. Single file upload
3. AcquireLock from the same client returns 409
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: I9218e8263f31e519d76683822290dfe259c57192
|
|
Dbus calls to dbus objects 'xyz.openbmc_project.Inventory.Item' and
and 'xyz.openbmc_project.State.Decorator.OperationalStatus' are nested
under a call for 'xyz.openbmc_project.Inventory.Item.Cpu' properties.
This change seperates these calls for clarity and to minimize variables
Testing:
1. Redfish Validator Testing passed
2. Curl testing:
curl -k -H "X-Auth-Token: $tok" https://$bmc/redfish/v1/Systems/system
...
"ProcessorSummary": {
"CoreCount": 0,
"Count": 2,
"Model": "",
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "Enabled"
}
},
...
Signed-off-by: Ali Ahmed <ama213000@gmail.com>
Change-Id: Id4657836607a2261a188db8d565aaa2b1a414c5a
Signed-off-by: Shantappa Teekappanavar <shantappa.teekappanavar@ibm.com>
|
|
It looks like one of the patchset merge conflicts got rebased poorly (ie
commented out). In the meantime, a number of these structures have gone
from map->vector, so modify the algorithm to account for that.
Tested:
1. Redfish validator - passed with this change.
2. Verified from Redfish by patching "StepwiseControllers" sensors.
Signed-off-by: Ed Tanous <edtanous@google.com>
Signed-off-by: Jayaprakash Mutyala <mutyalax.jayaprakash@intel.com>
Change-Id: I89d36f1b0b3699b6bf07a17af029e5a2504c85de
|
|
The property known as "PredictedMediaLifeLeftPercent" was not
implemented in bmcweb. This change adds that property to bmcweb, and
exposes the value of PredictedMediaLifeLeftPercent to redfish clients.
More information about the interface can be found at the link below, or
in yaml/xyz/openbmc_project/Inventory/Item/Drive.interface.yaml.
https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/47657/10/yaml/xyz/openbmc_project/Inventory/Item/Drive.interface.yaml#31
Tested:
Redfish Validator did not show any errors from this change.
$ wget -qO- http://localhost:80/redfish/v1/Chassis/DC_SCM/Drives/mmcblk0
{
"@odata.context": "/redfish/v1/$metadata#Drive.Drive",
"@odata.id": "/redfish/v1/Chassis/Drives/mmcblk0",
"@odata.type": "#Drive.v1_7_0.Drive",
"CapacityBytes": 15634268160,
"EncryptionStatus": "Unencrypted",
"Id": "mmcblk0",
"Links": {
"Chassis": "Enabled"
},
"Name": "mmcblk0",
"PredictedMediaLifeLeftPercent": 100,
"Status": {
"State": "Enabled"
}
}
Signed-off-by: John Edward Broadbent <jebr@google.com>
Change-Id: I360ce393707ba7a876810fc80f8a2d6fb484759c
|
|
used sdbusplus::unpackPropertiesNoThrow in openbmc_dbus_rest.hpp,
memory.hpp and sensors.hpp, also replaced all usages of "GetAll" with
sdbusplus::asio::getAllProperties
bmcweb size: 2697624 -> 2697624 (0)
compressed size: 1129645 -> 1130037 (+392)
Tested:
Performed get on:
- /redfish/v1/Systems/system/Memory/dimm0
Performed get one of the members of:
- /redfish/v1/Chassis/chassis/Sensors
Get result before and after the change was in same format.
Change-Id: I05efcedfd905ea2c8d1d663e909cb59ebc2cf2b7
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
|
|
This is a minor reorganization of helper code from ethernet (which is
redfish specific) into utils. This function is generic, and should be
in ip utils.
Tested: Code compiles. Code move only.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4aac5a98379c9844aea6c21d2294d1ed7ae2ea9b
|
|
Redfish has specific error messages for OperationNotSupported in the
Base registry. This commit allows bmcweb to return both the correct
return code (405) and the correct error message, while not effecting the
rest of the tree.
We didn't have the equivalent call in error_messages, so this adds the
required call.
Tested:
GET /redfish/v1 returns ServiceRoot
GET /redfish/v1/foo Returns 404
PATCH /redfish/v1 returns 405 OperationNotSupported
POST /redfish/v1/Chassis returns 405 OperationNotSupported
DELETE /redfish/v1/Chassis returns 405 ResourceCannotBeDeleted
POST /redfish/v1/foo/bar Returns 404
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I6f980af7307af602344b65a12a2b7589cc9ec959
Signed-off-by: Carson Labrado <clabrado@google.com>
|
|
Similar to the 404 handler, add a 405 handler for registering custom 405
handlers for a given tree. The primary use case is for protocols like
redfish that support specific messages for 405 handlers that don't have
an empty body.
Tested: Unit tests pass.
PATCH /redfish/v1 returns 405 Method Not Allowed
POST /redfish/v1/Chassis returns 405 Method Not Allowed
POST /redfish/v1/Chassis/foo returns 405 Method Not Allowed
PATCH /redfish/v1/foo/bar returns 404 Not Found
GET /redfish/v1 returns ServiceRoot
GET /redfish/v1/Chassis returns Chassis collection
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib0afd23d46bb5b88f89cf1e3f4e0606a48ae47ca
Signed-off-by: Carson Labrado <clabrado@google.com>
|
|
Different HTTP protocols have different http responses for 404. This
commit adds support for registering a route designed to host a handler
meant for when a response would otherwise return. This allows
registering a custom 404 handler for Redfish, for which all routes will
now return a Redfish response.
This was in response to the 404 handler not working in all cases (in the
case of POST/PATCH/DELETE). Allowing an explicit registration helps to
give the intended behavior in all cases.
Tested:
GET /redfish/v1/foo returns 404 Not found
PATCH /redfish/v1/foo returns 404 Not found
GET /redfish/v1 returns 200 OK, and content
PATCH /redfish/v1 returns 405 Method Not Allowed
With Redfish Aggregation:
GET /redfish/v1/foo gets forwarded to satellite BMC
PATCH /redfish/v1/foo does not get forwarded and returns 404
PATCH /redfish/v1/foo/5B247A_bar gets forwarded
Unit tests pass
Redfish-service-validator passes
Redfish-Protocol-Validator fails 7 tests (same as before)
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I731a5b4e736a2480700d8f3e81f9c9c6cbe6efca
Signed-off-by: Carson Labrado <clabrado@google.com>
|
|
It was accidentally missed in previous round of iwyu clean up.
Tested: build.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I6857409848a99594d5c91168cf4a760e456e8c90
|
|
The BMC Manager MACAddress ethernet property is a Read/Write property
in the Redfish spec. There are OpenBMC users that configure the
property to be Read Only. The phosphor-network build includes a
'persist-mac' configuration switch which controls MAC address
assignment. An attempt to set a R/O MACAddress causes D-Bus to return
an error. The exact error is available in a sdbus message.
The EthernetInterface XML indicates this is acceptable behavior:
"If an assignable MAC address is not supported, this value is a
read-only alias of the PermanentMACAddress."
As this condition is considered proper behavior it is incorrect to
return an internalError(). It is better behavior to return a
"Read-only" error message.
Tested:
Performed a Redfish PATCH for the MACAddress property.
The PATCH command returns a 403 error code, and a message body
indicating that the MACAddress is not writable.
Change-Id: Ice97affe694f4bee15436293c9e5944bcae7f4cc
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
|
|
This patch cleans up a few edge cases that aren't handled properly.
We need to break out of the aggregation code earlier when there are
no satellite configs. The logs are showing mixed messages of
Aggregation not being enabled due to no found satellite configs
followed by processing the request anyway until we fail to actually
find a satellite BMC to forward the request to.
When we don't have any satellite configs, but a request is sent to
what should be a valid satellite URI such as
/redfish/v1/Chassis/5B247A_ChassisID then we need to make sure we
return a 404 within the aggregation code since we won't locally
handle the request. We don't have to worry about collection
requests since by design we will also locally handle the request.
This patch is also prep to allow forwarding non-GET requests to
resources that are not supported by BMCWeb. The aggregation code
will get to handle all such requests and we need to make sure that
we do not forward non-GET requests to top level collections.
Tested:
Without any satellite configs the aggregation code exited before
it began trying to send a request to all satellites for
/redfish/v1/Chassis. The same occurred for a request for a satellite
resource. In the latter case the aggregation code also returned a
404.
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Idd1a71ebb485a77795ba47b873624c8e53c36a4c
|
|
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
|
|
SensorsAsyncResp has existed for a long time, and has slowly morphed
from its intended usage (as an RAII response object) into a
conglomeration of all possible things that a sensor request could want.
This leads to a ton of inefficient queries, and lots of data being held
for much longer than we'd like.
This commit tries to start breaking things apart, and follow the
patterns we use elsewhere, passing AsyncResp where a response object is
needed, and passing specialized data structures only into the scopes
where they're needed. This significantly increases the performance of
the /redfish/v1/Chassis/<>/Sensors/<sensor> URI.
The optimization changes the URI such that in includes both the sensor
type as well as the sensor name in the URI, meaning that from a given
tree, we can directly look up the sensor path, instead of having to look
up all sensor paths, and do a filename() compare on them.
Implementation-wise, there is one main difference in user-facing
behavior, in that instead of using a mechanized version of the sensor
name for the URI (aka /redfish/v1/Chassis/my_chassis/Sensors/my_sensor)
the URI now contains the sensor type (ex
/redfish/v1/Chassis/my_chassis/Sensors/temperature_my_sensor). One
implementation note: because fan_pwm and fan_tach namespaces have an
underscore in them, we normalize these in the URI to fanpwm and fantach
respectively such that we can differentiate between the two without
looping, and special case them on the other side. This seems like a
reasonable compromise.
The above means that when a request comes in to query the sensor, we no
longer have to pull all sensors to identify the one that matches the
name, and we can go directly to the mapper to determine which sensor we
need, with a GetObject query. This significantly reduces the amount of
time to grab the information from a single sensor.
To accomplish this, the per-sensor methods needed broken down into
pieces that allowed loading a single sensor at a time, rather than a
complete GetManagedObjects call. In practice, this just means breaking
out one helper function, such that the new code can directly call
GetAll.
In a few places, const std::string& had to be replaced with
std::string_view, because the new sensor API can directly inline its
const char* parameters for types, which allows it to avoid constructing
a string copy to do it.
Tested:
Redfish service validator passes on a S7106 system, and shows a timing
of ~40-50ms per sensor request, which is in line with what we'd expect
for a keepalive function using Session auth.
'''
curl --insecure -w "@curl-format.txt" -H "X-Auth-Token: nOIarWLRFkFN14qVONs0" https://192.168.10.140/redfish/v1/Chassis/Tyan_S7106_Baseboard/Sensors/temperature_sys_air_inlet
'''
returns timing that is on the order of 125ms. On this setup,
ServiceRoot (which should do no dbus calls) returns in 90ms, so the
sensor implementation itself is on the order of 40% of the timing.
TelemetryService functions as expected
'''
curl -k --user "root:0penBmc" -X POST https://$bmc/redfish/v1/TelemetryService/MetricReportDefinitions/ -d '{"Id": "lxw1", "Metrics": [{"MetricId": "123", "MetricProperties": ["/redfish/v1/Chassis/Tyan_S7106_Baseboard/Power#/Voltages/0"]}], "MetricReportDefinitionType": "OnRequest", "ReportActions": ["LogToMetricReportsCollection"], "Schedule": {"RecurrenceInterval": "100"}}'
'''
Succeeds.
Also succeeds with MetricProperties set to:
/redfish/v1/Chassis/Tyan_S7106_Baseboard/Sensors/voltage_vcc5
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If42f531b385c3b611b100c1c485a1e4e877c5512
|
|
Refactor more functions out of sensors such that they don't require the
use of SensorAsyncResp in all cases. This is to prepare for
implementations that might not rely on SensorAsyncResp at all.
Tested:
Tested at end of series. Merge together.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie359f449c0f7ace57a09c0db0c4f1b347b5d5030
|
|
Refactor some methods of the sensor code to not rely directly on
SensorAsyncResponse, or to require an ObjectManager response. This is
done to allow later patches to implement individual sensor collection
using GetAll instead of GetManagedObjects for a single sensor
This is done by making the underlying implementations rely on the
properties alone, as we do in other locations.
Behavior should not change with this patchset.
Tested:
Testing in later patch that behavior is the same. Merge at the same
time
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I77838ee14c14dcc9d2bc50375b2a5b0ffbc4573c
|
|
used sdbusplus::unpackPropertiesNoThrow in chassis.hpp and sensors.hpp,
also replaced all usages of "GetAll" with
sdbusplus::asio::getAllProperties
bmcweb size: 2701720 -> 2697624 (-4096)
compressed size: 1131481 -> 1129725 (-1756)
Tested:
Performed get on:
- /redfish/v1/Chassis/chassis
- /redfish/v1/Chassis/chassis/Thermal
Get result before and after the change was in same format.
Change-Id: I76377710cd037f7c54cb0639c011b64c73a719ab
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
|
|
used sdbusplus::unpackPropertiesNoThrow in certificate_service.hpp, also
replaced all usages of "GetAll" with sdbusplus::asio::getAllProperties
bmcweb size: 2668888 -> 2664776 (-4112)
compressed size: 1127280 -> 1125100 (-2180)
Tested:
Performed get on:
- /redfish/v1/Managers/bmc/NetworkProtocol/HTTPS/Certificates/1
Get result before and after the change was in same format, values for:
ValidNotAfter, ValidNotBefore and CertificateString were different
(but it is expected behaiour)
Change-Id: If94adea317dea18cb984788dc1515778ce4097c6
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
|
|
CI recently has added a feature for enforcing git commit message rules.
For the moment, this is optional and opt-in per repo. Enable it for
OpenBMC.
Tested:
CI only.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9cfe426bbcfd3f8fec34e2f92d29c03631eaee32
|
|
Similar to the other patches in the series, add a link header to all
ComputerSystem resources, along with an explicit HEAD operator.
Tested:
All of the below return a link header
GET /redfish/v1/Systems
HEAD /redfish/v1/Systems
GET /redfish/v1/Systems/system
HEAD /redfish/v1/Systems/system
GET /redfish/v1/Systems/system/ResetActionInfo
HEAD /redfish/v1/Systems/system/ResetActionInfo
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie2fc6b16be70604613ba5322e1a358974e4e6cde
|
|
I found that error_messages.cpp missed headers when adding
insufficientResource errors. This commit is like the other incremental
iwyu effort. Now error message library is fixed.
Tetsted: compiles.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ia5f4716d21a98cad56ec2ae0b842a40ed825cb17
|
|
tcp_stream has several problems in its current implementation. First,
it takes up a significant amount of binary size, for features that we
don't use. Next, it has some implied guarantees about timeouts that we
erronously rely on, namely that async_connect will timeout
appropriately given the tcp_stream timeout (it doesn't).
We already have a timer present in the ConnectionInfo class anyway, this
commit just makes use of it for ALL timeout operations, rather than just
the connect timeout operations. This flow is roughly analogous to what
we do in http_connection.hpp already, so the pattern is well troden.
This saves 2.8% of the binary size of bmcweb, and solves several race
conditions.
Tested:
Relying on Carson: Aggregated a sub-bmc, and ensured that top level
collections returned correctly under GET /redfish/v1/Chassis
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I72e8e97b667826f272bb4921afc2b16907f3b271
|
|
This commit adds the initial SSL support for http_client which can be
used for sending asynchronous Events/MetricReports to subscribed Event
Listener servers over secure channel.
Current implementation of http client only works for http protocol.
With current implementation, http client can be configured to work
with secure http (HTTPS). As part of implementation it adds the SSL
handshake mechanism and enforces the peer ceritificate verification.
The http-client uses the cipher suites which are supported by mozilla
browser and as recommended by OWASP. For better security enforcement
its disables the SSLv2, SSLv3, TLSv1, TLSv1.1 as described in below
OWASP cheetsheet.
It is validated with RootCA certificate(PEM) for now. Adding support
for different certificates can be looked in future as need arises.
[1]: https://cheatsheetseries.owasp.org/cheatsheets/TLS_Cipher_String_Cheat_Sheet.html
Tested:
- Created new subscription with SSL destination(https) and confirmed
that events are seen on EventListener side.
URI: /redfish/v1/EventService/Subscriptions
Method: POST
Body:
{
"Context": "CustomText",
"Destination": "https://<IP>:4000/service/collector/event_logs",
"EventFormatType": "Event",
"DeliveryRetryPolicy": "RetryForever",
"Protocol": "Redfish"
}
- Unit tested the non-SSL connection by disabling the check in code
(Note: EventService blocks all Non-SSL destinations). Verified that
all events are properly shown on EventListener.
URI: /redfish/v1/EventService/Subscriptions
Method: POST
Body:
{
"Context": "CustomText",
"Destination": "http://<IP>:4001/service/collector/event_logs",
"EventFormatType": "Event",
"Protocol": "Redfish"
}
- Combined above two tests and verified both SSL & Non-SSL work fine in
congention.
- Created subscription with different URI paths on same IP, Port and
protocol and verified that events sent as expected.
Change-Id: I13b2fc942c9ce6c55cd7348aae1e088a3f3d7fd9
Signed-off-by: AppaRao Puli <apparao.puli@intel.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
Reduces the number of retry attempts to 1. The aggregating BMC
cannot generate a response until the retry policy has been exhausted.
We want to minimize the amount of time it takes for the aggregating
BMC to respond in the event of an unreachable satellite BMC.
Also explicity sets Redfish Aggregation's retry policy action as
"TerminateAfterRetries". Previously it relied on this being the
default action.
Tested:
Requests sent to unreachable satellite BMC only attempted to resend
a single time.
Signed-off-by: Carson Labrado <clabrado@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If1819389affc96f49908d586459b03b1bb2689c2
|
|
Adds aggregation support for resource collections that take the form
of "/redfish/v1/<resource collection>". Collection URIs are
identified by the precense of a "Members" array in the response.
Resources from satellite BMCs are added to the "Members" array of
the response and the "Members@odata.count" value is updated to
denote the new array size.
These satellite resource URIs that are added also include the
prefix associated with that satellite.
Note that as a first step this patch assumes a single satellite BMC.
There are some potential race conditions that could occur for setups
with multiple satellite BMCs. This has been commented in the code
and is better left to its own patch.
Tested:
Queried various collection URIs and the aggregated resources
appeared in the response's "Members" array.
Querying 'localhost:80/redfish/v1/Chassis?$expand=.($levels=1)'
resulted in $expand correctly returning the outputs from querying
the URIs of all local and satellite Chassis resources. This would
have failed if the satellite Chassis resources were omitted from the
"Members" array or the satellite's prefix was not correctly added to
the URI.
Also queried a collection URI that only existed on the satellite BMC.
The AsyncResp was completely overwritten by the response from the
satellite BMC.
Queries to non-collection URIs resulted in no attempts to add
satellite responses to the AsyncResp.
Signed-off-by: Carson Labrado <clabrado@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I3b379cd57e5a121eb4a344d88fc8e43170ca78a6
|
|
URIs in the responses returned with Redfish Aggregation enabled will
potentially be incorrect since ones from satellite BMCs will not
include the associated prefix such as "5B247A_" in the resource ID
portion of the URIs.
This patch fixes those links so that they include their BMC's
associated prefix. Note that a future patch will be needed to add
prefixes to aggregated resources that would appear under collection
URIs such as "/redfish/v1/Chassis".
Tested:
Requests were sent to URIs associated with the aggregating BMC and a
satellite BMC denoted as "5B247A". The URIs in the responses
were successfully updated such that "5B247A_" was added for
satellite resources.
Signed-off-by: Carson Labrado <clabrado@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib4f976fab1ca1e8603f7cf55292732ffb71cd03e
|
|
Adds ability to route requests to either native resources or
resources that belong to a satellite BMC as part of Redfish
Aggregation. A prefix in the URI denotes if the resource is actually
from a satellite BMC. Prefixes are only used to denote satellite
resources. The URI of resources on the local/aggregating BMC will
remain unchanged.
Prefixes are separated from the resource ID by an underscore. This
means that underscores cannot be used in the prefix name itself.
The prefixes used by satellite BMCs are revealed via D-Bus as well as
the config information needed to connect to that BMC.
Requests for satellite resources will not be handled locally.
Care should be taken to not name any local resources in a way that
could cause a collision (e.g. having a Chassis object named
"aggregated0_1U" on the aggregating BMC).
The patch only covers routing requests. Requests to collection URIs
like /redfish/v1/Chassis will only return resources on the local BMC.
A future patch will cover adding satellite resources to collections.
Also note that URIs returned in the responses will not have the proper
prefix included. Fixing these URIs will be addressed in future
patches.
A number of TODO comments are included in the code to indicate that
this functionality (collections and URI fixup) still needs to be
implemented.
Example URIs w/o Redfish Aggregation:
/redfish/v1/Chassis/1U/
/redfish/v1/Systems/system/
/redfish/v1/Managers/bmc/
Example URIs after enabling Redfish Aggregation if the associated
resources are located on the local/aggregating BMC:
/redfish/v1/Chassis/1U/
/redfish/v1/Systems/system/
/redfish/v1/Managers/bmc/
Example URIs if resources are instead located on a satellite BMC
named "aggregated0":
/redfish/v1/Chassis/aggregated0_1U/
/redfish/v1/Systems/aggregated0_system/
/redfish/v1/Managers/aggregated0_bmc/
Tested:
I was able to query supported resources located on the local BMC
as well as on a satellite BMC. Requests with unknown prefixes return
a 404. Requests to resource collections only return the resources
that are located on the aggregating BMC.
Signed-off-by: Carson Labrado <clabrado@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I87a3deb730bda95e72ecd3144ea40b0e5ee7d491
|
|
We do not want to allow a HW config to set its own prefix since that
results in HW choosing and hardcoding resource URIs. Removes using
"Name" from the satellite config as the config's prefix.
For now assume there will be no more than one satellite bmc. We will
always assign that config to be "aggregated0". If more than one
config is present then we will not attempt to forward any requests.
In a future patch we will add support for aggregating multiple
satellite BMCs. The aggregator will be responsible for assigning the
prefixes to each satellite.
When we receive a request we parse the resource ID to see if it
begins with "aggregated" and thus should be forwarded to a satellite
BMC. In those cases we should not locally handle the request. We
return a 500 error, but in a future patch that will be replaced by
the actual code to forward the request to the appropriate satellite.
Requests for resource collections need to be both handled locally and
forwarded. Place holders are added for where the forwarding will
occur. A future patch will add that functionality.
Tested:
Exposed two configs in an entity-manager json:
"Exposes": [
{
"Hostname": "127.0.0.1",
"Port": "443",
"Name": "Sat1",
"Type": "SatelliteController",
"AuthType": "None"
},
{
"Hostname": "127.0.0.1",
"Port": "444",
"Name": "Sat2",
"Type": "SatelliteController",
"AuthType": "None"
},
It produced an error that only one satellite is supported and as a
result both configs were ignored. I removed the second config and
that resulted in the first (and only) config being added as
"aggregated0".
Requests for local resources were ignored by the aggregation code.
Requests for collections hit the forward collection endpoints and
return local results.
500 returned for satellite resources such as:
/redfish/v1/Chassis/aggregated0_Fake
/redfish/v1/UpdateService/FirmwareInventory/aggregated0_Fake
/redfish/v1/UpdateService/SoftwareInventory/aggregated0_Fake
Change-Id: I5c860c01534e7d5b1a37c95f75be5b3c1f695816
Signed-off-by: Carson Labrado <clabrado@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
Sometimes it is desirable to only parse a portion of a URL, and allow
any elements at the end. This comes up significantly in aggregation
where parsing "/redfish/v1/<collection>/anything is pretty common.
This commit adds a new class, OrMorePaths, that can be used as a
placeholder to indicate that more paths should be accepted.
Tested: Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If4fb3991a91560fd3b8b838f912aa36e79ddd2b3
|
|
bmcweb does not do anything with the q-factor weighting (;q=) so just
remove it from the encoding.
This is needed because routes like
"/redfish/v1/Systems/system/LogServices/EventLog/Entries/<str>/attachment"
have a check for isOctetAccepted. Even though */* is in the Accept
Header isOctetAccepted still fails due to the q-factor weighting.
On the system I tested, on firefox, Accept looks like:
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8
The GUI reported being unable to download a AdditionalDataURI (e.g.
...attachment/)
Here is the GUI code attempting to download the additional data:
https://github.com/openbmc/webui-vue/blob/9b79a6e7e3df3d3cbaf9a7750bbe343628022026/src/views/Logs/EventLogs/EventLogs.vue#L155
https://github.com/openbmc/webui-vue/blob/9b79a6e7e3df3d3cbaf9a7750bbe343628022026/src/locales/en-US.json#L251
Today this results in a 400 Bad Request due to isOctetAccepted.
See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept
Tested:
/redfish/v1/Systems/system/LogServices/PostCodes/Entries/<str>/attachment/
and .../EventLog/Entries/<str>/attachment now return correctly.
Change-Id: I969f5f2c32c4acccd4d80615f17c44d0c8fabd0d
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
Commit bc1d29d ("used sdbusplus::unpackPropertiesNoThrow part 3")
introduced a bug due to not passing the properties list to
unpackPropertiesNoThrow() in getProvisioningStatus(). This function
is only get compiled when redfish-provisioning-feature is enabled,
since it is not enabled in CI, it passed CI build.
Tested:
Local build with redfish-provisioning-feature enabled passed.
Change-Id: Ifcd19e0a8b830fe6d306919953aa01412c55043a
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
|
Renamed handleLogServicesCollectionGet() to
handleBMCLogServicesCollectionGet() (added "BMC").
handleLogServicesCollectionGet() was a function first introduced in
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53175 but the function
name was accidentally made to be too broad. The function only handles
outputting the collection of LogServices specifically under
/redfish/v1/Managers/bmc/LogServices.
No expected client impact.
Tested:
Redfish Service Validator succeeded on the following URI:
/redfish/v1/Managers/bmc/LogServices/
Signed-off-by: Claire Weinan <cweinan@google.com>
Change-Id: I4648db2eaa5417c2428b514fdda1dbf82af0b3f0
|
|
I developed a fair amount of the query parameter codes and had a fair
understanding of how it works, what the next steps are, how to review
related codes, how to test it. Thus, propose that I own these codes.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I9c5a4ca5f7d569f3523afb3d97aa367f46314220
|
|
Putting the maxEntriesPerPage next to the top parameter makes it more
clear about its intention as Ed suggested. Here it is also renamed to
maxTop to illustrate its relationship with top.
Tested:
Build, unit test and Redfish Service Validator passed.
Change-Id: I035eea7e33d78439685a81092a4dd9332cfc501a
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
|
clang14 complains about building this header.
Tested: clang14 builds successfully.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ib6be59babb0fdc9cc033385b4b71f1c9c88dfa44
|
|
In the spec, when a response is too large, we need a error messsage to
represent such error. The corresponding error is 507 Insufficient
Storage, which is already in the base registry.
This commit adds that utility to error messages library.
Reference: redfish spec Section 7.3.2
```
If a service cannot return the payload due to its size, it shall return
the HTTP 507 Insufficient Storage status code.
```
Tested: code compies.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I1fe840f015b3d67fa2092d1d3a1f8c3fcbc2c438
|
|
As bmcweb grows, it's important that we distribute the load amongst the
available community. This commit pivots slightly from our usual process
of giving full repo maintainership to a small list of one or two people,
and instead proposes passing out smaller pieces of maintainership. The
hope is that over time, these maintainers can grow into owning larger
portions of the stack.
In this way, we can increase our breadth of available maintainers for
the things that are straightforward, like adding new redfish schemas or
doing migrations, while still keeping a few core maintainers responsible
for the core.
I've reached out to both Krzysztof and Nan, and asked them which pieces
they feel that they're comfortable maintaining, and I've added all
components they listed to the OWNERS files. Each has a track record of
doing reviews in upstream, and given that Gunnar and myself will still
be present and active, this seems like a good thing for the project
overall.
The specific sections of maintainership for each person is laid out in
the OWNERS file, and we can modify over time as each person gets more
experience in reviewing and testing.
The expectations are that for the files explicitly listed, these new
maintainers will be able to handle all reviews that touch those sections
of functionality (including reviews not yet merged). Please continue to
communicate as you have previously.
To those being added, it is expected that if there are core issues
brought up in the modules that you maintain, you escalate them to the
entire group of maintainers, and remain active on our communication
channels. In the past, we've had significant issues where individual
modules tried to implement generalized solutions in a handler-specific
way, and while most of that has been cleaned up, it caused significant
thrash. Lets try to avoid that going forward.
I look forward to working with both of you more.
Change-Id: I7750a12703af68fc82f84a0e22496dabca582208
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
A continuation of 4d94272fe54c147974f86788092bbbdd950406aa,
increases the size of httpReadBodyLimit to 2^17 bytes (about 128 KB).
This is because with Redfish Aggregation we need to be able to handle
larger response sizes such as json schema files. The ComputerSystem
schema in particular is over 120 KB.
Going forward we will not be able to keep increasing the limit.
We need a better solution for handling larger response that are
larger than httpReadBodyLimit.
Tested:
Used https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310/68 to
successfully retrieve
/redfish/v1/JsonSchemas/ComputerSystem/ComputerSystem.json from a
satellite BMC.
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ida80b8fddbd1df1310d18a77ecfb44b7fdf292ef
|
|
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
|
|
used sdbusplus::unpackPropertiesNoThrow in systems.hpp, also replaced
all usages of "GetAll" with sdbusplus::asio::getAllProperties
bmcweb size: 2672984 -> 2668888 (-4096)
compressed size: 1128611 -> 1127280 (-1331)
Tested:
Performed get on:
- /redfish/v1/Systems/system
- /redfish/v1/Systems/system/Memory/dimm0
Get result before and after the change was the same
Change-Id: I11ca70bae07bdb17edd1935c539c68814d6ec172
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
|
|
We should do this because it means that variables are only initialized
once. In the simplest case:
struct MyClass{
std::string myString
MyClass(){
myString = "foo";
}
}
in the language, myString is constructed twice, once with empty string,
then a second time with "foo". If you do the construction in the
initializer list for the class the construction only happens once.
Now, the above case is contrived, the optimizer can see through it and
likely optimizes this case because std::string is relatively simple, but
for more complex structures, it's possible this generates less and
bettercompiled code, and this is worth having the check for, and making
our existing code correct.
Tested: cppcheck passing.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie5c7f293598408d437e7bf7b3fc93b0819e25f9f
|
|
We have two functions, leftZeroPadding, and padZeros that effectively do
the same thing. leftZeroPadding has only one usage, and padZeros is
debatably more efficient (given it doesn't need to construct an
intermediate string). Move the one usage of leftZeroPadding to
padZeros, and remove leftZeroPadding.
One minor change needs to be made to padZeros, in that it needs to
accept a long int instead of an int, given that some implementations use
long int as their duration object.
Tested: Unit tests pass. Good coverage on time functions.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Icbec45787ff940d098e18b61741b7476f4263558
|
|
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/55468 was a refactoring
task that was not intended to have any client impact. However it had a
bug where it unintentionally changed the “Created” timestamp of log
entries of all dump types to be smaller (earlier) than pre-refactoring.
Tested:
FaultLog entry before fix (timestamp too large--reported as max Redfish
time):
curl k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Managers/bmc/LogServices/FaultLog/Entries/1
{
"@odata.id": "/redfish/v1/Managers/bmc/LogServices/FaultLog/Entries/1",
"@odata.type": "#LogEntry.v1_8_0.LogEntry",
"Created": "9999-12-31T23:59:59+00:00",
"EntryType": "Event",
"Id": "1",
"Name": "FaultLog Dump Entry"
}
FaultLog entry after fix (timestamp reported correctly):
curl -k -H "X-Auth-Token: $token" -X GET http://${bmc}/redfish/v1/Managers/bmc/LogServices/FaultLog/Entries/1
{
"@odata.id": "/redfish/v1/Managers/bmc/LogServices/FaultLog/Entries/1",
"@odata.type": "#LogEntry.v1_8_0.LogEntry",
"Created": "2022-08-14T10:29:40+00:00",
"EntryType": "Event",
"Id": "1",
"Name": "FaultLog Dump Entry"
}
Tested similarly for a BMC Dump entry.
Interestingly, it seems like the System Dump entry timestamp before the
refactoring task was too small, and the bug actually helped to correct
it. In any case, this change reverts timestamp behavior to match what it
was pre-refactoring.
Redfish Service Validator succeeded on the following URI trees:
/redfish/v1/Managers/bmc/LogServices/FaultLog
/redfish/v1/Managers/bmc/LogServices/Dump
/redfish/v1/Systems/system/LogServices/Dump
Signed-off-by: Claire Weinan <cweinan@google.com>
Change-Id: I7badbd348595a7bcb61982a4cd6b7e82a16b0219
|
|
We've accumulated several time utility functions in the http classes.
Time isn't a core HTTP primitive, so http is not where those functions
below.
This commit moves all the time functions from the crow::utility
namespace into the redfish::time_utils namespace, as well as moves the
unit tests.
No code changes where made to the individual functions, with the
exception of changing the namespace on the unit tests.
Tested: Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8493375f60aea31899c84ae703e0f71a17dbdb73
|
|
ipsPropertiesType is identical to dbus::utility::DBusPropertiesMap,
change to the latter one for consistency.
Tested:
Build pass.
Change-Id: Ibe006a252a8eb9c2861917ee255fc6093f0176ea
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
|
