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.

Sign up for GitHub

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

Jump to bottom

net/wireguard: Add dashboard widget #1865

Merged
merged 1 commit into from
Jul 1, 2020
Merged

net/wireguard: Add dashboard widget #1865

merged 1 commit into from
Jul 1, 2020

Conversation

jokay
Copy link
Contributor

@jokay jokay commented May 28, 2020

Adds a widget for WireGuard.

Relates #1862.

AdSchellevis
AdSchellevis requested changes Jun 3, 2020
View reviewed changes
Copy link
Member

@AdSchellevis AdSchellevis left a comment

Choose a reason for hiding this comment

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

see inline notes. If people start using this, I do expect the next feature request will be to refresh the widget content on changes. you might want to consider adding that as well, although I'm not against adding a static widget if @mimugmail is ok with it.

@jokay
Copy link
Contributor Author

jokay commented Jun 3, 2020

I do expect the next feature request will be to refresh the widget content on changes.

Absolutely, but this is what I can provide atm. To be honest, PHP is not really my strong point 😉
The widget will be updated on each load of the dashboard, this is imho better than nothing?

@AdSchellevis
Copy link
Member

@x-jokay no problem, let's ask @mimugmail if he accepts the feature as is so we can pull it in. The effort is certainly appreciated, we all have to start somewhere :)

@jokay jokay requested a review from AdSchellevis June 3, 2020 19:33
@mimugmail
Copy link
Member

Hi,

I installed your branch but I can't find the widget in the UI?
Did you also test you patch on a clean install?

@jokay
Copy link
Contributor Author

jokay commented Jun 4, 2020 •
edited
Loading

Did you also test you patch on a clean install?

No, it's untested. I have no idea what the requirements for an OPNsense plugin are.

This PR is only an "interpretation" based on some other plugins of what might be the correct change(s) to get this widget working.

@AdSchellevis
Copy link
Member

@x-jokay just checkout the plugins repo on an OPNsense and execute a make upgrade in the wireguard directory. Please make sure to test stuff before asking others for feedback, people are quite busy and you want to make sure their time is properly spend.

@mimugmail sorry about the noise, I was under the impression that it was tested, but had issues due to the configd bug.

@jokay jokay changed the title Add WireGuard widget WIP: Add WireGuard widget Jun 4, 2020
@jokay
Copy link
Contributor Author

jokay commented Jun 4, 2020

@AdSchellevis like this?

user@debian:~/GitHub/core/plugins/net/wireguard$ make upgrade
Makefile:7: *** missing separator.  Stop.

@AdSchellevis
Copy link
Member

on an OPNsense/HardenedBSD machine, yes, see https://github.com/opnsense/tools for information about our toolchain.

@mimugmail
Copy link
Member

On your OPNsense installation:

pkg install git
git clone https://github.com/x-jokay/plugins/
cd plugins && git checkout feature/wireguard-widget
cd net/wireguard
make upgrade

If you do any changes in github you can easily git pull && make upgrade

@jokay
Copy link
Contributor Author

jokay commented Jun 4, 2020

Thx, will setup a VM for testing.

@jokay jokay changed the title WIP: Add WireGuard widget Add WireGuard widget Jun 4, 2020
@jokay
Copy link
Contributor Author

jokay commented Jun 4, 2020

wireguard-widget-empty

wireguard-widget

@jokay
Copy link
Contributor Author

jokay commented Jun 4, 2020

@AdSchellevis & @mimugmail got it working now 👍

@jokay jokay changed the title Add WireGuard widget net/wireguard: Add dashboard widget Jun 6, 2020
@mimugmail
Copy link
Member

@x-jokay Is it now ready for testing? :)

@jokay
Copy link
Contributor Author

jokay commented Jun 9, 2020

@mimugmail yes, can be tested now.

@mimugmail
Copy link
Member

It works, thanks, but as soon as you have more than one column it misses a line break:

image

@jokay
Copy link
Contributor Author

jokay commented Jun 9, 2020

May be this is because of this trim().

@jokay jokay changed the title net/wireguard: Add dashboard widget WIP: net/wireguard: Add dashboard widget Jun 9, 2020
@jokay
Copy link
Contributor Author

jokay commented Jun 9, 2020

current

It worked for me using multiple interfaces, will do some more tests.

@jokay
Copy link
Contributor Author

jokay commented Jun 9, 2020

Well, this is indeed a problem 😉

ui-probs

@mimugmail may be you got an idea on how to get this one responsive?

@mimugmail
Copy link
Member

I would only display first 8 chars or endpoint, I'd guess noone is interested in full output

@jokay
Copy link
Contributor Author

jokay commented Jun 9, 2020

Good idea, will do it like this.

@jokay jokay changed the title WIP: net/wireguard: Add dashboard widget net/wireguard: Add dashboard widget Jun 10, 2020
@jokay
Copy link
Contributor Author

jokay commented Jun 10, 2020

ui-current

@jokay
Copy link
Contributor Author

jokay commented Jun 10, 2020

@mimugmail ready for the final verdict 😉

mimugmail
mimugmail approved these changes Jun 10, 2020
View reviewed changes
Copy link
Member

@mimugmail mimugmail left a comment

Choose a reason for hiding this comment

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

Great, thanks!

fichtner
fichtner reviewed Jun 11, 2020
View reviewed changes
net/wireguard/pkg-descr Outdated Show resolved Hide resolved
@fichtner fichtner self-assigned this Jun 11, 2020
@jokay jokay changed the title net/wireguard: Add dashboard widget WIP: net/wireguard: Add dashboard widget Jun 11, 2020
@jokay jokay changed the title WIP: net/wireguard: Add dashboard widget net/wireguard: Add dashboard widget Jun 11, 2020
@jokay
Copy link
Contributor Author

jokay commented Jun 11, 2020

@fichtner ready for another review.

@jokay
Copy link
Contributor Author

jokay commented Jun 20, 2020

@fichtner This is how it looks now.

current

@mimugmail
Copy link
Member

Looks good 👍

@DasSkelett
Copy link
Contributor

Also tried it out on my firewall, seems to work fine!

@jokay
Copy link
Contributor Author

jokay commented Jul 1, 2020

@AdSchellevis @fichtner please let me know if anything is missing so it can be merged.

@AdSchellevis
Copy link
Member

@x-jokay I'm very sorry, it has been extremely busy so I totally forgot about this one.

@AdSchellevis AdSchellevis merged commit 8353a29 into opnsense:master Jul 1, 2020
@fichtner fichtner assigned AdSchellevis and unassigned fichtner Jul 2, 2020
@jokay jokay deleted the feature/wireguard-widget branch July 24, 2020 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fichtner fichtner fichtner left review comments

@mimugmail mimugmail mimugmail approved these changes

@AdSchellevis AdSchellevis Awaiting requested review from AdSchellevis

Assignees

@AdSchellevis AdSchellevis

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jokay @AdSchellevis @mimugmail @DasSkelett @fichtner