fix: default to not mounting docker socket in exec

drewcassidy commented

2025-10-14 06:24:03 +00:00

Contributor

forgejo-runner exec currently defaults to mounting /var/run/docker.sock in the job container, which might not exist. This PR changes the default to not mounting anything in the job container. This mirrors the default value of the docker_host config value

If mounting a docker socket in the job container is desired, the user can use --container-daemon-socket

bug fixes
- PR: fix: default to not mounting docker socket in exec

Fixes #1073 `forgejo-runner exec` currently defaults to mounting `/var/run/docker.sock` in the job container, which might not exist. This PR changes the default to not mounting anything in the job container. This mirrors the default value of the `docker_host` config value If mounting a docker socket in the job container is desired, the user can use `--container-daemon-socket`   - bug fixes - [PR](https://code.forgejo.org/forgejo/runner/pulls/1090): fix: default to not mounting docker socket in exec

drewcassidy added 1 commit

2025-10-14 06:24:03 +00:00

Default to not mounting docker socket in exec

checks / validate pre-commit-hooks file (pull_request) Successful in 31s

Details

checks / validate mocks (pull_request) Successful in 39s

Details

checks / build and test (pull_request) Successful in 52s

Details

checks / runner exec tests (pull_request) Successful in 25s

Details

checks / runner integration tests (pull_request) Successful in 4m51s

Details

cascade / forgejo (pull_request_target) Has been skipped

Details

cascade / debug (pull_request_target) Has been skipped

Details

cascade / end-to-end (pull_request_target) Has been skipped

Details

issue-labels / release-notes (pull_request_target) Successful in 13s

Details

checks / integration tests (pull_request) Failing after 17m39s

Details

eca5e67c0b

When running `forgejo-runner exec`, default to not mounting anything in the job container instead of mounting /var/run/docker.sock. This mirrors the behavior of the docker_host config value

If mounting a docker socket in the job container is desired, the user can use --container-daemon-socket

drewcassidy commented

2025-10-14 06:42:16 +00:00

Author

Contributor

looks like some of the tests were written with the assumption that when ContainerDaemonSocket is set to an empty string it should mount /var/run/docker.sock. Not sure if I should update the tests or put the code I pulled from act/runner back. personally I think the default should always be not to mount anything but understand this could be a breaking change for some.

looks like some of the tests were written with the assumption that when `ContainerDaemonSocket` is set to an empty string it should mount `/var/run/docker.sock`. Not sure if I should update the tests or put the code I pulled from `act/runner` back. personally I think the default should always be not to mount anything but understand this could be a breaking change for some.

mfenniak commented

2025-10-15 01:41:15 +00:00

Owner

I'm not so sure of this, since it's a breaking change for users who access exec today and may have existing workflows around it. What would you think of detecting the absence of /var/run/docker.sock and proceeding with this behaviour only in that case, maybe with an info-level log message?

I'm not so sure of this, since it's a breaking change for users who access `exec` today and may have existing workflows around it. What would you think of detecting the absence of `/var/run/docker.sock` and proceeding with this behaviour only in that case, maybe with an info-level log message?

drewcassidy commented

2025-10-15 05:44:53 +00:00

Author

Contributor

its not just about it existing, but also about it being accessible to the current user. the docker socket is only usable by users in the docker group by default. Thats what I was running into when I filed the issue (I said it didn't exist, I later realized it did exist thanks to the Podman-docker shim I installed because the docs told me to, even though I was using the podman user socket. I just didn't have write access to it)

Manually running workflows that themselves deal with docker doesnt seem like it would be a super common use case, but it is possible. maybe this could be delayed to a more major update when breaking changes are more palatable?

its not just about it existing, but also about it being accessible to the current user. the docker socket is only usable by users in the docker group by default. Thats what I was running into when I filed the issue (I said it didn't exist, I later realized it did exist thanks to the Podman-docker shim I installed because the docs told me to, even though I was using the podman user socket. I just didn't have write access to it) Manually running workflows that themselves deal with docker doesnt seem like it would be a super common use case, but it is possible. maybe this could be delayed to a more major update when breaking changes are more palatable?

earl-warren added the

Kind/Bug

label

2025-10-15 07:50:17 +00:00

mfenniak commented

2025-10-17 03:18:30 +00:00

Owner

Manually running workflows that themselves deal with docker doesnt seem like it would be a super common use case, but it is possible. maybe this could be delayed to a more major update when breaking changes are more palatable?

Hm, I develop almost all my workflows with forgejo-runner exec because it's a faster dev cycle, including those that build containers, but I don't do that daily. I could counter-argue that using forgejo-runner exec on a system without access to the docker socket doesn't seem like a super common use case. I think we'd both be right that neither case is common. 😉

Thinking back to making it automatic -- rather than checking if the socket exists, could we make an attempt to access the socket? I believe there's a Ping method on the docker client Go API...

> Manually running workflows that themselves deal with docker doesnt seem like it would be a super common use case, but it is possible. maybe this could be delayed to a more major update when breaking changes are more palatable? Hm, I develop almost all my workflows with `forgejo-runner exec` because it's a faster dev cycle, including those that build containers, but I don't do that daily. I could counter-argue that using `forgejo-runner exec` on a system without access to the docker socket doesn't seem like a super common use case. I think we'd both be right that neither case is common. 😉 Thinking back to making it automatic -- rather than checking if the socket exists, could we make an attempt to access the socket? I believe there's a `Ping` method on the docker client Go API...

checks / validate pre-commit-hooks file (pull_request) Successful in 31s

Details

checks / validate mocks (pull_request) Successful in 39s

Details

checks / build and test (pull_request) Successful in 52s

Details

checks / runner exec tests (pull_request) Successful in 25s

Required

Details

checks / runner integration tests (pull_request) Successful in 4m51s

Details

cascade / forgejo (pull_request_target) Has been skipped

Required

Details

cascade / debug (pull_request_target) Has been skipped

Required

Details

cascade / end-to-end (pull_request_target) Has been skipped

Required

Details

issue-labels / release-notes (pull_request_target) Successful in 13s

Details

checks / integration tests (pull_request) Failing after 17m39s

Details

checks / Build Forgejo Runner *

Required

checks / Run integration tests with *

Required

This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u fix-dont-mount-socket:drewcassidy-fix-dont-mount-socket

git switch drewcassidy-fix-dont-mount-socket

Rows
Columns

fix: default to not mounting docker socket in exec #1090

Checkout