diff --git a/cmd/oci-runtime-tool/validate.go b/cmd/oci-runtime-tool/validate.go index 9e08f4525..fd206514d 100644 --- a/cmd/oci-runtime-tool/validate.go +++ b/cmd/oci-runtime-tool/validate.go @@ -27,9 +27,9 @@ var bundleValidateCommand = cli.Command{ return err } - errMsgs := v.CheckAll() - if len(errMsgs) > 0 { - return fmt.Errorf("%d Errors detected:\n%s", len(errMsgs), strings.Join(errMsgs, "\n")) + err := v.CheckAll() + if err != nil { + return err } fmt.Println("Bundle validation succeeded.") diff --git a/error/error.go b/error/error.go index c2345c1d8..2b4ab6267 100644 --- a/error/error.go +++ b/error/error.go @@ -48,6 +48,7 @@ type Error struct { Level Level Reference string Err error + ErrCode int } // ParseLevel takes a string level and returns the OCI compliance level constant. diff --git a/validate/error.go b/validate/error.go index 0311c2304..074839bc6 100644 --- a/validate/error.go +++ b/validate/error.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" + "github.com/hashicorp/go-multierror" rfc2119 "github.com/opencontainers/runtime-tools/error" ) @@ -13,8 +14,36 @@ const referenceTemplate = "https://github.com/opencontainers/runtime-spec/blob/v type ErrorCode int const ( + // NonError represents that an input is not an error + NonError ErrorCode = iota + // NonRFCError represents that an error is not a rfc2119 error + NonRFCError + + // ConfigFileExistence represents the error code of 'config.json' existence test. + ConfigFileExistence + // ArtifactsInSignleDir represents the error code of artifacts place test. + ArtifactsInSingleDir + + // SpecVersion represents the error code of specfication version test. + SpecVersion + + // RootOnNonHyperV represents the error code of root setting test on non hyper-v containers + RootOnNonHyperV + // RootOnNonHyperV represents the error code of root setting test on hyper-v containers + RootOnHyperV + // PathFormatOnwindows represents the error code of the path format test on Window + PathFormatOnWindows + // PathName represents the error code of the path name test + PathName + // PathExistence represents the error code of the path existence test + PathExistence + // ReadonlyFilesystem represents the error code of readonly test + ReadonlyFilesystem + // ReadonlyOnWindows represents the error code of readonly setting test on Windows + ReadonlyOnWindows + // DefaultFilesystems represents the error code of default filesystems test. - DefaultFilesystems ErrorCode = iota + DefaultFilesystems ) type errorTemplate struct { @@ -22,13 +51,44 @@ type errorTemplate struct { Reference func(version string) (reference string, err error) } +var ( + containerFormatRef = func(version string) (reference string, err error) { + return fmt.Sprintf(referenceTemplate, version, "bundle.md#container-format"), nil + } + specVersionRef = func(version string) (reference string, err error) { + return fmt.Sprintf(referenceTemplate, version, "config.md#specification-version"), nil + } + rootRef = func(version string) (reference string, err error) { + return fmt.Sprintf(referenceTemplate, version, "config.md#root"), nil + } + defaultFSRef = func(version string) (reference string, err error) { + return fmt.Sprintf(referenceTemplate, version, "config-linux.md#default-filesystems"), nil + } +) + var ociErrors = map[ErrorCode]errorTemplate{ - DefaultFilesystems: errorTemplate{ - Level: rfc2119.Should, - Reference: func(version string) (reference string, err error) { - return fmt.Sprintf(referenceTemplate, version, "config-linux.md#default-filesystems"), nil - }, - }, + // NonRFCError represents that an error is not a rfc2119 error + // Bundle.md + // Container Format + ConfigFileExistence: errorTemplate{Level: rfc2119.Must, Reference: containerFormatRef}, + ArtifactsInSingleDir: errorTemplate{Level: rfc2119.Must, Reference: containerFormatRef}, + + // Config.md + // Specification Version + SpecVersion: errorTemplate{Level: rfc2119.Must, Reference: specVersionRef}, + // Root + RootOnNonHyperV: errorTemplate{Level: rfc2119.Required, Reference: rootRef}, + RootOnHyperV: errorTemplate{Level: rfc2119.Must, Reference: rootRef}, + // TODO + PathFormatOnWindows: errorTemplate{Level: rfc2119.Must, Reference: rootRef}, + PathName: errorTemplate{Level: rfc2119.Should, Reference: rootRef}, + PathExistence: errorTemplate{Level: rfc2119.Must, Reference: rootRef}, + ReadonlyFilesystem: errorTemplate{Level: rfc2119.Must, Reference: rootRef}, + ReadonlyOnWindows: errorTemplate{Level: rfc2119.Must, Reference: rootRef}, + + // Config-Linux.md + // Default Filesystems + DefaultFilesystems: errorTemplate{Level: rfc2119.Should, Reference: defaultFSRef}, } // NewError creates an Error referencing a spec violation. The error @@ -48,5 +108,30 @@ func NewError(code ErrorCode, msg string, version string) (err error) { Level: template.Level, Reference: reference, Err: errors.New(msg), + ErrCode: int(code), + } +} + +// FindError finds an error from a source error (mulitple error) and +// returns the error code if founded. +// If the source error is nil or empty, return NonErr. +// If the source error is not a multiple error, return NonRFCErr. +func FindError(err error, code ErrorCode) ErrorCode { + if err == nil { + return NonError + } + + if merr, ok := err.(*multierror.Error); ok { + if merr.ErrorOrNil() == nil { + return NonError + } + for _, e := range merr.Errors { + if rfcErr, ok := e.(*rfc2119.Error); ok { + if rfcErr.ErrCode == int(code) { + return code + } + } + } } + return NonRFCError } diff --git a/validate/validate.go b/validate/validate.go index 4b79e24b4..8e171e596 100644 --- a/validate/validate.go +++ b/validate/validate.go @@ -3,6 +3,7 @@ package validate import ( "bufio" "encoding/json" + "errors" "fmt" "io/ioutil" "net" @@ -17,6 +18,7 @@ import ( "unicode/utf8" "github.com/blang/semver" + "github.com/hashicorp/go-multierror" rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" "github.com/syndtr/gocapability/capability" @@ -82,7 +84,7 @@ func NewValidatorFromPath(bundlePath string, hostSpecific bool, platform string) configPath := filepath.Join(bundlePath, specConfig) content, err := ioutil.ReadFile(configPath) if err != nil { - return Validator{}, err + return Validator{}, NewError(ConfigFileExistence, err.Error(), rspec.Version) } if !utf8.Valid(content) { return Validator{}, fmt.Errorf("%q is not encoded in UTF-8", configPath) @@ -96,37 +98,44 @@ func NewValidatorFromPath(bundlePath string, hostSpecific bool, platform string) } // CheckAll checks all parts of runtime bundle -func (v *Validator) CheckAll() (msgs []string) { - msgs = append(msgs, v.CheckPlatform()...) - msgs = append(msgs, v.CheckRoot()...) - msgs = append(msgs, v.CheckMandatoryFields()...) - msgs = append(msgs, v.CheckSemVer()...) - msgs = append(msgs, v.CheckMounts()...) - msgs = append(msgs, v.CheckProcess()...) - msgs = append(msgs, v.CheckHooks()...) - msgs = append(msgs, v.CheckLinux()...) +func (v *Validator) CheckAll() (errs error) { + errs = multierror.Append(errs, v.CheckPlatform()) + errs = multierror.Append(errs, v.CheckRoot()) + errs = multierror.Append(errs, v.CheckMandatoryFields()) + errs = multierror.Append(errs, v.CheckSemVer()) + errs = multierror.Append(errs, v.CheckMounts()) + errs = multierror.Append(errs, v.CheckProcess()) + errs = multierror.Append(errs, v.CheckHooks()) + errs = multierror.Append(errs, v.CheckLinux()) return } // CheckRoot checks status of v.spec.Root -func (v *Validator) CheckRoot() (msgs []string) { +func (v *Validator) CheckRoot() (errs error) { logrus.Debugf("check root") if v.platform == "windows" && v.spec.Windows != nil && v.spec.Windows.HyperV != nil { if v.spec.Root != nil { - msgs = append(msgs, fmt.Sprintf("for Hyper-V containers, Root must not be set")) + errs = multierror.Append(errs, + NewError(RootOnHyperV, "for Hyper-V containers, Root must not be set", rspec.Version)) return } return } else if v.spec.Root == nil { - msgs = append(msgs, fmt.Sprintf("for non-Hyper-V containers, Root must be set")) + errs = multierror.Append(errs, + NewError(RootOnNonHyperV, "for non-Hyper-V containers, Root must be set", rspec.Version)) return } absBundlePath, err := filepath.Abs(v.bundlePath) if err != nil { - msgs = append(msgs, fmt.Sprintf("unable to convert %q to an absolute path", v.bundlePath)) + errs = multierror.Append(errs, fmt.Errorf("unable to convert %q to an absolute path", v.bundlePath)) + } + + if filepath.Base(v.spec.Root.Path) != "rootfs" { + errs = multierror.Append(errs, + NewError(PathName, "Path name should be the conventional 'rootfs'", rspec.Version)) } var rootfsPath string @@ -139,24 +148,28 @@ func (v *Validator) CheckRoot() (msgs []string) { rootfsPath = filepath.Join(v.bundlePath, v.spec.Root.Path) absRootPath, err = filepath.Abs(rootfsPath) if err != nil { - msgs = append(msgs, fmt.Sprintf("unable to convert %q to an absolute path", rootfsPath)) + errs = multierror.Append(errs, fmt.Errorf("unable to convert %q to an absolute path", rootfsPath)) } } if fi, err := os.Stat(rootfsPath); err != nil { - msgs = append(msgs, fmt.Sprintf("Cannot find the root path %q", rootfsPath)) + errs = multierror.Append(errs, + NewError(PathExistence, fmt.Sprintf("Cannot find the root path %q", rootfsPath), rspec.Version)) } else if !fi.IsDir() { - msgs = append(msgs, fmt.Sprintf("The root path %q is not a directory.", rootfsPath)) + errs = multierror.Append(errs, + NewError(PathExistence, fmt.Sprintf("The root path %q is not a directory.", rootfsPath), rspec.Version)) } rootParent := filepath.Dir(absRootPath) if absRootPath == string(filepath.Separator) || rootParent != absBundlePath { - msgs = append(msgs, fmt.Sprintf("root.path is %q, but it MUST be a child of %q", v.spec.Root.Path, absBundlePath)) + errs = multierror.Append(errs, + NewError(ArtifactsInSingleDir, fmt.Sprintf("root.path is %q, but it MUST be a child of %q", v.spec.Root.Path, absBundlePath), rspec.Version)) } if v.platform == "windows" { if v.spec.Root.Readonly { - msgs = append(msgs, "root.readonly field MUST be omitted or false when target platform is windows") + errs = multierror.Append(errs, + NewError(ReadonlyOnWindows, "root.readonly field MUST be omitted or false when target platform is windows", rspec.Version)) } } @@ -164,53 +177,54 @@ func (v *Validator) CheckRoot() (msgs []string) { } // CheckSemVer checks v.spec.Version -func (v *Validator) CheckSemVer() (msgs []string) { +func (v *Validator) CheckSemVer() (errs error) { logrus.Debugf("check semver") version := v.spec.Version _, err := semver.Parse(version) if err != nil { - msgs = append(msgs, fmt.Sprintf("%q is not valid SemVer: %s", version, err.Error())) + errs = multierror.Append(errs, + NewError(SpecVersion, fmt.Sprintf("%q is not valid SemVer: %s", version, err.Error()), rspec.Version)) } if version != rspec.Version { - msgs = append(msgs, fmt.Sprintf("internal error: validate currently only handles version %s, but the supplied configuration targets %s", rspec.Version, version)) + errs = multierror.Append(errs, fmt.Errorf("internal error: validate currently only handles version %s, but the supplied configuration targets %s", rspec.Version, version)) } return } // CheckHooks check v.spec.Hooks -func (v *Validator) CheckHooks() (msgs []string) { +func (v *Validator) CheckHooks() (errs error) { logrus.Debugf("check hooks") if v.spec.Hooks != nil { - msgs = append(msgs, checkEventHooks("pre-start", v.spec.Hooks.Prestart, v.HostSpecific)...) - msgs = append(msgs, checkEventHooks("post-start", v.spec.Hooks.Poststart, v.HostSpecific)...) - msgs = append(msgs, checkEventHooks("post-stop", v.spec.Hooks.Poststop, v.HostSpecific)...) + errs = multierror.Append(errs, checkEventHooks("pre-start", v.spec.Hooks.Prestart, v.HostSpecific)) + errs = multierror.Append(errs, checkEventHooks("post-start", v.spec.Hooks.Poststart, v.HostSpecific)) + errs = multierror.Append(errs, checkEventHooks("post-stop", v.spec.Hooks.Poststop, v.HostSpecific)) } return } -func checkEventHooks(hookType string, hooks []rspec.Hook, hostSpecific bool) (msgs []string) { +func checkEventHooks(hookType string, hooks []rspec.Hook, hostSpecific bool) (errs error) { for _, hook := range hooks { if !filepath.IsAbs(hook.Path) { - msgs = append(msgs, fmt.Sprintf("The %s hook %v: is not absolute path", hookType, hook.Path)) + errs = multierror.Append(errs, fmt.Errorf("The %s hook %v: is not absolute path", hookType, hook.Path)) } if hostSpecific { fi, err := os.Stat(hook.Path) if err != nil { - msgs = append(msgs, fmt.Sprintf("Cannot find %s hook: %v", hookType, hook.Path)) + errs = multierror.Append(errs, fmt.Errorf("Cannot find %s hook: %v", hookType, hook.Path)) } if fi.Mode()&0111 == 0 { - msgs = append(msgs, fmt.Sprintf("The %s hook %v: is not executable", hookType, hook.Path)) + errs = multierror.Append(errs, fmt.Errorf("The %s hook %v: is not executable", hookType, hook.Path)) } } for _, env := range hook.Env { if !envValid(env) { - msgs = append(msgs, fmt.Sprintf("Env %q for hook %v is in the invalid form.", env, hook.Path)) + errs = multierror.Append(errs, fmt.Errorf("Env %q for hook %v is in the invalid form.", env, hook.Path)) } } } @@ -219,7 +233,7 @@ func checkEventHooks(hookType string, hooks []rspec.Hook, hostSpecific bool) (ms } // CheckProcess checks v.spec.Process -func (v *Validator) CheckProcess() (msgs []string) { +func (v *Validator) CheckProcess() (errs error) { logrus.Debugf("check process") if v.spec.Process == nil { @@ -228,17 +242,17 @@ func (v *Validator) CheckProcess() (msgs []string) { process := v.spec.Process if !filepath.IsAbs(process.Cwd) { - msgs = append(msgs, fmt.Sprintf("cwd %q is not an absolute path", process.Cwd)) + errs = multierror.Append(errs, fmt.Errorf("cwd %q is not an absolute path", process.Cwd)) } for _, env := range process.Env { if !envValid(env) { - msgs = append(msgs, fmt.Sprintf("env %q should be in the form of 'key=value'. The left hand side must consist solely of letters, digits, and underscores '_'.", env)) + errs = multierror.Append(errs, fmt.Errorf("env %q should be in the form of 'key=value'. The left hand side must consist solely of letters, digits, and underscores '_'.", env)) } } if len(process.Args) == 0 { - msgs = append(msgs, fmt.Sprintf("args must not be empty")) + errs = multierror.Append(errs, fmt.Errorf("args must not be empty")) } else { if filepath.IsAbs(process.Args[0]) { var rootfsPath string @@ -252,27 +266,27 @@ func (v *Validator) CheckProcess() (msgs []string) { if os.IsNotExist(err) { logrus.Warnf("executable %q is not available in rootfs currently", process.Args[0]) } else if err != nil { - msgs = append(msgs, err.Error()) + errs = multierror.Append(errs, err) } else { m := fileinfo.Mode() if m.IsDir() || m&0111 == 0 { - msgs = append(msgs, fmt.Sprintf("arg %q is not executable", process.Args[0])) + errs = multierror.Append(errs, fmt.Errorf("arg %q is not executable", process.Args[0])) } } } } if v.spec.Process.Capabilities != nil { - msgs = append(msgs, v.CheckCapabilities()...) + errs = multierror.Append(errs, v.CheckCapabilities()) } - msgs = append(msgs, v.CheckRlimits()...) + errs = multierror.Append(errs, v.CheckRlimits()) if v.platform == "linux" { if len(process.ApparmorProfile) > 0 { profilePath := filepath.Join(v.bundlePath, v.spec.Root.Path, "/etc/apparmor.d", process.ApparmorProfile) _, err := os.Stat(profilePath) if err != nil { - msgs = append(msgs, err.Error()) + errs = multierror.Append(errs, err) } } } @@ -281,7 +295,7 @@ func (v *Validator) CheckProcess() (msgs []string) { } // CheckCapabilities checks v.spec.Process.Capabilities -func (v *Validator) CheckCapabilities() (msgs []string) { +func (v *Validator) CheckCapabilities() (errs error) { process := v.spec.Process if v.platform == "linux" { var effective, permitted, inheritable, ambient bool @@ -305,7 +319,7 @@ func (v *Validator) CheckCapabilities() (msgs []string) { for capability, owns := range caps { if err := CapValid(capability, v.HostSpecific); err != nil { - msgs = append(msgs, fmt.Sprintf("capability %q is not valid, man capabilities(7)", capability)) + errs = multierror.Append(errs, fmt.Errorf("capability %q is not valid, man capabilities(7)", capability)) } effective, permitted, ambient, inheritable = false, false, false, false @@ -324,10 +338,10 @@ func (v *Validator) CheckCapabilities() (msgs []string) { } } if effective && !permitted { - msgs = append(msgs, fmt.Sprintf("effective capability %q is not allowed, as it's not permitted", capability)) + errs = multierror.Append(errs, fmt.Errorf("effective capability %q is not allowed, as it's not permitted", capability)) } if ambient && !(effective && inheritable) { - msgs = append(msgs, fmt.Sprintf("ambient capability %q is not allowed, as it's not permitted and inheribate", capability)) + errs = multierror.Append(errs, fmt.Errorf("ambient capability %q is not allowed, as it's not permitted and inheribate", capability)) } } } else { @@ -338,15 +352,15 @@ func (v *Validator) CheckCapabilities() (msgs []string) { } // CheckRlimits checks v.spec.Process.Rlimits -func (v *Validator) CheckRlimits() (msgs []string) { +func (v *Validator) CheckRlimits() (errs error) { process := v.spec.Process for index, rlimit := range process.Rlimits { for i := index + 1; i < len(process.Rlimits); i++ { if process.Rlimits[index].Type == process.Rlimits[i].Type { - msgs = append(msgs, fmt.Sprintf("rlimit can not contain the same type %q.", process.Rlimits[index].Type)) + errs = multierror.Append(errs, fmt.Errorf("rlimit can not contain the same type %q.", process.Rlimits[index].Type)) } } - msgs = append(msgs, v.rlimitValid(rlimit)...) + errs = multierror.Append(errs, v.rlimitValid(rlimit)) } return @@ -394,29 +408,29 @@ func supportedMountTypes(OS string, hostSpecific bool) (map[string]bool, error) } // CheckMounts checks v.spec.Mounts -func (v *Validator) CheckMounts() (msgs []string) { +func (v *Validator) CheckMounts() (errs error) { logrus.Debugf("check mounts") supportedTypes, err := supportedMountTypes(v.platform, v.HostSpecific) if err != nil { - msgs = append(msgs, err.Error()) + errs = multierror.Append(errs, err) return } for i, mountA := range v.spec.Mounts { if supportedTypes != nil && !supportedTypes[mountA.Type] { - msgs = append(msgs, fmt.Sprintf("Unsupported mount type %q", mountA.Type)) + errs = multierror.Append(errs, fmt.Errorf("Unsupported mount type %q", mountA.Type)) } if v.platform == "windows" { if err := pathValid(v.platform, mountA.Destination); err != nil { - msgs = append(msgs, err.Error()) + errs = multierror.Append(errs, err) } if err := pathValid(v.platform, mountA.Source); err != nil { - msgs = append(msgs, err.Error()) + errs = multierror.Append(errs, err) } } else { if err := pathValid(v.platform, mountA.Destination); err != nil { - msgs = append(msgs, err.Error()) + errs = multierror.Append(errs, err) } } for j, mountB := range v.spec.Mounts { @@ -426,12 +440,12 @@ func (v *Validator) CheckMounts() (msgs []string) { // whether B.Desination is nested within A.Destination nested, err := nestedValid(v.platform, mountA.Destination, mountB.Destination) if err != nil { - msgs = append(msgs, err.Error()) + errs = multierror.Append(errs, err) continue } if nested { if v.platform == "windows" && i < j { - msgs = append(msgs, fmt.Sprintf("On Windows, %v nested within %v is forbidden", mountB.Destination, mountA.Destination)) + errs = multierror.Append(errs, fmt.Errorf("On Windows, %v nested within %v is forbidden", mountB.Destination, mountA.Destination)) } if i > j { logrus.Warnf("%v will be covered by %v", mountB.Destination, mountA.Destination) @@ -444,17 +458,17 @@ func (v *Validator) CheckMounts() (msgs []string) { } // CheckPlatform checks v.platform -func (v *Validator) CheckPlatform() (msgs []string) { +func (v *Validator) CheckPlatform() (errs error) { logrus.Debugf("check platform") if v.platform != "linux" && v.platform != "solaris" && v.platform != "windows" { - msgs = append(msgs, fmt.Sprintf("platform %q is not supported", v.platform)) + errs = multierror.Append(errs, fmt.Errorf("platform %q is not supported", v.platform)) return } if v.platform == "windows" { if v.spec.Windows == nil { - msgs = append(msgs, "'windows' MUST be set when platform is `windows`") + errs = multierror.Append(errs, errors.New("'windows' MUST be set when platform is `windows`")) } } @@ -462,7 +476,7 @@ func (v *Validator) CheckPlatform() (msgs []string) { } // CheckLinux checks v.spec.Linux -func (v *Validator) CheckLinux() (msgs []string) { +func (v *Validator) CheckLinux() (errs error) { logrus.Debugf("check linux") if v.spec.Linux == nil { @@ -485,13 +499,13 @@ func (v *Validator) CheckLinux() (msgs []string) { for index := 0; index < len(v.spec.Linux.Namespaces); index++ { ns := v.spec.Linux.Namespaces[index] if !namespaceValid(ns) { - msgs = append(msgs, fmt.Sprintf("namespace %v is invalid.", ns)) + errs = multierror.Append(errs, fmt.Errorf("namespace %v is invalid.", ns)) } tmpItem := nsTypeList[ns.Type] tmpItem.num = tmpItem.num + 1 if tmpItem.num > 1 { - msgs = append(msgs, fmt.Sprintf("duplicated namespace %q", ns.Type)) + errs = multierror.Append(errs, fmt.Errorf("duplicated namespace %q", ns.Type)) } if len(ns.Path) == 0 { @@ -501,26 +515,26 @@ func (v *Validator) CheckLinux() (msgs []string) { } if (len(v.spec.Linux.UIDMappings) > 0 || len(v.spec.Linux.GIDMappings) > 0) && !nsTypeList[rspec.UserNamespace].newExist { - msgs = append(msgs, "UID/GID mappings requires a new User namespace to be specified as well") + errs = multierror.Append(errs, errors.New("UID/GID mappings requires a new User namespace to be specified as well")) } else if len(v.spec.Linux.UIDMappings) > 5 { - msgs = append(msgs, "Only 5 UID mappings are allowed (linux kernel restriction).") + errs = multierror.Append(errs, errors.New("Only 5 UID mappings are allowed (linux kernel restriction).")) } else if len(v.spec.Linux.GIDMappings) > 5 { - msgs = append(msgs, "Only 5 GID mappings are allowed (linux kernel restriction).") + errs = multierror.Append(errs, errors.New("Only 5 GID mappings are allowed (linux kernel restriction).")) } for k := range v.spec.Linux.Sysctl { if strings.HasPrefix(k, "net.") && !nsTypeList[rspec.NetworkNamespace].newExist { - msgs = append(msgs, fmt.Sprintf("Sysctl %v requires a new Network namespace to be specified as well", k)) + errs = multierror.Append(errs, fmt.Errorf("Sysctl %v requires a new Network namespace to be specified as well", k)) } if strings.HasPrefix(k, "fs.mqueue.") { if !nsTypeList[rspec.MountNamespace].newExist || !nsTypeList[rspec.IPCNamespace].newExist { - msgs = append(msgs, fmt.Sprintf("Sysctl %v requires a new IPC namespace and Mount namespace to be specified as well", k)) + errs = multierror.Append(errs, fmt.Errorf("Sysctl %v requires a new IPC namespace and Mount namespace to be specified as well", k)) } } } if v.platform == "linux" && !nsTypeList[rspec.UTSNamespace].newExist && v.spec.Hostname != "" { - msgs = append(msgs, fmt.Sprintf("On Linux, hostname requires a new UTS namespace to be specified as well")) + errs = multierror.Append(errs, fmt.Errorf("On Linux, hostname requires a new UTS namespace to be specified as well")) } // Linux devices validation @@ -529,11 +543,11 @@ func (v *Validator) CheckLinux() (msgs []string) { for index := 0; index < len(v.spec.Linux.Devices); index++ { device := v.spec.Linux.Devices[index] if !deviceValid(device) { - msgs = append(msgs, fmt.Sprintf("device %v is invalid.", device)) + errs = multierror.Append(errs, fmt.Errorf("device %v is invalid.", device)) } if _, exists := devList[device.Path]; exists { - msgs = append(msgs, fmt.Sprintf("device %s is duplicated", device.Path)) + errs = multierror.Append(errs, fmt.Errorf("device %s is duplicated", device.Path)) } else { var rootfsPath string if filepath.IsAbs(v.spec.Root.Path) { @@ -546,11 +560,11 @@ func (v *Validator) CheckLinux() (msgs []string) { if os.IsNotExist(err) { devList[device.Path] = true } else if err != nil { - msgs = append(msgs, err.Error()) + errs = multierror.Append(errs, err) } else { fStat, ok := fi.Sys().(*syscall.Stat_t) if !ok { - msgs = append(msgs, fmt.Sprintf("cannot determine state for device %s", device.Path)) + errs = multierror.Append(errs, fmt.Errorf("cannot determine state for device %s", device.Path)) continue } var devType string @@ -565,7 +579,7 @@ func (v *Validator) CheckLinux() (msgs []string) { devType = "unmatched" } if devType != device.Type || (devType == "c" && device.Type == "u") { - msgs = append(msgs, fmt.Sprintf("unmatched %s already exists in filesystem", device.Path)) + errs = multierror.Append(errs, fmt.Errorf("unmatched %s already exists in filesystem", device.Path)) continue } if devType != "p" { @@ -573,7 +587,7 @@ func (v *Validator) CheckLinux() (msgs []string) { major := (dev >> 8) & 0xfff minor := (dev & 0xff) | ((dev >> 12) & 0xfff00) if int64(major) != device.Major || int64(minor) != device.Minor { - msgs = append(msgs, fmt.Sprintf("unmatched %s already exists in filesystem", device.Path)) + errs = multierror.Append(errs, fmt.Errorf("unmatched %s already exists in filesystem", device.Path)) continue } } @@ -581,19 +595,19 @@ func (v *Validator) CheckLinux() (msgs []string) { expectedPerm := *device.FileMode & os.ModePerm actualPerm := fi.Mode() & os.ModePerm if expectedPerm != actualPerm { - msgs = append(msgs, fmt.Sprintf("unmatched %s already exists in filesystem", device.Path)) + errs = multierror.Append(errs, fmt.Errorf("unmatched %s already exists in filesystem", device.Path)) continue } } if device.UID != nil { if *device.UID != fStat.Uid { - msgs = append(msgs, fmt.Sprintf("unmatched %s already exists in filesystem", device.Path)) + errs = multierror.Append(errs, fmt.Errorf("unmatched %s already exists in filesystem", device.Path)) continue } } if device.GID != nil { if *device.GID != fStat.Gid { - msgs = append(msgs, fmt.Sprintf("unmatched %s already exists in filesystem", device.Path)) + errs = multierror.Append(errs, fmt.Errorf("unmatched %s already exists in filesystem", device.Path)) continue } } @@ -616,13 +630,11 @@ func (v *Validator) CheckLinux() (msgs []string) { } if v.spec.Linux.Resources != nil { - ms := v.CheckLinuxResources() - msgs = append(msgs, ms...) + errs = multierror.Append(errs, v.CheckLinuxResources()) } if v.spec.Linux.Seccomp != nil { - ms := v.CheckSeccomp() - msgs = append(msgs, ms...) + errs = multierror.Append(errs, v.CheckSeccomp()) } switch v.spec.Linux.RootfsPropagation { @@ -636,18 +648,18 @@ func (v *Validator) CheckLinux() (msgs []string) { case "unbindable": case "runbindable": default: - msgs = append(msgs, "rootfsPropagation must be empty or one of \"private|rprivate|slave|rslave|shared|rshared|unbindable|runbindable\"") + errs = multierror.Append(errs, errors.New("rootfsPropagation must be empty or one of \"private|rprivate|slave|rslave|shared|rshared|unbindable|runbindable\"")) } for _, maskedPath := range v.spec.Linux.MaskedPaths { if !strings.HasPrefix(maskedPath, "/") { - msgs = append(msgs, fmt.Sprintf("maskedPath %v is not an absolute path", maskedPath)) + errs = multierror.Append(errs, fmt.Errorf("maskedPath %v is not an absolute path", maskedPath)) } } for _, readonlyPath := range v.spec.Linux.ReadonlyPaths { if !strings.HasPrefix(readonlyPath, "/") { - msgs = append(msgs, fmt.Sprintf("readonlyPath %v is not an absolute path", readonlyPath)) + errs = multierror.Append(errs, fmt.Errorf("readonlyPath %v is not an absolute path", readonlyPath)) } } @@ -655,23 +667,23 @@ func (v *Validator) CheckLinux() (msgs []string) { } // CheckLinuxResources checks v.spec.Linux.Resources -func (v *Validator) CheckLinuxResources() (msgs []string) { +func (v *Validator) CheckLinuxResources() (errs error) { logrus.Debugf("check linux resources") r := v.spec.Linux.Resources if r.Memory != nil { if r.Memory.Limit != nil && r.Memory.Swap != nil && uint64(*r.Memory.Limit) > uint64(*r.Memory.Swap) { - msgs = append(msgs, fmt.Sprintf("Minimum memoryswap should be larger than memory limit")) + errs = multierror.Append(errs, fmt.Errorf("Minimum memoryswap should be larger than memory limit")) } if r.Memory.Limit != nil && r.Memory.Reservation != nil && uint64(*r.Memory.Reservation) > uint64(*r.Memory.Limit) { - msgs = append(msgs, fmt.Sprintf("Minimum memory limit should be larger than memory reservation")) + errs = multierror.Append(errs, fmt.Errorf("Minimum memory limit should be larger than memory reservation")) } } if r.Network != nil && v.HostSpecific { var exist bool interfaces, err := net.Interfaces() if err != nil { - msgs = append(msgs, err.Error()) + errs = multierror.Append(errs, err) return } for _, prio := range r.Network.Priorities { @@ -683,7 +695,7 @@ func (v *Validator) CheckLinuxResources() (msgs []string) { } } if !exist { - msgs = append(msgs, fmt.Sprintf("Interface %s does not exist currently", prio.Name)) + errs = multierror.Append(errs, fmt.Errorf("Interface %s does not exist currently", prio.Name)) } } } @@ -692,16 +704,16 @@ func (v *Validator) CheckLinuxResources() (msgs []string) { } // CheckSeccomp checkc v.spec.Linux.Seccomp -func (v *Validator) CheckSeccomp() (msgs []string) { +func (v *Validator) CheckSeccomp() (errs error) { logrus.Debugf("check linux seccomp") s := v.spec.Linux.Seccomp if !seccompActionValid(s.DefaultAction) { - msgs = append(msgs, fmt.Sprintf("seccomp defaultAction %q is invalid.", s.DefaultAction)) + errs = multierror.Append(errs, fmt.Errorf("seccomp defaultAction %q is invalid.", s.DefaultAction)) } for index := 0; index < len(s.Syscalls); index++ { if !syscallValid(s.Syscalls[index]) { - msgs = append(msgs, fmt.Sprintf("syscall %v is invalid.", s.Syscalls[index])) + errs = multierror.Append(errs, fmt.Errorf("syscall %v is invalid.", s.Syscalls[index])) } } for index := 0; index < len(s.Architectures); index++ { @@ -725,7 +737,7 @@ func (v *Validator) CheckSeccomp() (msgs []string) { case rspec.ArchPARISC: case rspec.ArchPARISC64: default: - msgs = append(msgs, fmt.Sprintf("seccomp architecture %q is invalid", s.Architectures[index])) + errs = multierror.Append(errs, fmt.Errorf("seccomp architecture %q is invalid", s.Architectures[index])) } } @@ -782,9 +794,9 @@ func envValid(env string) bool { return true } -func (v *Validator) rlimitValid(rlimit rspec.POSIXRlimit) (msgs []string) { +func (v *Validator) rlimitValid(rlimit rspec.POSIXRlimit) (errs error) { if rlimit.Hard < rlimit.Soft { - msgs = append(msgs, fmt.Sprintf("hard limit of rlimit %s should not be less than soft limit", rlimit.Type)) + errs = multierror.Append(errs, fmt.Errorf("hard limit of rlimit %s should not be less than soft limit", rlimit.Type)) } if v.platform == "linux" { @@ -793,7 +805,7 @@ func (v *Validator) rlimitValid(rlimit rspec.POSIXRlimit) (msgs []string) { return } } - msgs = append(msgs, fmt.Sprintf("rlimit type %q is invalid", rlimit.Type)) + errs = multierror.Append(errs, fmt.Errorf("rlimit type %q is invalid", rlimit.Type)) } else { logrus.Warnf("process.rlimits validation not yet implemented for platform %q", v.platform) } @@ -939,38 +951,38 @@ func isStructPtr(t reflect.Type) bool { return t.Kind() == reflect.Ptr && t.Elem().Kind() == reflect.Struct } -func checkMandatoryUnit(field reflect.Value, tagField reflect.StructField, parent string) (msgs []string) { +func checkMandatoryUnit(field reflect.Value, tagField reflect.StructField, parent string) (errs error) { mandatory := !strings.Contains(tagField.Tag.Get("json"), "omitempty") switch field.Kind() { case reflect.Ptr: if mandatory && field.IsNil() { - msgs = append(msgs, fmt.Sprintf("'%s.%s' should not be empty.", parent, tagField.Name)) + errs = multierror.Append(errs, fmt.Errorf("'%s.%s' should not be empty.", parent, tagField.Name)) } case reflect.String: if mandatory && (field.Len() == 0) { - msgs = append(msgs, fmt.Sprintf("'%s.%s' should not be empty.", parent, tagField.Name)) + errs = multierror.Append(errs, fmt.Errorf("'%s.%s' should not be empty.", parent, tagField.Name)) } case reflect.Slice: if mandatory && (field.IsNil() || field.Len() == 0) { - msgs = append(msgs, fmt.Sprintf("'%s.%s' should not be empty.", parent, tagField.Name)) + errs = multierror.Append(errs, fmt.Errorf("'%s.%s' should not be empty.", parent, tagField.Name)) return } for index := 0; index < field.Len(); index++ { mValue := field.Index(index) if mValue.CanInterface() { - msgs = append(msgs, checkMandatory(mValue.Interface())...) + errs = multierror.Append(errs, checkMandatory(mValue.Interface())) } } case reflect.Map: if mandatory && (field.IsNil() || field.Len() == 0) { - msgs = append(msgs, fmt.Sprintf("'%s.%s' should not be empty.", parent, tagField.Name)) - return msgs + errs = multierror.Append(errs, fmt.Errorf("'%s.%s' should not be empty.", parent, tagField.Name)) + return } keys := field.MapKeys() for index := 0; index < len(keys); index++ { mValue := field.MapIndex(keys[index]) if mValue.CanInterface() { - msgs = append(msgs, checkMandatory(mValue.Interface())...) + errs = multierror.Append(errs, checkMandatory(mValue.Interface())) } } default: @@ -979,7 +991,7 @@ func checkMandatoryUnit(field reflect.Value, tagField reflect.StructField, paren return } -func checkMandatory(obj interface{}) (msgs []string) { +func checkMandatory(obj interface{}) (errs error) { objT := reflect.TypeOf(obj) objV := reflect.ValueOf(obj) if isStructPtr(objT) { @@ -993,12 +1005,12 @@ func checkMandatory(obj interface{}) (msgs []string) { t := objT.Field(i).Type if isStructPtr(t) && objV.Field(i).IsNil() { if !strings.Contains(objT.Field(i).Tag.Get("json"), "omitempty") { - msgs = append(msgs, fmt.Sprintf("'%s.%s' should not be empty", objT.Name(), objT.Field(i).Name)) + errs = multierror.Append(errs, fmt.Errorf("'%s.%s' should not be empty", objT.Name(), objT.Field(i).Name)) } } else if (isStruct(t) || isStructPtr(t)) && objV.Field(i).CanInterface() { - msgs = append(msgs, checkMandatory(objV.Field(i).Interface())...) + errs = multierror.Append(errs, checkMandatory(objV.Field(i).Interface())) } else { - msgs = append(msgs, checkMandatoryUnit(objV.Field(i), objT.Field(i), objT.Name())...) + errs = multierror.Append(errs, checkMandatoryUnit(objV.Field(i), objT.Field(i), objT.Name())) } } @@ -1006,7 +1018,7 @@ func checkMandatory(obj interface{}) (msgs []string) { } // CheckMandatoryFields checks mandatory field of container's config file -func (v *Validator) CheckMandatoryFields() []string { +func (v *Validator) CheckMandatoryFields() error { logrus.Debugf("check mandatory fields") return checkMandatory(v.spec) diff --git a/validate/validate_test.go b/validate/validate_test.go index 7a3931d8a..844805005 100644 --- a/validate/validate_test.go +++ b/validate/validate_test.go @@ -1,20 +1,32 @@ package validate import ( + "fmt" "io/ioutil" "os" "path/filepath" - "strings" + "runtime" "testing" rspec "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" ) -func checkErrors(t *testing.T, title string, msgs []string, valid bool) { - if valid && len(msgs) > 0 { - t.Fatalf("%s: expected not to get error, but get %d errors:\n%s", title, len(msgs), strings.Join(msgs, "\n")) - } else if !valid && len(msgs) == 0 { - t.Fatalf("%s: expected to get error, but actually not", title) +func TestNewValidator(t *testing.T) { + testSpec := &rspec.Spec{} + testBundle := "" + testPlatform := "not" + runtime.GOOS + cases := []struct { + val Validator + expected Validator + }{ + {Validator{testSpec, testBundle, true, testPlatform}, Validator{testSpec, testBundle, true, runtime.GOOS}}, + {Validator{testSpec, testBundle, true, runtime.GOOS}, Validator{testSpec, testBundle, true, runtime.GOOS}}, + {Validator{testSpec, testBundle, false, testPlatform}, Validator{testSpec, testBundle, false, testPlatform}}, + } + + for _, c := range cases { + assert.Equal(t, c.expected, NewValidator(c.val.spec, c.val.bundlePath, c.val.HostSpecific, c.val.platform)) } } @@ -25,7 +37,7 @@ func TestCheckRoot(t *testing.T) { } defer os.RemoveAll(tmpBundle) - rootfsDir := "rootfs" + rootfsDir := "rootfs/rootfs" rootfsNonDir := "rootfsfile" rootfsNonExists := "rootfsnil" if err := os.MkdirAll(filepath.Join(tmpBundle, rootfsDir), 0700); err != nil { @@ -35,36 +47,44 @@ func TestCheckRoot(t *testing.T) { t.Fatalf("Failed to create a non-directory rootfs in 'CheckRoot'") } + // Note: Abs error is not tested cases := []struct { - val string - expected bool + val rspec.Spec + platform string + expected ErrorCode }{ - {rootfsDir, true}, - {rootfsNonDir, false}, - {rootfsNonExists, false}, - {filepath.Join(tmpBundle, rootfsDir), true}, - {filepath.Join(tmpBundle, rootfsNonDir), false}, - {filepath.Join(tmpBundle, rootfsNonExists), false}, + {rspec.Spec{Windows: &rspec.Windows{HyperV: &rspec.WindowsHyperV{}}, Root: &rspec.Root{}}, "windows", RootOnHyperV}, + {rspec.Spec{Windows: &rspec.Windows{HyperV: &rspec.WindowsHyperV{}}, Root: nil}, "windows", NonError}, + {rspec.Spec{Root: nil}, "linux", RootOnNonHyperV}, + {rspec.Spec{Root: &rspec.Root{Path: "maverick-rootfs"}}, "linux", PathName}, + {rspec.Spec{Root: &rspec.Root{Path: "rootfs"}}, "linux", NonError}, + {rspec.Spec{Root: &rspec.Root{Path: filepath.Join(tmpBundle, rootfsNonExists)}}, "linux", PathExistence}, + {rspec.Spec{Root: &rspec.Root{Path: filepath.Join(tmpBundle, rootfsNonDir)}}, "linux", PathExistence}, + {rspec.Spec{Root: &rspec.Root{Path: filepath.Join(tmpBundle, "rootfs")}}, "linux", NonError}, + {rspec.Spec{Root: &rspec.Root{Path: "rootfs/rootfs"}}, "linux", ArtifactsInSingleDir}, + {rspec.Spec{Root: &rspec.Root{Readonly: true}}, "windows", ReadonlyOnWindows}, } for _, c := range cases { - v := NewValidator(&rspec.Spec{Root: &rspec.Root{Path: c.val}}, tmpBundle, false, "linux") - checkErrors(t, "CheckRoot "+c.val, v.CheckRoot(), c.expected) + v := NewValidator(&c.val, tmpBundle, false, c.platform) + err := v.CheckRoot() + assert.Equal(t, c.expected, FindError(err, c.expected), fmt.Sprintf("Fail to check Root: %v %d", err, c.expected)) } } func TestCheckSemVer(t *testing.T) { cases := []struct { val string - expected bool + expected ErrorCode }{ - {rspec.Version, true}, + {rspec.Version, NonError}, //FIXME: validate currently only handles rpsec.Version - {"0.0.1", false}, - {"invalid", false}, + {"0.0.1", NonRFCError}, + {"invalid", SpecVersion}, } for _, c := range cases { v := NewValidator(&rspec.Spec{Version: c.val}, "", false, "linux") - checkErrors(t, "CheckSemVer "+c.val, v.CheckSemVer(), c.expected) + err := v.CheckSemVer() + assert.Equal(t, c.expected, FindError(err, c.expected), "Fail to check SemVer "+c.val) } }