Add Relative function to Locator interface

This adds a Relative function to the Locator interface and uses
this to determine the host and container paths for located files
(and devices). This ensures that the root (e.g. the nvidia driver
root) is stripped from the container path.

Signed-off-by: Evan Lezar <elezar@nvidia.com>

internal/discover/csv_test.go

@@ -49,7 +49,7 @@ func TestCharDevices(t *testing.T) {
 					required: []string{"required"},
 				},
 			),
-			expectedDevices: []Device{{Path: "located"}},
+			expectedDevices: []Device{{Path: "located", HostPath: "located"}},
 		},
 		{
 			description:          "dev devices returns error for nil lookup",

internal/discover/discover.go

@@ -24,12 +24,14 @@ type Config struct {
 
 // Device represents a discovered character device.
 type Device struct {
-	Path string
+	HostPath string
+	Path     string
 }
 
 // Mount represents a discovered mount.
 type Mount struct {
-	Path string
+	HostPath string
+	Path     string
 }
 
 // Hook represents a discovered hook.

internal/discover/mounts.go

@@ -51,7 +51,7 @@ func (d *mounts) Mounts() ([]Mount, error) {
 	d.Lock()
 	defer d.Unlock()
 
-	paths := make(map[string]bool)
+	uniqueMounts := make(map[string]Mount)
 
 	for _, candidate := range d.required {
 		d.logger.Debugf("Locating %v", candidate)
@@ -66,20 +66,34 @@ func (d *mounts) Mounts() ([]Mount, error) {
 		}
 		d.logger.Debugf("Located %v as %v", candidate, located)
 		for _, p := range located {
-			paths[p] = true
+			if _, ok := uniqueMounts[p]; ok {
+				d.logger.Debugf("Skipping duplicate mount %v", p)
+				continue
+			}
+
+			r, err := d.lookup.Relative(p)
+			if err != nil {
+				d.logger.Warnf("Failed to get relative path of %v: %v", p, err)
+				continue
+			}
+			if r == "" {
+				r = p
+			}
+
+			d.logger.Infof("Selecting %v as %v", p, r)
+			uniqueMounts[p] = Mount{
+				HostPath: p,
+				Path:     r,
+			}
 		}
 	}
 
 	var mounts []Mount
-	for path := range paths {
-		d.logger.Infof("Selecting %v", path)
-		mount := Mount{
-			Path: path,
-		}
-		mounts = append(mounts, mount)
+	for _, m := range uniqueMounts {
+		mounts = append(mounts, m)
 	}
 
 	d.cache = mounts
 
-	return mounts, nil
+	return d.cache, nil
 }

internal/discover/mounts_test.go

@@ -70,7 +70,7 @@ func TestMounts(t *testing.T) {
 				},
 				required: []string{"required"},
 			},
-			expectedMounts: []Mount{{Path: "located"}},
+			expectedMounts: []Mount{{Path: "located", HostPath: "located"}},
 		},
 		{
 			description:   "mounts removes located duplicates",
@@ -83,7 +83,7 @@ func TestMounts(t *testing.T) {
 				},
 				required: []string{"required0", "required1"},
 			},
-			expectedMounts: []Mount{{Path: "located"}},
+			expectedMounts: []Mount{{Path: "located", HostPath: "located"}},
 		},
 		{
 			description: "mounts skips located errors",
@@ -98,7 +98,7 @@ func TestMounts(t *testing.T) {
 				},
 				required: []string{"required0", "error", "required1"},
 			},
-			expectedMounts: []Mount{{Path: "required0"}, {Path: "required1"}},
+			expectedMounts: []Mount{{Path: "required0", HostPath: "required0"}, {Path: "required1", HostPath: "required1"}},
 		},
 		{
 			description: "mounts skips unlocated",
@@ -113,10 +113,10 @@ func TestMounts(t *testing.T) {
 				},
 				required: []string{"required0", "empty", "required1"},
 			},
-			expectedMounts: []Mount{{Path: "required0"}, {Path: "required1"}},
+			expectedMounts: []Mount{{Path: "required0", HostPath: "required0"}, {Path: "required1", HostPath: "required1"}},
 		},
 		{
-			description: "mounts skips unlocated",
+			description: "mounts adds multiple",
 			input: &mounts{
 				lookup: &lookup.LocatorMock{
 					LocateFunc: func(s string) ([]string, error) {
@@ -129,10 +129,48 @@ func TestMounts(t *testing.T) {
 				required: []string{"required0", "multiple", "required1"},
 			},
 			expectedMounts: []Mount{
-				{Path: "required0"},
-				{Path: "multiple0"},
-				{Path: "multiple1"},
-				{Path: "required1"},
+				{Path: "required0", HostPath: "required0"},
+				{Path: "multiple0", HostPath: "multiple0"},
+				{Path: "multiple1", HostPath: "multiple1"},
+				{Path: "required1", HostPath: "required1"},
+			},
+		},
+		{
+			description: "mounts uses relative path",
+			input: &mounts{
+				lookup: &lookup.LocatorMock{
+					LocateFunc: func(s string) ([]string, error) {
+						return []string{"located"}, nil
+					},
+					RelativeFunc: func(s string) (string, error) {
+						return "relative", nil
+					},
+				},
+				required: []string{"required0", "multiple", "required1"},
+			},
+			expectedMounts: []Mount{
+				{Path: "relative", HostPath: "located"},
+			},
+		},
+		{
+			description: "mounts skips relative error",
+			input: &mounts{
+				lookup: &lookup.LocatorMock{
+					LocateFunc: func(s string) ([]string, error) {
+						return []string{s}, nil
+					},
+					RelativeFunc: func(s string) (string, error) {
+						if s == "error" {
+							return "", fmt.Errorf("no relative path")
+						}
+						return "relative" + s, nil
+					},
+				},
+				required: []string{"required0", "error", "required1"},
+			},
+			expectedMounts: []Mount{
+				{Path: "relativerequired0", HostPath: "required0"},
+				{Path: "relativerequired1", HostPath: "required1"},
 			},
 		},
 	}

internal/edits/device.go

@@ -37,6 +37,9 @@ func (d device) toEdits() *cdi.ContainerEdits {
 // toSpec converts a discovered Device to a CDI Spec Device. Note
 // that missing info is filled in when edits are applied by querying the Device node.
 func (d device) toSpec() *specs.DeviceNode {
+	// NOTE: We may want to mirror what is done in cri-o w.r.t src (Host) and dst (Container) paths
+	// to ensure that the right permissions are included.
+	// https://github.com/cri-o/cri-o/blob/ca3bb80a3dda0440659fcf8da8ed6f23211de94e/internal/config/device/device.go#L93
 	s := specs.DeviceNode{
 		Path: d.Path,
 	}

internal/edits/mount.go

@@ -38,8 +38,7 @@ func (d mount) toEdits() *cdi.ContainerEdits {
 // that missing info is filled in when edits are applied by querying the Mount node.
 func (d mount) toSpec() *specs.Mount {
 	s := specs.Mount{
-		HostPath: d.Path,
-		// TODO: We need to allow the container path to be customised
+		HostPath:      d.HostPath,
 		ContainerPath: d.Path,
 		Options: []string{
 			"ro",

internal/lookup/file.go

@@ -28,6 +28,7 @@ import (
 // prefixes. The validity of a file is determined by a filter function.
 type file struct {
 	logger   *log.Logger
+	root     string
 	prefixes []string
 	filter   func(string) error
 }
@@ -77,6 +78,15 @@ func (p file) Locate(pattern string) ([]string, error) {
 	return filenames, nil
 }
 
+// Relative returns the path relative to the root for the file locator
+func (p file) Relative(path string) (string, error) {
+	if p.root == "" || p.root == "/" {
+		return path, nil
+	}
+
+	return filepath.Rel(p.root, path)
+}
+
 // assertFile checks whether the specified path is a regular file
 func assertFile(filename string) error {
 	info, err := os.Stat(filename)

internal/lookup/locator.go

@@ -21,4 +21,5 @@ package lookup
 // Locator defines the interface for locating files on a system.
 type Locator interface {
 	Locate(string) ([]string, error)
+	Relative(string) (string, error)
 }

internal/lookup/locator_mock.go

@@ -20,6 +20,9 @@ var _ Locator = &LocatorMock{}
 // 			LocateFunc: func(s string) ([]string, error) {
 // 				panic("mock out the Locate method")
 // 			},
+// 			RelativeFunc: func(s string) (string, error) {
+// 				panic("mock out the Relative method")
+// 			},
 // 		}
 //
 // 		// use mockedLocator in code that requires Locator
@@ -30,6 +33,9 @@ type LocatorMock struct {
 	// LocateFunc mocks the Locate method.
 	LocateFunc func(s string) ([]string, error)
 
+	// RelativeFunc mocks the Relative method.
+	RelativeFunc func(s string) (string, error)
+
 	// calls tracks calls to the methods.
 	calls struct {
 		// Locate holds details about calls to the Locate method.
@@ -37,8 +43,14 @@ type LocatorMock struct {
 			// S is the s argument value.
 			S string
 		}
+		// Relative holds details about calls to the Relative method.
+		Relative []struct {
+			// S is the s argument value.
+			S string
+		}
 	}
-	lockLocate sync.RWMutex
+	lockLocate   sync.RWMutex
+	lockRelative sync.RWMutex
 }
 
 // Locate calls LocateFunc.
@@ -75,3 +87,38 @@ func (mock *LocatorMock) LocateCalls() []struct {
 	mock.lockLocate.RUnlock()
 	return calls
 }
+
+// Relative calls RelativeFunc.
+func (mock *LocatorMock) Relative(s string) (string, error) {
+	callInfo := struct {
+		S string
+	}{
+		S: s,
+	}
+	mock.lockRelative.Lock()
+	mock.calls.Relative = append(mock.calls.Relative, callInfo)
+	mock.lockRelative.Unlock()
+	if mock.RelativeFunc == nil {
+		var (
+			sOut   string
+			errOut error
+		)
+		return sOut, errOut
+	}
+	return mock.RelativeFunc(s)
+}
+
+// RelativeCalls gets all the calls that were made to Relative.
+// Check the length with:
+//     len(mockedLocator.RelativeCalls())
+func (mock *LocatorMock) RelativeCalls() []struct {
+	S string
+} {
+	var calls []struct {
+		S string
+	}
+	mock.lockRelative.RLock()
+	calls = mock.calls.Relative
+	mock.lockRelative.RUnlock()
+	return calls
+}