From 687c61c0f889d10efd29cc346cc64c41be26f68a Mon Sep 17 00:00:00 2001 From: liang chenye Date: Thu, 1 Sep 2016 21:28:58 +0800 Subject: [PATCH] add CheckSemVer unit test Signed-off-by: liang chenye --- cmd/ocitools/validate.go | 35 +--------- validate/validate.go | 132 ++++++++++++++++++++++++-------------- validate/validate_test.go | 33 ++++++++++ 3 files changed, 119 insertions(+), 81 deletions(-) create mode 100644 validate/validate_test.go diff --git a/cmd/ocitools/validate.go b/cmd/ocitools/validate.go index 18d1405c9..b894dd54e 100644 --- a/cmd/ocitools/validate.go +++ b/cmd/ocitools/validate.go @@ -1,15 +1,9 @@ package main import ( - "encoding/json" "fmt" - "io/ioutil" - "os" - "path" "strings" - "unicode/utf8" - rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/urfave/cli" "github.com/opencontainers/ocitools/validate" @@ -26,37 +20,12 @@ var bundleValidateCommand = cli.Command{ Before: before, Action: func(context *cli.Context) error { inputPath := context.String("path") - if inputPath == "" { - return fmt.Errorf("Bundle path shouldn't be empty") - } - - if _, err := os.Stat(inputPath); err != nil { - return err - } - - configPath := path.Join(inputPath, "config.json") - content, err := ioutil.ReadFile(configPath) + hostSpecific := context.GlobalBool("host-specific") + v, err := validate.NewValidatorFromPath(inputPath, hostSpecific) if err != nil { return err } - if !utf8.Valid(content) { - return fmt.Errorf("%q is not encoded in UTF-8", configPath) - } - var spec rspec.Spec - if err = json.Unmarshal(content, &spec); err != nil { - return err - } - - rootfsPath := path.Join(inputPath, spec.Root.Path) - if fi, err := os.Stat(rootfsPath); err != nil { - return fmt.Errorf("Cannot find the root path %q", rootfsPath) - } else if !fi.IsDir() { - return fmt.Errorf("The root path %q is not a directory.", rootfsPath) - } - - hostSpecific := context.GlobalBool("host-specific") - v := validate.NewValidator(spec, rootfsPath, hostSpecific) errMsgs := v.CheckAll() if len(errMsgs) > 0 { return fmt.Errorf("%d Errors detected:\n%s", len(errMsgs), strings.Join(errMsgs, "\n")) diff --git a/validate/validate.go b/validate/validate.go index ab6801db5..d79999888 100644 --- a/validate/validate.go +++ b/validate/validate.go @@ -2,13 +2,16 @@ package validate import ( "bufio" + "encoding/json" "fmt" + "io/ioutil" "os" "path" "path/filepath" "reflect" "strings" "unicode" + "unicode/utf8" "github.com/Sirupsen/logrus" "github.com/blang/semver" @@ -53,13 +56,45 @@ var ( ) type Validator struct { - spec rspec.Spec - rootfs string - hostSpecific bool + Spec *rspec.Spec + RootfsPath string + HostSpecific bool } -func NewValidator(spec rspec.Spec, rootfs string, hostSpecific bool) Validator { - return Validator{spec: spec, rootfs: rootfs, hostSpecific: hostSpecific} +func NewValidator(spec *rspec.Spec, rootfsPath string, hostSpecific bool) Validator { + return Validator{Spec: spec, RootfsPath: rootfsPath, HostSpecific: hostSpecific} +} + +func NewValidatorFromPath(bundlePath string, hostSpecific bool) (Validator, error) { + if bundlePath == "" { + return Validator{}, fmt.Errorf("Bundle path shouldn't be empty") + } + + if _, err := os.Stat(bundlePath); err != nil { + return Validator{}, err + } + + configPath := path.Join(bundlePath, "config.json") + content, err := ioutil.ReadFile(configPath) + if err != nil { + return Validator{}, err + } + if !utf8.Valid(content) { + return Validator{}, fmt.Errorf("%q is not encoded in UTF-8", configPath) + } + var spec rspec.Spec + if err = json.Unmarshal(content, &spec); err != nil { + return Validator{}, err + } + + rootfsPath := path.Join(bundlePath, spec.Root.Path) + if fi, err := os.Stat(rootfsPath); err != nil { + return Validator{}, fmt.Errorf("Cannot find the root path %q", rootfsPath) + } else if !fi.IsDir() { + return Validator{}, fmt.Errorf("The root path %q is not a directory.", rootfsPath) + } + + return NewValidator(&spec, rootfsPath, hostSpecific), nil } func (v *Validator) CheckAll() (msgs []string) { @@ -77,7 +112,7 @@ func (v *Validator) CheckAll() (msgs []string) { func (v *Validator) CheckSemVer() (msgs []string) { logrus.Debugf("check semver") - version := v.spec.Version + 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())) @@ -102,7 +137,7 @@ func (v *Validator) CheckPlatform() (msgs []string) { "plan9": {"386", "amd64"}, "solaris": {"amd64"}, "windows": {"386", "amd64"}} - platform := v.spec.Platform + platform := v.Spec.Platform for os, archs := range validCombins { if os == platform.OS { for _, arch := range archs { @@ -118,6 +153,16 @@ func (v *Validator) CheckPlatform() (msgs []string) { return } +func (v *Validator) CheckHooks() (msgs []string) { + logrus.Debugf("check hooks") + + 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)...) + + return +} + func checkEventHooks(hookType string, hooks []rspec.Hook, hostSpecific bool) (msgs []string) { for _, hook := range hooks { if !filepath.IsAbs(hook.Path) { @@ -143,20 +188,11 @@ func checkEventHooks(hookType string, hooks []rspec.Hook, hostSpecific bool) (ms return } -func (v *Validator) CheckHooks() (msgs []string) { - logrus.Debugf("check hooks") - - 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)...) - - return -} func (v *Validator) CheckProcess() (msgs []string) { logrus.Debugf("check process") - process := v.spec.Process + process := v.Spec.Process if !path.IsAbs(process.Cwd) { msgs = append(msgs, fmt.Sprintf("cwd %q is not an absolute path", process.Cwd)) } @@ -181,7 +217,7 @@ func (v *Validator) CheckProcess() (msgs []string) { } if len(process.ApparmorProfile) > 0 { - profilePath := filepath.Join(v.rootfs, "/etc/apparmor.d", process.ApparmorProfile) + profilePath := filepath.Join(v.RootfsPath, "/etc/apparmor.d", process.ApparmorProfile) _, err := os.Stat(profilePath) if err != nil { msgs = append(msgs, err.Error()) @@ -195,7 +231,7 @@ func supportedMountTypes(OS string, hostSpecific bool) (map[string]bool, error) supportedTypes := make(map[string]bool) if OS != "linux" && OS != "windows" { - logrus.Warnf("%v is not supported to (v *Validator) Check mount type", OS) + logrus.Warnf("%v is not supported to check mount type", OS) return nil, nil } else if OS == "windows" { supportedTypes["ntfs"] = true @@ -235,14 +271,14 @@ func supportedMountTypes(OS string, hostSpecific bool) (map[string]bool, error) func (v *Validator) CheckMounts() (msgs []string) { logrus.Debugf("check mounts") - supportedTypes, err := supportedMountTypes(v.spec.Platform.OS, v.hostSpecific) + supportedTypes, err := supportedMountTypes(v.Spec.Platform.OS, v.HostSpecific) if err != nil { msgs = append(msgs, err.Error()) return } if supportedTypes != nil { - for _, mount := range v.spec.Mounts { + for _, mount := range v.Spec.Mounts { if !supportedTypes[mount.Type] { msgs = append(msgs, fmt.Sprintf("Unsupported mount type %q", mount.Type)) } @@ -262,33 +298,33 @@ func (v *Validator) CheckLinux() (msgs []string) { netExists := false userExists := false - for index := 0; index < len(v.spec.Linux.Namespaces); index++ { - if !namespaceValid(v.spec.Linux.Namespaces[index]) { - msgs = append(msgs, fmt.Sprintf("namespace %v is invalid.", v.spec.Linux.Namespaces[index])) - } else if len(v.spec.Linux.Namespaces[index].Path) == 0 { - if v.spec.Linux.Namespaces[index].Type == rspec.UTSNamespace { + for index := 0; index < len(v.Spec.Linux.Namespaces); index++ { + if !namespaceValid(v.Spec.Linux.Namespaces[index]) { + msgs = append(msgs, fmt.Sprintf("namespace %v is invalid.", v.Spec.Linux.Namespaces[index])) + } else if len(v.Spec.Linux.Namespaces[index].Path) == 0 { + if v.Spec.Linux.Namespaces[index].Type == rspec.UTSNamespace { utsExists = true - } else if v.spec.Linux.Namespaces[index].Type == rspec.IPCNamespace { + } else if v.Spec.Linux.Namespaces[index].Type == rspec.IPCNamespace { ipcExists = true - } else if v.spec.Linux.Namespaces[index].Type == rspec.NetworkNamespace { + } else if v.Spec.Linux.Namespaces[index].Type == rspec.NetworkNamespace { netExists = true - } else if v.spec.Linux.Namespaces[index].Type == rspec.MountNamespace { + } else if v.Spec.Linux.Namespaces[index].Type == rspec.MountNamespace { mountExists = true - } else if v.spec.Linux.Namespaces[index].Type == rspec.UserNamespace { + } else if v.Spec.Linux.Namespaces[index].Type == rspec.UserNamespace { userExists = true } } } - if (len(v.spec.Linux.UIDMappings) > 0 || len(v.spec.Linux.GIDMappings) > 0) && !userExists { + if (len(v.Spec.Linux.UIDMappings) > 0 || len(v.Spec.Linux.GIDMappings) > 0) && !userExists { msgs = append(msgs, "UID/GID mappings requires a new User namespace to be specified as well") - } else if len(v.spec.Linux.UIDMappings) > 5 { + } else if len(v.Spec.Linux.UIDMappings) > 5 { msgs = append(msgs, "Only 5 UID mappings are allowed (linux kernel restriction).") - } else if len(v.spec.Linux.GIDMappings) > 5 { + } else if len(v.Spec.Linux.GIDMappings) > 5 { msgs = append(msgs, "Only 5 GID mappings are allowed (linux kernel restriction).") } - for k := range v.spec.Linux.Sysctl { + for k := range v.Spec.Linux.Sysctl { if strings.HasPrefix(k, "net.") && !netExists { msgs = append(msgs, fmt.Sprintf("Sysctl %v requires a new Network namespace to be specified as well", k)) } @@ -299,27 +335,27 @@ func (v *Validator) CheckLinux() (msgs []string) { } } - if v.spec.Platform.OS == "linux" && !utsExists && v.spec.Hostname != "" { + if v.Spec.Platform.OS == "linux" && !utsExists && v.Spec.Hostname != "" { msgs = append(msgs, fmt.Sprintf("On Linux, hostname requires a new UTS namespace to be specified as well")) } - for index := 0; index < len(v.spec.Linux.Devices); index++ { - if !deviceValid(v.spec.Linux.Devices[index]) { - msgs = append(msgs, fmt.Sprintf("device %v is invalid.", v.spec.Linux.Devices[index])) + for index := 0; index < len(v.Spec.Linux.Devices); index++ { + if !deviceValid(v.Spec.Linux.Devices[index]) { + msgs = append(msgs, fmt.Sprintf("device %v is invalid.", v.Spec.Linux.Devices[index])) } } - if v.spec.Linux.Resources != nil { - ms := v.checkLinuxResources() + if v.Spec.Linux.Resources != nil { + ms := v.CheckLinuxResources() msgs = append(msgs, ms...) } - if v.spec.Linux.Seccomp != nil { - ms := v.checkSeccomp() + if v.Spec.Linux.Seccomp != nil { + ms := v.CheckSeccomp() msgs = append(msgs, ms...) } - switch v.spec.Linux.RootfsPropagation { + switch v.Spec.Linux.RootfsPropagation { case "": case "private": case "rprivate": @@ -334,10 +370,10 @@ func (v *Validator) CheckLinux() (msgs []string) { return } -func (v *Validator) checkLinuxResources() (msgs []string) { +func (v *Validator) CheckLinuxResources() (msgs []string) { logrus.Debugf("check linux resources") - r := v.spec.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")) @@ -350,10 +386,10 @@ func (v *Validator) checkLinuxResources() (msgs []string) { return } -func (v *Validator) checkSeccomp() (msgs []string) { +func (v *Validator) CheckSeccomp() (msgs []string) { logrus.Debugf("check linux seccomp") - s := v.spec.Linux.Seccomp + s := v.Spec.Linux.Seccomp if !seccompActionValid(s.DefaultAction) { msgs = append(msgs, fmt.Sprintf("seccomp defaultAction %q is invalid.", s.DefaultAction)) } @@ -570,5 +606,5 @@ func checkMandatory(obj interface{}) (msgs []string) { func (v *Validator) CheckMandatoryFields() []string { logrus.Debugf("check mandatory fields") - return checkMandatory(v.spec) + return checkMandatory(v.Spec) } diff --git a/validate/validate_test.go b/validate/validate_test.go new file mode 100644 index 000000000..522628213 --- /dev/null +++ b/validate/validate_test.go @@ -0,0 +1,33 @@ +package validate + +import ( + "strings" + "testing" + + rspec "github.com/opencontainers/runtime-spec/specs-go" +) + +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 TestCheckSemVer(t *testing.T) { + cases := []struct { + val string + expected bool + }{ + {rspec.Version, true}, + //FIXME: validate currently only handles rpsec.Version + {"0.0.1", false}, + {"invalid", false}, + } + + for _, c := range cases { + v := NewValidator(&rspec.Spec{Version: c.val}, "", false) + checkErrors(t, "checkSemVer "+c.val, v.CheckSemVer(), c.expected) + } +}