-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 b2a8394. * 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>
- Loading branch information
1 parent
6e80373
commit f2e65e1
Showing
3 changed files
with
292 additions
and
63 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| package container | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| log "github.com/sirupsen/logrus" | ||
| ) | ||
|
|
||
| var CommonSocketLocations = []string{ | ||
| "/var/run/docker.sock", | ||
| "/run/podman/podman.sock", | ||
| "$HOME/.colima/docker.sock", | ||
| "$XDG_RUNTIME_DIR/docker.sock", | ||
| "$XDG_RUNTIME_DIR/podman/podman.sock", | ||
| `\\.\pipe\docker_engine`, | ||
| "$HOME/.docker/run/docker.sock", | ||
| } | ||
|
|
||
| // returns socket URI or false if not found any | ||
| func socketLocation() (string, bool) { | ||
| if dockerHost, exists := os.LookupEnv("DOCKER_HOST"); exists { | ||
| return dockerHost, true | ||
| } | ||
|
|
||
| for _, p := range CommonSocketLocations { | ||
| if _, err := os.Lstat(os.ExpandEnv(p)); err == nil { | ||
| if strings.HasPrefix(p, `\\.\`) { | ||
| return "npipe://" + filepath.ToSlash(os.ExpandEnv(p)), true | ||
| } | ||
| return "unix://" + filepath.ToSlash(os.ExpandEnv(p)), true | ||
| } | ||
| } | ||
|
|
||
| return "", false | ||
| } | ||
|
|
||
| // This function, `isDockerHostURI`, takes a string argument `daemonPath`. It checks if the | ||
| // `daemonPath` is a valid Docker host URI. It does this by checking if the scheme of the URI (the | ||
| // part before "://") contains only alphabetic characters. If it does, the function returns true, | ||
| // indicating that the `daemonPath` is a Docker host URI. If it doesn't, or if the "://" delimiter | ||
| // is not found in the `daemonPath`, the function returns false. | ||
| func isDockerHostURI(daemonPath string) bool { | ||
| if protoIndex := strings.Index(daemonPath, "://"); protoIndex != -1 { | ||
| scheme := daemonPath[:protoIndex] | ||
| if strings.IndexFunc(scheme, func(r rune) bool { | ||
| return (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') | ||
| }) == -1 { | ||
| return true | ||
| } | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| type SocketAndHost struct { | ||
| Socket string | ||
| Host string | ||
| } | ||
|
|
||
| func GetSocketAndHost(containerSocket string) (SocketAndHost, error) { | ||
| log.Debugf("Handling container host and socket") | ||
|
|
||
| // Prefer DOCKER_HOST, don't override it | ||
| dockerHost, hasDockerHost := socketLocation() | ||
| socketHost := SocketAndHost{Socket: containerSocket, Host: dockerHost} | ||
|
|
||
| // ** socketHost.Socket cases ** | ||
| // Case 1: User does _not_ want to mount a daemon socket (passes a dash) | ||
| // Case 2: User passes a filepath to the socket; is that even valid? | ||
| // Case 3: User passes a valid socket; do nothing | ||
| // Case 4: User omitted the flag; set a sane default | ||
|
|
||
| // ** DOCKER_HOST cases ** | ||
| // Case A: DOCKER_HOST is set; use it, i.e. do nothing | ||
| // Case B: DOCKER_HOST is empty; use sane defaults | ||
|
|
||
| // Set host for sanity's sake, when the socket isn't useful | ||
| if !hasDockerHost && (socketHost.Socket == "-" || !isDockerHostURI(socketHost.Socket) || socketHost.Socket == "") { | ||
| // Cases: 1B, 2B, 4B | ||
| socket, found := socketLocation() | ||
| socketHost.Host = socket | ||
| hasDockerHost = found | ||
| } | ||
|
|
||
| // A - (dash) in socketHost.Socket means don't mount, preserve this value | ||
| // otherwise if socketHost.Socket is a filepath don't use it as socket | ||
| // Exit early if we're in an invalid state (e.g. when no DOCKER_HOST and user supplied "-", a dash or omitted) | ||
| if !hasDockerHost && socketHost.Socket != "" && !isDockerHostURI(socketHost.Socket) { | ||
| // Cases: 1B, 2B | ||
| // Should we early-exit here, since there is no host nor socket to talk to? | ||
| 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) | ||
| } | ||
|
|
||
| // Default to DOCKER_HOST if set | ||
| if socketHost.Socket == "" && hasDockerHost { | ||
| // Cases: 4A | ||
| log.Debugf("Defaulting container socket to DOCKER_HOST") | ||
| socketHost.Socket = socketHost.Host | ||
| } | ||
| // Set sane default socket location if user omitted it | ||
| if socketHost.Socket == "" { | ||
| // Cases: 4B | ||
| socket, _ := socketLocation() | ||
| // socket is empty if it isn't found, so assignment here is at worst a no-op | ||
| log.Debugf("Defaulting container socket to default '%s'", socket) | ||
| socketHost.Socket = socket | ||
| } | ||
|
|
||
| // Exit if both the DOCKER_HOST and socket are fulfilled | ||
| if hasDockerHost { | ||
| // Cases: 1A, 2A, 3A, 4A | ||
| if !isDockerHostURI(socketHost.Socket) { | ||
| // Cases: 1A, 2A | ||
| log.Debugf("DOCKER_HOST is set, but socket is invalid '%s'", socketHost.Socket) | ||
| } | ||
| return socketHost, nil | ||
| } | ||
|
|
||
| // Set a sane DOCKER_HOST default if we can | ||
| if isDockerHostURI(socketHost.Socket) { | ||
| // Cases: 3B | ||
| log.Debugf("Setting DOCKER_HOST to container socket '%s'", socketHost.Socket) | ||
| socketHost.Host = socketHost.Socket | ||
| // Both DOCKER_HOST and container socket are valid; short-circuit exit | ||
| return socketHost, nil | ||
| } | ||
|
|
||
| // Here there is no DOCKER_HOST _and_ the supplied container socket is not a valid URI (either invalid or a file path) | ||
| // Cases: 2B <- but is already handled at the top | ||
| // I.e. this path should never be taken | ||
| return SocketAndHost{}, fmt.Errorf("no DOCKER_HOST and an invalid container socket '%s'", socketHost.Socket) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| package container | ||
|
|
||
| import ( | ||
| "os" | ||
| "testing" | ||
|
|
||
| log "github.com/sirupsen/logrus" | ||
| assert "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func init() { | ||
| log.SetLevel(log.DebugLevel) | ||
| } | ||
|
|
||
| var originalCommonSocketLocations = CommonSocketLocations | ||
|
|
||
| func TestGetSocketAndHostWithSocket(t *testing.T) { | ||
| // Arrange | ||
| CommonSocketLocations = originalCommonSocketLocations | ||
| dockerHost := "unix:///my/docker/host.sock" | ||
| socketURI := "/path/to/my.socket" | ||
| os.Setenv("DOCKER_HOST", dockerHost) | ||
|
|
||
| // Act | ||
| ret, err := GetSocketAndHost(socketURI) | ||
|
|
||
| // Assert | ||
| assert.Nil(t, err) | ||
| assert.Equal(t, SocketAndHost{socketURI, dockerHost}, ret) | ||
| } | ||
|
|
||
| func TestGetSocketAndHostNoSocket(t *testing.T) { | ||
| // Arrange | ||
| dockerHost := "unix:///my/docker/host.sock" | ||
| os.Setenv("DOCKER_HOST", dockerHost) | ||
|
|
||
| // Act | ||
| ret, err := GetSocketAndHost("") | ||
|
|
||
| // Assert | ||
| assert.Nil(t, err) | ||
| assert.Equal(t, SocketAndHost{dockerHost, dockerHost}, ret) | ||
| } | ||
|
|
||
| func TestGetSocketAndHostOnlySocket(t *testing.T) { | ||
| // Arrange | ||
| socketURI := "/path/to/my.socket" | ||
| os.Unsetenv("DOCKER_HOST") | ||
| CommonSocketLocations = originalCommonSocketLocations | ||
| defaultSocket, defaultSocketFound := socketLocation() | ||
|
|
||
| // Act | ||
| ret, err := GetSocketAndHost(socketURI) | ||
|
|
||
| // Assert | ||
| assert.NoError(t, err, "Expected no error from GetSocketAndHost") | ||
| assert.Equal(t, true, defaultSocketFound, "Expected to find default socket") | ||
| assert.Equal(t, socketURI, ret.Socket, "Expected socket to match common location") | ||
| assert.Equal(t, defaultSocket, ret.Host, "Expected ret.Host to match default socket location") | ||
| } | ||
|
|
||
| func TestGetSocketAndHostDontMount(t *testing.T) { | ||
| // Arrange | ||
| CommonSocketLocations = originalCommonSocketLocations | ||
| dockerHost := "unix:///my/docker/host.sock" | ||
| os.Setenv("DOCKER_HOST", dockerHost) | ||
|
|
||
| // Act | ||
| ret, err := GetSocketAndHost("-") | ||
|
|
||
| // Assert | ||
| assert.Nil(t, err) | ||
| assert.Equal(t, SocketAndHost{"-", dockerHost}, ret) | ||
| } | ||
|
|
||
| func TestGetSocketAndHostNoHostNoSocket(t *testing.T) { | ||
| // Arrange | ||
| CommonSocketLocations = originalCommonSocketLocations | ||
| os.Unsetenv("DOCKER_HOST") | ||
| defaultSocket, found := socketLocation() | ||
|
|
||
| // Act | ||
| ret, err := GetSocketAndHost("") | ||
|
|
||
| // Assert | ||
| assert.Equal(t, true, found, "Expected a default socket to be found") | ||
| assert.Nil(t, err, "Expected no error from GetSocketAndHost") | ||
| assert.Equal(t, SocketAndHost{defaultSocket, defaultSocket}, ret, "Expected to match default socket location") | ||
| } | ||
|
|
||
| // Catch | ||
| // > Your code breaks setting DOCKER_HOST if shouldMount is false. | ||
| // > This happens if neither DOCKER_HOST nor --container-daemon-socket has a value, but socketLocation() returns a URI | ||
| func TestGetSocketAndHostNoHostNoSocketDefaultLocation(t *testing.T) { | ||
| // Arrange | ||
| mySocketFile, tmpErr := os.CreateTemp("", "act-*.sock") | ||
| mySocket := mySocketFile.Name() | ||
| unixSocket := "unix://" + mySocket | ||
| defer os.RemoveAll(mySocket) | ||
| assert.NoError(t, tmpErr) | ||
| os.Unsetenv("DOCKER_HOST") | ||
|
|
||
| CommonSocketLocations = []string{mySocket} | ||
| defaultSocket, found := socketLocation() | ||
|
|
||
| // Act | ||
| ret, err := GetSocketAndHost("") | ||
|
|
||
| // Assert | ||
| assert.Equal(t, unixSocket, defaultSocket, "Expected default socket to match common socket location") | ||
| assert.Equal(t, true, found, "Expected default socket to be found") | ||
| assert.Nil(t, err, "Expected no error from GetSocketAndHost") | ||
| assert.Equal(t, SocketAndHost{unixSocket, unixSocket}, ret, "Expected to match default socket location") | ||
| } | ||
|
|
||
| func TestGetSocketAndHostNoHostInvalidSocket(t *testing.T) { | ||
| // Arrange | ||
| os.Unsetenv("DOCKER_HOST") | ||
| mySocket := "/my/socket/path.sock" | ||
| CommonSocketLocations = []string{"/unusual", "/socket", "/location"} | ||
| defaultSocket, found := socketLocation() | ||
|
|
||
| // Act | ||
| ret, err := GetSocketAndHost(mySocket) | ||
|
|
||
| // Assert | ||
| assert.Equal(t, false, found, "Expected no default socket to be found") | ||
| assert.Equal(t, "", defaultSocket, "Expected no default socket to be found") | ||
| assert.Equal(t, SocketAndHost{}, ret, "Expected to match default socket location") | ||
| assert.Error(t, err, "Expected an error in invalid state") | ||
| } | ||
|
|
||
| func TestGetSocketAndHostOnlySocketValidButUnusualLocation(t *testing.T) { | ||
| // Arrange | ||
| socketURI := "unix:///path/to/my.socket" | ||
| CommonSocketLocations = []string{"/unusual", "/location"} | ||
| os.Unsetenv("DOCKER_HOST") | ||
| defaultSocket, found := socketLocation() | ||
|
|
||
| // Act | ||
| ret, err := GetSocketAndHost(socketURI) | ||
|
|
||
| // Assert | ||
| // Default socket locations | ||
| assert.Equal(t, "", defaultSocket, "Expect default socket location to be empty") | ||
| assert.Equal(t, false, found, "Expected no default socket to be found") | ||
| // Sane default | ||
| assert.Nil(t, err, "Expect no error from GetSocketAndHost") | ||
| assert.Equal(t, socketURI, ret.Host, "Expect host to default to unusual socket") | ||
| } |