Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtimetest: add validation of mount order #88

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 127 additions & 0 deletions cmd/runtimetest/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"syscall"
Expand Down Expand Up @@ -459,6 +460,131 @@ func validateMountsExist(spec *rspec.Spec) error {
return nil
}

func isParent(parent, child string) bool {
if parent == child {
return false
}

parent = filepath.ToSlash(parent)
child = filepath.ToSlash(child)

cparts := strings.Split(child, "/")
for i, part := range strings.Split(parent, "/") {
if cparts[i] != part {
return false

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we use filepath.Rel() can reduce the loop

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I remember that I didn't use it for /some/ reason but I can't remember what it was -- so we'll just switch it.

}
}

return true
}

func isMountPoint(path string, mountinfos []*mount.Info) (bool, error) {
// Find the mountpoint for path.
var mounts []string
pathindex := -1
for idx, mi := range mountinfos {
if mi.Mountpoint == path {
pathindex = idx
}
mounts = append(mounts, mi.Mountpoint)
}

// It isn't in mountinfo.
if pathindex < 0 {
return false, nil
}

// Check that the mount isn't followed by a mount on a parent directory.
hasParent := false
for _, other := range mounts[pathindex+1:] {
// If we see our mountpoint again, we reset the assumption.
if other == path {
hasParent = false
continue
}

// If there's a case where something was mounted over then we
// invalidate the assumption.
if isParent(other, path) {
hasParent = true
}
}

return !hasParent, nil
}

// Finds and returns any two paths in the given slice where pathA is a parent of
// pathB. Otherwise it returns "", "", false.
func findNestedPaths(paths []string) (string, string, bool) {
for _, parent := range paths {
for _, child := range paths {
if isParent(parent, child) {
return parent, child, true
}
}
}

return "", "", false
}

func validateMountOrder(spec *rspec.Spec) error {
// Windows doesn't support the concept of nested mounts, so this test
// doesn't make any sense on that platform.
if runtime.GOOS == "windows" {
return nil
}

fmt.Println("validating mount order")

var mounts []string
for _, m := range spec.Mounts {
mounts = append(mounts, filepath.Clean(m.Destination))
}

// Get the mountinfo for us.
mountinfos, err := mount.GetMounts()
if err != nil {
return err
}

// If there are two mount options where A is a parent of B, then we can
// verify that the right order is maintained no matter which order they are
// in the mounts.
A, B, ok := findNestedPaths(mounts)
if !ok {
return nil
}

// Figure out the order of A and B.
var first string
for _, m := range mounts {
if A == m || B == m {
first = m
break
}
}

// A must *always* be a mountpoint.
if ok, err := isMountPoint(A, mountinfos); err != nil {
return fmt.Errorf("failed to get whether %q is a mountpoint: %q", A, err)
} else if !ok {
return fmt.Errorf("expected %q to be a mountpoint", A)
}

// B must be a mountpoint iff A was first.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iff -> if

Copy link
Member Author

@cyphar cyphar Oct 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iff == if and only if. It's a mathematical statement which is like a two-way if-then statement. https://en.wikipedia.org/wiki/If_and_only_if

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, got it.
But I think using general and easy understood expression may be better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll write out the words "if and only if" if it'll make you feel better. The reason I wrote it like that is because the test is testing that precise statement, namely that B will only be a mountpoint if A was first (and that if A is first then B will be a mountpoint).

if ok, err := isMountPoint(B, mountinfos); err != nil {
return fmt.Errorf("failed to get whether %q is a mountpoint: %q", A, err)
} else {
if first == A && !ok {
return fmt.Errorf("expected %q to be a mountpoint", B)
} else if first == B && ok {
return fmt.Errorf("expected %q to not be a mountpoint", B)
}
}

return nil
}

func validate(context *cli.Context) error {
logLevelString := context.String("log-level")
logLevel, err := logrus.ParseLevel(logLevelString)
Expand All @@ -479,6 +605,7 @@ func validate(context *cli.Context) error {
validateHostname,
validateRlimits,
validateMountsExist,
validateMountOrder,
}

linuxValidations := []validation{
Expand Down