luci-app-nordvpnlite: update to meet all feature. by pesa1234 · Pull Request #1801 · NordSecurity/libtelio

pesa1234 · 2026-06-03T19:15:12Z

Dear NordVPN team, as suggest I reopen it and I hope now it will be reviewed by the team.

Extend the NordVPN Lite LuCI backend ACLs and RPC handlers so the settings page can query service state, runtime status, country lists and per-server WireGuard metadata.

Rework the settings view to support recommended, country and specific server selection, fetch country and server data on demand, expose service start/restart/stop/enable/disable controls, and restart the service after Save & Apply while polling for the updated runtime state.

Example:

Changes:

Port the expanded LuCI settings view from the standalone luci-app-nordvpnlite app.
Add service enabled flag handling through UCI.
Add service status and control actions for start, restart, stop, enable and disable.
Add runtime status lookup and display for Telio state, tunnel IP and exit node details.
Add VPN selection modes for recommended server, country code and specific server.
Add country list lookup from the nordvpnlite command.
Add NordVPN server lookup using the public API, hostname normalization and automatic IP/public key fill.
Expand the rpcd ucode backend with the new config, service, lookup and runtime status methods.
Expand the LuCI ACL grants and update the menu ACL dependency for the app.
Add runtime dependencies required by the new LuCI backend: curl, jq and ca-bundle.

Problem

Not so useful Luci app

Solution

After merged is more complete

Run tested on openwrt snapshot

jjanowsk · 2026-06-09T13:42:55Z

Hi! Just to let you know, I've just started looking at it. Most likely I'll send you my feedback tomorrow. I've also send the ipk to our QA engineer so he can give his opinion on the UI/UX. Thanks again for the contribution and for your patience

jjanowsk

Hi! I'll leave my comments free standing because they are mostly UI/UX related. Once we polish that I'll review the code itself.

Major:

I'm having trouble understanding what is the point of "Enabled" button. Can you please explain that?
It's a little counter intuitive when you install the app, paste the "Authentication token" and try to start the service for the first time. It will fail, because the token is not yet saved in the config file. It's obvious to us what's happening, but not very clear to the user. It might be hard to figure out that he needs to first click the "Save & Apply" button, and only then he should click the "Start" button. Moreover the app just hangs in that situation.
Similar situation happens when the user changes configuration e.g. from recommended server to the server from the country list. Changing those settings and clicking restart does not apply the new configuration. That's again counter-intuitive.
Touching on the server configuration. I'm not sure I'm sold on the idea of selecting "Specific server" via hostname. Right now the nordvpnlite config allows only configuration via pub key and IP address and we could leave it this way in the OpenWRT UI. The problem I see with the current solution is that you need to query ~30MB server list and filter it in memory on the device. It can be slow. Also, by dropping this feature we can get rid of extra dependencies for our app.
Why is clicking "Disable autostart", blocking "Start", "Restart", "Stop" buttons? It seems to be a bug.

Minor:

Status texts should not end with ".". E.g. "Stopped" not "Stopped."
Field names and field values are unaligned, at least in chrome:

pesa1234 · 2026-06-20T11:39:39Z

Hi @jjanowsk,
thank you for the detailed feedback. I’ve addressed all the points.

The service switch is a runtime master switch stored in /etc/config/nordvpnlite. Unlike autostart, it determines whether the init script is allowed to start the daemon.
I renamed it to “Enable service”, clarified its description and aligned the embedded init script with the OpenWrt package implementation.
Start and Restart now validate and save the current form configuration before controlling the service. A startup failure also produces a notification instead of leaving the UI apparently hanging.

Consequently, changing the selected country or other settings and clicking Restart now applies the new configuration.

I removed hostname-based server lookup. Specific server configuration now accepts the IP address and WireGuard public key directly, matching the native nordvpnlite configuration. The full server-list download and the curl, jq and ca-bundle dependencies were removed.

Autostart is now independent from runtime service controls. Disabling autostart no longer blocks Start, Restart or Stop.

I also removed trailing periods from status texts and adjusted the status field alignment.

Thanks again for the review!

jjanowsk · 2026-07-07T10:51:10Z

Hi @pesa1234! Thanks for the updates. I was on vacation, but I'm back now and I'll review it this week

jjanowsk

Hi! Found a few technical issues. The main issue for me though is this enable button. I still don't understand why we would need that tbh. You can try to convince me, because maybe I'm missing someting, but in my opinion it is just adding unnecessary complications without any benefit.

jjanowsk · 2026-07-28T16:12:38Z

+            if (view.vpn_mode_option.formvalue('config') !== 'server')
+                return true;
+
+            return String(value || '').trim() !== '' ? true : _('Please provide the server public key.');


Similar problem with _ shadowing here.

jjanowsk · 2026-07-29T04:57:27Z


+function has_service() {
+	let st = fs.stat(INIT_SCRIPT);
+	return st && st.type == 'file' && st.user_exec != false;


st.perm.user_exec

jjanowsk · 2026-07-29T05:06:12Z

+	return trim(output);
+}
+
+function config_enabled() {


I still don't know what we are gaining by adding this enabled flag to be honest. I would gladly get rid of it unless you have some valid reason for keeping it. It would clean up the logic in this PR by not having to deal with weird states like: Running (service disabled).

jjanowsk · 2026-07-29T05:12:22Z

-    handleSaveApply: null,
+    handleSave: null,
+
+    handleSaveApply: function () {


Save and apply previously was only saving the configuration and I think it was a correct decision. I've asked you to also save the configuration when you click start, so the configuration visible in the UI is applied to the start. But now you've also added restarting to the save button, even when the service is not running. This I think is incorrect. (Maybe you have not added it now, maybe I've missed that during the first review.)

jjanowsk · 2026-07-29T08:01:58Z

+        if (typeof widget.addChoices === 'function')
+            widget.addChoices(keys, choices);
+
+        widget.setValue(code !== '' ? code : '');


it's just equivalent to: widget.setValue(code);

jjanowsk · 2026-07-29T08:11:44Z

+		};
+	}
+
+	let output = read_command_output(sprintf('%s countries 2>/dev/null', SERVICE_NAME));


In case of lack of internet this can hang for a long time. I think we should add some kind of timeout mechanism here.

asledzinskiy · 2026-07-29T08:44:06Z

One minor scenario that is not working for me:

Pre-condition - nordvpnlite is not running
Go to UI and click Get status button

Actual result - banner with message 'Failed to get status'
Expected - Service Status is displaying Not Running

asledzinskiy · 2026-07-29T12:23:00Z

And also another failed test case:

Open Services → NordVPN Lite.
Set VPN Selection = Specific server.
Leave Server Public Key empty
Click Save & Apply.
Expected: validation error. Actual: red TypeError: _ is not a function banner

Was able to fix it with:

sed -i 's/function (_, value)/function (section_id, value)/g' \
  /www/luci-static/resources/view/nordvpnlite/settings.js

pesa1234 · 2026-08-09T09:44:25Z

Hi! Found a few technical issues. The main issue for me though is this enable button. I still don't understand why we would need that tbh. You can try to convince me, because maybe I'm missing someting, but in my opinion it is just adding unnecessary complications without any benefit.

All the request should be addressed, thanks for point it... please check

jjanowsk

Hi @pesa1234! Thanks again for your contribution. I've checked the code and it's +1 from me. There are now few minor things left, and I can handle them for you if you agree. Of course your commits will be left marked as authored by you, but I can just move them to my branch and do those administrative things. If you prefer to do it then I need you to:

Rebase to fix deny and black lints
Create .unreleased/luci_improvements file, and add short description there e.g.: nordvpnlite: OpenWRT UI improvements: Added service control
Take content of dod.yml file from the root of libtelio's repo and put it in the top of your PR description. Mark all three points as done. (This part TBH I'm not sure will work for external contribution, so that's why I would prefer to just do it for you to avoid any hustle. We don't have many external contributors;))

Few small issues left which I can also fix:

You've changed the behaviour of Save&Apply as I've asked you, but I think now the name does not match. We should use Save handler for that.
timeout is not default on all openwrt images. Please add +coreutils-timeout to the dependency list.
logout ucode method is unreachable. Let's get rid of it.

Let me know if you want me to handle that. If so I'll prepare new PR and link this one there.

pesa1234 · 2026-09-01T06:56:51Z

Hi @jjanowsk, thanks a lot for the review and for the +1!

I can take care of the remaining technical fixes on my side if you want — just let me know:

use the correct Save handler;
add coreutils-timeout;
remove the logout call.

If you’re happy with that, I’ll update this PR accordingly.

For the repository-specific/admin parts, especially the DoD workflow and anything that is easier to handle internally, I’d really appreciate your help.

I can also test the updated package on a real OpenWrt device afterwards, including the LuCI UI, service controls, Save/Start/Restart behaviour, and runtime status.

Thanks again for helping me get this into shape for merging!

One more thing: as long as there is no free plan, even with limited bandwidth or a restricted server selection, I won’t be able to prepare a PR for the official OpenWrt packages repository.

It might be worth considering a small free tier, as that could make it easier for more OpenWrt users and contributors to test the package and potentially increase adoption.

Andrea

jjanowsk · 2026-09-01T08:36:19Z

Ok, so please do those 3 + rebase, and I'll take if from there. Thanks again! Regarding your suggestion, it's outside of my scope, but I'll pass the information to the relevant people.

Just one note about the rebase: Please do it first, and force push here, and only then do your fixes. This way I'll have nice "compare" button here on github to just see your latest changes. Thanks!

Changes: - Port the expanded LuCI settings view from the standalone luci-app-nordvpnlite app. - Add service enabled flag handling through UCI. - Add service status and control actions for start, restart, stop, enable and disable. - Add runtime status lookup and display for Telio state, tunnel IP and exit node details. - Add VPN selection modes for recommended server, country code and specific server. - Add country list lookup from the nordvpnlite command. - Add NordVPN server lookup using the public API, hostname normalization and automatic IP/public key fill. - Expand the rpcd ucode backend with the new config, service, lookup and runtime status methods. - Expand the LuCI ACL grants and update the menu ACL dependency for the app. - Add runtime dependencies required by the new LuCI backend: curl, jq and ca-bundle.

Save pending settings before starting or restarting the service while keeping normal Save & Apply side-effect free and runtime controls independent from autostart. Align the LuCI backend with the separate auth file and login API, preserve the upstream reload flow, remove the redundant service-enabled switch, bound external lookups, handle stopped status cleanly, and update the OpenWrt UI page object.

pesa1234 · 2026-09-01T09:42:48Z

@jjanowsk can you please check now, all the things should be done

jjanowsk · 2026-09-11T13:17:22Z

Merged. Thanks again @pesa1234 for your contribution!

pesa1234 requested a review from a team as a code owner June 3, 2026 19:15

pesa1234 mentioned this pull request Jun 3, 2026

luci: add nordvpnlite service controls and server lookup #1747

Closed

olekoliinyk added the run tests PR is ready for CI label Jun 10, 2026

olekoliinyk temporarily deployed to External June 10, 2026 08:30 — with GitHub Actions Inactive

jjanowsk reviewed Jun 11, 2026

View reviewed changes

olekoliinyk added run tests PR is ready for CI and removed run tests PR is ready for CI labels Jul 2, 2026

olekoliinyk deployed to External July 2, 2026 09:49 — with GitHub Actions Active

jjanowsk requested changes Jul 29, 2026

View reviewed changes

pesa1234 force-pushed the luci-fix-nordvpnlit branch from 4fbbd2c to 52b055a Compare August 9, 2026 09:37

pesa1234 requested a review from jjanowsk August 10, 2026 15:23

jjanowsk reviewed Sep 1, 2026

View reviewed changes

pesa1234 added 2 commits September 1, 2026 11:36

pesa1234 force-pushed the luci-fix-nordvpnlit branch from 52b055a to b9f97ce Compare September 1, 2026 09:37

luci: address remaining review feedback

2e4f099

olekoliinyk added run tests PR is ready for CI and removed run tests PR is ready for CI labels Sep 7, 2026

olekoliinyk deployed to External September 7, 2026 09:45 — with GitHub Actions Active

jjanowsk mentioned this pull request Sep 8, 2026

Llt 7057 openwrt UI improvements #1979

Merged

3 tasks

github-merge-queue Bot closed this pull request by merging all changes into NordSecurity:main in e1eb075 Sep 11, 2026

pesa1234 deleted the luci-fix-nordvpnlit branch September 11, 2026 19:32

Uh oh!

Conversation

pesa1234 commented Jun 3, 2026

Problem

Solution

Uh oh!

jjanowsk commented Jun 9, 2026

Uh oh!

jjanowsk left a comment

Choose a reason for hiding this comment

Uh oh!

pesa1234 commented Jun 20, 2026

Uh oh!

jjanowsk commented Jul 7, 2026

Uh oh!

jjanowsk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jjanowsk Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

jjanowsk Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

jjanowsk Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

jjanowsk Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

jjanowsk Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

jjanowsk Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

asledzinskiy commented Jul 29, 2026

Uh oh!

asledzinskiy commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pesa1234 commented Aug 9, 2026

Uh oh!

jjanowsk left a comment

Choose a reason for hiding this comment

Uh oh!

pesa1234 commented Sep 1, 2026

Uh oh!

jjanowsk commented Sep 1, 2026

Uh oh!

pesa1234 commented Sep 1, 2026

Uh oh!

jjanowsk commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asledzinskiy commented Jul 29, 2026 •

edited

Loading