From d5be1524667c6d816ea73eaa143fb594e6aab074 Mon Sep 17 00:00:00 2001
From: Alban Crequy <alban@kinvolk.io>
Date: Wed, 20 Jun 2018 17:03:01 +0200
Subject: [PATCH] validation: mounts: fix condition of source & type check

The runtime-spec says that [mount source is optional](
https://github.com/opencontainers/runtime-spec/blob/v1.0.1/config.md#mounts).
So let's relax condition of the mount source path check, so that it only
checks for an empty mount source. Ditto for the type field.

The 'mount' test now tries different mount options: bind and not bind,
recursive or not, different mount propagation modes.

runc passes the test after https://github.com/opencontainers/runc/pull/1845

Closes: https://github.com/opencontainers/runtime-tools/issues/651

Based on work from: Dongsu Park <dongsu@kinvolk.io>
Signed-off-by: Alban Crequy <alban@kinvolk.io>
---
 cmd/runtimetest/main.go     | 26 ++++++++++--
 validation/mounts/mounts.go | 85 ++++++++++++++++++++++++++++++++-----
 2 files changed, 96 insertions(+), 15 deletions(-)

diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go
index ad30bef92..77e3e15b8 100644
--- a/cmd/runtimetest/main.go
+++ b/cmd/runtimetest/main.go
@@ -1091,14 +1091,32 @@ func mountMatch(configMount rspec.Mount, sysMount *mount.Info) error {
 		return fmt.Errorf("mount destination expected: %v, actual: %v", configMount.Destination, sys.Destination)
 	}
 
-	if configMount.Type != sys.Type {
+	isBind := false
+	for _, opt := range configMount.Options {
+		if opt == "bind" || opt == "rbind" {
+			isBind = true
+			break
+		}
+	}
+	// Type is an optional field in the spec: only check if it is set
+	if configMount.Type != "" && configMount.Type != sys.Type {
 		return fmt.Errorf("mount %v type expected: %v, actual: %v", configMount.Destination, configMount.Type, sys.Type)
 	}
 
-	if filepath.Clean(configMount.Source) != sys.Source {
-		return fmt.Errorf("mount %v source expected: %v, actual: %v", configMount.Destination, configMount.Source, sys.Source)
+	// For bind mounts, the source is not the block device but the path on the host that is being bind mounted.
+	// sysMount.Root is that path.
+	if isBind {
+		// Source is an optional field in the spec: only check if it is set
+		// We only test the base name here, in case the tests are being run in a chroot environment
+		if configMount.Source != "" && filepath.Base(configMount.Source) != filepath.Base(sysMount.Root) {
+			return fmt.Errorf("mount %v source expected: %v, actual: %v", configMount.Destination, filepath.Base(configMount.Source), filepath.Base(sysMount.Root))
+		}
+	} else {
+		// Source is an optional field in the spec: only check if it is set
+		if configMount.Source != "" && filepath.Clean(configMount.Source) != sys.Source {
+			return fmt.Errorf("mount %v source expected: %v, actual: %v", configMount.Destination, configMount.Source, sys.Source)
+		}
 	}
-
 	return nil
 }
 
diff --git a/validation/mounts/mounts.go b/validation/mounts/mounts.go
index 416014824..8cd373805 100644
--- a/validation/mounts/mounts.go
+++ b/validation/mounts/mounts.go
@@ -6,22 +6,85 @@ import (
 )
 
 func main() {
+	defaultOptions := []string{
+		"nosuid",
+		"strictatime",
+		"mode=755",
+		"size=1k",
+	}
+
+	// Different combinations of mount types, mount options, mount propagation modes
+	mounts := []rspec.Mount{
+		rspec.Mount{
+			Destination: "/tmp/test-shared",
+			Type:        "tmpfs",
+			Source:      "tmpfs",
+			Options:     []string{"shared"},
+		},
+		rspec.Mount{
+			Destination: "/tmp/test-slave",
+			Type:        "tmpfs",
+			Source:      "tmpfs",
+			Options:     []string{"slave"},
+		},
+		rspec.Mount{
+			Destination: "/tmp/test-private",
+			Type:        "tmpfs",
+			Source:      "tmpfs",
+			Options:     []string{"private"},
+		},
+		rspec.Mount{
+			Destination: "/mnt/etc-shared",
+			Source:      "/etc",
+			Options:     []string{"bind", "shared"},
+		},
+		rspec.Mount{
+			Destination: "/mnt/etc-rshared",
+			Source:      "/etc",
+			Options:     []string{"rbind", "rshared"},
+		},
+		rspec.Mount{
+			Destination: "/mnt/etc-slave",
+			Source:      "/etc",
+			Options:     []string{"bind", "slave"},
+		},
+		rspec.Mount{
+			Destination: "/mnt/etc-rslave",
+			Source:      "/etc",
+			Options:     []string{"rbind", "rslave"},
+		},
+		rspec.Mount{
+			Destination: "/mnt/etc-private",
+			Source:      "/etc",
+			Options:     []string{"bind", "private"},
+		},
+		rspec.Mount{
+			Destination: "/mnt/etc-rprivate",
+			Source:      "/etc",
+			Options:     []string{"rbind", "rprivate"},
+		},
+		rspec.Mount{
+			Destination: "/mnt/etc-unbindable",
+			Source:      "/etc",
+			Options:     []string{"bind", "unbindable"},
+		},
+		rspec.Mount{
+			Destination: "/mnt/etc-runbindable",
+			Source:      "/etc",
+			Options:     []string{"rbind", "runbindable"},
+		},
+	}
+
 	g, err := util.GetDefaultGenerator()
 	if err != nil {
 		util.Fatal(err)
 	}
-	mount := rspec.Mount{
-		Destination: "/tmp",
-		Type:        "tmpfs",
-		Source:      "tmpfs",
-		Options: []string{
-			"nosuid",
-			"strictatime",
-			"mode=755",
-			"size=1k",
-		},
+
+	for _, m := range mounts {
+		m.Options = append(defaultOptions, m.Options...)
+
+		g.AddMount(m)
 	}
-	g.AddMount(mount)
 	err = util.RuntimeInsideValidate(g, nil, nil)
 	if err != nil {
 		util.Fatal(err)