Merge pull request #93 from elezar/min-spec-version

Add functions to query and validate minimum spec version

Author: bart0sh

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/spf13/cobra v1.6.0
 	github.com/stretchr/testify v1.7.0
 	github.com/xeipuuv/gojsonschema v1.2.0
+	golang.org/x/mod v0.4.2
 	golang.org/x/sys v0.0.0-20211116061358-0a5406a5449c
 	sigs.k8s.io/yaml v1.3.0
 )

go.sum

@@ -72,7 +72,16 @@ github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 h1:EzJWgHo
 github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ=
 github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17UxZ74=
 github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y=
+golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
+golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
+golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
+golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
 golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
+golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
+golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20190606203320-7fc4e5ec1444/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191115151921-52ab43148777/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
@@ -84,8 +93,12 @@ golang.org/x/sys v0.0.0-20211025201205-69cdffdb9359/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20211116061358-0a5406a5449c h1:DHcbWVXeY+0Y8HHKR+rbLwnoh2F4tNCY7rTiHJ30RmA=
 golang.org/x/sys v0.0.0-20211116061358-0a5406a5449c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
+golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
+golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
+golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
 google.golang.org/protobuf v1.27.1/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=

pkg/cdi/container-edits.go

@@ -298,7 +298,8 @@ func sortMounts(specgen *ocigen.Generator) {
 // orderedMounts defines how to sort an OCI Spec Mount slice.
 // This is the almost the same implementation sa used by CRI-O and Docker,
 // with a minor tweak for stable sorting order (easier to test):
-//   https://github.com/moby/moby/blob/17.05.x/daemon/volumes.go#L26
+//
+//	https://github.com/moby/moby/blob/17.05.x/daemon/volumes.go#L26
 type orderedMounts []oci.Mount
 
 // Len returns the number of mounts. Used in sorting.

pkg/cdi/regressions_test.go

@@ -43,7 +43,7 @@ func TestCDIInjectionRace(t *testing.T) {
 		{description: "expect properly injected resolvable CDI devices",
 			cdiSpecFiles: []string{
 				`
-cdiVersion: "0.2.0"
+cdiVersion: "0.3.0"
 kind: "vendor1.com/device"
 devices:
   - name: foo
@@ -60,7 +60,7 @@ containerEdits:
     - "VENDOR1=present"
 `,
 				`
-cdiVersion: "0.2.0"
+cdiVersion: "0.3.0"
 kind: "vendor2.com/device"
 devices:
   - name: bar

pkg/cdi/spec.go

@@ -32,23 +32,11 @@ import (
 )
 
 const (
-	// CurrentVersion is the current vesion of the CDI Spec.
-	CurrentVersion = cdi.CurrentVersion
-
 	// defaultSpecExt is the file extension for the default encoding.
 	defaultSpecExt = ".yaml"
 )
 
 var (
-	// Valid CDI Spec versions.
-	validSpecVersions = map[string]struct{}{
-		"0.1.0": {},
-		"0.2.0": {},
-		"0.3.0": {},
-		"0.4.0": {},
-		"0.5.0": {},
-	}
-
 	// Externally set CDI Spec validation function.
 	specValidator func(*cdi.Spec) error
 	validatorLock sync.RWMutex
@@ -216,6 +204,15 @@ func (s *Spec) validate() (map[string]*Device, error) {
 	if err := validateVersion(s.Version); err != nil {
 		return nil, err
 	}
+
+	minVersion, err := MinimumRequiredVersion(s.Spec)
+	if err != nil {
+		return nil, fmt.Errorf("could not determine minumum required version: %v", err)
+	}
+	if newVersion(minVersion).IsGreaterThan(newVersion(s.Version)) {
+		return nil, fmt.Errorf("the spec version must be at least v%v", minVersion)
+	}
+
 	if err := ValidateVendorName(s.vendor); err != nil {
 		return nil, err
 	}
@@ -243,7 +240,7 @@ func (s *Spec) validate() (map[string]*Device, error) {
 
 // validateVersion checks whether the specified spec version is supported.
 func validateVersion(version string) error {
-	if _, ok := validSpecVersions[version]; !ok {
+	if !validSpecVersions.isValidVersion(version) {
 		return fmt.Errorf("invalid version %q", version)
 	}
 

pkg/cdi/spec_test.go

@@ -527,3 +527,117 @@ func specType(content []byte) string {
 func TestCurrentVersionIsValid(t *testing.T) {
 	require.NoError(t, validateVersion(cdi.CurrentVersion))
 }
+
+func TestRequiredVersion(t *testing.T) {
+
+	testCases := []struct {
+		description     string
+		spec            *cdi.Spec
+		expectedVersion string
+	}{
+		{
+			description:     "empty spec returns lowest version",
+			spec:            &cdi.Spec{},
+			expectedVersion: "0.3.0",
+		},
+		{
+			description: "hostPath set returns version 0.5.0",
+			spec: &cdi.Spec{
+				ContainerEdits: cdi.ContainerEdits{
+					DeviceNodes: []*cdi.DeviceNode{
+						{
+							HostPath: "/host/path/set",
+						},
+					},
+				},
+			},
+			expectedVersion: "0.5.0",
+		},
+		{
+			description: "hostPath equal to Path required v0.5.0",
+			spec: &cdi.Spec{
+				ContainerEdits: cdi.ContainerEdits{
+					DeviceNodes: []*cdi.DeviceNode{
+						{
+							HostPath: "/some/path",
+							Path:     "/some/path",
+						},
+					},
+				},
+			},
+			expectedVersion: "0.5.0",
+		},
+		{
+			description: "mount type set returns version 0.4.0",
+			spec: &cdi.Spec{
+				ContainerEdits: cdi.ContainerEdits{
+					Mounts: []*cdi.Mount{
+						{
+							Type: "bind",
+						},
+					},
+				},
+			},
+			expectedVersion: "0.4.0",
+		},
+		{
+			description: "newest required version is selected",
+			spec: &cdi.Spec{
+				ContainerEdits: cdi.ContainerEdits{
+					DeviceNodes: []*cdi.DeviceNode{
+						{
+							HostPath: "/host/path/set",
+						},
+					},
+					Mounts: []*cdi.Mount{
+						{
+							Type: "bind",
+						},
+					},
+				},
+			},
+			expectedVersion: "0.5.0",
+		},
+		{
+			description: "device with name starting with digit requires v0.5.0",
+			spec: &cdi.Spec{
+				Devices: []cdi.Device{
+					{
+						Name: "0",
+						ContainerEdits: cdi.ContainerEdits{
+							Env: []string{
+								"FOO=bar",
+							},
+						},
+					},
+				},
+			},
+			expectedVersion: "0.5.0",
+		},
+		{
+			description: "device with name starting with letter requires minimum version",
+			spec: &cdi.Spec{
+				Devices: []cdi.Device{
+					{
+						Name: "device0",
+						ContainerEdits: cdi.ContainerEdits{
+							Env: []string{
+								"FOO=bar",
+							},
+						},
+					},
+				},
+			},
+			expectedVersion: "0.3.0",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			v, err := MinimumRequiredVersion(tc.spec)
+			require.NoError(t, err)
+
+			require.Equal(t, tc.expectedVersion, v)
+		})
+	}
+}

pkg/cdi/version.go

@@ -0,0 +1,160 @@
+/*
+   Copyright © The CDI Authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package cdi
+
+import (
+	"strings"
+
+	"golang.org/x/mod/semver"
+
+	cdi "github.com/container-orchestrated-devices/container-device-interface/specs-go"
+)
+
+const (
+	// CurrentVersion is the current version of the CDI Spec.
+	CurrentVersion = cdi.CurrentVersion
+
+	// vCurrent is the current version as a semver-comparable type
+	vCurrent version = "v" + CurrentVersion
+
+	// These represent the released versions of the CDI specification
+	v010 version = "v0.1.0"
+	v020 version = "v0.2.0"
+	v030 version = "v0.3.0"
+	v040 version = "v0.4.0"
+	v050 version = "v0.5.0"
+
+	// vEarliest is the earliest supported version of the CDI specification
+	vEarliest version = v030
+)
+
+// validSpecVersions stores a map of spec versions to functions to check the required versions.
+// Adding new fields / spec versions requires that a `requiredFunc` be implemented and
+// this map be updated.
+var validSpecVersions = requiredVersionMap{
+	v010: nil,
+	v020: nil,
+	v030: nil,
+	v040: requiresV040,
+	v050: requiresV050,
+}
+
+// MinimumRequiredVersion determines the minumum spec version for the input spec.
+func MinimumRequiredVersion(spec *cdi.Spec) (string, error) {
+	minVersion := validSpecVersions.requiredVersion(spec)
+	return minVersion.String(), nil
+}
+
+// version represents a semantic version string
+type version string
+
+// newVersion creates a version that can be used for semantic version comparisons.
+func newVersion(v string) version {
+	return version("v" + strings.TrimPrefix(v, "v"))
+}
+
+// String returns the string representation of the version.
+// This trims a leading v if present.
+func (v version) String() string {
+	return strings.TrimPrefix(string(v), "v")
+}
+
+// IsGreaterThan checks with a version is greater than the specified version.
+func (v version) IsGreaterThan(o version) bool {
+	return semver.Compare(string(v), string(o)) > 0
+}
+
+// IsLatest checks whether the version is the latest supported version
+func (v version) IsLatest() bool {
+	return v == vCurrent
+}
+
+type requiredFunc func(*cdi.Spec) bool
+
+type requiredVersionMap map[version]requiredFunc
+
+// isValidVersion checks whether the specified version is valid.
+// A version is valid if it is contained in the required version map.
+func (r requiredVersionMap) isValidVersion(specVersion string) bool {
+	_, ok := validSpecVersions[newVersion(specVersion)]
+
+	return ok
+}
+
+// requiredVersion returns the minimum version required for the given spec
+func (r requiredVersionMap) requiredVersion(spec *cdi.Spec) version {
+	minVersion := vEarliest
+
+	for v, isRequired := range validSpecVersions {
+		if isRequired == nil {
+			continue
+		}
+		if isRequired(spec) && v.IsGreaterThan(minVersion) {
+			minVersion = v
+		}
+		// If we have already detected the latest version then no later version could be detected
+		if minVersion.IsLatest() {
+			break
+		}
+	}
+
+	return minVersion
+}
+
+// requiresV050 returns true if the spec uses v0.5.0 features
+func requiresV050(spec *cdi.Spec) bool {
+	var edits []*cdi.ContainerEdits
+
+	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])) {
+			return true
+		}
+		edits = append(edits, &d.ContainerEdits)
+	}
+
+	edits = append(edits, &spec.ContainerEdits)
+	for _, e := range edits {
+		for _, dn := range e.DeviceNodes {
+			// The HostPath field was added in v0.5.0
+			if dn.HostPath != "" {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// requiresV040 returns true if the spec uses v0.4.0 features
+func requiresV040(spec *cdi.Spec) bool {
+	var edits []*cdi.ContainerEdits
+
+	for _, d := range spec.Devices {
+		edits = append(edits, &d.ContainerEdits)
+	}
+
+	edits = append(edits, &spec.ContainerEdits)
+	for _, e := range edits {
+		for _, m := range e.Mounts {
+			// The Type field was added in v0.4.0
+			if m.Type != "" {
+				return true
+			}
+		}
+	}
+	return false
+}