pkg/ns: introduce error types indicate NS verification

plugins: don't require CNI_NETNS for DEL command

pkg/ns/ns.go

@@ -20,6 +20,7 @@ import (
 	"os"
 	"path"
 	"runtime"
+	"strings"
 	"sync"
 	"syscall"
 
@@ -74,38 +75,66 @@ func GetCurrentNS() (NetNS, error) {
 	return GetNS(getCurrentThreadNetNSPath())
 }
 
-// Returns an object representing the namespace referred to by @path
+const (
-func GetNS(nspath string) (NetNS, error) {
+	// https://github.com/torvalds/linux/blob/master/include/uapi/linux/magic.h
-	fd, err := os.Open(nspath)
+	NSFS_MAGIC   = 0x6e736673
-	if err != nil {
+	PROCFS_MAGIC = 0x9fa0
-		return nil, fmt.Errorf("Failed to open %v: %v", nspath, err)
+)
+
+type NSPathNotExistErr struct{ msg string }
+
+func (e NSPathNotExistErr) Error() string { return e.msg }
+
+type NSPathNotNSErr struct{ msg string }
+
+func (e NSPathNotNSErr) Error() string { return e.msg }
+
+func IsNSorErr(nspath string) error {
+	stat := syscall.Statfs_t{}
+	if err := syscall.Statfs(nspath, &stat); err != nil {
+		if os.IsNotExist(err) {
+			err = NSPathNotExistErr{msg: fmt.Sprintf("failed to Statfs %q: %v", nspath, err)}
+		} else {
+			err = fmt.Errorf("failed to Statfs %q: %v", nspath, err)
+		}
+		return err
 	}
 
-	isNSFS, err := IsNSFS(nspath)
+	switch stat.Type {
+	case PROCFS_MAGIC:
+		// Kernel < 3.19
+
+		validPathContent := "ns/"
+		validName := strings.Contains(nspath, validPathContent)
+		if !validName {
+			return NSPathNotNSErr{msg: fmt.Sprintf("path %q doesn't contain %q", nspath, validPathContent)}
+		}
+
+		return nil
+	case NSFS_MAGIC:
+		// Kernel >= 3.19
+
+		return nil
+	default:
+		return NSPathNotNSErr{msg: fmt.Sprintf("unknown FS magic on %q: %x", nspath, stat.Type)}
+	}
+}
+
+// Returns an object representing the namespace referred to by @path
+func GetNS(nspath string) (NetNS, error) {
+	err := IsNSorErr(nspath)
 	if err != nil {
-		fd.Close()
 		return nil, err
 	}
-	if !isNSFS {
+
-		fd.Close()
+	fd, err := os.Open(nspath)
-		return nil, fmt.Errorf("%v is not of type NSFS", nspath)
+	if err != nil {
+		return nil, err
 	}
 
 	return &netNS{file: fd}, nil
 }
 
-// Returns whether or not the nspath argument points to a network namespace
-func IsNSFS(nspath string) (bool, error) {
-	const NSFS_MAGIC = 0x6e736673
-
-	stat := syscall.Statfs_t{}
-	if err := syscall.Statfs(nspath, &stat); err != nil {
-		return false, fmt.Errorf("failed to Statfs %q: %v", nspath, err)
-	}
-
-	return stat.Type == NSFS_MAGIC, nil
-}
-
 // Creates a new persistent network namespace and returns an object
 // representing that namespace, without switching to it
 func NewNS() (NetNS, error) {

pkg/ns/ns_test.go

@@ -180,7 +180,9 @@ var _ = Describe("Linux namespace operations", func() {
 				defer os.Remove(nspath)
 
 				_, err = ns.GetNS(nspath)
-				Expect(err).To(MatchError(fmt.Sprintf("%v is not of type NSFS", nspath)))
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(BeAssignableToTypeOf(ns.NSPathNotNSErr{}))
+				Expect(err).NotTo(BeAssignableToTypeOf(ns.NSPathNotExistErr{}))
 			})
 		})
 
@@ -212,12 +214,11 @@ var _ = Describe("Linux namespace operations", func() {
 		})
 	})
 
-	Describe("IsNSFS", func() {
+	Describe("IsNSorErr", func() {
 		It("should detect a namespace", func() {
 			createdNetNS, err := ns.NewNS()
-			isNSFS, err := ns.IsNSFS(createdNetNS.Path())
+			err = ns.IsNSorErr(createdNetNS.Path())
 			Expect(err).NotTo(HaveOccurred())
-			Expect(isNSFS).To(Equal(true))
 		})
 
 		It("should refuse other paths", func() {
@@ -228,9 +229,17 @@ var _ = Describe("Linux namespace operations", func() {
 			nspath := tempFile.Name()
 			defer os.Remove(nspath)
 
-			isNSFS, err := ns.IsNSFS(nspath)
+			err = ns.IsNSorErr(nspath)
-			Expect(err).NotTo(HaveOccurred())
+			Expect(err).To(HaveOccurred())
-			Expect(isNSFS).To(Equal(false))
+			Expect(err).To(BeAssignableToTypeOf(ns.NSPathNotNSErr{}))
+			Expect(err).NotTo(BeAssignableToTypeOf(ns.NSPathNotExistErr{}))
+		})
+
+		It("should error on non-existing paths", func() {
+			err := ns.IsNSorErr("/tmp/IDoNotExist")
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(BeAssignableToTypeOf(ns.NSPathNotExistErr{}))
+			Expect(err).NotTo(BeAssignableToTypeOf(ns.NSPathNotNSErr{}))
 		})
 	})
 })

pkg/skel/skel.go

@@ -36,28 +36,72 @@ type CmdArgs struct {
 	StdinData   []byte
 }
 
+type reqForCmdEntry map[string]bool
+
 // PluginMain is the "main" for a plugin. It accepts
 // two callback functions for add and del commands.
 func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) {
 	var cmd, contID, netns, ifName, args, path string
 
 	vars := []struct {
-		name string
+		name      string
-		val  *string
+		val       *string
-		req  bool
+		reqForCmd reqForCmdEntry
 	}{
-		{"CNI_COMMAND", &cmd, true},
+		{
-		{"CNI_CONTAINERID", &contID, false},
+			"CNI_COMMAND",
-		{"CNI_NETNS", &netns, true},
+			&cmd,
-		{"CNI_IFNAME", &ifName, true},
+			reqForCmdEntry{
-		{"CNI_ARGS", &args, false},
+				"ADD": true,
-		{"CNI_PATH", &path, true},
+				"DEL": true,
+			},
+		},
+		{
+			"CNI_CONTAINERID",
+			&contID,
+			reqForCmdEntry{
+				"ADD": false,
+				"DEL": false,
+			},
+		},
+		{
+			"CNI_NETNS",
+			&netns,
+			reqForCmdEntry{
+				"ADD": true,
+				"DEL": false,
+			},
+		},
+		{
+			"CNI_IFNAME",
+			&ifName,
+			reqForCmdEntry{
+				"ADD": true,
+				"DEL": true,
+			},
+		},
+		{
+			"CNI_ARGS",
+			&args,
+			reqForCmdEntry{
+				"ADD": false,
+				"DEL": false,
+			},
+		},
+		{
+			"CNI_PATH",
+			&path,
+			reqForCmdEntry{
+				"ADD": true,
+				"DEL": true,
+			},
+		},
 	}
 
 	argsMissing := false
 	for _, v := range vars {
 		*v.val = os.Getenv(v.name)
-		if v.req && *v.val == "" {
+		if v.reqForCmd[cmd] && *v.val == "" {
 			log.Printf("%v env variable missing", v.name)
 			argsMissing = true
 		}

pkg/skel/skel_test.go

@@ -71,5 +71,14 @@ var _ = Describe("Skel", func() {
 		// 	Expect(err).NotTo(HaveOccurred())
 		// 	PluginMain(fErr, nil)
 		// })
+
+		It("should not fail with DEL and no NETNS and noop callback", func() {
+			err := os.Setenv("CNI_COMMAND", "DEL")
+			Expect(err).NotTo(HaveOccurred())
+			err = os.Unsetenv("CNI_NETNS")
+			Expect(err).NotTo(HaveOccurred())
+			PluginMain(nil, fNoop)
+		})
+
 	})
 })

plugins/main/bridge/bridge.go

@@ -289,6 +289,10 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if args.Netns == "" {
+		return nil
+	}
+
 	var ipn *net.IPNet
 	err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		var err error

plugins/main/ipvlan/ipvlan.go

@@ -152,6 +152,10 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if args.Netns == "" {
+		return nil
+	}
+
 	return ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		return ip.DelLinkByName(args.IfName)
 	})

plugins/main/macvlan/macvlan.go

@@ -170,6 +170,10 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if args.Netns == "" {
+		return nil
+	}
+
 	return ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		return ip.DelLinkByName(args.IfName)
 	})

plugins/main/ptp/ptp.go

@@ -199,6 +199,10 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
+	if args.Netns == "" {
+		return nil
+	}
+
 	var ipn *net.IPNet
 	err := ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		var err error