pinns: pin to /var/run/*ns instead of /var/run/crio/ns/*

haircommander · haircommander · commit c443e9b887c8 · 2020-04-03T14:10:14.000-04:00
This change is largely needed for third party networking plugins. These plugins often bind mount /var/run/netns and expect to be able to operate on the namespaces of the pods. Unfortunately, by pinning to /var/run/crio/ns*, we lose this capability. Rather than pinning everything to /var/run/netns/ns/uts or something odd, change the way pinns builds the pinned namespace. Now, we bind namespaces to /var/run/$NS_NAMEns/$ID

Signed-off-by: Peter Hunt &lt;pehunt@redhat.com&gt;
diff --git a/docs/crio.8.md b/docs/crio.8.md
@@ -232,7 +232,7 @@ crio [GLOBAL OPTIONS] command [COMMAND OPTIONS] [ARGUMENTS...]
 
 **--metrics-port**="": Port for the metrics endpoint (default: 9090)
 
-**--namespaces-dir**="": The directory where the state of the managed namespaces gets tracked. Only used when manage-ns-lifecycle is true (default: /var/run/crio/ns)
+**--namespaces-dir**="": The directory where the state of the managed namespaces gets tracked. Only used when manage-ns-lifecycle is true (default: /var/run)
 
 **--no-pivot**: If true, the runtime will not use `pivot_root`, but instead use `MS_MOVE` (default: false)
 
diff --git a/docs/crio.conf.5.md b/docs/crio.conf.5.md
@@ -210,7 +210,7 @@ The `crio.runtime` table contains settings pertaining to the OCI runtime used an
 **manage_ns_lifecycle**=false
   Determines whether we pin and remove namespaces and manage their lifecycle
 
-**namespaces_dir**="/var/run/crio/ns"
+**namespaces_dir**="/var/run"
   The directory where the state of the managed namespaces gets tracked. Only used when manage_ns_lifecycle is true
 
 **pinns_path**=""
diff --git a/internal/lib/sandbox/namespaces_linux.go b/internal/lib/sandbox/namespaces_linux.go
@@ -63,13 +63,11 @@ func pinNamespaces(nsTypes []NSType, cfg *config.Config) ([]NamespaceIface, erro
 		NETNS:  "-n",
 	}
 
-	pinDir := filepath.Join(cfg.NamespacesDir, uuid.New().String())
-
-	if err := os.MkdirAll(pinDir, 0755); err != nil {
-		return nil, err
+	pinnedNamespace := uuid.New().String()
+	pinnsArgs := []string{
+		"-d", cfg.NamespacesDir,
+		"-f", pinnedNamespace,
 	}
-
-	pinnsArgs := []string{"-d", pinDir}
 	type namespaceInfo struct {
 		path   string
 		nsType NSType
@@ -83,7 +81,7 @@ func pinNamespaces(nsTypes []NSType, cfg *config.Config) ([]NamespaceIface, erro
 		}
 		pinnsArgs = append(pinnsArgs, arg)
 		mountedNamespaces = append(mountedNamespaces, namespaceInfo{
-			path:   filepath.Join(pinDir, string(nsType)),
+			path:   filepath.Join(cfg.NamespacesDir, fmt.Sprintf("%sns", string(nsType)), pinnedNamespace),
 			nsType: nsType,
 		})
 	}
diff --git a/pinns/pinns.c b/pinns/pinns.c
@@ -14,33 +14,15 @@
 
 #include "utils.h"
 
-static int bind_ns(const char *pin_path, const char *ns_name) {
-  char bind_path[PATH_MAX];
-  char ns_path[PATH_MAX];
-  int fd;
-
-  snprintf(bind_path, PATH_MAX - 1, "%s/%s", pin_path, ns_name);
-  fd = open(bind_path, O_RDONLY | O_CREAT | O_EXCL, 0);
-  if (fd < 0) {
-    pwarn("Failed to create ns file");
-    return -1;
-  }
-  close(fd);
-
-  snprintf(ns_path, PATH_MAX - 1, "/proc/self/ns/%s", ns_name);
-  if (mount(ns_path, bind_path, NULL, MS_BIND, NULL) < 0) {
-    pwarnf("Failed to bind mount ns: %s", ns_path);
-    return -1;
-  }
-
-  return 0;
-}
+static int bind_ns(const char *pin_path, const char *filename, const char *ns_name);
+static int directory_exists_or_create(const char* path);
 
 int main(int argc, char **argv) {
   int num_unshares = 0;
   int unshare_flags = 0;
   int c;
   char *pin_path = NULL;
+  char *filename = NULL;
   bool bind_net = false;
   bool bind_uts = false;
   bool bind_ipc = false;
@@ -55,9 +37,10 @@ int main(int argc, char **argv) {
       {"user", optional_argument, NULL, 'U'},
       {"cgroup", optional_argument, NULL, 'c'},
       {"dir", required_argument, NULL, 'd'},
+      {"filename", required_argument, NULL, 'f'},
   };
 
-  while ((c = getopt_long(argc, argv, "pchuUind:", long_options, NULL)) != -1) {
+  while ((c = getopt_long(argc, argv, "pchuUind:f:", long_options, NULL)) != -1) {
     switch (c) {
     case 'u':
       unshare_flags |= CLONE_NEWUTS;
@@ -90,6 +73,9 @@ int main(int argc, char **argv) {
     case 'd':
       pin_path = optarg;
       break;
+    case 'f':
+      filename = optarg;
+      break;
     case 'h':
       // usage();
     default:
@@ -102,13 +88,12 @@ int main(int argc, char **argv) {
     pexit("Path for pinning namespaces not specified");
   }
 
-  struct stat sb;
-  if (stat(pin_path, &sb) != 0) {
-    pexitf("Failed to check if directory %s exists", pin_path);
+  if (!filename) {
+    pexit("Filename for pinning namespaces not specified");
   }
 
-  if (!S_ISDIR(sb.st_mode)) {
-    nexitf("%s is not a directory", pin_path);
+  if (directory_exists_or_create(pin_path) < 0) {
+    nexitf("%s exists but is not a directory", pin_path);
   }
 
   if (num_unshares == 0) {
@@ -120,34 +105,78 @@ int main(int argc, char **argv) {
   }
 
   if (bind_uts) {
-    if (bind_ns(pin_path, "uts") < 0) {
+    if (bind_ns(pin_path, filename, "uts") < 0) {
       return EXIT_FAILURE;
     }
   }
 
   if (bind_ipc) {
-    if (bind_ns(pin_path, "ipc") < 0) {
+    if (bind_ns(pin_path, filename, "ipc") < 0) {
       return EXIT_FAILURE;
     }
   }
 
   if (bind_net) {
-    if (bind_ns(pin_path, "net") < 0) {
+    if (bind_ns(pin_path, filename, "net") < 0) {
       return EXIT_FAILURE;
     }
   }
 
   if (bind_user) {
-    if (bind_ns(pin_path, "user") < 0) {
+    if (bind_ns(pin_path, filename, "user") < 0) {
       return EXIT_FAILURE;
     }
   }
 
   if (bind_cgroup) {
-    if (bind_ns(pin_path, "cgroup") < 0) {
+    if (bind_ns(pin_path, filename, "cgroup") < 0) {
       return EXIT_FAILURE;
     }
   }
 
   return EXIT_SUCCESS;
 }
+
+static int bind_ns(const char *pin_path, const char *filename, const char *ns_name) {
+  char bind_path[PATH_MAX];
+  char ns_path[PATH_MAX];
+  int fd;
+
+  // first, verify the /$PATH/$NSns directory exists
+  snprintf(bind_path, PATH_MAX - 1, "%s/%sns", pin_path, ns_name);
+  if (directory_exists_or_create(bind_path) < 0) {
+    pwarnf("%s exists and is not a directory", bind_path);
+    return -1;
+  }
+
+  // now, get the real path we want
+  snprintf(bind_path, PATH_MAX - 1, "%s/%sns/%s", pin_path, ns_name, filename);
+
+  fd = open(bind_path, O_RDONLY | O_CREAT | O_EXCL, 0);
+  if (fd < 0) {
+    pwarn("Failed to create ns file");
+    return -1;
+  }
+  close(fd);
+
+  snprintf(ns_path, PATH_MAX - 1, "/proc/self/ns/%s", ns_name);
+  if (mount(ns_path, bind_path, NULL, MS_BIND, NULL) < 0) {
+    pwarnf("Failed to bind mount ns: %s", ns_path);
+    return -1;
+  }
+
+  return 0;
+}
+
+static int directory_exists_or_create(const char* path) {
+  struct stat sb;
+  if (stat(path, &sb) != 0) {
+    mkdir(path, 0755);
+	return 0;
+  }
+
+  if (!S_ISDIR(sb.st_mode)) {
+    return -1;
+  }
+  return 0;
+}
diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -552,7 +552,7 @@ func DefaultConfig() (*Config, error) {
 			DefaultCapabilities:      DefaultCapabilities,
 			LogLevel:                 "info",
 			HooksDir:                 []string{hooks.DefaultDir},
-			NamespacesDir:            "/var/run/crio/ns",
+			NamespacesDir:            "/var/run",
 			seccompConfig:            seccomp.New(),
 			apparmorConfig:           apparmor.New(),
 		},
@@ -787,7 +787,7 @@ func (c *RuntimeConfig) Validate(systemContext *types.SystemContext, onExecution
 			return errors.Wrap(err, "pinns validation")
 		}
 
-		if err := os.MkdirAll(c.NamespacesDir, 0700); err != nil {
+		if err := os.MkdirAll(c.NamespacesDir, 0755); err != nil {
 			return errors.Wrap(err, "invalid namespaces_dir")
 		}
 
