-
Notifications
You must be signed in to change notification settings - Fork 352
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
ipoe_server: T6872: Add the ability to configure LUA scripts and username #4196
Conversation
👍 |
<help>Username function</help> | ||
<valueHelp> | ||
<format>txt</format> | ||
<description>Name of function in lua file to construct username</description> |
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.
<description>Name of function in lua file to construct username</description> | |
<description>Name of the function in the Lua file to construct usernames with</description> |
"Lua" is a proper name and should be capitalized.
#include <include/accel-ppp/vlan.xml.i> | ||
#include <include/accel-ppp/vlan-mon.xml.i> | ||
</children> | ||
</tagNode> | ||
<leafNode name="lua-file"> | ||
<properties> | ||
<help>File containing lua function for create username</help> |
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.
<help>File containing lua function for create username</help> | |
<help>Lua script file for constructing user names</help> |
<help>File containing lua function for create username</help> | ||
<valueHelp> | ||
<format>filename</format> | ||
<description>File with LUA script</description> |
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.
<description>File with LUA script</description> | |
<description>File with Lua script</description> |
"Lua" is not an acronym, it's a Portugese word that means "the moon".
src/conf_mode/service_ipoe-server.py
Outdated
'use "client-ip-pool" instead!') | ||
if 'vlan_mon' in iface_config and not 'vlan' in iface_config: | ||
raise ConfigError( | ||
'Option "client-subnet" and "vlan" are mutually exclusive, ' |
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.
'Option "client-subnet" and "vlan" are mutually exclusive, ' | |
'Options "client-subnet" and "vlan" are mutually exclusive, ' |
Missed plural.
src/conf_mode/service_ipoe-server.py
Outdated
raise ConfigError(f'File {ipoe["lua_file"]} does not exist') | ||
if dict_search('authentication.mode', ipoe) != 'radius': | ||
raise ConfigError( | ||
'Can configure username with LUA script only for RADIUS authentication' |
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.
'Can configure username with LUA script only for RADIUS authentication' | |
'Can configure username with Lua script only for RADIUS authentication' |
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.
Looks good to me now. I think the motivation for hard restart should be documented in the comments, so that no one wonders about it in the future — or, when accel-ppp implements a soft reload, can spot it as untrue and change it.
CI integration 👍 passed! Details
|
Change Summary
Types of changes
Related Task(s)
Related PR(s)
Component(s) name
service ipoe-server
Proposed changes
added ability to configure username with LUA script
Also changed systemctl action from
reload-or-restart
torestart
because accel-ppp doesn't apply changes to the configuration withreload-or-restart
actionHow to test
Example of the lua file:
Config:
Logs
Smoketest result
Checklist: