Created attachment 267865 [details] Update to 2.2.0 this patch can close bug #292818: - updates www/garage to v2.2.0 - adds DOCS option - adds garage:483 to UIDs and GIDs to run as an unprivileged user
I've tested the patch but something is a miss with user and group creation: [1/1] Installing garage-2.2.0... ===> Creating users Creating user 'garage' with uid '483' pw: group `483' does not exist pkg: PRE-INSTALL script failed 15.0-RELEASE
FWIW, it patched cleanly against HEAD and built without any issues.
Sorry, I missed this update. I'm building it, and if no noticeable issues, I'll commit. (In reply to yds from comment #0) Would you like to maintain this port ? As I don't use Garage, I don't really have good way to test it. Thanks!
(In reply to Ashish SHUKLA from comment #3) > Would you like to maintain this port ? version bumps are easy enough, but I'm kinda in the same boat as you needing to test something I do not use on the regular.. I created this patch to try out `garage` and kick the tires with the latest version. I'm not sure I could really do justice to the port as a maintainer since I'm not sure I'll end up as a regular user of the port. (In reply to Jason Tubnor from comment #1) FWIW, this the first patch I've made adding a new UID/GID -- not sure the way I submitted this patch is the "right way" to add a new UID/GID to the apropos files.
I can take maintainership once the patch has the noted items fixed as I do use Garage but I don't have commit access.
(In reply to Jason Tubnor from comment #5) I'm not a committer either. I'm a maintainer of several ports without having commit access. commit access is not a prerequisite to maintain a port. I may submit further patches to this port tho. Jason, if you take over as maintainer and have questions about ports, I'm happy to help, I've been hacking on FreeBSD ports for over 25 years.
(In reply to yds from comment #4) @Ashish could you please look and correct the UID/GID creation in the Makefile. You'll find it patches, builds and packages cleanly but won't install on a fresh system. Thanks @yds. I do use the port and happy to take it on as I want to see this software stack grow and be a viable option for Minio users.
+USERS?= ${PORTNAME} +GROUPS?= ${PORTNAME} +SUB_LIST= USER=${USERS:[1]} +SUB_FILES= ${PORTNAME} + +OPTIONS_DEFINE= DOCS +
I can confirm the addition of: GROUPS?= ${PORTNAME} Fixes the issue. The port packages and installs correctly and functions as intended.
(In reply to yds from comment #4) Is there a particular reason you went for: USERS?= ... GROUPS?= ... instead of: USERS= ... GROUPS= ... Even porter's handbook advises the latter[0]. Thanks! References: [0] https://docs.freebsd.org/en/books/porters-handbook/special/#users-and-groups
(In reply to Ashish SHUKLA from comment #10) yes, as a courtesy for anyone building the port locally using `?=` allows the admin to install a port with a bespoke USER/GROUP without having to hack the Makefile to do it. USERS?= ... GROUPS?= ... this is the convention used in countless other ports, and I personally rely on the `?=` convention in many of the other ports to get the install tailored to just the way I need to run it. there's no harm using `?=` but '=' alone /does/ take away a much needed knob for those of us who use it.
(In reply to yds from comment #11) Fair enough. So, that's on purpose. The patch worked fine (after including the missing GROUPS contributed by Jason). (In reply to Jason Tubnor from comment #7) @Jason — I'm happy to pass on the maintainership torch to you, and can help commit the updates. Since you're an existing user, I will appreciate if you can contribute instructions for upgrade (esp. to a user/group situation), and how that affects existing uploaded content ? I believe it'll require chown command. Do you mind helping with this ? Essentially, a patch to UPDATING (for ports users), and pkg-message (for pkg users) Thanks!
Created attachment 268463 [details] Update to 2.2.0 (In reply to Ashish SHUKLA from comment #12) new patch adds `GROUPS?= ${PORTNAME}` and a somewhat opinionated FreeBSDish `DATADIR?= /var/db/garage` there's a new `start_precmd="chown -Rh ${user}:${group} %%DATADIR%%"` to ensure everything in `/var/db/garage` is owned by `garage` `garage` does not have any default location for DATADIR nor any sample config -- I figure adding the DATADIR and ensuring it's got the right permissions at startup is a worthy, albeit opinionated quality of life improvement for end users. @Jason — what are your thoughts about adding this DATADIR tracked by the pkg? we could also add a etc/garage.toml.sample using the DATADIR to help document the intent of the DATADIR
Created attachment 268464 [details] Update to 2.2.0 my bad -- forgot to include new UIDs/GIDs in the patch
Created attachment 268466 [details] Update to 2.2.0 another minor edit — another patch
Created attachment 268467 [details] Update to 2.2.0 one mowr edit — set home dir for the new `garage` user to `/var/db/garage` — same as DATADIR
Thanks. I'll reflect this for the UPDATING reference patch.
(In reply to Jason Tubnor from comment #17) @Jason, another thing worth mentioning in UPDATING: for the most secure permissions, the config file should have these applied: chmod 0640 /usr/local/etc/garage.toml chown root:garage /usr/local/etc/garage.toml that way only `root` can edit the config, and the `garage` user can read the config but not edit. this could be added to the `start_precmd` but that may be /too/ opinionated? perhaps just this is enough: chmod 0600 /usr/local/etc/garage.toml chown garage:garage /usr/local/etc/garage.toml tho IMHO the first variant is mowr secure.
(In reply to yds from comment #18) I'd rather the first option and also avoid putting any adjustments into garage.in file. Noted and adjusting.
(In reply to yds from comment #13) Quote: there's a new `start_precmd="chown -Rh ${user}:${group} %%DATADIR%%"` to ensure everything in `/var/db/garage` is owned by `garage` `garage` does not have any default location for DATADIR nor any sample config -- I figure adding the DATADIR and ensuring it's got the right permissions at startup is a worthy, albeit opinionated quality of life improvement for end users. --- Users will have a variety of ways how they will configure garage and storage locations (along with varying ZFS options). I'd be happy for DATADIR defined in the Makefile to assist users but wouldn't want it explicitly created or managed via garage.in
Created attachment 268468 [details] Update to 2.2.0 (In reply to Jason Tubnor from comment #20) > Users will have a variety of ways how they will configure garage > and storage locations (along with varying ZFS options). > I'd be happy for DATADIR defined in the Makefile to assist users > but wouldn't want it explicitly created or managed via garage.in not sure it makes sense to decouple how the Makefile packages the port creating the DATADIR with the right ownership and permissions and garage.in making sure those permissions are applied upon every startup. ZFS can be configured with a bespoke dataset and `mountpoint=DATADIR` and nullfs can be used to further tweak mount points if need be. also, if an admin still want to stray from all the pre-configured opinionated goodness of the default DATADIR location, there's always the option of building the port locally with the DATADIR set to any path the admin chooses: e.g. `env DATADIR=/tmp/foo/bar/garage portmaster -v www/garage` that's the main reason for making DATADIR a build time tunable knob -- the pkg installs an opinionated default, but those admins who prefer to go their own way still can via the build time knobs like the new DATADIR, USERS, and GROUPS vars -- all three of these use `?=` in the Makefile so they can be tweaked to whatever an admin may need. setting DATADIR at build time preserves most of the pre-configured opinionated goodness of having garage.in fix the ownership of DATADIR I really don't see a downside of managing the DATADIR via Makefile and garage.in -- if an admin really does not want to use that path they can delete it or ignore it and use whatever other path they wish via the config file settings. ignoring it just means there's an empty /var/db/garage/ dir -- not really all that huge of a waste. the upside is if an admin chooses the default DATADIR path in their config then it JustWorks without further tweaking -- hence the quality of life improvement for new users of the pkg. Cheers! them are my $0.02 ;)
(In reply to yds from comment #21) Ok. Thinking about it, I think this suggestion would work. I'd like to see something like this in garage.in though: : ${garage_datadir:="/var/db/garage"} So then it can be overridden by the user in /etc/rc.conf and no other hacks are required post install. Thoughts?
Created attachment 268470 [details] UPDATING and pkg-message patch First cut for UPDATING and www/garage/pkg-message
Created attachment 268483 [details] Update to 2.2.0 (In reply to Jason Tubnor from comment #22) I forgot to mention I had already written a pkg-install blurb in my last patch. in this latest patch I used some of your wording for the pkg-install msg and added %%SUBS%% where apropos. I also tightened up the pkg-install msg to the minimum needed for the end user to do manually. e.g. I moved the config file instructions into the start-precmd to ensure those are always correct without needing to tell the user to do anything extra manually. I changed start-precmd from attempting to fix DATADIR permissions to fixing only the config file permissions: ``` start_precmd="/bin/chmod 0640 ${garage_config} && /usr/sbin/chown root:%%GROUP%% ${garage_config}" ``` I figure it's always better to just do the right thing vs telling the user they need to do it. ;) in this case it always works since ${garage_config} is already a user settable knob -- doesn't matter if it's not the default location, the config file will always get fixed up to the correct permissions before startup. DATADIR permissions and ownership get fixed up every time the pkg is installed or upgraded. the pkg-install advice is only needed for admins who chose location for their DATADIR somewhere *other* than /var/db/garage/ -- I changed the example path in pkg-install to make that unambiguous. > Ok. Thinking about it, I think this suggestion would work. > I'd like to see something like this in garage.in though: > > : ${garage_datadir:="/var/db/garage"} > > So then it can be overridden by the user in /etc/rc.conf > and no other hacks are required > post install. Thoughts? given that garage does not offer any "default" config file nor data dir location, I think it's best to not add any new config knobs to garage.in especially if they then need to be kept in sync with the same setting in the config file itself. less is more, so to speak. as you suggested earlier, let's leave the creation and permissions setting of the new default /var/db/garage/ datadir location strictly in the Makefile, pkg-plist and UIDs files where pkg install/upgrade triggers setting all the right permissions. if an admin chooses to go with a different data dir path for their deployment -- it'll work exactly as it did before, it's upto the admin to ensure the custom data dir path they choose has the correct ownership. that's about the best we can do.
(In reply to yds from comment #24) I'm happy with all of that except where permissions are update on upgrade to /var/db/garage. For extremely large data stores, it could take considerable time to walk the tree of 1MB file blocks (think PB where S3 storage is typically used). Can we just set the permissions correctly on install and let pre 2.2.0 users manually update permission for DATADIR through instructions? Happy for garage.in to ensure root:garage 750 for /usr/local/etc/garage.toml. Cheers.
Created attachment 268489 [details] Update to 2.2.0 (In reply to Jason Tubnor from comment #25) > Can we just set the permissions correctly on install > and let pre 2.2.0 users manually update permission > for DATADIR through instructions? yes, that's pretty much how this latest version of the patch works. only the config file permissions and ownership get fixed up via start-precmd -- getting the DATADIR permissions set is up to the admin via pkg-message instructions. I added a pkg-message `type: install` blurb to announce and encourage new deployments to use the pre-installed /var/db/garage/ as the data_dir. try this latest patch, see if there's anything I missed.
I got a staging fail on a build run which needs further investigation on why it failed when all the patches bound correctly. [06:21:53] [01] [00:13:51] Status www/garage | garage-2.2.0: stage [06:21:54] [01] [00:13:52] Finished www/garage | garage-2.2.0: Failed: stage This may take a bit unless something is standing out.
It appears the 'garage' group isn't being created and causing issues for post install. I don't know if this is a me issue or something else. I'll do a preen and go completely from start to see if I hit the issue again. FWIW install -o garage -g garage -m 0750 -d /wrkdirs/usr/ports/www/garage/work/stage/var/db/garage install: unknown group garage *** Error code 1 Stop. make: stopped making "stage" in /usr/ports/www/garage =>> Cleaning up wrkdir ===> Cleaning for garage-2.2.0 build of www/garage | garage-2.2.0 ended at Wed Mar 4 09:40:03 AEDT 2026 build time: 00:13:53 !!! build failure encountered !!!
Yeap, issue remains. I need to work out why the Makefile isn't creating the garage group: install -m 0755 -d /wrkdirs/usr/ports/www/garage/work/stage/usr/local/share/doc/garage install -m 444 /wrkdirs/usr/ports/www/garage/work/garage-2.2.0/README.md /wrkdirs/usr/ports/www/garage/work/stage/usr/local/share/doc/garage cd /wrkdirs/usr/ports/www/garage/work/garage-2.2.0/doc && /bin/sh -c '(/usr/bin/find -Ed $1 $3 | /usr/bin/cpio -dumpl $2 >/dev/null 2>&1) && /usr/bin/find -Ed $1 $3 \( -type d -exec /bin/sh -c '\''cd '\''$2'\'' && chmod 755 "$@"'\'' . {} + -o -type f -exec /bin/sh -c '\''cd '\''$2'\'' && chmod 0644 "$@"'\'' . {} + \)' COPYTREE_SHARE . /wrkdirs/usr/ports/www/garage/work/stage/usr/local/share/doc/garage "! -name *\.bak" install -o garage -g garage -m 0750 -d /wrkdirs/usr/ports/www/garage/work/stage/var/db/garage install: unknown group garage *** Error code 1 Stop. make: stopped making "stage" in /usr/ports/www/garage =>> Cleaning up wrkdir ===> Cleaning for garage-2.2.0 build of www/garage | garage-2.2.0 ended at Thu Mar 5 15:18:12 AEDT 2026 build time: 00:22:17 !!! build failure encountered !!!
Looking through how other ports do this, they lean on MKDIR for post-install and update the permissions through pkg-plist which you already have correct: @dir(%%USER%%,%%GROUP%%,750) %%DATADIR%% @postexec chown -Rh %%USER%%:%%GROUP%% %%DATADIR%% So I propose the following diff on the patch above: --- www/garage/Makefile.new 2026-03-05 20:52:04.084778000 +1100 +++ www/garage/Makefile 2026-03-07 14:50:40.049183000 +1100 @@ -48,7 +48,7 @@ post-install: -e 's,/etc/garage,${ETCDIR},g' post-install: - ${INSTALL} -o ${USERS:[1]} -g ${GROUPS:[1]} -m 0750 -d ${STAGEDIR}${DATADIR} + ${MKDIR} ${STAGEDIR}${DATADIR} do-install-DOCS-on: ${INSTALL} -m 0755 -d ${STAGEDIR}${DOCSDIR} Thoughts @yds ?? If we are all good, we are go for this to be added and committed to the tree.
Created attachment 268834 [details] Update to 2.2.0 (In reply to Jason Tubnor from comment #30) Jason, I believe it's a chicken-n-egg situation where the new user is not yet created and cannot be referenced by the Makefile -- so I changed the target to not set the USER/GROUP: ``` post-install: ${INSTALL} -m 0750 -d ${STAGEDIR}${DATADIR} ``` this way permissions are still set. got rid of `pkg-plist` replaced by auto-generated PLIST
Created attachment 268835 [details] Update to 2.2.0 again my bad -- forgot to include new UIDs/GIDs in the patch
Created attachment 268836 [details] Update to 2.2.0 moved `load_rc_config` back to the top, otherwise defaults don't work as expected.. again my bad -- I broke it earlier in this patch cycle..
Created attachment 268838 [details] Update to 2.2.0 some more minor Makefile edits to make it read more consistently.
Created attachment 268839 [details] Update to 2.2.0 yet another edit: no need for PLIST_SUB with an auto-generated PLIST
Created attachment 268855 [details] Update to 2.2.0 yet another single char edit, there's no need to escape the `.` in '! -name *\.bak' -- without the surrounding quotes `*` would need to be escaped, but not the `.`, that's not a regex.
Created attachment 268859 [details] garage-2.2.0.diff I use this port for my backups and for some apps on a daily basis, so I might maintain it if ashish@ allows me to. And after reading the progress of this bug, I think we can keep it simple. It’s a shame that Garage runs as root first when it could run as a dedicated user, but in this case I think a polite pkg-message is sufficient. We don’t want to run chown(8) recursively, and we prefer not to run stuff in pkg-plist entries whenever possible. And for an auto plist, I don’t think this is necessary. I’d rather, in these cases, have poudriere inform me that a missing file is no longer needed or that an existing file should be added to pkg-plist. Finally, for users who want to run garage as another user, let’s simply let them do so at runtime instead of using USER?= or GROUP?=. A user who wants to do that should know what they’re doing and apply chown(8) to their existing data. --- * Take MAINTAINERSHIP. * Pet portclippy/portfmt. * Run garage as a unprivileged user. * Create a sample configuration file to use /var/db/garage as default meta and data directory. * Add DOCS option. * Fix pkg-message formatting.
(In reply to Jesús Daniel Colmenares Oviedo from comment #37) As a garage user, I already acknowledged that I would take over the port. To have someone just ignore that is a bit poor honestly. We have been curating this to meet our use and take better care of systems from a security perspective. Your totally reworked diff ignores the hours of testing that has already been performed here and is very disappointing.
(In reply to Jesús Daniel Colmenares Oviedo from comment #37) Quote: Finally, for users who want to run garage as another user, let’s simply let them do so at runtime instead of using USER?= or GROUP?=. A user who wants to do that should know what they’re doing and apply chown(8) to their existing data. This is totally missing any new installs. Don't keep running as root, fix the security when deficiencies are noticed.
(In reply to Jason Tubnor from comment #38) If you want to maintain this port, why not submit a patch? The only person I've seen working on it is yds, who has told us that he isn't a regular user. I have no problem with someone else taking over this port, but anyone who wants to maintain it should, at the very least, submit patches.
(In reply to Jason Tubnor from comment #39) I don't understand this, now the rc script runs Garage as garage (the dedicated user). What I didn't include in the patch I sent was applying chown(8) to /var/db/garage.
It is in the patches that we have been working with above. We've come over here from a previous bug to work in this thread (previous patch was just to move garage to an unprivileged user). @yds built on this to scratch an itch and include the move to unprivileged and then was happy to hand it over. Why duplicate work with multiple people running multiple patches making it harder to test. Anyway, seems you are keen (but I'm not happy with some of your technical choices/thoughts) so honestly I just can't be bothered arguing.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=325797c15b4be3b8f17c9f678f68b34895103546 commit 325797c15b4be3b8f17c9f678f68b34895103546 Author: Jesús Daniel Colmenares Oviedo <dtxdf@FreeBSD.org> AuthorDate: 2026-03-26 07:10:37 +0000 Commit: Jesús Daniel Colmenares Oviedo <dtxdf@FreeBSD.org> CommitDate: 2026-03-26 07:14:10 +0000 www/garage: Update to 2.2.0 * Take MAINTAINERSHIP. * Pet portclippy/portfmt. * Run garage as a unprivileged user. * Create a sample configuration file to use /var/db/garage as default meta and data directory. * Add DOCS option. * Fix pkg-message formatting. ChangeLog: https://git.deuxfleurs.fr/Deuxfleurs/garage/releases/tag/v2.2.0 PR: 293011 Reported by: yds@Necessitu.de Approved by: ashish@ (email) GIDs | 2 +- UIDs | 2 +- www/garage/Makefile | 37 +- www/garage/Makefile.crates | 442 ++++++++--------- www/garage/distinfo | 890 +++++++++++++++++----------------- www/garage/files/garage.in | 18 +- www/garage/files/garage.toml.in (new) | 16 + www/garage/files/pkg-message.in | 44 +- www/garage/pkg-plist | 599 ++++++++++++++++++----- 9 files changed, 1251 insertions(+), 799 deletions(-)
Committed, thanks!