247357 – net/librespeed-go: Update to 1.0.1 and simplify port

Bug 247357 - net/librespeed-go: Update to 1.0.1 and simplify port

Summary: net/librespeed-go: Update to 1.0.1 and simplify port

Status:	Closed FIXED

Alias:	None

Product:	Ports & Packages
Classification:	Unclassified
Component:	Individual Port(s) (show other bugs)
Version:	Latest
Hardware:	Any Any

Importance:	--- Affects Only Me
Assignee:	Koichiro Iwao

URL:	https://github.com/librespeed/speedte...
Keywords:	buildisok, patch

Depends on:
Blocks:

Reported:	2020-06-17 20:02 UTC by Dries Michiels
Modified:	2020-06-22 14:55 UTC (History)
CC List:	2 users (show)

See Also:

Attachments
librespeed-go.diff (13.13 KB, patch) 2020-06-17 20:02 UTC, Dries Michiels	driesm: maintainer-approval+	Details \| Diff
librespeedgo.diff (67.13 KB, patch) 2020-06-18 17:34 UTC, Dries Michiels	driesm: maintainer-approval+	Details \| Diff
librespeedgo.diff (67.13 KB, patch) 2020-06-18 17:37 UTC, Dries Michiels	driesm: maintainer-approval+	Details \| Diff
librespeedgo.diff (8.43 KB, patch) 2020-06-18 17:40 UTC, Dries Michiels	driesm: maintainer-approval+	Details \| Diff
librespeedgo.diff (8.43 KB, patch) 2020-06-18 17:43 UTC, Dries Michiels	driesm: maintainer-approval+	Details \| Diff
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Dries Michiels freebsd_committer

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 freebsd_committer

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 freebsd_committer

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 freebsd_committer

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 freebsd_committer

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 freebsd_committer

2020-06-18 17:37:15 UTC

Created attachment 215738 [details]
librespeedgo.diff

Fix typo in rc script.

Comment 9 Dries Michiels freebsd_committer

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 freebsd_committer

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 freebsd_committer

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 freebsd_committer

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