-
Notifications
You must be signed in to change notification settings - Fork 142
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import ( | |
"io/ioutil" | ||
"os" | ||
"path/filepath" | ||
"runtime" | ||
"strconv" | ||
"strings" | ||
"syscall" | ||
|
@@ -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 | ||
} | ||
} | ||
|
||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. iff -> if There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, got it. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -479,6 +605,7 @@ func validate(context *cli.Context) error { | |
validateHostname, | ||
validateRlimits, | ||
validateMountsExist, | ||
validateMountOrder, | ||
} | ||
|
||
linuxValidations := []validation{ | ||
|
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.