Bug 247357

Summary: net/librespeed-go: Update to 1.0.1 and simplify port
Product: Ports & Packages Reporter: Dries Michiels <driesm.michiels>
Component: Individual Port(s)Assignee: Koichiro Iwao <meta>
Status: Closed FIXED    
Severity: Affects Only Me CC: lcook, meta
Priority: --- Keywords: buildisok, patch
Version: Latest   
Hardware: Any   
OS: Any   
URL: https://github.com/librespeed/speedtest-go/releases/tag/v1.0.1
Attachments:
Description Flags
librespeed-go.diff
driesm.michiels: maintainer-approval+
librespeedgo.diff
driesm.michiels: maintainer-approval+
librespeedgo.diff
driesm.michiels: maintainer-approval+
librespeedgo.diff
driesm.michiels: maintainer-approval+
librespeedgo.diff driesm.michiels: maintainer-approval+

Description Dries Michiels 2020-06-17 20:02:26 UTC
Created attachment 215672 [details]
librespeed-go.diff

Changelog: https://github.com/librespeed/speedtest-go/releases/tag/v1.0.1

QA: Poudriere testport @ current amd 64

Because of the assets directory now being configurable, we can stick to hier(8) more better. I went for libexec for the initial port because binary and assets directory had to be in the same directory. This is not the case anymore.
Also fix some typo's from rc file copy from librenms :-).

I don't think a UPDATING entry is required here since the port is 2 days brand new. Breakage should be limited in location of configured landing pages. Furthermore, pkg-message also echo's the assets directory which is under %%WWWDIR%%.
Comment 1 Automation User 2020-06-17 20:28:49 UTC
Build info is available at https://gitlab.com/swills/freebsd-ports/pipelines/157368199
Comment 2 Koichiro Iwao freebsd_committer 2020-06-18 02:35:43 UTC
Overall, LGTM but I have some thoughts.

/var/db/speedtest.db might not be unique. It reminds me of other speedtest server database than LibreSpeed. What about creating subdirectory /var/db/librespeed or name like librespeed.db?
Comment 3 Dries Michiels 2020-06-18 06:18:35 UTC
Yea good idea, I see two solutions, either make a seperate dir eg /var/db/librespeed-go/speedtest.db, or chose another name for the database. eg /var/db/librespeed-go.db. 

What do we prefer? 

Both are clean, so I'm not in strong favor of any solution.
Comment 4 Dries Michiels 2020-06-18 06:20:56 UTC
Woops you already stated both solution I see :-) looked over that. Typically we select a separate dir in /var/db/ when we need separate permissions on it (IIRC). Since this program is running as root, I think I'll go with renaming the database file to something more unique. Will attach a patch later this day. Good catch btw!
Comment 5 Koichiro Iwao freebsd_committer 2020-06-18 08:17:50 UTC
(In reply to Dries Michiels from comment #4)

Another thought, 

The librespeed-go daemon runs under root privilege so far however I don't think it's necessary as far as daemon doesn't listen on tcp port less than 1024. It can be run under user privilege such as 'librespeed' user.

Some people including myself don't like giving unnecessary privileges to the daemon. So there's a chance that librespeed server is run in user privilege to be more secure. If users want to expose port 80 to librespeed server, the port can be forwarded by ipfw like this. 

> ipfw add 100 fwd 127.0.0.1,8989 tcp from any to me 80 in 

At this point, subdirectory is more flexible.

I'm not enforcing you to run the daemon under user privilege but what do you think?
Comment 6 Dries Michiels 2020-06-18 11:14:56 UTC
This is a good improvement for the port in terms of security, I'll prepare a patch later today :-).
Comment 7 Dries Michiels 2020-06-18 17:34:41 UTC
Created attachment 215737 [details]
librespeedgo.diff

This adds the feature to run the daemon as non-root user; with a new user librespeed.
Comment 8 Dries Michiels 2020-06-18 17:37:15 UTC
Created attachment 215738 [details]
librespeedgo.diff

Fix typo in rc script.
Comment 9 Dries Michiels 2020-06-18 17:40:23 UTC
Created attachment 215739 [details]
librespeedgo.diff

Really fix the typo .... (I introduced another one LOL).
Comment 10 Dries Michiels 2020-06-18 17:43:06 UTC
Created attachment 215740 [details]
librespeedgo.diff

I'm so sorry for the spam ... This hidden character didn't want to go away!
Comment 11 Dries Michiels 2020-06-18 17:44:01 UTC
I can't believe it, its still there? 
#				Default /var/run/%%PORTNAME%%/%%PĀ¹RTNAME%%.pid

I give up.
Comment 12 Koichiro Iwao freebsd_committer 2020-06-19 00:38:07 UTC
(In reply to Dries Michiels from comment #11)
No problem, I can remove it.
Comment 13 Koichiro Iwao freebsd_committer 2020-06-22 13:13:49 UTC
Committed with minor changes, thanks!

https://svnweb.freebsd.org/changeset/ports/539808
Comment 14 Dries Michiels 2020-06-22 13:32:46 UTC
Thanks Meta, I think we still need to commit UID, GID additions of the librespeed user/group. Thanks in advance! Dries
Comment 15 Koichiro Iwao freebsd_committer 2020-06-22 14:35:34 UTC
Oops, indeed.
Comment 16 commit-hook freebsd_committer 2020-06-22 14:55:09 UTC
A commit references this bug:

Author: meta
Date: Mon Jun 22 14:54:49 UTC 2020
New revision: 539810
URL: https://svnweb.freebsd.org/changeset/ports/539810

Log:
  net/librespeed-go: commit UIDs/GIDs

  missed in the previous commit r539808.

  PR:		247357

Changes:
  head/GIDs
  head/UIDs