From fb588c5c152ff1bfe9b108f59f903374da7a6d70 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Thu, 21 Nov 2024 16:47:18 +0100 Subject: [PATCH] Add api/validator submodule Signed-off-by: Evan Lezar --- .../validator/annotations.go | 4 +- api/validator/api.go | 28 ++ api/validator/go.mod | 17 + api/validator/go.sum | 15 + api/validator/identifiers.go | 139 ++++++++ .../validator}/k8s/objectmeta.go | 0 .../validator}/k8s/validation.go | 0 api/validator/validator-default.go | 254 +++++++++++++++ api/validator/validator-default_test.go | 302 ++++++++++++++++++ api/validator/validator-disabled.go | 29 ++ cmd/cdi/go.mod | 9 +- cmd/validate/go.mod | 11 +- cmd/validate/go.sum | 1 + go.mod | 6 +- pkg/cdi/annotations.go | 7 +- pkg/cdi/container-edits.go | 100 +----- pkg/cdi/device.go | 38 +-- pkg/cdi/device_test.go | 34 +- pkg/cdi/spec.go | 51 +-- pkg/parser/parser.go | 83 ++--- schema/schema.go | 7 +- specs-go/config.go | 4 +- 22 files changed, 889 insertions(+), 250 deletions(-) rename internal/validation/validate.go => api/validator/annotations.go (94%) create mode 100644 api/validator/api.go create mode 100644 api/validator/go.mod create mode 100644 api/validator/go.sum create mode 100644 api/validator/identifiers.go rename {internal/validation => api/validator}/k8s/objectmeta.go (100%) rename {internal/validation => api/validator}/k8s/validation.go (100%) create mode 100644 api/validator/validator-default.go create mode 100644 api/validator/validator-default_test.go create mode 100644 api/validator/validator-disabled.go diff --git a/internal/validation/validate.go b/api/validator/annotations.go similarity index 94% rename from internal/validation/validate.go rename to api/validator/annotations.go index 5d9b55ff..d2aad316 100644 --- a/internal/validation/validate.go +++ b/api/validator/annotations.go @@ -14,13 +14,13 @@ limitations under the License. */ -package validation +package validator import ( "fmt" "strings" - "tags.cncf.io/container-device-interface/internal/validation/k8s" + "tags.cncf.io/container-device-interface/api/validator/k8s" ) // ValidateSpecAnnotations checks whether spec annotations are valid. diff --git a/api/validator/api.go b/api/validator/api.go new file mode 100644 index 00000000..9883048b --- /dev/null +++ b/api/validator/api.go @@ -0,0 +1,28 @@ +/* + Copyright © 2024 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 validator + +import "errors" + +// Validators as constants. +const ( + Default = defaultValidator("default") + Disabled = disabledValidator("disabled") +) + +// ErrInvalid can be returned if CDI spec validation fails. +var ErrInvalid = errors.New("invalid CDI specification") diff --git a/api/validator/go.mod b/api/validator/go.mod new file mode 100644 index 00000000..973172ad --- /dev/null +++ b/api/validator/go.mod @@ -0,0 +1,17 @@ +module tags.cncf.io/container-device-interface/api/validator + +go 1.20 + +require ( + github.com/stretchr/testify v1.7.0 + tags.cncf.io/container-device-interface/specs-go v0.8.0 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + golang.org/x/mod v0.19.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) + +replace tags.cncf.io/container-device-interface/specs-go => ../../specs-go diff --git a/api/validator/go.sum b/api/validator/go.sum new file mode 100644 index 00000000..16c08315 --- /dev/null +++ b/api/validator/go.sum @@ -0,0 +1,15 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +golang.org/x/mod v0.19.0 h1:fEdghXQSo20giMthA7cd28ZC+jts4amQ3YMXiP5oMQ8= +golang.org/x/mod v0.19.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/api/validator/identifiers.go b/api/validator/identifiers.go new file mode 100644 index 00000000..5e40d04b --- /dev/null +++ b/api/validator/identifiers.go @@ -0,0 +1,139 @@ +/* + Copyright © 2024 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 validator + +import ( + "fmt" + "strings" +) + +// ValidateKind checks the validity of a CDI kind. +// The syntax for a device kind“ is +// +// "/" +func ValidateKind(kind string) error { + parts := strings.SplitN(kind, "/", 2) + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return fmt.Errorf("kind %s does not contain a / %w", kind, ErrInvalid) + } + if err := ValidateVendorName(parts[0]); err != nil { + return err + } + if err := ValidateClassName(parts[1]); err != nil { + return err + } + return nil +} + +// 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 '.') +func ValidateVendorName(vendor string) error { + err := validateVendorOrClassName(vendor) + if err != nil { + err = fmt.Errorf("invalid vendor. %w", err) + } + return err +} + +// 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, dash, and dot ('_', '-', and '.') +func ValidateClassName(class string) error { + err := validateVendorOrClassName(class) + if err != nil { + err = fmt.Errorf("invalid class. %w", err) + } + return err +} + +// validateVendorOrClassName checks the validity of vendor or class name. +// A name may contain the following ASCII characters: +// - upper- and lowercase letters ('A'-'Z', 'a'-'z') +// - digits ('0'-'9') +// - underscore, dash, and dot ('_', '-', and '.') +func validateVendorOrClassName(name string) error { + if name == "" { + return fmt.Errorf("empty name") + } + if !IsLetter(rune(name[0])) { + return fmt.Errorf("%q, should start with letter", name) + } + for _, c := range string(name[1 : len(name)-1]) { + switch { + case IsAlphaNumeric(c): + case c == '_' || c == '-' || c == '.': + default: + return fmt.Errorf("invalid character '%c' in name %q", + c, name) + } + } + if !IsAlphaNumeric(rune(name[len(name)-1])) { + return fmt.Errorf("%q, should end with a letter or digit", name) + } + + return nil +} + +// 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 ('_', '-', '.', ':') +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 +} + +// IsLetter reports whether the rune is a letter. +func IsLetter(c rune) bool { + return ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z') +} + +// IsDigit reports whether the rune is a digit. +func IsDigit(c rune) bool { + return '0' <= c && c <= '9' +} + +// IsAlphaNumeric reports whether the rune is a letter or digit. +func IsAlphaNumeric(c rune) bool { + return IsLetter(c) || IsDigit(c) +} diff --git a/internal/validation/k8s/objectmeta.go b/api/validator/k8s/objectmeta.go similarity index 100% rename from internal/validation/k8s/objectmeta.go rename to api/validator/k8s/objectmeta.go diff --git a/internal/validation/k8s/validation.go b/api/validator/k8s/validation.go similarity index 100% rename from internal/validation/k8s/validation.go rename to api/validator/k8s/validation.go diff --git a/api/validator/validator-default.go b/api/validator/validator-default.go new file mode 100644 index 00000000..1b6b4285 --- /dev/null +++ b/api/validator/validator-default.go @@ -0,0 +1,254 @@ +/* + Copyright © 2024 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 validator + +import ( + "errors" + "fmt" + "strings" + + cdi "tags.cncf.io/container-device-interface/specs-go" +) + +type defaultValidator string + +// ValidateAny implements a generic validation handler for the defaultValidator. +func (v defaultValidator) ValidateAny(o interface{}) (rerr error) { + defer func() { + if rerr != nil { + rerr = errors.Join(rerr, ErrInvalid) + } + }() + + switch o := o.(type) { + case *cdi.ContainerEdits: + return v.validateEdits(o) + case *cdi.Device: + return v.ValidateDevice("", o) + case *cdi.DeviceNode: + return v.validateDeviceNode(o) + case *cdi.Hook: + return v.validateHook(o) + case *cdi.IntelRdt: + return v.validateIntelRdt(o) + case *cdi.Mount: + return v.validateMount(o) + case *cdi.Spec: + return v.Validate(o) + default: + return fmt.Errorf("unsupported validation type: %T", o) + } +} + +// Validate performs a default validation on a CDI spec. +func (v defaultValidator) Validate(s *cdi.Spec) (rerr error) { + defer func() { + if rerr != nil { + rerr = errors.Join(rerr, ErrInvalid) + } + }() + + if err := cdi.ValidateVersion(s); err != nil { + return err + } + if err := ValidateKind(s.Kind); err != nil { + return err + } + if err := ValidateSpecAnnotations(s.Kind, s.Annotations); err != nil { + return err + } + if err := v.validateEdits(&s.ContainerEdits); err != nil { + return err + } + + seen := make(map[string]bool) + for _, d := range s.Devices { + if seen[d.Name] { + return fmt.Errorf("invalid spec, multiple device %q", d.Name) + } + seen[d.Name] = true + if err := v.ValidateDevice(s.Kind, &d); err != nil { + return fmt.Errorf("invalid device %q: %w", d.Name, err) + } + } + return nil +} + +func (v defaultValidator) ValidateDevice(kind string, d *cdi.Device) error { + if err := ValidateDeviceName(d.Name); err != nil { + return err + } + + name := d.Name + if kind != "" { + name = kind + "=" + name + } + if err := ValidateSpecAnnotations(name, d.Annotations); err != nil { + return err + } + + if err := v.assertNonEmptyEdits(&d.ContainerEdits); err != nil { + return err + } + if err := v.validateEdits(&d.ContainerEdits); err != nil { + return err + } + return nil +} + +func (v defaultValidator) assertNonEmptyEdits(e *cdi.ContainerEdits) error { + if e == nil { + return nil + } + if len(e.Env) > 0 { + return nil + } + if len(e.DeviceNodes) > 0 { + return nil + } + if len(e.Hooks) > 0 { + return nil + } + if len(e.Mounts) > 0 { + return nil + } + if len(e.AdditionalGIDs) > 0 { + return nil + } + if e.IntelRdt != nil { + return nil + } + return errors.New("empty container edits") +} + +func (v defaultValidator) validateEdits(e *cdi.ContainerEdits) error { + if e == nil { + return nil + } + if err := v.validateEnv(e.Env); err != nil { + return fmt.Errorf("invalid container edits: %w", err) + } + for _, d := range e.DeviceNodes { + if err := v.validateDeviceNode(d); err != nil { + return err + } + } + for _, h := range e.Hooks { + if err := v.validateHook(h); err != nil { + return err + } + } + for _, m := range e.Mounts { + if err := v.validateMount(m); err != nil { + return err + } + } + if err := v.validateIntelRdt(e.IntelRdt); err != nil { + return err + } + return nil +} + +func (v defaultValidator) validateEnv(env []string) error { + for _, v := range env { + if strings.IndexByte(v, byte('=')) <= 0 { + return fmt.Errorf("invalid environment variable %q", v) + } + } + return nil +} + +func (v defaultValidator) validateDeviceNode(d *cdi.DeviceNode) error { + validTypes := map[string]struct{}{ + "": {}, + "b": {}, + "c": {}, + "u": {}, + "p": {}, + } + + if d.Path == "" { + return errors.New("invalid (empty) device path") + } + if _, ok := validTypes[d.Type]; !ok { + return fmt.Errorf("device %q: invalid type %q", d.Path, d.Type) + } + for _, bit := range d.Permissions { + if bit != 'r' && bit != 'w' && bit != 'm' { + return fmt.Errorf("device %q: invalid permissions %q", + d.Path, d.Permissions) + } + } + return nil +} + +func (v defaultValidator) validateHook(h *cdi.Hook) error { + const ( + // PrestartHook is the name of the OCI "prestart" hook. + PrestartHook = "prestart" + // CreateRuntimeHook is the name of the OCI "createRuntime" hook. + CreateRuntimeHook = "createRuntime" + // CreateContainerHook is the name of the OCI "createContainer" hook. + CreateContainerHook = "createContainer" + // StartContainerHook is the name of the OCI "startContainer" hook. + StartContainerHook = "startContainer" + // PoststartHook is the name of the OCI "poststart" hook. + PoststartHook = "poststart" + // PoststopHook is the name of the OCI "poststop" hook. + PoststopHook = "poststop" + ) + validHookNames := map[string]struct{}{ + PrestartHook: {}, + CreateRuntimeHook: {}, + CreateContainerHook: {}, + StartContainerHook: {}, + PoststartHook: {}, + PoststopHook: {}, + } + + if _, ok := validHookNames[h.HookName]; !ok { + return fmt.Errorf("invalid hook name %q", h.HookName) + } + if h.Path == "" { + return fmt.Errorf("invalid hook %q with empty path", h.HookName) + } + if err := v.validateEnv(h.Env); err != nil { + return fmt.Errorf("invalid hook %q: %w", h.HookName, err) + } + return nil +} + +func (v defaultValidator) validateMount(m *cdi.Mount) error { + if m.HostPath == "" { + return errors.New("invalid mount, empty host path") + } + if m.ContainerPath == "" { + return errors.New("invalid mount, empty container path") + } + return nil +} + +func (v defaultValidator) validateIntelRdt(i *cdi.IntelRdt) error { + if i == nil { + return nil + } + // ClosID must be a valid Linux filename + if len(i.ClosID) >= 4096 || i.ClosID == "." || i.ClosID == ".." || strings.ContainsAny(i.ClosID, "/\n") { + return errors.New("invalid ClosID") + } + return nil +} diff --git a/api/validator/validator-default_test.go b/api/validator/validator-default_test.go new file mode 100644 index 00000000..b90d2f65 --- /dev/null +++ b/api/validator/validator-default_test.go @@ -0,0 +1,302 @@ +/* + Copyright © 2024 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 validator + +import ( + "testing" + + "github.com/stretchr/testify/require" + + cdi "tags.cncf.io/container-device-interface/specs-go" +) + +func TestValidateContainerEdits(t *testing.T) { + type testCase struct { + name string + edits *cdi.ContainerEdits + invalid bool + } + for _, tc := range []*testCase{ + { + name: "valid, empty edits", + edits: nil, + }, + { + name: "valid, env var", + edits: &cdi.ContainerEdits{ + Env: []string{"BAR=BARVALUE1"}, + }, + }, + { + name: "invalid env, empty var", + edits: &cdi.ContainerEdits{ + Env: []string{""}, + }, + invalid: true, + }, + { + name: "invalid env, no var name", + edits: &cdi.ContainerEdits{ + Env: []string{"=foo"}, + }, + invalid: true, + }, + { + name: "invalid env, no assignment", + edits: &cdi.ContainerEdits{ + Env: []string{"FOOBAR"}, + }, + invalid: true, + }, + { + name: "valid device, path only", + edits: &cdi.ContainerEdits{ + DeviceNodes: []*cdi.DeviceNode{ + { + Path: "/dev/null", + }, + }, + }, + }, + { + name: "valid device, path+type", + edits: &cdi.ContainerEdits{ + DeviceNodes: []*cdi.DeviceNode{ + { + Path: "/dev/null", + Type: "c", + }, + }, + }, + }, + { + name: "valid device, path+type+permissions", + edits: &cdi.ContainerEdits{ + DeviceNodes: []*cdi.DeviceNode{ + { + Path: "/dev/null", + Type: "b", + Permissions: "rwm", + }, + }, + }, + }, + { + name: "invalid device, empty path", + edits: &cdi.ContainerEdits{ + DeviceNodes: []*cdi.DeviceNode{ + { + Path: "", + }, + }, + }, + invalid: true, + }, + { + name: "invalid device, wrong type", + edits: &cdi.ContainerEdits{ + DeviceNodes: []*cdi.DeviceNode{ + { + Path: "/dev/vendorctl", + Type: "f", + }, + }, + }, + invalid: true, + }, + { + name: "invalid device, wrong permissions", + edits: &cdi.ContainerEdits{ + DeviceNodes: []*cdi.DeviceNode{ + { + Path: "/dev/vendorctl", + Type: "b", + Permissions: "to land", + }, + }, + }, + invalid: true, + }, + { + name: "valid mount", + edits: &cdi.ContainerEdits{ + Mounts: []*cdi.Mount{ + { + HostPath: "/dev/vendorctl", + ContainerPath: "/dev/vendorctl", + }, + }, + }, + }, + { + name: "invalid mount, empty host path", + edits: &cdi.ContainerEdits{ + Mounts: []*cdi.Mount{ + { + HostPath: "", + ContainerPath: "/dev/vendorctl", + }, + }, + }, + invalid: true, + }, + { + name: "invalid mount, empty container path", + edits: &cdi.ContainerEdits{ + Mounts: []*cdi.Mount{ + { + HostPath: "/dev/vendorctl", + ContainerPath: "", + }, + }, + }, + invalid: true, + }, + { + name: "valid hooks", + edits: &cdi.ContainerEdits{ + Hooks: []*cdi.Hook{ + { + HookName: "prestart", + Path: "/usr/local/bin/prestart-vendor-hook", + Args: []string{"--verbose"}, + Env: []string{"VENDOR_ENV1=value1"}, + }, + { + HookName: "createRuntime", + Path: "/usr/local/bin/cr-vendor-hook", + Args: []string{"--debug"}, + Env: []string{"VENDOR_ENV2=value2"}, + }, + { + HookName: "createContainer", + Path: "/usr/local/bin/cc-vendor-hook", + Args: []string{"--create"}, + Env: []string{"VENDOR_ENV3=value3"}, + }, + { + HookName: "startContainer", + Path: "/usr/local/bin/sc-vendor-hook", + Args: []string{"--start"}, + Env: []string{"VENDOR_ENV4=value4"}, + }, + { + HookName: "poststart", + Path: "/usr/local/bin/poststart-vendor-hook", + Env: []string{"VENDOR_ENV5=value5"}, + }, + { + HookName: "poststop", + Path: "/usr/local/bin/poststop-vendor-hook", + }, + }, + }, + }, + { + name: "invalid hook, empty path", + edits: &cdi.ContainerEdits{ + Hooks: []*cdi.Hook{ + { + HookName: "prestart", + }, + }, + }, + invalid: true, + }, + { + name: "invalid hook, wrong hook name", + edits: &cdi.ContainerEdits{ + Hooks: []*cdi.Hook{ + { + HookName: "misCreateRuntime", + Path: "/usr/local/bin/cr-vendor-hook", + Args: []string{"--debug"}, + Env: []string{"VENDOR_ENV2=value2"}, + }, + }, + }, + invalid: true, + }, + { + name: "invalid hook, wrong env", + edits: &cdi.ContainerEdits{ + Hooks: []*cdi.Hook{ + { + HookName: "poststart", + Path: "/usr/local/bin/cr-vendor-hook", + Args: []string{"--debug"}, + Env: []string{"=value2"}, + }, + }, + }, + invalid: true, + }, + { + name: "valid rdt config", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "foo.bar", + }, + }, + }, + { + name: "invalid rdt config, invalid closID (slash)", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "foo/bar", + }, + }, + invalid: true, + }, + { + name: "invalid rdt config, invalid closID (dot)", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: ".", + }, + }, + invalid: true, + }, + { + name: "invalid rdt config, invalid closID (double dot)", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "..", + }, + }, + invalid: true, + }, + { + name: "invalid rdt config, invalid closID (newline)", + edits: &cdi.ContainerEdits{ + IntelRdt: &cdi.IntelRdt{ + ClosID: "foo\nbar", + }, + }, + invalid: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + err := Default.ValidateAny(tc.edits) + if tc.invalid { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/api/validator/validator-disabled.go b/api/validator/validator-disabled.go new file mode 100644 index 00000000..9323dc7e --- /dev/null +++ b/api/validator/validator-disabled.go @@ -0,0 +1,29 @@ +/* + Copyright © 2024 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 validator + +import ( + cdi "tags.cncf.io/container-device-interface/specs-go" +) + +// A disabledValidator performs no validation. +type disabledValidator string + +// Validate always passes for a disabledValidator. +func (v disabledValidator) Validate(*cdi.Spec) error { + return nil +} diff --git a/cmd/cdi/go.mod b/cmd/cdi/go.mod index c9356145..6370a806 100644 --- a/cmd/cdi/go.mod +++ b/cmd/cdi/go.mod @@ -22,9 +22,12 @@ require ( golang.org/x/mod v0.19.0 // indirect golang.org/x/sys v0.19.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect + tags.cncf.io/container-device-interface/api/validator v0.0.0 // indirect tags.cncf.io/container-device-interface/specs-go v0.8.0 // indirect ) -replace tags.cncf.io/container-device-interface => ../.. - -replace tags.cncf.io/container-device-interface/specs-go => ../../specs-go +replace ( + tags.cncf.io/container-device-interface => ../.. + tags.cncf.io/container-device-interface/api/validator => ../../api/validator + tags.cncf.io/container-device-interface/specs-go => ../../specs-go +) diff --git a/cmd/validate/go.mod b/cmd/validate/go.mod index 1d5c8701..5713a517 100644 --- a/cmd/validate/go.mod +++ b/cmd/validate/go.mod @@ -8,10 +8,15 @@ require ( github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xeipuuv/gojsonschema v1.2.0 // indirect + golang.org/x/mod v0.19.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect sigs.k8s.io/yaml v1.3.0 // indirect + tags.cncf.io/container-device-interface/api/validator v0.0.0-00010101000000-000000000000 // indirect + tags.cncf.io/container-device-interface/specs-go v0.8.0 // indirect ) -replace tags.cncf.io/container-device-interface => ../.. - -replace tags.cncf.io/container-device-interface/specs-go => ../../specs-go +replace ( + tags.cncf.io/container-device-interface => ../.. + tags.cncf.io/container-device-interface/api/validator => ../../api/validator + tags.cncf.io/container-device-interface/specs-go => ../../specs-go +) diff --git a/cmd/validate/go.sum b/cmd/validate/go.sum index dabe4784..9f83b553 100644 --- a/cmd/validate/go.sum +++ b/cmd/validate/go.sum @@ -17,6 +17,7 @@ github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1: 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/mod v0.19.0 h1:fEdghXQSo20giMthA7cd28ZC+jts4amQ3YMXiP5oMQ8= +golang.org/x/mod v0.19.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/go.mod b/go.mod index 6251be87..9a7ce77d 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/xeipuuv/gojsonschema v1.2.0 golang.org/x/sys v0.19.0 sigs.k8s.io/yaml v1.3.0 + tags.cncf.io/container-device-interface/api/validator v0.0.0-00010101000000-000000000000 tags.cncf.io/container-device-interface/specs-go v0.8.0 ) @@ -24,4 +25,7 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect ) -replace tags.cncf.io/container-device-interface/specs-go => ./specs-go +replace ( + tags.cncf.io/container-device-interface/api/validator => ./api/validator + tags.cncf.io/container-device-interface/specs-go => ./specs-go +) diff --git a/pkg/cdi/annotations.go b/pkg/cdi/annotations.go index a596c610..c344cd75 100644 --- a/pkg/cdi/annotations.go +++ b/pkg/cdi/annotations.go @@ -21,6 +21,7 @@ import ( "fmt" "strings" + "tags.cncf.io/container-device-interface/api/validator" "tags.cncf.io/container-device-interface/pkg/parser" ) @@ -103,14 +104,14 @@ func AnnotationKey(pluginName, deviceID string) (string, error) { return "", fmt.Errorf("invalid plugin+deviceID %q, too long", name) } - if c := rune(name[0]); !parser.IsAlphaNumeric(c) { + if c := rune(name[0]); !validator.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 parser.IsAlphaNumeric(c): + case validator.IsAlphaNumeric(c): case c == '_' || c == '-' || c == '.': default: return "", fmt.Errorf("invalid name %q, invalid character '%c'", @@ -118,7 +119,7 @@ func AnnotationKey(pluginName, deviceID string) (string, error) { } } } - if c := rune(name[len(name)-1]); !parser.IsAlphaNumeric(c) { + if c := rune(name[len(name)-1]); !validator.IsAlphaNumeric(c) { return "", fmt.Errorf("invalid name %q, last '%c' should be alphanumeric", name, c) } diff --git a/pkg/cdi/container-edits.go b/pkg/cdi/container-edits.go index 4744eff8..ebce3b8c 100644 --- a/pkg/cdi/container-edits.go +++ b/pkg/cdi/container-edits.go @@ -26,6 +26,7 @@ import ( oci "github.com/opencontainers/runtime-spec/specs-go" ocigen "github.com/opencontainers/runtime-tools/generate" + "tags.cncf.io/container-device-interface/api/validator" cdi "tags.cncf.io/container-device-interface/specs-go" ) @@ -44,18 +45,6 @@ const ( PoststopHook = "poststop" ) -var ( - // Names of recognized hooks. - validHookNames = map[string]struct{}{ - PrestartHook: {}, - CreateRuntimeHook: {}, - CreateContainerHook: {}, - StartContainerHook: {}, - PoststartHook: {}, - PoststopHook: {}, - } -) - // ContainerEdits represent updates to be applied to an OCI Spec. // These updates can be specific to a CDI device, or they can be // specific to a CDI Spec. In the former case these edits should @@ -163,36 +152,13 @@ func (e *ContainerEdits) Apply(spec *oci.Spec) error { } // Validate container edits. +// +// Deprecated: Use validator.Default.ValidateAny instead. func (e *ContainerEdits) Validate() error { if e == nil || e.ContainerEdits == nil { return nil } - - if err := ValidateEnv(e.Env); err != nil { - return fmt.Errorf("invalid container edits: %w", err) - } - for _, d := range e.DeviceNodes { - if err := (&DeviceNode{d}).Validate(); err != nil { - return err - } - } - for _, h := range e.Hooks { - if err := (&Hook{h}).Validate(); err != nil { - return err - } - } - for _, m := range e.Mounts { - if err := (&Mount{m}).Validate(); err != nil { - return err - } - } - if e.IntelRdt != nil { - if err := (&IntelRdt{e.IntelRdt}).Validate(); err != nil { - return err - } - } - - return nil + return validator.Default.ValidateAny(e.ContainerEdits) } // Append other edits into this one. If called with a nil receiver, @@ -220,33 +186,6 @@ func (e *ContainerEdits) Append(o *ContainerEdits) *ContainerEdits { return e } -// isEmpty returns true if these edits are empty. This is valid in a -// global Spec context but invalid in a Device context. -func (e *ContainerEdits) isEmpty() bool { - if e == nil { - return false - } - if len(e.Env) > 0 { - return false - } - if len(e.DeviceNodes) > 0 { - return false - } - if len(e.Hooks) > 0 { - return false - } - if len(e.Mounts) > 0 { - return false - } - if len(e.AdditionalGIDs) > 0 { - return false - } - if e.IntelRdt != nil { - return false - } - return true -} - // ValidateEnv validates the given environment variables. func ValidateEnv(env []string) error { for _, v := range env { @@ -293,17 +232,10 @@ type Hook struct { } // Validate a hook. +// +// Deprecated: Use validator.Default.ValidateAny instead. func (h *Hook) Validate() error { - if _, ok := validHookNames[h.HookName]; !ok { - return fmt.Errorf("invalid hook name %q", h.HookName) - } - if h.Path == "" { - return fmt.Errorf("invalid hook %q with empty path", h.HookName) - } - if err := ValidateEnv(h.Env); err != nil { - return fmt.Errorf("invalid hook %q: %w", h.HookName, err) - } - return nil + return validator.Default.ValidateAny(h.Hook) } // Mount is a CDI Mount wrapper, used for validating mounts. @@ -312,14 +244,10 @@ type Mount struct { } // Validate a mount. +// +// Deprecated: Use validator.Default.ValidateAny instead. func (m *Mount) Validate() error { - if m.HostPath == "" { - return errors.New("invalid mount, empty host path") - } - if m.ContainerPath == "" { - return errors.New("invalid mount, empty container path") - } - return nil + return validator.Default.ValidateAny(m.Mount) } // IntelRdt is a CDI IntelRdt wrapper. @@ -336,12 +264,10 @@ func ValidateIntelRdt(i *cdi.IntelRdt) error { } // Validate validates the IntelRdt configuration. +// +// Deprecated: Use validator.Default.ValidateAny instead. func (i *IntelRdt) Validate() error { - // ClosID must be a valid Linux filename - if len(i.ClosID) >= 4096 || i.ClosID == "." || i.ClosID == ".." || strings.ContainsAny(i.ClosID, "/\n") { - return errors.New("invalid ClosID") - } - return nil + return validator.Default.ValidateAny(i.IntelRdt) } // Ensure OCI Spec hooks are not nil so we can add hooks. diff --git a/pkg/cdi/device.go b/pkg/cdi/device.go index 2e5fa57f..0b54ddb5 100644 --- a/pkg/cdi/device.go +++ b/pkg/cdi/device.go @@ -17,10 +17,9 @@ package cdi import ( - "fmt" - oci "github.com/opencontainers/runtime-spec/specs-go" - "tags.cncf.io/container-device-interface/internal/validation" + + "tags.cncf.io/container-device-interface/api/validator" "tags.cncf.io/container-device-interface/pkg/parser" cdi "tags.cncf.io/container-device-interface/specs-go" ) @@ -33,15 +32,18 @@ type Device struct { // Create a new Device, associate it with the given Spec. func newDevice(spec *Spec, d cdi.Device) (*Device, error) { + var kind string + if spec != nil { + kind = spec.Kind + } + if err := validator.Default.ValidateDevice(kind, &d); err != nil { + return nil, err + } dev := &Device{ Device: &d, spec: spec, } - if err := dev.validate(); err != nil { - return nil, err - } - return dev, nil } @@ -64,25 +66,3 @@ func (d *Device) ApplyEdits(ociSpec *oci.Spec) error { func (d *Device) edits() *ContainerEdits { return &ContainerEdits{&d.ContainerEdits} } - -// Validate the device. -func (d *Device) validate() error { - if err := parser.ValidateDeviceName(d.Name); err != nil { - return err - } - name := d.Name - if d.spec != nil { - name = d.GetQualifiedName() - } - if err := validation.ValidateSpecAnnotations(name, d.Annotations); err != nil { - return err - } - edits := d.edits() - if edits.isEmpty() { - return fmt.Errorf("invalid device, empty device edits") - } - if err := edits.Validate(); err != nil { - return fmt.Errorf("invalid device %q: %w", d.Name, err) - } - return nil -} diff --git a/pkg/cdi/device_test.go b/pkg/cdi/device_test.go index 07fb97c4..393be772 100644 --- a/pkg/cdi/device_test.go +++ b/pkg/cdi/device_test.go @@ -26,48 +26,42 @@ import ( func TestDeviceValidate(t *testing.T) { type testCase struct { name string - device *Device + device cdi.Device invalid bool } for _, tc := range []*testCase{ { name: "valid name, valid edits", - device: &Device{ - Device: &cdi.Device{ - Name: "dev", - ContainerEdits: cdi.ContainerEdits{ - Env: []string{"FOO=BAR"}, - }, + device: cdi.Device{ + Name: "dev", + ContainerEdits: cdi.ContainerEdits{ + Env: []string{"FOO=BAR"}, }, }, }, { name: "valid name, invalid edits", - device: &Device{ - Device: &cdi.Device{ - Name: "dev", - ContainerEdits: cdi.ContainerEdits{ - Env: []string{"=BAR"}, - }, + device: cdi.Device{ + Name: "dev", + ContainerEdits: cdi.ContainerEdits{ + Env: []string{"=BAR"}, }, }, invalid: true, }, { name: "invalid name, valid edits", - device: &Device{ - Device: &cdi.Device{ - Name: "a dev ice", - ContainerEdits: cdi.ContainerEdits{ - Env: []string{"FOO=BAR"}, - }, + device: cdi.Device{ + Name: "a dev ice", + ContainerEdits: cdi.ContainerEdits{ + Env: []string{"FOO=BAR"}, }, }, invalid: true, }, } { t.Run(tc.name, func(t *testing.T) { - err := tc.device.validate() + _, err := newDevice(nil, tc.device) if tc.invalid { require.Error(t, err) } else { diff --git a/pkg/cdi/spec.go b/pkg/cdi/spec.go index f0231d81..fa018333 100644 --- a/pkg/cdi/spec.go +++ b/pkg/cdi/spec.go @@ -27,7 +27,7 @@ import ( oci "github.com/opencontainers/runtime-spec/specs-go" "sigs.k8s.io/yaml" - "tags.cncf.io/container-device-interface/internal/validation" + "tags.cncf.io/container-device-interface/api/validator" "tags.cncf.io/container-device-interface/pkg/parser" cdi "tags.cncf.io/container-device-interface/specs-go" ) @@ -108,10 +108,24 @@ func newSpec(raw *cdi.Spec, path string, priority int) (*Spec, error) { spec.vendor, spec.class = parser.ParseQualifier(spec.Kind) - if spec.devices, err = spec.validate(); err != nil { + if err := validator.Default.Validate(spec.Spec); err != nil { return nil, fmt.Errorf("invalid CDI Spec: %w", err) } + // We construct a map of device names to devices to associate with the spec. + // At this point we have validated that there are no duplicate devices. + spec.devices = make(map[string]*Device) + for _, d := range spec.Devices { + dev, err := newDevice(spec, d) + if err != nil { + return nil, fmt.Errorf("failed add device %q: %w", d.Name, err) + } + if _, conflict := spec.devices[d.Name]; conflict { + return nil, fmt.Errorf("invalid spec, multiple device %q", d.Name) + } + spec.devices[d.Name] = dev + } + return spec, nil } @@ -207,39 +221,6 @@ func MinimumRequiredVersion(spec *cdi.Spec) (string, error) { return cdi.MinimumRequiredVersion(spec) } -// Validate the Spec. -func (s *Spec) validate() (map[string]*Device, error) { - if err := cdi.ValidateVersion(s.Spec); err != nil { - return nil, err - } - if err := parser.ValidateVendorName(s.vendor); err != nil { - return nil, err - } - if err := parser.ValidateClassName(s.class); err != nil { - return nil, err - } - if err := validation.ValidateSpecAnnotations(s.Kind, s.Annotations); err != nil { - return nil, err - } - if err := s.edits().Validate(); err != nil { - return nil, err - } - - devices := make(map[string]*Device) - for _, d := range s.Devices { - dev, err := newDevice(s, d) - if err != nil { - return nil, fmt.Errorf("failed add device %q: %w", d.Name, err) - } - if _, conflict := devices[d.Name]; conflict { - return nil, fmt.Errorf("invalid spec, multiple device %q", d.Name) - } - devices[d.Name] = dev - } - - return devices, nil -} - // ParseSpec parses CDI Spec data into a raw CDI Spec. func ParseSpec(data []byte) (*cdi.Spec, error) { var raw *cdi.Spec diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index 53259895..513202e9 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -19,6 +19,8 @@ package parser import ( "fmt" "strings" + + "tags.cncf.io/container-device-interface/api/validator" ) // QualifiedName returns the qualified name for a device. @@ -117,12 +119,10 @@ func ParseQualifier(kind string) (string, string) { // - upper- and lowercase letters ('A'-'Z', 'a'-'z') // - digits ('0'-'9') // - underscore, dash, and dot ('_', '-', and '.') +// +// Deprecated: use validator.ValidateVendorName instead func ValidateVendorName(vendor string) error { - err := validateVendorOrClassName(vendor) - if err != nil { - err = fmt.Errorf("invalid vendor. %w", err) - } - return err + return validator.ValidateVendorName(vendor) } // ValidateClassName checks the validity of class name. @@ -130,40 +130,10 @@ func ValidateVendorName(vendor string) error { // - upper- and lowercase letters ('A'-'Z', 'a'-'z') // - digits ('0'-'9') // - underscore, dash, and dot ('_', '-', and '.') +// +// Deprecated: use validator.ValidateClassName instead func ValidateClassName(class string) error { - err := validateVendorOrClassName(class) - if err != nil { - err = fmt.Errorf("invalid class. %w", err) - } - return err -} - -// validateVendorOrClassName checks the validity of vendor or class name. -// A name may contain the following ASCII characters: -// - upper- and lowercase letters ('A'-'Z', 'a'-'z') -// - digits ('0'-'9') -// - underscore, dash, and dot ('_', '-', and '.') -func validateVendorOrClassName(name string) error { - if name == "" { - return fmt.Errorf("empty name") - } - if !IsLetter(rune(name[0])) { - return fmt.Errorf("%q, should start with letter", name) - } - for _, c := range string(name[1 : len(name)-1]) { - switch { - case IsAlphaNumeric(c): - case c == '_' || c == '-' || c == '.': - default: - return fmt.Errorf("invalid character '%c' in name %q", - c, name) - } - } - if !IsAlphaNumeric(rune(name[len(name)-1])) { - return fmt.Errorf("%q, should end with a letter or digit", name) - } - - return nil + return validator.ValidateClassName(class) } // ValidateDeviceName checks the validity of a device name. @@ -171,42 +141,29 @@ func validateVendorOrClassName(name string) error { // - upper- and lowercase letters ('A'-'Z', 'a'-'z') // - digits ('0'-'9') // - underscore, dash, dot, colon ('_', '-', '.', ':') +// +// Deprecated: use validator.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 + return validator.ValidateDeviceName(name) } // IsLetter reports whether the rune is a letter. +// +// Deprecated: use validator.IsLetter instead func IsLetter(c rune) bool { - return ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z') + return validator.IsLetter(c) } // IsDigit reports whether the rune is a digit. +// +// Deprecated: use validator.IsDigit instead func IsDigit(c rune) bool { - return '0' <= c && c <= '9' + return validator.IsDigit(c) } // IsAlphaNumeric reports whether the rune is a letter or digit. +// +// Deprecated: use validator.IsAlphaNumeric instead func IsAlphaNumeric(c rune) bool { - return IsLetter(c) || IsDigit(c) + return validator.IsAlphaNumeric(c) } diff --git a/schema/schema.go b/schema/schema.go index 6a582ca1..45d56e93 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -31,7 +31,8 @@ import ( "sigs.k8s.io/yaml" schema "github.com/xeipuuv/gojsonschema" - "tags.cncf.io/container-device-interface/internal/validation" + + "tags.cncf.io/container-device-interface/api/validator" ) const ( @@ -305,7 +306,7 @@ func (s *Schema) validateContents(any map[string]interface{}) error { contents := schemaContents(any) if specAnnotations, ok := contents.getAnnotations(); ok { - if err := validation.ValidateSpecAnnotations("", specAnnotations); err != nil { + if err := validator.ValidateSpecAnnotations("", specAnnotations); err != nil { return err } } @@ -318,7 +319,7 @@ func (s *Schema) validateContents(any map[string]interface{}) error { for _, device := range devices { name, _ := device.getFieldAsString("name") if annotations, ok := device.getAnnotations(); ok { - if err := validation.ValidateSpecAnnotations(name, annotations); err != nil { + if err := validator.ValidateSpecAnnotations(name, annotations); err != nil { return err } } diff --git a/specs-go/config.go b/specs-go/config.go index 77f095c0..19652440 100644 --- a/specs-go/config.go +++ b/specs-go/config.go @@ -1,6 +1,8 @@ package specs -import "os" +import ( + "os" +) // Spec is the base configuration for CDI type Spec struct {