[openthread_border_router] Move web UI to Ingress #4346
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| diff --git a/src/web/web-service/frontend/index.html b/src/web/web-service/frontend/index.html | ||
| index cb052f26..992f03d6 100644 | ||
| --- a/src/web/web-service/frontend/index.html | ||
| +++ b/src/web/web-service/frontend/index.html | ||
| @@ -89,7 +89,7 @@ | ||
| </header> | ||
| <nav class="demo-navigation mdl-navigation mdl-color--blue-grey-800"> | ||
| <md-list flex> | ||
| - <md-list-item md-ink-ripple class="mdl-navigation__link" ng-repeat="item in menu" ng-click="showPanels($index)" layout="row"> | ||
| + <md-list-item md-ink-ripple class="mdl-navigation__link" ng-repeat="item in menu" ng-click="showPanels($index)" ng-if="!item.hidden" layout="row"> | ||
| <div><i class="mdl-color-text--blue-grey-400 material-icons" role="presentation">{{item.icon}}</i>{{item.title}} | ||
| </div> | ||
| </md-list-item> | ||
| @@ -130,7 +130,6 @@ | ||
| <th>PAN ID</th> | ||
| <th>channel</th> | ||
| <th>Hardware Address</th> | ||
| - <th>Action</th> | ||
|
|
||
| </tr> | ||
| </thead> | ||
| @@ -142,9 +141,6 @@ | ||
| <td>{{item.pi}}</td> | ||
| <td>{{item.ch}}</td> | ||
| <td>{{item.ha}}</td> | ||
| - <td> | ||
| - <button class="mdl-button mdl-js-button mdl-button--raised mdl-button--colored mdl-button show-modal" ng-click="showJoinDialog($event, $index, item)">Join</button> | ||
| - </td> | ||
| </tr> | ||
| </tbody> | ||
| </table> | ||
| diff --git a/src/web/web-service/frontend/res/js/app.js b/src/web/web-service/frontend/res/js/app.js | ||
| index e8453f6c..e0645f83 100644 | ||
| --- a/src/web/web-service/frontend/res/js/app.js | ||
| +++ b/src/web/web-service/frontend/res/js/app.js | ||
| @@ -57,7 +57,7 @@ | ||
| show: true, | ||
| }, | ||
| { | ||
| - title: 'Join', | ||
| + title: 'Scan', | ||
| icon: 'add_circle_outline', | ||
| show: false, | ||
| }, | ||
| @@ -65,6 +65,7 @@ | ||
| title: 'Form', | ||
| icon: 'open_in_new', | ||
| show: false, | ||
| + hidden: true, | ||
| }, | ||
| { | ||
| title: 'Status', | ||
| @@ -124,7 +125,7 @@ | ||
| }; | ||
| $scope.showPanels = async function(index) { | ||
| $scope.headerTitle = $scope.menu[index].title; | ||
| - for (var i = 0; i < 7; i++) { | ||
| + for (var i = 0; i < $scope.menu.length; i++) { | ||
| $scope.menu[i].show = false; | ||
| } | ||
| $scope.menu[index].show = true; | ||
| @@ -1226,3 +1227,15 @@ | ||
| } | ||
| }; | ||
| })(); | ||
| + | ||
| +// Use relative URLs for ingress compatibility | ||
| +angular.module('StarterApp').config(['$httpProvider', function($httpProvider) { | ||
| + $httpProvider.interceptors.push(function() { | ||
| + return { | ||
| + request: function(config) { | ||
| + config.url = config.url.replace(/^http:\/\/[^/]+\//, ''); | ||
| + return config; | ||
| + } | ||
| + }; | ||
| + }); | ||
| +}]); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,16 @@ | ||||||||||||||||||
| diff --git a/src/rest/rest_web_server.cpp b/src/rest/rest_web_server.cpp | ||||||||||||||||||
| index 29ee6b506be..9b34948981a 100644 | ||||||||||||||||||
| --- a/src/rest/rest_web_server.cpp | ||||||||||||||||||
| +++ b/src/rest/rest_web_server.cpp | ||||||||||||||||||
| @@ -1839,6 +1839,11 @@ void RestWebServer::Init(const std::string &aRestListenAddress, int aRestListenP | ||||||||||||||||||
| { | ||||||||||||||||||
| otbrLogInfo("RestWebServer listening on %s:%u", aRestListenAddress.c_str(), aRestListenPort); | ||||||||||||||||||
| self->mServer.set_ipv6_v6only(false); | ||||||||||||||||||
| + self->mServer.set_socket_options([](int sock) { | ||||||||||||||||||
| + int opt = 1; | ||||||||||||||||||
| + // cpp-httplib defaults to SO_REUSEPORT instead of SO_REUSEADDR | ||||||||||||||||||
| + setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)); | ||||||||||||||||||
|
Comment on lines
+11
to
+12
There was a problem hiding this comment. The patch changes the REST API socket from SO_REUSEPORT to SO_REUSEADDR. According to the comment, cpp-httplib defaults to SO_REUSEPORT. However, this patch is setting SO_REUSEADDR via set_socket_options, but it's unclear if this actually overrides cpp-httplib's default SO_REUSEPORT setting or if both options will be set. If both are set, SO_REUSEPORT behavior will still apply. The patch should explicitly disable SO_REUSEPORT if that's the intent, or the comment should clarify the expected behavior.
Suggested change
|
||||||||||||||||||
| + }); | ||||||||||||||||||
| const httplib::Headers defaultHeaders = { | ||||||||||||||||||
| {"Access-Control-Allow-Origin", OTBR_REST_ACCESS_CONTROL_ALLOW_ORIGIN}, | ||||||||||||||||||
| {"Access-Control-Allow-Methods", OTBR_REST_ACCESS_CONTROL_ALLOW_METHODS}, | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ ENV DOCKER=1 | |
|
|
||
|
|
||
| COPY openthread-core-ha-config-posix.h /usr/src/ | ||
| COPY 0003-ha-web-ui-customizations.patch /usr/src/ | ||
| COPY 0004-socket-reuseport.patch /usr/src/ | ||
|
|
||
| WORKDIR /usr/src | ||
| RUN \ | ||
|
|
@@ -37,6 +39,8 @@ WORKDIR /usr/src/ot-br-posix | |
| RUN \ | ||
| set -x \ | ||
| && mv /usr/src/openthread-core-ha-config-posix.h /usr/src/ot-br-posix/third_party/openthread/repo/ \ | ||
| && patch -p1 < /usr/src/0003-ha-web-ui-customizations.patch \ | ||
| && patch -p1 < /usr/src/0004-socket-reuseport.patch \ | ||
|
Comment on lines
+42
to
+43
There was a problem hiding this comment. The 0004-socket-reuseport.patch is only applied to the beta build (line 43) but not included in the stable build's patch list (lines 116-118). This inconsistency means the stable and beta builds will have different socket behavior for the REST API. Either both builds should apply this patch, or there should be a comment explaining why only beta needs it. |
||
| && ./script/cmake-build \ | ||
| -DBUILD_TESTING=OFF \ | ||
| -DCMAKE_INSTALL_PREFIX=/opt/otbr-beta \ | ||
|
|
@@ -85,6 +89,7 @@ ENV DOCKER=1 | |
| COPY openthread-core-ha-config-posix.h /usr/src/ | ||
| COPY 0001-channel-monitor-disable-by-default.patch /usr/src/ | ||
| COPY 0002-spinel-Clear-source-match-tables-before-restoring.patch /usr/src/ | ||
| COPY 0003-ha-web-ui-customizations.patch /usr/src/ | ||
|
|
||
| WORKDIR /usr/src | ||
| RUN \ | ||
|
|
@@ -111,6 +116,7 @@ RUN \ | |
| && patch -p1 < /usr/src/0001-channel-monitor-disable-by-default.patch \ | ||
| && patch -p1 < /usr/src/0002-spinel-Clear-source-match-tables-before-restoring.patch \ | ||
| ) \ | ||
| && patch -p1 < /usr/src/0003-ha-web-ui-customizations.patch \ | ||
| && ./script/cmake-build \ | ||
| -DBUILD_TESTING=OFF \ | ||
| -DCMAKE_INSTALL_PREFIX=/opt/otbr-stable \ | ||
|
|
@@ -167,6 +173,7 @@ RUN \ | |
| libncurses6 \ | ||
| libprotobuf-lite32 \ | ||
| libjsoncpp26 \ | ||
| nginx \ | ||
| && pip install --break-system-packages --no-cache-dir \ | ||
| universal-silabs-flasher==${UNIVERSAL_SILABS_FLASHER_VERSION} \ | ||
| serialx==${SERIALX_VERSION} \ | ||
|
|
@@ -183,6 +190,7 @@ COPY --from=otbr-stable-builder /opt/otbr-stable /opt/otbr-stable | |
| COPY --from=otbr-stable-builder /usr/sbin/mdnsd /opt/otbr-stable/sbin/mdnsd | ||
| COPY --from=otbr-stable-builder /usr/lib/libdns_sd.so /usr/lib/libdns_sd.so | ||
| COPY --from=otbr-stable-builder /usr/lib/libdns_sd.so.1 /usr/lib/libdns_sd.so.1 | ||
|
|
||
| COPY rootfs / | ||
|
|
||
| ENV \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,7 +2,7 @@ | |||||
| version: 2.16.4 | ||||||
| slug: openthread_border_router | ||||||
| name: OpenThread Border Router | ||||||
| description: OpenThread Border Router | ||||||
|
There was a problem hiding this comment. The description field was shortened from "OpenThread Border Router add-on" to "OpenThread Border Router". While this change aligns with the rebranding from "add-on" to "app" seen in banner.sh, it's worth noting that this description is user-facing and removing "add-on" may make it less clear to users what this component is. Consider if "OpenThread Border Router App" would be clearer, or keep the original description for continuity.
Suggested change
|
||||||
| url: >- | ||||||
| https://github.com/home-assistant/addons/tree/master/openthread_border_router | ||||||
| arch: | ||||||
|
|
@@ -23,6 +23,8 @@ privileged: | |||||
| devices: | ||||||
| - /dev/net/tun | ||||||
| image: homeassistant/{arch}-addon-otbr | ||||||
| ingress: true | ||||||
| ingress_port: 8080 | ||||||
|
Comment on lines
+26
to
+27
There was a problem hiding this comment. While ingress is correctly enabled, the ports 8080/tcp and 8081/tcp are still defined in the configuration (lines 37-39, not shown in this diff). With ingress enabled and services now bound to localhost/addon IP, these port definitions should be removed to prevent users from accidentally exposing the services on the host network, which would bypass the ingress proxy and its security benefits. This is especially important given the PR description mentions ensuring nothing is listening on the host network interface.
Member
There was a problem hiding this comment. I think for now we want to keep them in case users want to access from extern. |
||||||
| init: false | ||||||
| options: | ||||||
| device: null | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,33 @@ | ||||||
| worker_processes 1; | ||||||
| pid /tmp/nginx.pid; | ||||||
| error_log /dev/stdout info; | ||||||
|
|
||||||
| events { | ||||||
| worker_connections 1024; | ||||||
| } | ||||||
|
|
||||||
| http { | ||||||
| include /etc/nginx/mime.types; | ||||||
| default_type application/octet-stream; | ||||||
| access_log /dev/stdout; | ||||||
|
|
||||||
| server { | ||||||
| listen 8080; | ||||||
|
There was a problem hiding this comment. The nginx server is listening on port 8080 without specifying a bind address, which means it will listen on all interfaces (0.0.0.0). For ingress to work securely, nginx should bind to 127.0.0.1 only, preventing direct external access and ensuring all traffic goes through the Home Assistant ingress proxy. Change line 15 to:
Suggested change
|
||||||
|
|
||||||
| # Proxy REST API requests to otbr-agent | ||||||
| location /api/ { | ||||||
| proxy_pass http://ADDON_IP:8081; | ||||||
| proxy_http_version 1.1; | ||||||
| proxy_set_header Host $host; | ||||||
| proxy_set_header X-Real-IP $remote_addr; | ||||||
| } | ||||||
|
|
||||||
| # Proxy everything else to otbr-web | ||||||
| location / { | ||||||
| proxy_pass http://127.0.0.1:8082; | ||||||
| proxy_http_version 1.1; | ||||||
| proxy_set_header Host $host; | ||||||
| proxy_set_header X-Real-IP $remote_addr; | ||||||
| } | ||||||
|
Comment on lines
+18
to
+31
There was a problem hiding this comment. The nginx proxy configuration is missing important security headers and proxy settings. Consider adding: 1) |
||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,13 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| #!/usr/bin/with-contenv bashio | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # vim: ft=bash | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # shellcheck shell=bash | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # ============================================================================== | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # Start nginx reverse proxy for OTBR web interface | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # ============================================================================== | ||||||||||||||||||||||||||||||||||||||||||||||||||
| bashio::log.info "Starting nginx..." | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| # REST API listens on app IP, so we need to know it for proxying | ||||||||||||||||||||||||||||||||||||||||||||||||||
| addon_ip="$(bashio::addon.ip_address)" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| sed "s/ADDON_IP/${addon_ip}/g" /etc/nginx/nginx.conf > /tmp/nginx.conf | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. The nginx service depends on otbr-web but not on otbr-agent, even though it proxies API requests to otbr-agent on port 8081 (line 19 of nginx.conf). This could cause nginx to start before otbr-agent is ready, resulting in failed API requests during startup. Consider adding an otbr-agent dependency file in the nginx/dependencies.d/ directory.
Suggested change
Member
There was a problem hiding this comment. There is
Comment on lines
+10
to
+12
There was a problem hiding this comment. The sed command on line 11 should include error handling. If bashio::addon.ip_address returns an empty value or sed fails, nginx will start with an invalid configuration containing the literal string "ADDON_IP", causing runtime errors. Consider adding validation:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| exec nginx -g "daemon off;" -c /tmp/nginx.conf | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| longrun |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,25 @@ | ||
| #!/usr/bin/with-contenv bashio | ||
| # shellcheck shell=bash | ||
| # ============================================================================== | ||
| # Home Assistant Community App: Base Images | ||
| # Displays a simple app banner on startup | ||
| # ============================================================================== | ||
| if bashio::supervisor.ping; then | ||
| bashio::log.blue \ | ||
| '-----------------------------------------------------------' | ||
| bashio::log.blue " App: $(bashio::addon.name)" | ||
| bashio::log.blue " $(bashio::addon.description)" | ||
| bashio::log.blue \ | ||
| '-----------------------------------------------------------' | ||
|
|
||
| bashio::log.blue " App version: $(bashio::addon.version)" | ||
| if bashio::var.true "$(bashio::addon.update_available)"; then | ||
| bashio::log.magenta ' There is an update available for this app!' | ||
| bashio::log.magenta \ | ||
| " Latest app version: $(bashio::addon.version_latest)" | ||
| bashio::log.magenta ' Please consider upgrading as soon as possible.' | ||
| else | ||
| bashio::log.green ' You are running the latest version of this app.' | ||
|
Comment on lines
+4
to
+22
There was a problem hiding this comment. The terminology has been changed from "Add-on" to "App" in multiple places. However, this is inconsistent with the rest of the codebase which still uses "add-on" terminology (e.g., in config.yaml comments line 16: "# IPC is only used within the Add-on"). This inconsistency may confuse users. Consider keeping consistent terminology throughout the codebase or updating all references together. |
||
| fi | ||
|
|
||
| bashio::log.blue " System: $(bashio::info.operating_system)" \ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The URL rewriting logic strips absolute HTTP URLs to make them relative for ingress compatibility. However, this regex only handles HTTP URLs (
http://), not HTTPS URLs (https://). If the web UI ever generates HTTPS absolute URLs, they won't be rewritten. Consider using a protocol-agnostic pattern:config.url.replace(/^https?:\/\/[^/]+/, '')to handle both HTTP and HTTPS URLs.