luci icon indicating copy to clipboard operation
luci copied to clipboard

luci-app-tailscale-community: Init checkin

Open Tokisaki-Galaxy opened this issue 4 months ago • 22 comments

add new package. use to gui manage tailscale configuare https://github.com/Tokisaki-Galaxy/luci-app-tailscale-community/

  • [x] This PR is not from my main or master branch :poop:, but a separate branch :white_check_mark:

  • [x] Each commit has a valid :black_nib: Signed-off-by: <[email protected]> row (via git commit --signoff)

  • [x] Each commit and PR title has a valid :memo: <package name>: title first line subject for packages

  • [x] Incremented :up: any PKG_VERSION in the Makefile

  • [x] Tested on: (aarch64, openwrt 24.10.03, Edge) :white_check_mark:

  • [ ] ( Preferred ) Mention: @ the original code author for feedback

  • [x] ( Preferred ) Screenshot or mp4 of changes: setting status

  • [ ] ( Optional ) Closes: e.g. openwrt/luci#issue-number

  • [x] ( Optional ) Depends on: e.g. openwrt/packages#pr-number in sister repo LUCI_DEPENDS:=+tailscale +coreutils-base64

  • [x] Description: (describe the changes proposed in this PR)

luci-app-tailscale: Add Tailscale WebUI application

This Pull Request introduces a comprehensive LuCI Web Interface for the Tailscale VPN service. This application provides users with a user-friendly way to configure their Tailscale node settings and monitor the status of their virtual network (tailnet) directly from the OpenWrt administration panel.

I confirm that I have tested this application thoroughly and it adheres to the LuCI coding guidelines, including full i18n support and use of the modern luasrc structure.

Key Features

  • Status Overview: Displays core node information (running status, IPv4/IPv6 address, Tailnet name).
  • Peer Status Table: Lists all connected network devices (peers) in the tailnet, showing their hostname, IPs, OS, connection status (Direct/Relay), and last seen time.
  • Node Settings Configuration: Allows users to manage instant node configurations via tailscale set commands, including:
    • Accept/Advertise Routes
    • Advertise as Exit Node
    • Use Exit Node / Allow LAN Access
    • SSH, Shields Up, Auto-Updates, Custom Hostname
  • Daemon Environment Settings: Provides an interface to set service-level environment variables (e.g., Custom MTU, Memory Reduction optimization GOGC=10), which trigger a service restart upon commit.

Technical Implementation Details

  1. CBI & Status Polling: The application uses the CBI framework for configuration and relies on polling data from the tailscale status and tailscale status --json commands via a custom data model (luci.model.tailscale_data).
  2. Configuration Separation:
    • Instant Settings: Node settings are applied immediately on on_after_commit using tailscale set ....
    • Daemon Settings: Environment variables are written to /etc/profile.d/tailscale-env.sh and require a service restart. The PR code correctly handles the service restart only when these daemon settings are changed.
  3. Dependency: This application requires the tailscale core package to be installed to function. The Makefile correctly sets +tailscale +coreutils-base64.
  4. Internationalization: Full i18n support is implemented with po/ files included.

Test Notes

Tested on Openwrt 24.10.03 on a aarch64 device using the latest tailscale package.

Tokisaki-Galaxy avatar Oct 19 '25 04:10 Tokisaki-Galaxy

Problem: LUA

No new lua additions are accepted into luci. See #3378 and #7310 Migrate to ucode, or javascript. Both of which provide base64 en/decode facilities inbuilt.

systemcrash avatar Oct 19 '25 11:10 systemcrash

First, let me express my sincere gratitude for your incredible work on the LuCI framework. It's a powerful and flexible platform.

I am currently in the process of refactoring lua into the ucode+js framework. I've successfully implemented the core functionality, but I've hit a stubborn roadblock that I'm hoping you can provide some guidance on.

My application's goal is not to simply manage a UCI config file in the traditional sense. Instead, its primary purpose is to execute a specific command (tailscale set --option=value ...) via a ucode RPC call. The UCI file is essentially used as a temporary vehicle to pass the user's settings from the frontend form to the backend script.

The process works flawlessly: the user changes settings, clicks "Save & Apply," my custom handleSaveApply function is triggered, it calls the ucode RPC, the tailscale command is executed successfully, and the service is configured.

The Problem: However, after the handleSaveApply function completes and the page reloads, the "Unsaved Changes: N" warning persists at the top of the page. This is confusing for the end-user, as their changes have already been successfully applied.

I understand this is happening because the form.Map object still considers itself to be in a "dirty" state. I have tried several methods to resolve this:

  1. map.reset(): My first thought was to call this.map.reset() within the handleSaveApply function after the RPC call succeeds. Unfortunately, this does not seem to have the desired effect, and the warning remains after the page reloads.

  2. Command-Line UCI: I also considered attempting to clear the changes via a command-line approach, but I quickly realized that the shell's UCI context (and its buffer) is separate from the one used by the LuCI web interface, making this path unviable.

My question is: What is the correct, canonical method within a custom handleSaveApply function to signal to the LuCI framework that the custom actions have completed successfully and that the form's state should be considered 'clean'?

Here is a simplified version of my handleSaveApply function for context:

handleSaveApply: function (ev) {
    var map = this.map;
    return map.save().then(function () {
        var data = uci.get('tailscale', 'settings');
        ui.showModal(_('Applying changes...'), E('em', {}, _('Please wait.')));

        return callSetSettings(data).then(function (response) {
            if (response.success) {
                // I have tried map.reset() and this.map.reset() here,
                // but the "Unsaved Changes" warning persists after reload.
                map.reset(); 

                ui.hideModal();
                ui.addNotification(null, E('p', _('Settings applied successfully.')), 'info');
                setTimeout(function () { window.location.reload(); }, 2000);
            } else {
                // Error handling...
                ui.hideModal();
                ui.addNotification(null, E('p', _('Error: %s').format(response.error)), 'error');
            }
        });
    }).catch(function(err) {
        // General error handling...
        ui.hideModal();
        ui.addNotification(null, E('p', _('Failed to save: %s').format(err.message)), 'error');
    });
},

As a side note, I'm learning the hard way how critical getting these details right is. During my development process, a misconfigured backend script (triggered by my LuCI view) unfortunately caused a system lock-up, which prevented all remote access (including SSH) and required physical intervention to recover my router. sad:(

Any guidance or example you could offer on the proper way to handle this "stateless" save-and-apply scenario would be immensely helpful and deeply appreciated.

Thank you again for your time and for maintaining such a fantastic platform.

Tokisaki-Galaxy avatar Oct 20 '25 13:10 Tokisaki-Galaxy

You're overriding handleSaveApply, so any of the original functionality which performs the save and commit actions is now replaced by your version (meaning nothing was committed that you changed in the form). I'm not sure why you'd get the settings when you're saving. The save action reloads the page. You should be loading the settings at page load in the load promise => (uci.load('tailscale'),)actions.

systemcrash avatar Oct 20 '25 13:10 systemcrash

The 'get the settings and send them to the daemon' should generally happen in an init (procd) script, i.e. in its package in the packages repo.

systemcrash avatar Oct 20 '25 13:10 systemcrash

Thank you so much for your insightful reply. It was extremely helpful and clarified the "standard" OpenWrt development pattern perfectly. I now understand that the ideal place for the configuration logic is within the service's init script.

I realize I didn't provide the full context in my first message, which is my oversight. The reason I am using a custom handleSaveApply is that my project is not a new package, but rather a LuCI view for the official, pre-existing Tailscale package.

This official package provides its own service file (/etc/init.d/tailscale), which I should not modify. This service file does not read from the UCI configuration file I am managing.

Therefore, my custom handleSaveApply and RPC backend are intentionally designed to act as a "bridge" between LuCI's UCI-based world and the command-based reality of the Tailscale daemon (tailscale set ...). This seems to be the only way to make them work together without altering the official package's files.

Given this "adapter" architecture, I am still facing the original challenge: the "Unsaved Changes" warning persists after my custom logic has successfully applied the settings.

I have re-attempted to use map.reset() inside the success handler of handleSaveApply, but it doesn't seem to clear the dirty state after the page reloads.

So, my question is: Within this specific architecture where a custom handleSaveApply is necessary, what is the correct method to programmatically clear the form's dirty state and prevent the "Unsaved Changes" warning from appearing after a successful apply action?

Any insight you could provide would be the final key to solving this puzzle. Thank you again for your time and invaluable help

Tokisaki-Galaxy avatar Oct 20 '25 14:10 Tokisaki-Galaxy

You're better off writing a ucode script to handle this, so it takes your uci settings and writes out the JSON. ucode is basically javascript.

systemcrash avatar Oct 20 '25 18:10 systemcrash

image

Thank you for the guidance. Following your feedback, I have completed the refactoring from Lua to ucode and JavaScript, addressing the policy against new Lua additions. The pull request has been updated accordingly and is now ready for your review. Please let me know if further changes are required.

Tokisaki-Galaxy avatar Oct 21 '25 13:10 Tokisaki-Galaxy

Apologies for the extra merge commit. I used the 'Update branch' button to sync with the latest master, and I realize now that this created a merge commit. If it's easier for you, please feel free to squash all commits into one when you merge. I will use a rebase workflow for future updates. Thank you for your time and guidance!

Tokisaki-Galaxy avatar Oct 21 '25 13:10 Tokisaki-Galaxy

Apologies for the extra merge commit. I used the 'Update branch' button to sync with the latest master, and I realize now that this created a merge commit. If it's easier for you, please feel free to squash all commits into one when you merge. I will use a rebase workflow for future updates. Thank you for your time and guidance!

Yes - we don't accept merges. Just rebase in future. Thanks!

systemcrash avatar Oct 21 '25 14:10 systemcrash

Thank you again for your detailed review and guidance. I have addressed the feedback you provided and pushed the updated code. Please let me know if any further changes are needed.

Tokisaki-Galaxy avatar Oct 22 '25 10:10 Tokisaki-Galaxy

@Tokisaki-Galaxy Any plan to add support for netifd to this? See ZeroTier MR

miyatsu avatar Oct 28 '25 07:10 miyatsu

@Tokisaki-Galaxy Any plan to add support for netifd to this? See ZeroTier MR

good idea. but i think this is the tailscale task of package. I just provide luci GUI

Tokisaki-Galaxy avatar Oct 28 '25 07:10 Tokisaki-Galaxy

I modified the code as required. Thank you very much for your careful check.

Tokisaki-Galaxy avatar Oct 28 '25 14:10 Tokisaki-Galaxy

Thanks for pointing out that it has been added \n

Tokisaki-Galaxy avatar Oct 29 '25 04:10 Tokisaki-Galaxy

I missed it when I checked this morning. I'm very sorry.

Tokisaki-Galaxy avatar Oct 29 '25 15:10 Tokisaki-Galaxy

OK - squash all your commits and rebase on master please.

systemcrash avatar Oct 29 '25 18:10 systemcrash

done. I should have finished these, please check them for me.

Tokisaki-Galaxy avatar Oct 30 '25 08:10 Tokisaki-Galaxy

Deamon settings need further debugging, and it seems that this function is not available in tailscale in opkg source. Temporarily disable

Tokisaki-Galaxy avatar Oct 31 '25 13:10 Tokisaki-Galaxy

https://github.com/openwrt/packages/issues/27740 Due to the problem of tailscale package, this function cannot be provided for the time being. If it is forcibly used, tailscale will be completely unusable.

Tokisaki-Galaxy avatar Nov 01 '25 02:11 Tokisaki-Galaxy

I could not identify any issue with the packaging. There is likely some logic bug or missunderstanding here.

SuperSandro2000 avatar Nov 01 '25 03:11 SuperSandro2000

Hi @systemcrash,

Thanks again for all your detailed feedback.

Just wanted to post a quick update to clarify the current scope of this PR. Regarding the daemon functionality we discussed earlier, I've decided to leave that for a future PR to keep this initial version focused and easier to review. I believe I have addressed all the other points from your last review. Please let me know if there's anything else I can do to get this ready for merging.

Thanks!

Tokisaki-Galaxy avatar Nov 03 '25 03:11 Tokisaki-Galaxy

Hi @systemcrash I see the automated checks have skips. This is my first contribution to the LuCI project, and I'm very excited about it!

Could you please take a look when you get a chance, or let me know if there are any specific guidelines or improvements I should address? Thanks for your time and guidance

Tokisaki-Galaxy avatar Nov 15 '25 14:11 Tokisaki-Galaxy

update pr to lastest version.

  • add auto config tailscale firewall
  • fix config exit node bug

I configured copilot automatic review incorrectly before, and I'm very sorry.

Tokisaki-Galaxy avatar Jan 22 '26 02:01 Tokisaki-Galaxy

[!WARNING]

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an :x: are failing checks.

Commit 33a8cda597e9ccf106a79653ab745d46a62597a4

  • :large_orange_diamond: Author name (Tokisaki-Galaxy) seems to be a nickname or an alias
  • :large_orange_diamond: Committer name (Tokisaki-Galaxy) seems to be a nickname or an alias
  • :x: First word after prefix in subject should not be capitalized

For more details, see the full job log.

Something broken? Consider providing feedback.

github-actions[bot] avatar Jan 22 '26 06:01 github-actions[bot]

[!WARNING]

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an :x: are failing checks.

Commit fd56a03ef6edd51a71f2d910d435a2a86a9e17a2

  • :large_orange_diamond: Author name (Tokisaki-Galaxy) seems to be a nickname or an alias
  • :large_orange_diamond: Committer name (Tokisaki-Galaxy) seems to be a nickname or an alias
  • :x: First word after prefix in subject should not be capitalized

For more details, see the full job log.

Something broken? Consider providing feedback.

github-actions[bot] avatar Jan 22 '26 06:01 github-actions[bot]

[!WARNING]

Some formality checks failed.

Consider (re)reading submissions guidelines.

Failed checks

Issues marked with an :x: are failing checks.

Commit fc7de69f67b70b3954d3917508d6f7e4383f16f1

  • :large_orange_diamond: Committer name (Tokisaki-Galaxy) seems to be a nickname or an alias

For more details, see the full job log.

Something broken? Consider providing feedback.

github-actions[bot] avatar Jan 22 '26 09:01 github-actions[bot]

Merged. Thanks @Tokisaki-Galaxy !

systemcrash avatar Jan 23 '26 03:01 systemcrash