v2, util: Take the full binary path when starting the shimv2 process

fidencio · fidencio · commit d80dbdae6823 · 2021-02-05T13:35:22.000+01:00
The current code simply ignores the full binary path when starting the shimv2 process, and instead fallbacks to a binary in the path, and this is problematic (and confusing) for those using CRI-O, which has this bits vendored. The reason it's problematic with CRI-O is because the user can simply set the full binary path and, instead of having that executed, CRI-O will simply fail to create the container unless that binary is part of the path, which may not be case in a few different scenarios (testing being the most common one). Fixes: #5006 Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
diff --git a/runtime/v2/shim/util.go b/runtime/v2/shim/util.go
@@ -64,21 +64,28 @@ func Command(ctx context.Context, runtime, containerdAddress, containerdTTRPCAdd
 		cmdPath = cmdPathI.(string)
 	} else {
 		var lerr error
-		if cmdPath, lerr = exec.LookPath(name); lerr != nil {
-			if eerr, ok := lerr.(*exec.Error); ok {
-				if eerr.Err == exec.ErrNotFound {
-					// LookPath only finds current directory matches based on
-					// the callers current directory but the caller is not
-					// likely in the same directory as the containerd
-					// executables. Instead match the calling binaries path
-					// (containerd) and see if they are side by side. If so
-					// execute the shim found there.
-					testPath := filepath.Join(filepath.Dir(self), name)
-					if _, serr := os.Stat(testPath); serr == nil {
-						cmdPath = testPath
-					}
-					if cmdPath == "" {
-						return nil, errors.Wrapf(os.ErrNotExist, "runtime %q binary not installed %q", runtime, name)
+		binaryPath := BinaryPath(runtime)
+		if _, serr := os.Stat(binaryPath); serr == nil {
+			cmdPath = binaryPath
+		}
+
+		if cmdPath == "" {
+			if cmdPath, lerr = exec.LookPath(name); err != nil {
+				if eerr, ok := lerr.(*exec.Error); ok {
+					if eerr.Err == exec.ErrNotFound {
+						// LookPath only finds current directory matches based on
+						// the callers current directory but the caller is not
+						// likely in the same directory as the containerd
+						// executables. Instead match the calling binaries path
+						// (containerd) and see if they are side by side. If so
+						// execute the shim found there.
+						testPath := filepath.Join(filepath.Dir(self), name)
+						if _, serr := os.Stat(testPath); serr == nil {
+							cmdPath = testPath
+						}
+						if cmdPath == "" {
+							return nil, errors.Wrapf(os.ErrNotExist, "runtime %q binary not installed %q", runtime, name)
+						}
 					}
 				}
 			}
@@ -123,6 +130,20 @@ func BinaryName(runtime string) string {
 	return fmt.Sprintf(shimBinaryFormat, parts[len(parts)-2], parts[len(parts)-1])
 }
 
+// BinaryPath returns the full path for the shim binary from the runtime name,
+// empty string returns means runtime name is invalid
+func BinaryPath(runtime string) string {
+	dir := filepath.Dir(runtime)
+	binary := BinaryName(runtime)
+
+	path, err := filepath.Abs(filepath.Join(dir, binary))
+	if err != nil {
+		return ""
+	}
+
+	return path
+}
+
 // Connect to the provided address
 func Connect(address string, d func(string, time.Duration) (net.Conn, error)) (net.Conn, error) {
 	return d(address, 100*time.Second)
