Skip to content

Commit f2e65e1

Browse files
fix: Rootless error concerning /var/run/docker.sock (#2181)
* Use same socket defaulting strategy every time * Always default to DOCKER_HOST * Add more debug logs * Commenting, and massively simplified socket logic * Rever to upstream run_context.go * Fix EACCESS error regarding /opt/hostedtoolcache * Revert "Fix EACCESS error regarding /opt/hostedtoolcache" This reverts commit b2a8394d3358e1b5aab9dabe555d4a3f2bf0b2f9. * Revert CLI debug logs * Move socket and host handling to own function, and simplify logic * Move to container package * Make return be a struct * Write tests to verify functionality * Fix DOCKER_HOST being set to the string "DOCKER_HOST" * Always use struct * Use socketLocation, for DOCKER_HOST and more defaults * Fixup arguments to GetSocketAndHost in test and root.go * Un-struct hasDockerHost * Fixup logic and set hasDockerHost * Minor scoping & variable name change * Move functionality to a new file * Rename corresponding test * Reviewfix * Fix DOCKER_HOST expected * Fix test assertions and add comments * Swap comparison actual, expected * Fixed no-DOCKER_HOST env test * Fixed default socket test * Add test to verify review comments * Add more test for greater test coverage * Consistent comment references * Fix bug found while writing tests * Passing tests * NoMountNoHost testfix * Rename test appropriately * NoMount testfix * Fixed OnlySocket * Swap expected <-> actual in tests --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
1 parent 6e80373 commit f2e65e1

File tree

3 files changed

+292
-63
lines changed

3 files changed

+292
-63
lines changed

cmd/root.go

Lines changed: 8 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import (
3232
// Execute is the entry point to running the CLI
3333
func Execute(ctx context.Context, version string) {
3434
input := new(Input)
35-
var rootCmd = &cobra.Command{
35+
rootCmd := &cobra.Command{
3636
Use: "act [event name to run] [flags]\n\nIf no event name passed, will default to \"on: push\"\nIf actions handles only one event it will be used as default instead of \"on: push\"",
3737
Short: "Run GitHub actions locally by specifying the event name (e.g. `push`) or an action name directly.",
3838
Args: cobra.MaximumNArgs(1),
@@ -125,34 +125,6 @@ func configLocations() []string {
125125
return []string{specPath, homePath, invocationPath}
126126
}
127127

128-
var commonSocketPaths = []string{
129-
"/var/run/docker.sock",
130-
"/run/podman/podman.sock",
131-
"$HOME/.colima/docker.sock",
132-
"$XDG_RUNTIME_DIR/docker.sock",
133-
"$XDG_RUNTIME_DIR/podman/podman.sock",
134-
`\\.\pipe\docker_engine`,
135-
"$HOME/.docker/run/docker.sock",
136-
}
137-
138-
// returns socket path or false if not found any
139-
func socketLocation() (string, bool) {
140-
if dockerHost, exists := os.LookupEnv("DOCKER_HOST"); exists {
141-
return dockerHost, true
142-
}
143-
144-
for _, p := range commonSocketPaths {
145-
if _, err := os.Lstat(os.ExpandEnv(p)); err == nil {
146-
if strings.HasPrefix(p, `\\.\`) {
147-
return "npipe://" + filepath.ToSlash(os.ExpandEnv(p)), true
148-
}
149-
return "unix://" + filepath.ToSlash(os.ExpandEnv(p)), true
150-
}
151-
}
152-
153-
return "", false
154-
}
155-
156128
func args() []string {
157129
actrc := configLocations()
158130

@@ -185,7 +157,7 @@ func bugReport(ctx context.Context, version string) error {
185157

186158
report += sprintf("Docker host:", dockerHost)
187159
report += fmt.Sprintln("Sockets found:")
188-
for _, p := range commonSocketPaths {
160+
for _, p := range container.CommonSocketLocations {
189161
if _, err := os.Lstat(os.ExpandEnv(p)); err != nil {
190162
continue
191163
} else if _, err := os.Stat(os.ExpandEnv(p)); err != nil {
@@ -356,18 +328,6 @@ func parseMatrix(matrix []string) map[string]map[string]bool {
356328
return matrixes
357329
}
358330

359-
func isDockerHostURI(daemonPath string) bool {
360-
if protoIndex := strings.Index(daemonPath, "://"); protoIndex != -1 {
361-
scheme := daemonPath[:protoIndex]
362-
if strings.IndexFunc(scheme, func(r rune) bool {
363-
return (r < 'a' || r > 'z') && (r < 'A' || r > 'Z')
364-
}) == -1 {
365-
return true
366-
}
367-
}
368-
return false
369-
}
370-
371331
//nolint:gocyclo
372332
func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []string) error {
373333
return func(cmd *cobra.Command, args []string) error {
@@ -378,27 +338,12 @@ func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []str
378338
if ok, _ := cmd.Flags().GetBool("bug-report"); ok {
379339
return bugReport(ctx, cmd.Version)
380340
}
381-
382-
// Prefer DOCKER_HOST, don't override it
383-
socketPath, hasDockerHost := os.LookupEnv("DOCKER_HOST")
384-
if !hasDockerHost {
385-
// a - in containerDaemonSocket means don't mount, preserve this value
386-
// otherwise if input.containerDaemonSocket is a filepath don't use it as socketPath
387-
skipMount := input.containerDaemonSocket == "-" || !isDockerHostURI(input.containerDaemonSocket)
388-
if input.containerDaemonSocket != "" && !skipMount {
389-
socketPath = input.containerDaemonSocket
390-
} else {
391-
socket, found := socketLocation()
392-
if !found {
393-
log.Errorln("daemon Docker Engine socket not found and containerDaemonSocket option was not set")
394-
} else {
395-
socketPath = socket
396-
}
397-
if !skipMount {
398-
input.containerDaemonSocket = socketPath
399-
}
400-
}
401-
os.Setenv("DOCKER_HOST", socketPath)
341+
if ret, err := container.GetSocketAndHost(input.containerDaemonSocket); err != nil {
342+
log.Warnf("Couldn't get a valid docker connection: %+v", err)
343+
} else {
344+
os.Setenv("DOCKER_HOST", ret.Host)
345+
input.containerDaemonSocket = ret.Socket
346+
log.Infof("Using docker host '%s', and daemon socket '%s'", ret.Host, ret.Socket)
402347
}
403348

404349
if runtime.GOOS == "darwin" && runtime.GOARCH == "arm64" && input.containerArchitecture == "" {

pkg/container/docker_socket.go

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
package container
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"path/filepath"
7+
"strings"
8+
9+
log "github.com/sirupsen/logrus"
10+
)
11+
12+
var CommonSocketLocations = []string{
13+
"/var/run/docker.sock",
14+
"/run/podman/podman.sock",
15+
"$HOME/.colima/docker.sock",
16+
"$XDG_RUNTIME_DIR/docker.sock",
17+
"$XDG_RUNTIME_DIR/podman/podman.sock",
18+
`\\.\pipe\docker_engine`,
19+
"$HOME/.docker/run/docker.sock",
20+
}
21+
22+
// returns socket URI or false if not found any
23+
func socketLocation() (string, bool) {
24+
if dockerHost, exists := os.LookupEnv("DOCKER_HOST"); exists {
25+
return dockerHost, true
26+
}
27+
28+
for _, p := range CommonSocketLocations {
29+
if _, err := os.Lstat(os.ExpandEnv(p)); err == nil {
30+
if strings.HasPrefix(p, `\\.\`) {
31+
return "npipe://" + filepath.ToSlash(os.ExpandEnv(p)), true
32+
}
33+
return "unix://" + filepath.ToSlash(os.ExpandEnv(p)), true
34+
}
35+
}
36+
37+
return "", false
38+
}
39+
40+
// This function, `isDockerHostURI`, takes a string argument `daemonPath`. It checks if the
41+
// `daemonPath` is a valid Docker host URI. It does this by checking if the scheme of the URI (the
42+
// part before "://") contains only alphabetic characters. If it does, the function returns true,
43+
// indicating that the `daemonPath` is a Docker host URI. If it doesn't, or if the "://" delimiter
44+
// is not found in the `daemonPath`, the function returns false.
45+
func isDockerHostURI(daemonPath string) bool {
46+
if protoIndex := strings.Index(daemonPath, "://"); protoIndex != -1 {
47+
scheme := daemonPath[:protoIndex]
48+
if strings.IndexFunc(scheme, func(r rune) bool {
49+
return (r < 'a' || r > 'z') && (r < 'A' || r > 'Z')
50+
}) == -1 {
51+
return true
52+
}
53+
}
54+
return false
55+
}
56+
57+
type SocketAndHost struct {
58+
Socket string
59+
Host string
60+
}
61+
62+
func GetSocketAndHost(containerSocket string) (SocketAndHost, error) {
63+
log.Debugf("Handling container host and socket")
64+
65+
// Prefer DOCKER_HOST, don't override it
66+
dockerHost, hasDockerHost := socketLocation()
67+
socketHost := SocketAndHost{Socket: containerSocket, Host: dockerHost}
68+
69+
// ** socketHost.Socket cases **
70+
// Case 1: User does _not_ want to mount a daemon socket (passes a dash)
71+
// Case 2: User passes a filepath to the socket; is that even valid?
72+
// Case 3: User passes a valid socket; do nothing
73+
// Case 4: User omitted the flag; set a sane default
74+
75+
// ** DOCKER_HOST cases **
76+
// Case A: DOCKER_HOST is set; use it, i.e. do nothing
77+
// Case B: DOCKER_HOST is empty; use sane defaults
78+
79+
// Set host for sanity's sake, when the socket isn't useful
80+
if !hasDockerHost && (socketHost.Socket == "-" || !isDockerHostURI(socketHost.Socket) || socketHost.Socket == "") {
81+
// Cases: 1B, 2B, 4B
82+
socket, found := socketLocation()
83+
socketHost.Host = socket
84+
hasDockerHost = found
85+
}
86+
87+
// A - (dash) in socketHost.Socket means don't mount, preserve this value
88+
// otherwise if socketHost.Socket is a filepath don't use it as socket
89+
// Exit early if we're in an invalid state (e.g. when no DOCKER_HOST and user supplied "-", a dash or omitted)
90+
if !hasDockerHost && socketHost.Socket != "" && !isDockerHostURI(socketHost.Socket) {
91+
// Cases: 1B, 2B
92+
// Should we early-exit here, since there is no host nor socket to talk to?
93+
return SocketAndHost{}, fmt.Errorf("DOCKER_HOST was not set, couldn't be found in the usual locations, and the container daemon socket ('%s') is invalid", socketHost.Socket)
94+
}
95+
96+
// Default to DOCKER_HOST if set
97+
if socketHost.Socket == "" && hasDockerHost {
98+
// Cases: 4A
99+
log.Debugf("Defaulting container socket to DOCKER_HOST")
100+
socketHost.Socket = socketHost.Host
101+
}
102+
// Set sane default socket location if user omitted it
103+
if socketHost.Socket == "" {
104+
// Cases: 4B
105+
socket, _ := socketLocation()
106+
// socket is empty if it isn't found, so assignment here is at worst a no-op
107+
log.Debugf("Defaulting container socket to default '%s'", socket)
108+
socketHost.Socket = socket
109+
}
110+
111+
// Exit if both the DOCKER_HOST and socket are fulfilled
112+
if hasDockerHost {
113+
// Cases: 1A, 2A, 3A, 4A
114+
if !isDockerHostURI(socketHost.Socket) {
115+
// Cases: 1A, 2A
116+
log.Debugf("DOCKER_HOST is set, but socket is invalid '%s'", socketHost.Socket)
117+
}
118+
return socketHost, nil
119+
}
120+
121+
// Set a sane DOCKER_HOST default if we can
122+
if isDockerHostURI(socketHost.Socket) {
123+
// Cases: 3B
124+
log.Debugf("Setting DOCKER_HOST to container socket '%s'", socketHost.Socket)
125+
socketHost.Host = socketHost.Socket
126+
// Both DOCKER_HOST and container socket are valid; short-circuit exit
127+
return socketHost, nil
128+
}
129+
130+
// Here there is no DOCKER_HOST _and_ the supplied container socket is not a valid URI (either invalid or a file path)
131+
// Cases: 2B <- but is already handled at the top
132+
// I.e. this path should never be taken
133+
return SocketAndHost{}, fmt.Errorf("no DOCKER_HOST and an invalid container socket '%s'", socketHost.Socket)
134+
}
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
package container
2+
3+
import (
4+
"os"
5+
"testing"
6+
7+
log "github.com/sirupsen/logrus"
8+
assert "github.com/stretchr/testify/assert"
9+
)
10+
11+
func init() {
12+
log.SetLevel(log.DebugLevel)
13+
}
14+
15+
var originalCommonSocketLocations = CommonSocketLocations
16+
17+
func TestGetSocketAndHostWithSocket(t *testing.T) {
18+
// Arrange
19+
CommonSocketLocations = originalCommonSocketLocations
20+
dockerHost := "unix:///my/docker/host.sock"
21+
socketURI := "/path/to/my.socket"
22+
os.Setenv("DOCKER_HOST", dockerHost)
23+
24+
// Act
25+
ret, err := GetSocketAndHost(socketURI)
26+
27+
// Assert
28+
assert.Nil(t, err)
29+
assert.Equal(t, SocketAndHost{socketURI, dockerHost}, ret)
30+
}
31+
32+
func TestGetSocketAndHostNoSocket(t *testing.T) {
33+
// Arrange
34+
dockerHost := "unix:///my/docker/host.sock"
35+
os.Setenv("DOCKER_HOST", dockerHost)
36+
37+
// Act
38+
ret, err := GetSocketAndHost("")
39+
40+
// Assert
41+
assert.Nil(t, err)
42+
assert.Equal(t, SocketAndHost{dockerHost, dockerHost}, ret)
43+
}
44+
45+
func TestGetSocketAndHostOnlySocket(t *testing.T) {
46+
// Arrange
47+
socketURI := "/path/to/my.socket"
48+
os.Unsetenv("DOCKER_HOST")
49+
CommonSocketLocations = originalCommonSocketLocations
50+
defaultSocket, defaultSocketFound := socketLocation()
51+
52+
// Act
53+
ret, err := GetSocketAndHost(socketURI)
54+
55+
// Assert
56+
assert.NoError(t, err, "Expected no error from GetSocketAndHost")
57+
assert.Equal(t, true, defaultSocketFound, "Expected to find default socket")
58+
assert.Equal(t, socketURI, ret.Socket, "Expected socket to match common location")
59+
assert.Equal(t, defaultSocket, ret.Host, "Expected ret.Host to match default socket location")
60+
}
61+
62+
func TestGetSocketAndHostDontMount(t *testing.T) {
63+
// Arrange
64+
CommonSocketLocations = originalCommonSocketLocations
65+
dockerHost := "unix:///my/docker/host.sock"
66+
os.Setenv("DOCKER_HOST", dockerHost)
67+
68+
// Act
69+
ret, err := GetSocketAndHost("-")
70+
71+
// Assert
72+
assert.Nil(t, err)
73+
assert.Equal(t, SocketAndHost{"-", dockerHost}, ret)
74+
}
75+
76+
func TestGetSocketAndHostNoHostNoSocket(t *testing.T) {
77+
// Arrange
78+
CommonSocketLocations = originalCommonSocketLocations
79+
os.Unsetenv("DOCKER_HOST")
80+
defaultSocket, found := socketLocation()
81+
82+
// Act
83+
ret, err := GetSocketAndHost("")
84+
85+
// Assert
86+
assert.Equal(t, true, found, "Expected a default socket to be found")
87+
assert.Nil(t, err, "Expected no error from GetSocketAndHost")
88+
assert.Equal(t, SocketAndHost{defaultSocket, defaultSocket}, ret, "Expected to match default socket location")
89+
}
90+
91+
// Catch
92+
// > Your code breaks setting DOCKER_HOST if shouldMount is false.
93+
// > This happens if neither DOCKER_HOST nor --container-daemon-socket has a value, but socketLocation() returns a URI
94+
func TestGetSocketAndHostNoHostNoSocketDefaultLocation(t *testing.T) {
95+
// Arrange
96+
mySocketFile, tmpErr := os.CreateTemp("", "act-*.sock")
97+
mySocket := mySocketFile.Name()
98+
unixSocket := "unix://" + mySocket
99+
defer os.RemoveAll(mySocket)
100+
assert.NoError(t, tmpErr)
101+
os.Unsetenv("DOCKER_HOST")
102+
103+
CommonSocketLocations = []string{mySocket}
104+
defaultSocket, found := socketLocation()
105+
106+
// Act
107+
ret, err := GetSocketAndHost("")
108+
109+
// Assert
110+
assert.Equal(t, unixSocket, defaultSocket, "Expected default socket to match common socket location")
111+
assert.Equal(t, true, found, "Expected default socket to be found")
112+
assert.Nil(t, err, "Expected no error from GetSocketAndHost")
113+
assert.Equal(t, SocketAndHost{unixSocket, unixSocket}, ret, "Expected to match default socket location")
114+
}
115+
116+
func TestGetSocketAndHostNoHostInvalidSocket(t *testing.T) {
117+
// Arrange
118+
os.Unsetenv("DOCKER_HOST")
119+
mySocket := "/my/socket/path.sock"
120+
CommonSocketLocations = []string{"/unusual", "/socket", "/location"}
121+
defaultSocket, found := socketLocation()
122+
123+
// Act
124+
ret, err := GetSocketAndHost(mySocket)
125+
126+
// Assert
127+
assert.Equal(t, false, found, "Expected no default socket to be found")
128+
assert.Equal(t, "", defaultSocket, "Expected no default socket to be found")
129+
assert.Equal(t, SocketAndHost{}, ret, "Expected to match default socket location")
130+
assert.Error(t, err, "Expected an error in invalid state")
131+
}
132+
133+
func TestGetSocketAndHostOnlySocketValidButUnusualLocation(t *testing.T) {
134+
// Arrange
135+
socketURI := "unix:///path/to/my.socket"
136+
CommonSocketLocations = []string{"/unusual", "/location"}
137+
os.Unsetenv("DOCKER_HOST")
138+
defaultSocket, found := socketLocation()
139+
140+
// Act
141+
ret, err := GetSocketAndHost(socketURI)
142+
143+
// Assert
144+
// Default socket locations
145+
assert.Equal(t, "", defaultSocket, "Expect default socket location to be empty")
146+
assert.Equal(t, false, found, "Expected no default socket to be found")
147+
// Sane default
148+
assert.Nil(t, err, "Expect no error from GetSocketAndHost")
149+
assert.Equal(t, socketURI, ret.Host, "Expect host to default to unusual socket")
150+
}

0 commit comments

Comments
 (0)