Merge pull request opencontainers#3159 from thaJeztah/norunes

libct/devices: change devices.Type to be a string

Author: kolyshkin

libcontainer/cgroups/devices/devices_emulator.go

@@ -88,24 +88,22 @@ func parseLine(line string) (*deviceRule, error) {
 	}
 	var (
 		rule  deviceRule
-		node  = matches[1]
+		node  = devices.Type(matches[1])
 		major = matches[2]
 		minor = matches[3]
 		perms = matches[4]
 	)
 
 	// Parse the node type.
 	switch node {
-	case "a":
+	case devices.WildcardDevice:
 		// Super-special case -- "a" always means every device with every
 		// access mode. In fact, for devices.list this actually indicates that
 		// the cgroup is in black-list mode.
 		// TODO: Double-check that the entire file is "a *:* rwm".
 		return nil, nil
-	case "b":
-		rule.meta.node = devices.BlockDevice
-	case "c":
-		rule.meta.node = devices.CharDevice
+	case devices.BlockDevice, devices.CharDevice:
+		rule.meta.node = node
 	default:
 		// Should never happen!
 		return nil, fmt.Errorf("unknown device type %q", node)
@@ -139,6 +137,7 @@ func parseLine(line string) (*deviceRule, error) {
 		// Should never happen!
 		return nil, fmt.Errorf("parse access mode: contained unknown modes or is empty: %q", perms)
 	}
+
 	return &rule, nil
 }
 
@@ -320,10 +319,10 @@ func (source *Emulator) Transition(target *Emulator) ([]*devices.Rule, error) {
 	// black-list we also have to include a disruptive rule.
 	if source.IsBlacklist() || source.defaultAllow != target.defaultAllow {
 		transitionRules = append(transitionRules, &devices.Rule{
-			Type:        'a',
+			Type:        devices.WildcardDevice,
 			Major:       -1,
 			Minor:       -1,
-			Permissions: devices.Permissions("rwm"),
+			Permissions: "rwm",
 			Allow:       target.defaultAllow,
 		})
 		// The old rules are only relevant if we aren't starting out with a

libcontainer/cgroups/ebpf/devicefilter/devicefilter.go

@@ -119,7 +119,7 @@ func (p *program) appendRule(rule *devices.Rule) error {
 		bpfType = int32(unix.BPF_DEVCG_DEV_BLOCK)
 	default:
 		// We do not permit 'a', nor any other types we don't know about.
-		return fmt.Errorf("invalid type %q", string(rule.Type))
+		return fmt.Errorf("invalid type %q", rule.Type)
 	}
 	if rule.Major > math.MaxUint32 {
 		return fmt.Errorf("invalid major %d", rule.Major)

libcontainer/cgroups/ebpf/devicefilter/devicefilter_test.go

@@ -146,7 +146,7 @@ block-11:
 func TestDeviceFilter_Privileged(t *testing.T) {
 	devices := []*devices.Rule{
 		{
-			Type:        'a',
+			Type:        devices.WildcardDevice,
 			Major:       -1,
 			Minor:       -1,
 			Permissions: "rwm",
@@ -173,14 +173,14 @@ block-0:
 func TestDeviceFilter_PrivilegedExceptSingleDevice(t *testing.T) {
 	devices := []*devices.Rule{
 		{
-			Type:        'a',
+			Type:        devices.WildcardDevice,
 			Major:       -1,
 			Minor:       -1,
 			Permissions: "rwm",
 			Allow:       true,
 		},
 		{
-			Type:        'b',
+			Type:        devices.BlockDevice,
 			Major:       8,
 			Minor:       0,
 			Permissions: "rwm",
@@ -213,21 +213,21 @@ block-1:
 func TestDeviceFilter_Weird(t *testing.T) {
 	devices := []*devices.Rule{
 		{
-			Type:        'b',
+			Type:        devices.BlockDevice,
 			Major:       8,
 			Minor:       1,
 			Permissions: "rwm",
 			Allow:       false,
 		},
 		{
-			Type:        'a',
+			Type:        devices.WildcardDevice,
 			Major:       -1,
 			Minor:       -1,
 			Permissions: "rwm",
 			Allow:       true,
 		},
 		{
-			Type:        'b',
+			Type:        devices.BlockDevice,
 			Major:       8,
 			Minor:       2,
 			Permissions: "rwm",

libcontainer/devices/device.go

@@ -100,13 +100,13 @@ func (p Permissions) IsValid() bool {
 	return p == fromSet(p.toSet())
 }
 
-type Type rune
+type Type string
 
 const (
-	WildcardDevice Type = 'a'
-	BlockDevice    Type = 'b'
-	CharDevice     Type = 'c' // or 'u'
-	FifoDevice     Type = 'p'
+	WildcardDevice Type = "a"
+	BlockDevice    Type = "b"
+	CharDevice     Type = "c" // or 'u'
+	FifoDevice     Type = "p"
 )
 
 func (t Type) IsValid() bool {
@@ -166,7 +166,7 @@ func (d *Rule) CgroupString() string {
 	if d.Minor == Wildcard {
 		minor = "*"
 	}
-	return fmt.Sprintf("%c %s:%s %s", d.Type, major, minor, d.Permissions)
+	return fmt.Sprintf("%s %s:%s %s", d.Type, major, minor, d.Permissions)
 }
 
 func (d *Rule) Mkdev() (uint64, error) {

libcontainer/rootfs_linux.go

@@ -710,7 +710,7 @@ func mknodDevice(dest string, node *devices.Device) error {
 	case devices.FifoDevice:
 		fileMode |= unix.S_IFIFO
 	default:
-		return fmt.Errorf("%c is not a valid device type for device %s", node.Type, node.Path)
+		return fmt.Errorf("%s is not a valid device type for device %s", node.Type, node.Path)
 	}
 	dev, err := node.Mkdev()
 	if err != nil {

libcontainer/specconv/spec_linux.go

@@ -479,12 +479,15 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 		if r != nil {
 			for i, d := range spec.Linux.Resources.Devices {
 				var (
-					t     = "a"
+					dt    = devices.WildcardDevice
 					major = int64(-1)
 					minor = int64(-1)
 				)
 				if d.Type != "" {
-					t = d.Type
+					dt = devices.Type(d.Type)
+					if !dt.CanCgroup() {
+						return nil, fmt.Errorf("invalid cgroup device type %q", d.Type)
+					}
 				}
 				if d.Major != nil {
 					major = *d.Major
@@ -495,10 +498,6 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 				if d.Access == "" {
 					return nil, fmt.Errorf("device access at %d field cannot be empty", i)
 				}
-				dt, err := stringToCgroupDeviceRune(t)
-				if err != nil {
-					return nil, err
-				}
 				c.Resources.Devices = append(c.Resources.Devices, &devices.Rule{
 					Type:        dt,
 					Major:       major,
@@ -635,20 +634,7 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 	return c, nil
 }
 
-func stringToCgroupDeviceRune(s string) (devices.Type, error) {
-	switch s {
-	case "a":
-		return devices.WildcardDevice, nil
-	case "b":
-		return devices.BlockDevice, nil
-	case "c":
-		return devices.CharDevice, nil
-	default:
-		return 0, fmt.Errorf("invalid cgroup device type %q", s)
-	}
-}
-
-func stringToDeviceRune(s string) (devices.Type, error) {
+func stringToDeviceType(s string) (devices.Type, error) {
 	switch s {
 	case "p":
 		return devices.FifoDevice, nil
@@ -657,7 +643,7 @@ func stringToDeviceRune(s string) (devices.Type, error) {
 	case "b":
 		return devices.BlockDevice, nil
 	default:
-		return 0, fmt.Errorf("invalid device type %q", s)
+		return "", fmt.Errorf("invalid device type %q", s)
 	}
 }
 
@@ -693,7 +679,7 @@ next:
 			if d.GID != nil {
 				gid = *d.GID
 			}
-			dt, err := stringToDeviceRune(d.Type)
+			dt, err := stringToDeviceType(d.Type)
 			if err != nil {
 				return nil, err
 			}