diff --git a/internal/discover/csv_test.go b/internal/discover/csv_test.go index 28b1e021..d1b37d0e 100644 --- a/internal/discover/csv_test.go +++ b/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", diff --git a/internal/discover/discover.go b/internal/discover/discover.go index 64dbef85..e65b360e 100644 --- a/internal/discover/discover.go +++ b/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. diff --git a/internal/discover/mounts.go b/internal/discover/mounts.go index 07650509..006c7d56 100644 --- a/internal/discover/mounts.go +++ b/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 } diff --git a/internal/discover/mounts_test.go b/internal/discover/mounts_test.go index b443d18c..6d8e3d20 100644 --- a/internal/discover/mounts_test.go +++ b/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"}, }, }, } diff --git a/internal/edits/device.go b/internal/edits/device.go index 5e11f41d..4dd6a303 100644 --- a/internal/edits/device.go +++ b/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, } diff --git a/internal/edits/mount.go b/internal/edits/mount.go index 0697321e..46386322 100644 --- a/internal/edits/mount.go +++ b/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", diff --git a/internal/lookup/file.go b/internal/lookup/file.go index ab52d03d..1283f0ae 100644 --- a/internal/lookup/file.go +++ b/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) diff --git a/internal/lookup/locator.go b/internal/lookup/locator.go index 871e1b02..76ade332 100644 --- a/internal/lookup/locator.go +++ b/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) } diff --git a/internal/lookup/locator_mock.go b/internal/lookup/locator_mock.go index 0c50f345..349539ae 100644 --- a/internal/lookup/locator_mock.go +++ b/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 +}