From d80dbdae6823804cae096dec41542ec68145e0cd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
Date: Fri, 5 Feb 2021 13:20:54 +0100
Subject: [PATCH] v2, util: Take the full binary path when starting the shimv2
 process
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 runtime/v2/shim/util.go | 51 +++++++++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 15 deletions(-)

diff --git a/runtime/v2/shim/util.go b/runtime/v2/shim/util.go
index 2bb786d90..59f9ff21c 100644
--- 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)