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

Ntopng config UI changes to allow arbitrary local networks list #366

Merged
merged 4 commits into from
Jul 13, 2017
Merged

Ntopng config UI changes to allow arbitrary local networks list #366

merged 4 commits into from
Jul 13, 2017

Conversation

madpilot78
Copy link
Contributor

ntopng allows an arbitrary list of networks to be considered as "local".

I've added ability to configure such an arbitrary list to the pfsense ntopng UI.

To accommodate for such options I have reorganised a little the UI layout.

I'm submitting it hoping it can be accepted in your official tree. If any changes in UI layout or functionality is required please comment on this pull request.

Thanks!

@madpilot78 madpilot78 changed the title Ntopng config changes Ntopng config UI changes to allow arbitrary local networks list Jun 6, 2017
@rbgarga rbgarga requested a review from jim-p June 10, 2017 20:13
$nets = array();
foreach ($ntopng_config['row'] as $net) {
if (is_subnet($net['cidr'])) {
$nets[] = $net['cidr'];
Copy link
Contributor

Choose a reason for hiding this comment

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

To be on the safe side, you should use trim() around the right side, to be absolutely certain there is no whitespace around any of the values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I should have thought about this right away.

Fixing it shortly.

@madpilot78
Copy link
Contributor Author

I noticed the pfSense-pkg-ntopng package version has already been bumped to 0.8.7.

Should I add a commit bumping it further to 0.8.8?

@jim-p
Copy link
Contributor

jim-p commented Jun 12, 2017

Yes, you'll want to rebase and then bump it again.

@madpilot78
Copy link
Contributor Author

madpilot78 commented Jul 3, 2017

Oops, I just wanted to rebase and bump portrevision but made a git mess somehow.

Not sure how to fix it.

Any suggestions?

EDIT: Fixed it myself, sorry for the noise!

@doktornotor
Copy link
Contributor

Perhaps someone could commit this before it gets messed up again? 😛

@netgate-git-updates netgate-git-updates merged commit f65a634 into pfsense:devel Jul 13, 2017
@madpilot78 madpilot78 deleted the ntopng_config_changes branch July 14, 2017 15:33
@dennypage
Copy link
Contributor

Guys, there's a couple of bugs here. Even though the custom networks option is not selected, the custom networks input field is still shown. More significantly, the custom networks input field is checked for validity even if the custom networks option is not selected. An empty list is not considered valid. This means that everyone's prior config from 0.8.8 is considered invalid and they cannot re-save a configuration without entering a valid cidr.

@dennypage
Copy link
Contributor

And yes, putting a valid CIDR in the field does allow the config to be saved. I've only tested a couple, but a common one, "192.168.0.0/16" works fine.

@madpilot78
Copy link
Contributor Author

Just for the record, I have submitted pull request #372 which contains a fix for this issue.

netgate-git-updates pushed a commit that referenced this pull request Jan 6, 2018
ChangeLog:
 - Critical Bugfix: Syntax error in image script could break pip installs (#366)
 - CLI: Regression fix: English apps now load as fast as in 1.6.3 (#364)
 - CLI: Missing colon restored in group names
 - Regression fix: Restored non-setuptools installs (#367)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants