search

sablierapp / sablier

Start your containers on demand, shut them down automatically when there's no activity. Docker, Docker Swarm Mode and Kubernetes compatible.
https://sablierapp.dev/
GNU Affero General Public License v3.0
1.36k stars 46 forks source link

Sablier binary shouldn't be located on its current location #348

Closed gtherond closed 2 weeks ago

gtherond commented 3 months ago

Describe the bug The current container build use /etc/sablier/sablier as the binary location once it have been built

Context

Expected behavior As the Dockerfile is using an Alpine image for the final layer, it should try to respect Alpine and Linux FHS as much as possible.

If the build have to follow the requirements of either Linux or Alpine, the resulting binary should be located on one of the following available path:

Additional context Per see, there is no real big issue, BUT, this helps sablier to be more solid. I can do the patch and PR if you're willing to change the binary location.

TBN: The sablier yaml configuration file have to stay on /etc/sablier.yaml or /etc/sablier/sablier.yaml to match Linux FHS.

TBN2: Sablier being a static golang binary, the Dockerfile final layer could even be derivated from scratch and the binary placed on one of the expected upper suggested path, that would reduce even more the perimeter to support with this container image.

acouvreur commented 3 months ago

You can open a PR, I agree with all your points.

Maybe for the scratch image, this can be tedious to debug because you won't have a shell image inside.

What do you think ?

acouvreur commented 3 months ago

I simply changed the binary location for this PR.

For making the image scratch, I don't know yet. Depends if we still want to have a shell inside I guess.

I'm also interested in having this container rootless if you have any expertise. I think it's quite complicated given the fact we need docker group to access the socket.

I could something like Linuxservers.io images with PGID and PUID.

acouvreur commented 3 months ago

:tada: This issue has been resolved in version 1.8.0-beta.10 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

gtherond commented 3 months ago

I simply changed the binary location for this PR.

For making the image scratch, I don't know yet. Depends if we still want to have a shell inside I guess.

I'm also interested in having this container rootless if you have any expertise. I think it's quite complicated given the fact we need docker group to access the socket.

I could something like Linuxservers.io images with PGID and PUID.

Perfect! You're awesome, thanks a lot.

Yep, regarging the scratch base, it obviously isn't a light hearted decision and yes I didn't had those docker socket requirements in mind as I make it run on a kubernetes context. I could definitely have a look and find out how would it behave with a Wolfi or Google distroless base. I'll test it and let you know.

gtherond commented 3 months ago

You can open a PR, I agree with all your points.

Maybe for the scratch image, this can be tedious to debug because you won't have a shell image inside.

What do you think ?

Sorry I just saw your answer and find out you had done it two days ago, I'll enable notifications ^^

Thanks again!

acouvreur commented 2 weeks ago

:tada: This issue has been resolved in version 1.8.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: