Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: reject any OTA update/rollback request on ecu_info.yaml not properly loaded #465

Merged
merged 18 commits into from
Dec 26, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/otaclient/_status_monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,15 @@ def _status_collector_thread(self) -> None:
break

# ------ push status on load_report ------ #
if self.load_report(report) and self._status and _now > _next_shm_push:
# NOTE: always push OTAStatus change report
if (
self.load_report(report)
and self._status
and (
isinstance(report.payload, OTAStatusChangeReport)
or _now > _next_shm_push
)
):
Comment on lines +313 to +321
Copy link
Member Author

@Bodong-Yang Bodong-Yang Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, if the OTAStatusChangeReport reported too quickly, the newly coming report will be dropped, resulting in the OTA status not being updated. That is not proper, fixed in this PR.

try:
self._shm_status.write_msg(self._status)
_next_shm_push = _now + self.shm_push_interval
Expand Down
33 changes: 33 additions & 0 deletions src/otaclient/grpc/api_v2/servicer.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import multiprocessing.queues as mp_queue
from concurrent.futures import ThreadPoolExecutor

import otaclient.configs.cfg as otaclient_cfg
from otaclient._types import (
IPCRequest,
IPCResEnum,
Expand Down Expand Up @@ -111,6 +112,22 @@ async def update(
update_acked_ecus = set()
response = api_types.UpdateResponse()

# NOTE(20241220): due to the fact that OTA Service API doesn't have field
# in UpdateResponseEcu msg, the only way to pass the failure_msg
# to upper is by status API.
if not otaclient_cfg.ECU_INFO_LOADED_SUCCESSFULLY:
Bodong-Yang marked this conversation as resolved.
Show resolved Hide resolved
logger.error(
"ecu_info.yaml is not loaded properly, reject any update request"
)
for _update_req in request.iter_ecu():
response.add_ecu(
api_types.UpdateResponseEcu(
ecu_id=_update_req.ecu_id,
result=api_types.FailureType.UNRECOVERABLE,
Bodong-Yang marked this conversation as resolved.
Show resolved Hide resolved
)
)
return response

# first: dispatch update request to all directly connected subECUs
tasks: dict[asyncio.Task, ECUContact] = {}
for ecu_contact in self.sub_ecus:
Expand Down Expand Up @@ -225,6 +242,22 @@ async def rollback(
logger.info(f"receive rollback request: {request}")
response = api_types.RollbackResponse()

# NOTE(20241220): due to the fact that OTA Service API doesn't have field
# in UpdateResponseEcu msg, the only way to pass the failure_msg
# to upper is by status API.
if not otaclient_cfg.ECU_INFO_LOADED_SUCCESSFULLY:
logger.error(
"ecu_info.yaml is not loaded properly, reject any rollback request"
)
for _rollback_req in request.iter_ecu():
response.add_ecu(
api_types.RollbackResponseEcu(
ecu_id=_rollback_req.ecu_id,
result=api_types.FailureType.UNRECOVERABLE,
)
)
return response

Bodong-Yang marked this conversation as resolved.
Show resolved Hide resolved
# first: dispatch rollback request to all directly connected subECUs
tasks: dict[asyncio.Task, ECUContact] = {}
for ecu_contact in self.sub_ecus:
Expand Down
61 changes: 47 additions & 14 deletions src/otaclient/ota_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import requests.exceptions as requests_exc
from requests import Response

import otaclient.configs.cfg as otaclient_cfg
from ota_metadata.legacy import parser as ota_metadata_parser
from ota_metadata.legacy import types as ota_metadata_types
from ota_metadata.utils.cert_store import (
Expand Down Expand Up @@ -672,17 +673,7 @@ def __init__(
)
return

# load and report booted OTA status
_boot_ctrl_loaded_ota_status = self.boot_controller.get_booted_ota_status()
self._live_ota_status = _boot_ctrl_loaded_ota_status
status_report_queue.put_nowait(
StatusReport(
payload=OTAStatusChangeReport(
new_ota_status=_boot_ctrl_loaded_ota_status,
),
)
)

# ------ load firmware version ------ #
self.current_version = self.boot_controller.load_version()
status_report_queue.put_nowait(
StatusReport(
Expand All @@ -691,7 +682,9 @@ def __init__(
),
)
)
logger.info(f"firmware_version: {self.current_version}")

# ------ load CA store ------ #
self.ca_chains_store = None
try:
self.ca_chains_store = load_ca_cert_chains(cfg.CERT_DPATH)
Expand All @@ -701,9 +694,36 @@ def __init__(

self.ca_chains_store = CAChainStore()

self.started = True
logger.info("otaclient started")
logger.info(f"firmware_version: {self.current_version}")
# load and report booted OTA status
_boot_ctrl_loaded_ota_status = self.boot_controller.get_booted_ota_status()
if not otaclient_cfg.ECU_INFO_LOADED_SUCCESSFULLY:
logger.error(
"ecu_info.yaml file is not loaded properly, will reject any OTA request."
)
logger.error(f"set live_ota_status to {OTAStatus.FAILURE!r}")
self._live_ota_status = OTAStatus.FAILURE
status_report_queue.put_nowait(
StatusReport(
payload=OTAStatusChangeReport(
new_ota_status=OTAStatus.FAILURE,
failure_type=FailureType.UNRECOVERABLE,
failure_reason=f"ecu_info.yaml file is broken or missing, please check {cfg.ECU_INFO_FPATH}. "
"reject any OTA request.",
),
)
)
else:
self._live_ota_status = _boot_ctrl_loaded_ota_status
status_report_queue.put_nowait(
StatusReport(
payload=OTAStatusChangeReport(
new_ota_status=_boot_ctrl_loaded_ota_status,
),
)
)

self.started = True
logger.info("otaclient started")
Comment on lines +702 to +731
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If ecu_info.yaml is not loaded properly, we will just set the otaclient as NOT started, any requests coming to otaclient will be rejected when otaclient is not started.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that when ecu_info.yaml is broken, only the live_ota_status(in the memory) will be set to FAILURE, the status file will not be override, as ecu_info.yaml missing/broken is considered a soft failure.


def _on_failure(
self,
Expand Down Expand Up @@ -843,6 +863,19 @@ def main(
)
)

elif not self.started:
_err_msg = (
"otaclient is not started, might be due to broken ecu_info.yaml"
)
logger.error(_err_msg)
resp_queue.put_nowait(
IPCResponse(
res=IPCResEnum.REJECT_OTHER,
msg=_err_msg,
session_id=request.session_id,
)
)

elif isinstance(request, UpdateRequestV2):

_update_thread = threading.Thread(
Expand Down
Loading