Merge pull request #130 from elezar/better-is-qualified-name

Move parsing and name validation to cdi/parse package

Author: bart0sh

pkg/cdi/annotations.go

@@ -20,6 +20,8 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+
+	"github.com/container-orchestrated-devices/container-device-interface/pkg/parser"
 )
 
 const (
@@ -101,22 +103,22 @@ func AnnotationKey(pluginName, deviceID string) (string, error) {
 		return "", fmt.Errorf("invalid plugin+deviceID %q, too long", name)
 	}
 
-	if c := rune(name[0]); !isAlphaNumeric(c) {
+	if c := rune(name[0]); !parser.IsAlphaNumeric(c) {
 		return "", fmt.Errorf("invalid name %q, first '%c' should be alphanumeric",
 			name, c)
 	}
 	if len(name) > 2 {
 		for _, c := range name[1 : len(name)-1] {
 			switch {
-			case isAlphaNumeric(c):
+			case parser.IsAlphaNumeric(c):
 			case c == '_' || c == '-' || c == '.':
 			default:
 				return "", fmt.Errorf("invalid name %q, invalid charcter '%c'",
 					name, c)
 			}
 		}
 	}
-	if c := rune(name[len(name)-1]); !isAlphaNumeric(c) {
+	if c := rune(name[len(name)-1]); !parser.IsAlphaNumeric(c) {
 		return "", fmt.Errorf("invalid name %q, last '%c' should be alphanumeric",
 			name, c)
 	}

pkg/cdi/device.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"github.com/container-orchestrated-devices/container-device-interface/internal/validation"
+	"github.com/container-orchestrated-devices/container-device-interface/pkg/parser"
 	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 )
@@ -51,7 +52,7 @@ func (d *Device) GetSpec() *Spec {
 
 // GetQualifiedName returns the qualified name for this device.
 func (d *Device) GetQualifiedName() string {
-	return QualifiedName(d.spec.GetVendor(), d.spec.GetClass(), d.Name)
+	return parser.QualifiedName(d.spec.GetVendor(), d.spec.GetClass(), d.Name)
 }
 
 // ApplyEdits applies the device-speific container edits to an OCI Spec.

pkg/cdi/qualified-device.go

@@ -17,189 +17,101 @@
 package cdi
 
 import (
-	"fmt"
-	"strings"
+	"github.com/container-orchestrated-devices/container-device-interface/pkg/parser"
 )
 
 // QualifiedName returns the qualified name for a device.
 // The syntax for a qualified device names is
-//   "<vendor>/<class>=<name>".
+//
+//	"<vendor>/<class>=<name>".
+//
 // A valid vendor name may contain the following runes:
-//   'A'-'Z', 'a'-'z', '0'-'9', '.', '-', '_'.
+//
+//	'A'-'Z', 'a'-'z', '0'-'9', '.', '-', '_'.
+//
 // A valid class name may contain the following runes:
-//   'A'-'Z', 'a'-'z', '0'-'9', '-', '_'.
+//
+//	'A'-'Z', 'a'-'z', '0'-'9', '-', '_'.
+//
 // A valid device name may containe the following runes:
-//   'A'-'Z', 'a'-'z', '0'-'9', '-', '_', '.', ':'
+//
+//	'A'-'Z', 'a'-'z', '0'-'9', '-', '_', '.', ':'
+//
+// Deprecated: use parser.QualifiedName instead
 func QualifiedName(vendor, class, name string) string {
-	return vendor + "/" + class + "=" + name
+	return parser.QualifiedName(vendor, class, name)
 }
 
 // IsQualifiedName tests if a device name is qualified.
+//
+// Deprecated: use parser.IsQualifiedName instead
 func IsQualifiedName(device string) bool {
-	_, _, _, err := ParseQualifiedName(device)
-	return err == nil
+	return parser.IsQualifiedName(device)
 }
 
 // ParseQualifiedName splits a qualified name into device vendor, class,
 // and name. If the device fails to parse as a qualified name, or if any
 // of the split components fail to pass syntax validation, vendor and
 // class are returned as empty, together with the verbatim input as the
 // name and an error describing the reason for failure.
+//
+// Deprecated: use parser.ParseQualifiedName instead
 func ParseQualifiedName(device string) (string, string, string, error) {
-	vendor, class, name := ParseDevice(device)
-
-	if vendor == "" {
-		return "", "", device, fmt.Errorf("unqualified device %q, missing vendor", device)
-	}
-	if class == "" {
-		return "", "", device, fmt.Errorf("unqualified device %q, missing class", device)
-	}
-	if name == "" {
-		return "", "", device, fmt.Errorf("unqualified device %q, missing device name", device)
-	}
-
-	if err := ValidateVendorName(vendor); err != nil {
-		return "", "", device, fmt.Errorf("invalid device %q: %w", device, err)
-	}
-	if err := ValidateClassName(class); err != nil {
-		return "", "", device, fmt.Errorf("invalid device %q: %w", device, err)
-	}
-	if err := ValidateDeviceName(name); err != nil {
-		return "", "", device, fmt.Errorf("invalid device %q: %w", device, err)
-	}
-
-	return vendor, class, name, nil
+	return parser.ParseQualifiedName(device)
 }
 
 // ParseDevice tries to split a device name into vendor, class, and name.
 // If this fails, for instance in the case of unqualified device names,
 // ParseDevice returns an empty vendor and class together with name set
 // to the verbatim input.
+//
+// Deprecated: use parser.ParseDevice instead
 func ParseDevice(device string) (string, string, string) {
-	if device == "" || device[0] == '/' {
-		return "", "", device
-	}
-
-	parts := strings.SplitN(device, "=", 2)
-	if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
-		return "", "", device
-	}
-
-	name := parts[1]
-	vendor, class := ParseQualifier(parts[0])
-	if vendor == "" {
-		return "", "", device
-	}
-
-	return vendor, class, name
+	return parser.ParseDevice(device)
 }
 
 // ParseQualifier splits a device qualifier into vendor and class.
 // The syntax for a device qualifier is
-//     "<vendor>/<class>"
+//
+//	"<vendor>/<class>"
+//
 // If parsing fails, an empty vendor and the class set to the
 // verbatim input is returned.
+//
+// Deprecated: use parser.ParseQualifier instead
 func ParseQualifier(kind string) (string, string) {
-	parts := strings.SplitN(kind, "/", 2)
-	if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
-		return "", kind
-	}
-	return parts[0], parts[1]
+	return parser.ParseQualifier(kind)
 }
 
 // ValidateVendorName checks the validity of a vendor name.
 // A vendor name may contain the following ASCII characters:
 //   - upper- and lowercase letters ('A'-'Z', 'a'-'z')
 //   - digits ('0'-'9')
 //   - underscore, dash, and dot ('_', '-', and '.')
+//
+// Deprecated: use parser.ValidateVendorName instead
 func ValidateVendorName(vendor string) error {
-	if vendor == "" {
-		return fmt.Errorf("invalid (empty) vendor name")
-	}
-	if !isLetter(rune(vendor[0])) {
-		return fmt.Errorf("invalid vendor %q, should start with letter", vendor)
-	}
-	for _, c := range string(vendor[1 : len(vendor)-1]) {
-		switch {
-		case isAlphaNumeric(c):
-		case c == '_' || c == '-' || c == '.':
-		default:
-			return fmt.Errorf("invalid character '%c' in vendor name %q",
-				c, vendor)
-		}
-	}
-	if !isAlphaNumeric(rune(vendor[len(vendor)-1])) {
-		return fmt.Errorf("invalid vendor %q, should end with a letter or digit", vendor)
-	}
-
-	return nil
+	return parser.ValidateVendorName(vendor)
 }
 
 // ValidateClassName checks the validity of class name.
 // A class name may contain the following ASCII characters:
 //   - upper- and lowercase letters ('A'-'Z', 'a'-'z')
 //   - digits ('0'-'9')
 //   - underscore and dash ('_', '-')
+//
+// Deprecated: use parser.ValidateClassName instead
 func ValidateClassName(class string) error {
-	if class == "" {
-		return fmt.Errorf("invalid (empty) device class")
-	}
-	if !isLetter(rune(class[0])) {
-		return fmt.Errorf("invalid class %q, should start with letter", class)
-	}
-	for _, c := range string(class[1 : len(class)-1]) {
-		switch {
-		case isAlphaNumeric(c):
-		case c == '_' || c == '-':
-		default:
-			return fmt.Errorf("invalid character '%c' in device class %q",
-				c, class)
-		}
-	}
-	if !isAlphaNumeric(rune(class[len(class)-1])) {
-		return fmt.Errorf("invalid class %q, should end with a letter or digit", class)
-	}
-	return nil
+	return parser.ValidateClassName(class)
 }
 
 // ValidateDeviceName checks the validity of a device name.
 // A device name may contain the following ASCII characters:
 //   - upper- and lowercase letters ('A'-'Z', 'a'-'z')
 //   - digits ('0'-'9')
 //   - underscore, dash, dot, colon ('_', '-', '.', ':')
+//
+// Deprecated: use parser.ValidateDeviceName instead
 func ValidateDeviceName(name string) error {
-	if name == "" {
-		return fmt.Errorf("invalid (empty) device name")
-	}
-	if !isAlphaNumeric(rune(name[0])) {
-		return fmt.Errorf("invalid class %q, should start with a letter or digit", name)
-	}
-	if len(name) == 1 {
-		return nil
-	}
-	for _, c := range string(name[1 : len(name)-1]) {
-		switch {
-		case isAlphaNumeric(c):
-		case c == '_' || c == '-' || c == '.' || c == ':':
-		default:
-			return fmt.Errorf("invalid character '%c' in device name %q",
-				c, name)
-		}
-	}
-	if !isAlphaNumeric(rune(name[len(name)-1])) {
-		return fmt.Errorf("invalid name %q, should end with a letter or digit", name)
-	}
-	return nil
-}
-
-func isLetter(c rune) bool {
-	return ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z')
-}
-
-func isDigit(c rune) bool {
-	return '0' <= c && c <= '9'
-}
-
-func isAlphaNumeric(c rune) bool {
-	return isLetter(c) || isDigit(c)
+	return parser.ValidateDeviceName(name)
 }

pkg/cdi/registry.go

@@ -23,14 +23,12 @@ import (
 	oci "github.com/opencontainers/runtime-spec/specs-go"
 )
 
-//
 // Registry keeps a cache of all CDI Specs installed or generated on
 // the host. Registry is the primary interface clients should use to
 // interact with CDI.
 //
 // The most commonly used Registry functions are for refreshing the
 // registry and injecting CDI devices into an OCI Spec.
-//
 type Registry interface {
 	RegistryResolver
 	RegistryRefresher

pkg/cdi/spec_test.go

@@ -27,6 +27,7 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"github.com/container-orchestrated-devices/container-device-interface/pkg/cdi/validate"
+	"github.com/container-orchestrated-devices/container-device-interface/pkg/parser"
 	"github.com/container-orchestrated-devices/container-device-interface/schema"
 	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 	"github.com/stretchr/testify/require"
@@ -487,7 +488,7 @@ devices:
 			for name, d := range spec.devices {
 				require.Equal(t, spec, d.GetSpec())
 				require.Equal(t, d, spec.GetDevice(name))
-				require.Equal(t, QualifiedName(vendor, class, name), d.GetQualifiedName())
+				require.Equal(t, parser.QualifiedName(vendor, class, name), d.GetQualifiedName())
 			}
 		})
 	}

pkg/cdi/version.go

@@ -21,6 +21,7 @@ import (
 
 	"golang.org/x/mod/semver"
 
+	"github.com/container-orchestrated-devices/container-device-interface/pkg/parser"
 	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 )
 
@@ -140,7 +141,7 @@ func requiresV050(spec *cdi.Spec) bool {
 
 	for _, d := range spec.Devices {
 		// The v0.5.0 spec allowed device names to start with a digit instead of requiring a letter
-		if len(d.Name) > 0 && !isLetter(rune(d.Name[0])) {
+		if len(d.Name) > 0 && !parser.IsLetter(rune(d.Name[0])) {
 			return true
 		}
 		edits = append(edits, &d.ContainerEdits)