openthread / ot-br-posix

OpenThread Border Router, a Thread border router for POSIX-based platforms.
https://openthread.io/
BSD 3-Clause "New" or "Revised" License
420 stars 235 forks source link

[rest] add joiner to commissioner joiner table and get status of joiner … #2517

Open martinzi opened 1 month ago

martinzi commented 1 month ago

…by indirect 'addThreadDeviceTask' processing

This PR is a carve-out from PR2514 to ease review. It implements the 'addThreadDeviceTask' to add a Joiner and auto-start on-mesh commissioner after POST:

The commit also provides limited integration tests, see tests/restjsonapi.

Please follow below steps to reproduce and install/build OTBR.

  1. Checkout this PR

  2. Build and Install OTBR as usual, e.g. on a Raspberry Pi

  3. Restart the OTBR. sudo systemctl restart otbr-agent

  4. To monitor the log [Errors|Warnings|Info] please open a different terminal instance and use following command:

tail -f /var/log/syslog | grep otbr
  1. Send POST request using Bruno or cURL to join a new device into your network.
curl -X POST -H 'Content-Type: application/vnd.api+json' http://localhost:8081/api/actions -d '{"data": [{"type": "addThreadDeviceTask", "attributes": {"eui": "6234567890AACDEA", "pskd": "J01NME", "timeout": 3600}}]}' | jq

should return

{
  "data": [
    {
      "id": "2d5a8844-b1bc-4f02-93f0-d87b8c3b4e92",
      "type": "addThreadDeviceTask",
      "attributes": {
        "eui": "6234567890AACDEB",
        "pskd": "J01NME",
        "timeout": 3600,
        "status": "pending"
      },
    }
  ]
}
  1. You may check the status and get the full collection of actions.
curl -X GET -H 'Accept: application/vnd.api+json' http://localhost:8081/api/actions | jq

should return

{
  "data": [
    {
      "id": "2d5a8844-b1bc-4f02-93f0-d87b8c3b4e92",
      "type": "addThreadDeviceTask",
      "attributes": {
        "eui": "6234567890AACDEB",
        "pskd": "J01NME",
        "timeout": 3600,
        "status": "pending"
      }
    }
  ],
  "meta": {
    "collection": {
      "offset": 0,
      "limit": 100,
      "total": 1
    }
  }
}
  1. View the entry added to the commissioner's table sudo ot-ctl commissioner joiner table and expect
| ID                    | PSKd                             | Expiration |
+-----------------------+----------------------------------+------------+
|      6234567890aacdea |                           J01NME |    3459027 |
Done
  1. Start your joiner and after a few seconds repeat above steps 6. and 7.

  2. For running the included test script install Bruno-Cli and run the bash script on your border router

cd tests/restjsonapi
source ./install_bruno_cli
./test-restjsonapi-server
codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 7.61180% with 971 lines in your changes missing coverage. Please review.

Project coverage is 42.93%. Comparing base (2b41187) to head (3c37296). Report is 856 commits behind head on main.

Files with missing lines Patch % Lines
src/rest/extensions/commissioner_allow_list.cpp 0.00% 260 Missing :warning:
src/rest/extensions/rest_task_queue.cpp 2.64% 183 Missing and 1 partial :warning:
src/rest/resource.cpp 15.47% 141 Missing and 1 partial :warning:
...rc/rest/extensions/rest_task_add_thread_device.cpp 0.00% 111 Missing :warning:
src/rest/extensions/rest_server_common.cpp 0.00% 57 Missing :warning:
src/rest/extensions/rest_task_handler.cpp 0.00% 55 Missing :warning:
src/rest/extensions/uuid.cpp 0.00% 47 Missing :warning:
src/rest/extensions/linked_list.hpp 4.34% 44 Missing :warning:
src/rest/parser.cpp 63.79% 19 Missing and 2 partials :warning:
src/rest/extensions/commissioner_allow_list.hpp 0.00% 20 Missing :warning:
... and 4 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2517 +/- ## =========================================== - Coverage 55.77% 42.93% -12.85% =========================================== Files 87 113 +26 Lines 6890 13482 +6592 Branches 0 1055 +1055 =========================================== + Hits 3843 5788 +1945 - Misses 3047 7378 +4331 - Partials 0 316 +316 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

wgtdkp commented 1 month ago

@martinzi Thanks for splitting this out!

A few suggestions:

  1. could you please update https://github.com/openthread/ot-br-posix/blame/main/src/rest/openapi.yaml for the new REST APIs? We use the OpenAPI Specification for defining our REST API, see https://swagger.io/specification/ for details.
  2. could you fix the CI test failures to make sure everything works before we getting into code details? thanks
  3. Your code pretty test is failing, this is likely because you didn't format your code. You can use the command script/make-pretty to format your code.

BRs!

wgtdkp commented 1 month ago

Please be aware that there is also another PR adding the commissioner feature to REST API https://github.com/openthread/ot-br-posix/pull/2515