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.

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

Switch to official Gollum image (#410) #411

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

Hooverdan96
Copy link
Member

  • update docker image
  • update descriptions

Fixes #410 .

Information on docker image

Checklist

  • Passes JSONlint validation
  • Entry added to root.json in alphabetical order (for new rock-on only)
  • "description" object lists and links to the docker image used
  • "description" object provides information on the image's particularities (advantage over another existing rock-on for the same project, for instance)
  • "website" object links to project's main website

- update docker image
- update descriptions
@Hooverdan96 Hooverdan96 added the needs review Test install, function, on / off behaviour, all links / info. label Dec 16, 2024
Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@Hooverdan96 Thanks for yet more maintenance on our image use.

I get a failed Web-UI using: http://rleap15-6.lan:8084/ from the suggested default.

It may be we are looking at a changed along with the changed image:

Dec 17 18:30:58 rleap15-6 gollumwiki/gollum:latest/gollum-alpine[16236]: [2024-12-17 18:30:58] INFO  WEBrick 1.8.1
Dec 17 18:30:58 rleap15-6 gollumwiki/gollum:latest/gollum-alpine[16236]: [2024-12-17 18:30:58] INFO  ruby 3.1.6 (2024-05-29) [x86_64-linux-musl]
Dec 17 18:30:58 rleap15-6 gollumwiki/gollum:latest/gollum-alpine[16236]: == Sinatra (v4.0.0) has taken the stage on 4567 for production with backup from WEBrick
Dec 17 18:30:58 rleap15-6 gollumwiki/gollum:latest/gollum-alpine[16236]: [2024-12-17 18:30:58] INFO  WEBrick::HTTPServer#start: pid=1 port=4567

Yet it looks like we are mapping internal 8080 to 8084.

I.e.:

rleap15-6:~ # docker ps
CONTAINER ID   IMAGE                      COMMAND            CREATED         STATUS         PORTS                                       NAMES
aee4924f5b80   gollumwiki/gollum:latest   "/docker-run.sh"   4 minutes ago   Up 4 minutes   0.0.0.0:8084->8080/tcp, :::8084->8080/tcp   gollum-alpine

and from

docker inspect gollum-alpine

I see:

            "ExposedPorts": {
                "8080/tcp": {}
            },

So maybe we need a slug.

But: https://github.com/gollum/gollum/wiki/Gollum-via-Docker#running-gollum-via-docker

makes mention of port 4567 !

I'll make the suggested change so it's easier to try it out.

gollum.json Outdated Show resolved Hide resolved
@phillxnet phillxnet added question and removed needs review Test install, function, on / off behaviour, all links / info. labels Dec 17, 2024
@phillxnet
Copy link
Member

@Hooverdan96 I've also just checked their docker image:

https://github.com/gollum/gollum/blob/master/Dockerfile

Which does not explicitly export a port. So we are likely self-fulfilling our failure here. Hopefully that port change should do it, and we could change our suggested port to the internal one. Or not.

@Hooverdan96
Copy link
Member Author

I changed the port to test as you suggested. I think you're right. However, not having used Gollum before, at least I now end up with an error page (which, I guess, is progress):

image

Co-authored-by: Philip Guyton <[email protected]>
@Hooverdan96
Copy link
Member Author

Hooverdan96 commented Dec 17, 2024

alright, not sure what is going on. With a lower version this works flawlessly, but since 6.0 that seems to be an issue (or something is missing in the gollum documentation in terms of prep work).

I opened an issue on the gollum repo, to see whether I just can't figure it out or if there might be a bug with the docker image:

gollum/gollum#2084

For now I will just update the Rockon with the last working version 5.3 (which worked fine for me in testing), and once it's clear on how to get this to work on the later versions, go and update the Rockon definition (I guess, we then would want to add additional command parameters like the math typesetting which needs to be explicitly enabled post 6.0).

@FroggyFlox
Copy link
Member

FroggyFlox commented Dec 17, 2024

Could https://github.com/gollum/gollum/wiki/Gollum-via-Docker#podman give a clue here? It does relate to user permissions and the need to have the www-data user have appropriate permissions. It also seems to relate to v6 only after a quick look.

Hope this helps,

@Hooverdan96
Copy link
Member Author

Hooverdan96 commented Dec 17, 2024

Thanks @FroggyFlox, let me try that. If that's the solution, that would be good.

Update: the userns=keep-id is actually a podman specific option. Not sure whether there is anything corresponding in docker?

The error occurs within the container itself. The user www-data does not have access to enter the internal (and host-mapped) /wiki directory (owner in the container is root:root) to begin with.

Well, let's see, if I get an answer on the issue I've just opened. For now, let's stick with the lower version, I think.

@Hooverdan96 Hooverdan96 added needs review Test install, function, on / off behaviour, all links / info. and removed question labels Dec 17, 2024
Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@Hooverdan96 As per issue, you do look to have now moved this Rock-on over.
All-be-it to a non-latest variant. But this now looks to be working as intended and we can approach your upstream queries re using latest and user related privaledges in a follow-up
Issue/PR set which then clearly deals only with the required changes on that front.

Great to have this using an official docker image.

Bar some pending UI text elements affecting all our Rock-ons this looks to be working:

Gollum-rock-on-5 3-Web-UI

rleap15-6:~ # ls -la /mnt2/gollum-wiki/
total 4
drwxr-xr-x 1 root root  46 Dec 18 16:11 .
drwxr-xr-x 1 root root 276 Dec 18 16:05 ..
drwxr-xr-x 1 root root 116 Dec 18 16:11 .git
-rw-r--r-- 1 root root  94 Dec 18 16:11 Rock-on new page.md

@phillxnet phillxnet removed the needs review Test install, function, on / off behaviour, all links / info. label Dec 18, 2024
@phillxnet
Copy link
Member

@Hooverdan96 Re:

The user www-data does not have access to enter the internal (and host-mapped) /wiki directory (owner in the container is root:root) to begin with.

The newer docker image running as designed as the 'www-data' user (container internal user) would likely need to have a matching (by uuid) user on the outside to own the mapped share. Do you see this same inability to enter /wiki when it is not externally mapped. Docker only allows for uuid:guid mapping to host by number - as far as I know. So we would need to know the internal uuid for www-data to know how to setup a mapped Share ownership. Likely all you have already considered. But just mentioning this in case.

@phillxnet
Copy link
Member

At per review comment I'll get this merged as it has addressed the issue of moving us to the official image.

@phillxnet phillxnet merged commit e295404 into rockstor:master Dec 18, 2024
@phillxnet
Copy link
Member

The resulting Rock-on from this PR is another example exibiting the following bug:

@phillxnet
Copy link
Member

@Hooverdan96 Thanks for pushing this Rock-on along some. As always much appreciated.

PRODUCTION published

@Hooverdan96 Hooverdan96 deleted the 410_gollum_official branch December 18, 2024 19:12
@Hooverdan96
Copy link
Member Author

Hooverdan96 commented Dec 18, 2024

Well, what do you know, I got a response on the issue I reported to gollum. Indeed it was an authorization "challenge". 6.x seems to require that the permission is aligned with its container user (as it is not running as root now). While I didn't find that in the documentation, it is UID:GID=1000:1000.

I will likely update the just merged Rockon, update it to the latest version (i.e. master) and add something in the description for the share selection.

@phillxnet
Copy link
Member

@Hooverdan96 Maybe our under-used (and apparently un-documented!) Rock-on definition element of "uid" would be of use here:

Examples are in:

"jenkins": {
"image": "jenkins/jenkins",
"tag": "lts",
"launch_order": 1,
"uid": -1,

And the rockstor-core side:

https://github.com/rockstor/rockstor-core/blob/812f1cffe39b0c7b6005215355b7fa413feae4ea/src/rockstor/storageadmin/views/rockon.py#L245-L246

https://github.com/rockstor/rockstor-core/blob/812f1cffe39b0c7b6005215355b7fa413feae4ea/src/rockstor/storageadmin/views/rockon_helpers.py#L376-L383

It can optionally run a container as the owner of the first Volume mapped @FroggyFlox's use above. Or hardwired to a specific uuid as per in my example as Bareos may require specific hardware access that we have to take care with. And their rpms assume this level of control. Plus I wanted to explore having this way more tied down than we usually have, and I think this is becoming more prevalent in docker images (i.e. the last USER entry in the Dockerfile before ENTRYPOINT/CMD). As per this recent Gollum change in-fact.

Noteworthy is that one can only specify a single :group if specified; but that's from memory so take with a pinch of salt.

@Hooverdan96
Copy link
Member Author

Hooverdan96 commented Dec 19, 2024

I will take a look at the uid piece, as well as another suggestion that the gollum maintainer made about setting the volume mapping to rw explicitly in the mapping.
In their dockerfile they are explicitly setting the UID/GID, then set up a user with those attributes and then set and define the USER.

https://github.com/gollum/gollum/blob/e06121f914207c5e5af9cf82f1334c7afce3433d/Dockerfile

To me it's not obvious, but based on your comments it means I can still override the defined USER when using the -u option (or implementing the Rockon uid tag? UPDATE: yes, tested it, and that works.

At the same time, running a container without root would be a secure best practice wouldn't it?

Fundamentally, it could make sense to also have the ability to surface the uid option in the Rockon config wizard?

@phillxnet
Copy link
Member

@Hooverdan96 Re:

In their dockerfile they are explicitly setting the UID/GID, then set up a user with those attributes and then set and define the USER.

Pretty sure that is now considered best practice, as per the first of my prior links. It's also what I've done with the Bareos containers.

To me it's not obvious, but based on your comments it means I can still override the defined USER when using the -u option (or implementing the Rockon uid tag? UPDATE: yes, tested it, and that works.

I'm a little unclear on this one still as it goes. The Bareos container defaults (for non Director-local use) to root.bareos: but when run with -u 105 it also writes externally as 105. I'm just concerned about forcing a container to run under a user that it was not intended to run under. You can learn me as you go on this one hopefully.

At the same time, running a container without root would be a secure best practice wouldn't it?

It's a basic no-no now I think. But again one can jump up and down users within a Docker container. But ultimatly I also found that using -u on the docker command revoked rights, and in some cases if the internals of the container needed those rights it would fail. Had that during early development of one of the bareos images. I.e. RUN or ENTRYPOINT ideally need minimum privaledges - i.e. they can't install stuff for example.

Fundamentally, it could make sense to also have the ability to surface the uid option in the Rockon config wizard?

Agreed, I was thinking the same actually. But for next testing phase I think :). Especially as there is only @FroggyFlox's Rock-on currently using it in the published rock-on set.

@phillxnet
Copy link
Member

phillxnet commented Dec 19, 2024

@Hooverdan96 And remember we have the trick '-1' value for uid, where we run the container as the owner of the first Share mapped.

@Hooverdan96
Copy link
Member Author

Thanks, I just overlooked that. So, for this (and some of the other containers) it makes sense to use that trick, especially if only one share is mapped and no additional shares can be added. Otherwise, it will still have to be noted in the instructions.

@phillxnet
Copy link
Member

Possibly. But if folks leave the Share as default then it ends up running the container as root !

@Hooverdan96
Copy link
Member Author

but when run with -u 105 it also writes externally as 105
yes, I observed this behavior as well, which for this use case was fine (or even desired, as the contents of the directory aligned with the overall ownership of the directory).
I think, the risk using -u is lower if the container's default user is already a non-root user, as the designer of the container has (hopefully) already taken into account the permissions, etc.

However, you are probably right that there can be unintended consequences, especially if the default user was root and now it's forced to run with less privileges inside the container ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gollum Rockon - switch to official docker image
3 participants