dev-ml/ollama: a separate ollama user #210

CreeperMain · 2024-07-09T22:17:44Z

@vitaly-zdanevich Hello, I played around with the ollama package recently but i noticed that ollama serve in /etc/init.d/ollama (the daemon) ran that command as root. So i decided per the official documentation https://github.com/ollama/ollama/blob/main/docs/linux.md to implement a separate ollama user, my reasoning for this was for sandboxing's sake. Because I'm new to writing ebuilds and largely inexperienced please if you are in the position to check whether the user id I used is already used by anything, it's a simple change if its already is use by some other package.
PS: Yes this is my second pull request I messed up the first one

i had to do some tricky stuff i tried it in a vm it works fine now

CreeperMain · 2024-07-09T22:58:13Z

i also recorded a quick demo in a vm

2024-07-10.00-53-38.mp4

vitaly-zdanevich · 2024-07-09T23:15:46Z

Hi, thanks for your contributions, I do not know these GROUP_ID stuff, but greping /var/db/repos/gentoo I found the same number in another package https://gitweb.gentoo.org/repo/gentoo.git/tree/acct-group/murmur/murmur-0-r3.ebuild#n8

CreeperMain · 2024-07-10T11:33:40Z

Thank you I'll take a looksie

CreeperMain · 2024-07-10T13:41:29Z

125 was free in both guru and gentoo, I checked also pentoo and a few others. But I'm unaware if there is a proper convention around user and group ids.

vitaly-zdanevich · 2024-07-10T13:49:23Z

@MrRoy hi, can you please help here?

antecrescent · 2024-07-10T14:36:18Z

I'm currently investigating this as part of a code review.

MrRoy · 2024-07-10T16:21:56Z

Nice catch @vitaly-zdanevich

Users and group IDs outside of the official ::gentoo repository need to use ID -1 to avoid collisions with ::gentoo. -1 will dynamically allocate a free UID/GID.
I can't seem to find a source for that in the dev manual, but this wiki page says:

An assignment to variable ACCT_USER_ID, specifying the preferred user ID. It can be set to -1 in alternative ebuild repositories to dynamically allocate the UID.

Looking at other guru ebuilds, we find that only -1 is used in all acct-{user,group} ebuilds:

guru/acct-group $ grep -rh "GROUP_ID" | uniq
ACCT_GROUP_ID=-1
guru/acct-user $ grep -rh "USER_ID" | uniq
ACCT_USER_ID=-1

antecrescent · 2024-07-10T16:43:37Z

Users and group IDs outside of the official ::gentoo repository need to use ID -1 to avoid collisions with ::gentoo. -1 will dynamically allocate a free UID/GID. I can't seem to find a source for that in the dev manual, but this wiki page says:

An assignment to variable ACCT_USER_ID, specifying the preferred user ID. It can be set to -1 in alternative ebuild repositories to dynamically allocate the UID.

It's written in the eclass documentations for acct-{user,group}:

Overlays should set [ACCT_USER/GROUP_ID] to -1 to dynamically allocate [UID/GID]. Using -1 in ::gentoo is prohibited by policy.

CreeperMain · 2024-07-10T16:58:36Z

I changed them, I'll now check if it still works as intended

CreeperMain · 2024-07-10T17:22:13Z

It seems to work as intended if there any other issues please tell me
PS: From what I understand, Im guessing that 122 is free so it picked that

2024-07-10.19-17-56.mp4

vitaly-zdanevich · 2024-07-10T17:36:08Z

ffprobe says

moov atom not found
video.mp4: Invalid data found when processing input

vitaly-zdanevich · 2024-07-10T17:36:53Z

acct-user/ollama/metadata.xml

+	</maintainer>
+	<use>
+		<flag name="nvidia">Add support of nvidia</flag>
+		<flag name="amd">Add support of amd</flag>


Is it needed here?

vitaly-zdanevich · 2024-07-10T17:38:35Z

acct-group/ollama/metadata.xml

+	<use>
+		<flag name="nvidia">Add support of nvidia</flag>
+		<flag name="amd">Add support of amd</flag>
+	</use>


This use is for the ebuild, not for acct-group, right?

Thanks I'll sort it out rn

Should i change the description or no?

Actually I'll drop the description, I took a look at the other acct-user/group packages they didn't use a description tag

vitaly-zdanevich · 2024-07-10T22:10:43Z

Ok, is it ready to merge?

CreeperMain · 2024-07-10T22:22:06Z

I think so everything works as intended

antecrescent

Thanks for the contribution! Commits to (official) Gentoo ebuild repositories should adhere to GLEP 66.
To do so without much headache, follow the GURU contribution guide (create an OpenPGP key, setup the repo's git config and commit via dev-util/pkgdev).
And please change one package per commit, i.e.

acct-group/ollama: new package
acct-user/ollama: new package
dev-ml/ollama: support unprivileged ollama user

antecrescent · 2024-07-10T12:28:30Z

acct-group/ollama/ollama-0.ebuild

@@ -0,0 +1,9 @@
+# Copyright 1999-2024 Gentoo Authors


Suggested change

# Copyright 1999-2024 Gentoo Authors

# Copyright 2024 Gentoo Authors

antecrescent · 2024-07-10T12:29:50Z

acct-user/ollama/metadata.xml

+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE pkgmetadata SYSTEM "https://www.gentoo.org/dtd/metadata.dtd">
+<pkgmetadata>
+	<maintainer type="person" proxied="yes">


Remove the proxied attribute.

antecrescent · 2024-07-10T12:45:39Z

acct-user/ollama/ollama-0.ebuild

+
+DESCRIPTION="A user for ollama"
+ACCT_USER_ID=122
+ACCT_USER_SHELL=/bin/false


Remove this line in favor of the default /sbin/nologin.

antecrescent · 2024-07-10T13:01:21Z

acct-user/ollama/ollama-0.ebuild

+ACCT_USER_ID=122
+ACCT_USER_SHELL=/bin/false
+ACCT_USER_HOME=/usr/share/ollama
+ACCT_USER_HOME_PERMS=0755


Suggested change

ACCT_USER_HOME_PERMS=0755

ACCT_USER_HOME_PERMS=0700

Seems to work correctly and follows the principle of least privilege :)

antecrescent · 2024-07-10T13:36:00Z

dev-ml/ollama/ollama-9999.ebuild

+	touch /var/log/ollama.log
+	chown ollama:ollama /var/log/ollama.log


Move this function below src_install to reflect the correct phase function order and edit it like this

Suggested change

touch /var/log/ollama.log

chown ollama:ollama /var/log/ollama.log

touch /var/log/ollama.log || die

fowners ollama:ollama /var/log/ollama.log

antecrescent · 2024-07-10T15:59:05Z

dev-ml/ollama/ollama-9999.ebuild

+	acct-group/ollama
+	acct-user/ollama


Move these dependencies to RDEPEND and also add them to IDEPEND (because of fowners in pkg_preinst).

antecrescent · 2024-07-10T16:11:05Z

acct-user/ollama/ollama-0.ebuild

@@ -0,0 +1,15 @@
+# Copyright 1999-2024 Gentoo Authors


Suggested change

# Copyright 1999-2024 Gentoo Authors

# Copyright 2024 Gentoo Authors

antecrescent · 2024-07-10T17:48:12Z

acct-group/ollama/metadata.xml

+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE pkgmetadata SYSTEM "https://www.gentoo.org/dtd/metadata.dtd">
+<pkgmetadata>
+	<maintainer type="person" proxied="yes">


Suggested change

<maintainer type="person" proxied="yes">

<maintainer type="person">

This is not a proxy-maint package.

antecrescent · 2024-07-10T17:48:14Z

acct-group/ollama/metadata.xml

Remove the description, use and upstream tags. These belong to dev-ml/ollama, not to the user and group packages, like @vitaly-zdanevich pointed out.

antecrescent · 2024-07-10T23:29:38Z

acct-user/ollama/metadata.xml

Same applies here.

vitaly-zdanevich · 2024-07-11T00:09:54Z

Pushed, please review

6b22b60
0913afb

vitaly-zdanevich · 2024-07-11T00:10:50Z

f6b2371

antecrescent · 2024-07-11T01:09:42Z

@vitaly-zdanevich Thanks for (mostly) applying my suggestions. It would have been nice to give @CreeperMain a chance to author these changes, though ;)
Two remarks:

Please write more descriptive commit messages.
dev-ml/ollama: fixes from from https://github.com/gentoo/guru/pull/210/files does not cut it.
I gave an example of what the commit messages could look like earlier in this thread.
Generally speaking, you should never paste an URL into the commit summary line.
You did not close the PR in the last commit. I did it when I fixed your changes in ec9321f.

CreeperMain · 2024-07-11T09:29:15Z

Thanks for the contribution! Commits to (official) Gentoo ebuild repositories should adhere to GLEP 66. To do so without much headache, follow the GURU contribution guide (create an OpenPGP key, setup the repo's git config and commit via dev-util/pkgdev). And please change one package per commit, i.e.
1. `acct-group/ollama: new package`

2. `acct-user/ollama: new package`

3. `dev-ml/ollama: support unprivileged ollama user`

Thank you I'll keep that in mind next time

CreeperMain · 2024-07-11T09:35:39Z

@vitaly-zdanevich Thanks for (mostly) applying my suggestions. It would have been nice to give @CreeperMain a chance to author these changes, though ;) Two remarks:

1. Please write more descriptive commit messages.
   `dev-ml/ollama: fixes from from https://github.com/gentoo/guru/pull/210/files` does not cut it.
   I gave an example of what the commit messages could look like [earlier in this thread](https://github.com/gentoo/guru/pull/210#pullrequestreview-2168933467).
   Generally speaking, you should never paste an URL into the commit summary line.

2. You did not close the PR in the last commit. I did it when I fixed your changes in [ec9321f](https://github.com/gentoo/guru/commit/ec9321fa791adba089e1c378427309c594587e31).

Alright than I'm glad that a nonprivileged user was implemented into this package, should I close this pull request now?

antecrescent · 2024-07-11T10:13:40Z

Alright than I'm glad that a nonprivileged user was implemented into this package, should I close this pull request now?

No need, it'll get closed automatically once ec9321f gets merged into master.

a separate ollama user

44891d5

i had to do some tricky stuff i tried it in a vm it works fine now

found a suitable user and group id

f829cf5

changing user and group id to match gentoo conventions

b399dfe

vitaly-zdanevich reviewed Jul 10, 2024

View reviewed changes

Metadata set for the ollama user and group packages

97fd5b5

antecrescent suggested changes Jul 10, 2024

View reviewed changes

gentoo-bot closed this in ec9321f Jul 11, 2024

	# Copyright 1999-2024 Gentoo Authors
	# Copyright 2024 Gentoo Authors

		touch /var/log/ollama.log
		chown ollama:ollama /var/log/ollama.log

	<maintainer type="person" proxied="yes">
	<maintainer type="person">

dev-ml/ollama: a separate ollama user #210

dev-ml/ollama: a separate ollama user #210

Conversation

CreeperMain commented Jul 9, 2024

CreeperMain commented Jul 9, 2024

vitaly-zdanevich commented Jul 9, 2024

CreeperMain commented Jul 10, 2024

CreeperMain commented Jul 10, 2024

vitaly-zdanevich commented Jul 10, 2024

antecrescent commented Jul 10, 2024 • edited Loading

MrRoy commented Jul 10, 2024

antecrescent commented Jul 10, 2024 • edited Loading

CreeperMain commented Jul 10, 2024

CreeperMain commented Jul 10, 2024 • edited Loading

vitaly-zdanevich commented Jul 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaly-zdanevich commented Jul 10, 2024

CreeperMain commented Jul 10, 2024

antecrescent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vitaly-zdanevich commented Jul 11, 2024

vitaly-zdanevich commented Jul 11, 2024

antecrescent commented Jul 11, 2024

CreeperMain commented Jul 11, 2024

CreeperMain commented Jul 11, 2024

antecrescent commented Jul 11, 2024 • edited Loading

antecrescent commented Jul 10, 2024 •

edited

Loading

antecrescent commented Jul 10, 2024 •

edited

Loading

CreeperMain commented Jul 10, 2024 •

edited

Loading

antecrescent commented Jul 11, 2024 •

edited

Loading