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

DietPi-Software | frp #7292

Merged
merged 9 commits into from
Dec 1, 2024
Merged

DietPi-Software | frp #7292

merged 9 commits into from
Dec 1, 2024

Conversation

Joulinar
Copy link
Collaborator

  • switch from ini to toml configuration file because ini format is deprecated and the support will be removed in the future

DietPi-Software | frp - switch from ini to toml configuration file
@Joulinar Joulinar requested a review from MichaIng November 27, 2024 11:47
@Joulinar Joulinar self-assigned this Nov 27, 2024
@Joulinar Joulinar added this to the v9.9 milestone Nov 27, 2024
DietPi-Software | frp - add a separation for ARMv6 and ARMv7
@MichaIng
Copy link
Owner

MichaIng commented Dec 1, 2024

Shall we do some kind of migration from ini to toml?

@Joulinar
Copy link
Collaborator Author

Joulinar commented Dec 1, 2024

thougt about it, but we would need to check whole ini file for custom changes. Isn't it?

@MichaIng
Copy link
Owner

MichaIng commented Dec 1, 2024

Yeah, and the number of config keys is much too large to do all of this:

Sadly there is no internal conversion feature. So it looks like this is something we need to ask users to do manually. Best we can probably do is, checking whether the ini files exist, and if so, prompt an information, and rename them to *.bak (to not repeat the prompt).

Btw there is a way to generate bash completion files:

frpc completion bash
frps completion bash

Probably something for our docs?

- DietPi-Software | frp: Fix parsing client input, allow any characters for the token (as we do not know any limitations), and make use of the native G_WHIP_INPUTBOX input validation. Also expose the server bind address in our default config, which might be more commonly changed to limit access to particular networks.
- DietPi-Software | frp: Inform about new toml config file and leave old ini as backup in place. Do not offer client inputs, if the config exists already (and is hence not generated anyway). Allow an empty token input, which is generally possible. Allow unattended installs, using both server and client as default, and applying default values to all inputs and menus.
- DietPi-Globals | G_WHIP: Return default value for inputbox, checkbox and menu, if not interactive, but keep returning error code, so that it can be handled in parent script. Minor coding enhancements.
- CI | DietPi-Software: Enable frp checks now that it can be installed unattended
- CHANGELOG | Add frp changes
- DietPi-Software | frp: In case ESC is accidentally pressed, continue with default choice
- DietPi-Software | frp: G_WHIP_DEFAULT_ITEM is unset within G_WHIP call
- DietPi-Software | frp: Use default server address if ESC is accidentally hit to abort input
Copy link
Owner

@MichaIng MichaIng left a comment

Choose a reason for hiding this comment

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

Works well now. An unrelated issue is that the frp server can sometimes take longer to start than the frp client, and hence the client then fails to connect. It restarts up to 3 times, but during GitHub Actions test, for ARMv7 emulation, even that sometimes was not sufficient, but it failed ultimately. But I have no idea how to fix this properly. The daemon would need to return some systemd notify call to lock the start sequence until a defined state, like when the TCP listener is up. But as they removed their systemd examples a while ago, I don't think this has any priority.

@Joulinar
Copy link
Collaborator Author

Joulinar commented Dec 1, 2024

Yes have noticed this as well. At least until know we don't have a report this being an issue. Should be fine i guess

@MichaIng
Copy link
Owner

MichaIng commented Dec 1, 2024

Yeah, and if needed, we can raise RestartSec, to wait longer until a restart is schedulsd (currently 5 seconds), to give the server more time to start. This seems to be cleanest, without impacting common operation, like a ExecStartPre=/bin/sleep X would do.

@MichaIng MichaIng merged commit ec89b23 into dev Dec 1, 2024
2 checks passed
@MichaIng MichaIng deleted the frp branch December 1, 2024 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants