From 597c7d44eb9dce61574d2194953feb4a35714ae3 Mon Sep 17 00:00:00 2001 From: Qiang Huang Date: Wed, 5 Jul 2017 14:56:52 +0800 Subject: [PATCH 1/2] Remove platform As it's deleted from runtime-spec. Signed-off-by: Qiang Huang --- Makefile | 3 +- cmd/oci-runtime-tool/generate.go | 6 --- cmd/oci-runtime-tool/main.go | 5 ++ cmd/oci-runtime-tool/validate.go | 3 +- cmd/runtimetest/main.go | 11 ++++- generate/generate.go | 17 ------- man/oci-runtime-tool-generate.1.md | 7 --- man/oci-runtime-tool.1.md | 3 ++ validate/validate.go | 79 ++++++++++-------------------- validate/validate_test.go | 4 +- validation/validation_test.go | 6 ++- 11 files changed, 53 insertions(+), 91 deletions(-) diff --git a/Makefile b/Makefile index 49018dc7f..31b4ca109 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,7 @@ BINDIR ?= $(DESTDIR)/usr/bin BUILDTAGS= RUNTIME ?= runc +PLATFORM ?= linux all: tool runtimetest @@ -39,7 +40,7 @@ clean: rm -f oci-runtime-tool runtimetest *.1 localvalidation: runtimetest - RUNTIME=$(RUNTIME) go test -tags "$(BUILDTAGS)" ${TESTFLAGS} -v github.com/opencontainers/runtime-tools/validation + RUNTIME=$(RUNTIME) PLATFORM=$(PLATFORM) go test -tags "$(BUILDTAGS)" ${TESTFLAGS} -v github.com/opencontainers/runtime-tools/validation .PHONY: test .gofmt .govet .golint diff --git a/cmd/oci-runtime-tool/generate.go b/cmd/oci-runtime-tool/generate.go index db4174686..482256268 100644 --- a/cmd/oci-runtime-tool/generate.go +++ b/cmd/oci-runtime-tool/generate.go @@ -5,7 +5,6 @@ import ( "bytes" "fmt" "os" - "runtime" "strconv" "strings" "unicode" @@ -19,7 +18,6 @@ import ( var generateFlags = []cli.Flag{ cli.StringFlag{Name: "apparmor", Usage: "specifies the the apparmor profile for the container"}, - cli.StringFlag{Name: "arch", Value: runtime.GOARCH, Usage: "architecture the container is created for"}, cli.StringSliceFlag{Name: "args", Usage: "command to run in the container"}, cli.StringSliceFlag{Name: "bind", Usage: "bind mount directories src:dest[:options...]"}, cli.StringSliceFlag{Name: "cap-add", Usage: "add Linux capabilities"}, @@ -62,7 +60,6 @@ var generateFlags = []cli.Flag{ cli.StringFlag{Name: "mount-label", Usage: "selinux mount context label"}, cli.BoolFlag{Name: "no-new-privileges", Usage: "set no new privileges bit for the container process"}, cli.IntFlag{Name: "oom-score-adj", Usage: "oom_score_adj for the container"}, - cli.StringFlag{Name: "os", Value: runtime.GOOS, Usage: "operating system the container is created for"}, cli.StringFlag{Name: "output", Usage: "output file (defaults to stdout)"}, cli.StringSliceFlag{Name: "poststart", Usage: "set command to run in poststart hooks"}, cli.StringSliceFlag{Name: "poststop", Usage: "set command to run in poststop hooks"}, @@ -151,9 +148,6 @@ func setupSpec(g *generate.Generator, context *cli.Context) error { g.SetHostname(context.String("hostname")) } - g.SetPlatformOS(context.String("os")) - g.SetPlatformArch(context.String("arch")) - if context.IsSet("label") { annotations := context.StringSlice("label") for _, s := range annotations { diff --git a/cmd/oci-runtime-tool/main.go b/cmd/oci-runtime-tool/main.go index 600d540da..a164ad034 100644 --- a/cmd/oci-runtime-tool/main.go +++ b/cmd/oci-runtime-tool/main.go @@ -22,6 +22,11 @@ func main() { Value: "error", Usage: "Log level (panic, fatal, error, warn, info, or debug)", }, + cli.StringFlag{ + Name: "platform", + Value: "linux", + Usage: "Platform the tool targets", + }, } app.Commands = []cli.Command{ diff --git a/cmd/oci-runtime-tool/validate.go b/cmd/oci-runtime-tool/validate.go index a07741e93..bdf4d5eb2 100644 --- a/cmd/oci-runtime-tool/validate.go +++ b/cmd/oci-runtime-tool/validate.go @@ -20,7 +20,8 @@ var bundleValidateCommand = cli.Command{ Action: func(context *cli.Context) error { inputPath := context.String("path") hostSpecific := context.GlobalBool("host-specific") - v, err := validate.NewValidatorFromPath(inputPath, hostSpecific) + platform := context.GlobalString("platform") + v, err := validate.NewValidatorFromPath(inputPath, hostSpecific, platform) if err != nil { return err } diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go index f391d8d2f..340b50c48 100644 --- a/cmd/runtimetest/main.go +++ b/cmd/runtimetest/main.go @@ -588,6 +588,8 @@ func validate(context *cli.Context) error { return err } + platform := context.String("platform") + defaultValidations := []validation{ { test: validateRootFS, @@ -666,7 +668,7 @@ func validate(context *cli.Context) error { } } - if spec.Platform.OS == "linux" { + if platform == "linux" { for _, v := range linuxValidations { err := v.test(spec) t.Ok(err == nil, v.description) @@ -695,7 +697,12 @@ func main() { cli.StringFlag{ Name: "path", Value: ".", - Usage: "path to the configuration", + Usage: "Path to the configuration", + }, + cli.StringFlag{ + Name: "platform", + Value: "linux", + Usage: "Platform the runtime runs on", }, } diff --git a/generate/generate.go b/generate/generate.go index 565d06670..4a814e456 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "os" - "runtime" "strings" rspec "github.com/opencontainers/runtime-spec/specs-go" @@ -35,10 +34,6 @@ type ExportOptions struct { func New() Generator { spec := rspec.Spec{ Version: rspec.Version, - Platform: rspec.Platform{ - OS: runtime.GOOS, - Arch: runtime.GOARCH, - }, Root: rspec.Root{ Path: "", Readonly: false, @@ -346,18 +341,6 @@ func (g *Generator) RemoveAnnotation(key string) { delete(g.spec.Annotations, key) } -// SetPlatformOS sets g.spec.Process.OS. -func (g *Generator) SetPlatformOS(os string) { - g.initSpec() - g.spec.Platform.OS = os -} - -// SetPlatformArch sets g.spec.Platform.Arch. -func (g *Generator) SetPlatformArch(arch string) { - g.initSpec() - g.spec.Platform.Arch = arch -} - // SetProcessUID sets g.spec.Process.User.UID. func (g *Generator) SetProcessUID(uid uint32) { g.initSpec() diff --git a/man/oci-runtime-tool-generate.1.md b/man/oci-runtime-tool-generate.1.md index 6ec087faa..8df2d37d5 100644 --- a/man/oci-runtime-tool-generate.1.md +++ b/man/oci-runtime-tool-generate.1.md @@ -18,10 +18,6 @@ read the configuration from `config.json`. **--apparmor**=PROFILE Specifies the apparmor profile for the container -**--arch**=ARCH - Architecture used within the container. - "amd64" - **--args**=OPTION Arguments to run within the container. Can be specified multiple times. If you were going to run a command with multiple options, you would need @@ -200,9 +196,6 @@ read the configuration from `config.json`. **--oom-score-adj**=adj Specifies oom_score_adj for the container. -**--os**=OS - Operating system used within the container. - **--output**=PATH Instead of writing the configuration JSON to stdout, write it to a file at *PATH* (overwriting the existing content if a file already diff --git a/man/oci-runtime-tool.1.md b/man/oci-runtime-tool.1.md index 27b2e1708..47795300c 100644 --- a/man/oci-runtime-tool.1.md +++ b/man/oci-runtime-tool.1.md @@ -32,6 +32,9 @@ oci-runtime-tool is a collection of tools for working with the [OCI runtime spec **--log-level**=LEVEL Log level (panic, fatal, error, warn, info, or debug) (default: "error"). +**--platform** + Platform the tool targets. + **-v**, **--version** Print version information. diff --git a/validate/validate.go b/validate/validate.go index e35a76737..8710086f1 100644 --- a/validate/validate.go +++ b/validate/validate.go @@ -47,15 +47,21 @@ type Validator struct { spec *rspec.Spec bundlePath string HostSpecific bool + platform string } // NewValidator creates a Validator -func NewValidator(spec *rspec.Spec, bundlePath string, hostSpecific bool) Validator { - return Validator{spec: spec, bundlePath: bundlePath, HostSpecific: hostSpecific} +func NewValidator(spec *rspec.Spec, bundlePath string, hostSpecific bool, platform string) Validator { + return Validator{ + spec: spec, + bundlePath: bundlePath, + HostSpecific: hostSpecific, + platform: platform, + } } // NewValidatorFromPath creates a Validator with specified bundle path -func NewValidatorFromPath(bundlePath string, hostSpecific bool) (Validator, error) { +func NewValidatorFromPath(bundlePath string, hostSpecific bool, platform string) (Validator, error) { if bundlePath == "" { return Validator{}, fmt.Errorf("Bundle path shouldn't be empty") } @@ -77,7 +83,7 @@ func NewValidatorFromPath(bundlePath string, hostSpecific bool) (Validator, erro return Validator{}, err } - return NewValidator(&spec, bundlePath, hostSpecific), nil + return NewValidator(&spec, bundlePath, hostSpecific, platform), nil } // CheckAll checks all parts of runtime bundle @@ -86,7 +92,6 @@ func (v *Validator) CheckAll() (msgs []string) { msgs = append(msgs, v.CheckMandatoryFields()...) msgs = append(msgs, v.CheckSemVer()...) msgs = append(msgs, v.CheckMounts()...) - msgs = append(msgs, v.CheckPlatform()...) msgs = append(msgs, v.CheckProcess()...) msgs = append(msgs, v.CheckOS()...) msgs = append(msgs, v.CheckHooks()...) @@ -131,9 +136,9 @@ func (v *Validator) CheckRootfsPath() (msgs []string) { msgs = append(msgs, fmt.Sprintf("root.path is %q, but it MUST be a child of %q", v.spec.Root.Path, absBundlePath)) } - if v.spec.Platform.OS == "windows" { + if v.platform == "windows" { if v.spec.Root.Readonly { - msgs = append(msgs, "root.readonly field MUST be omitted or false when platform.os is windows") + msgs = append(msgs, "root.readonly field MUST be omitted or false when target platform is windows") } } @@ -156,37 +161,6 @@ func (v *Validator) CheckSemVer() (msgs []string) { return } -// CheckPlatform checks v.spec.Platform -func (v *Validator) CheckPlatform() (msgs []string) { - logrus.Debugf("check platform") - - validCombins := map[string][]string{ - "android": {"arm"}, - "darwin": {"386", "amd64", "arm", "arm64"}, - "dragonfly": {"amd64"}, - "freebsd": {"386", "amd64", "arm"}, - "linux": {"386", "amd64", "arm", "arm64", "mips", "mips64", "mips64le", "mipsle", "ppc64", "ppc64le", "s390x"}, - "netbsd": {"386", "amd64", "arm"}, - "openbsd": {"386", "amd64", "arm"}, - "plan9": {"386", "amd64"}, - "solaris": {"amd64"}, - "windows": {"386", "amd64"}} - platform := v.spec.Platform - for os, archs := range validCombins { - if os == platform.OS { - for _, arch := range archs { - if arch == platform.Arch { - return nil - } - } - msgs = append(msgs, fmt.Sprintf("Combination of %q and %q is invalid.", platform.OS, platform.Arch)) - } - } - msgs = append(msgs, fmt.Sprintf("Operation system %q of the bundle is not supported yet.", platform.OS)) - - return -} - // CheckHooks check v.spec.Hooks func (v *Validator) CheckHooks() (msgs []string) { logrus.Debugf("check hooks") @@ -271,8 +245,7 @@ func (v *Validator) CheckProcess() (msgs []string) { } msgs = append(msgs, v.CheckRlimits()...) - if v.spec.Platform.OS == "linux" { - + 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) @@ -288,7 +261,7 @@ func (v *Validator) CheckProcess() (msgs []string) { // CheckCapabilities checks v.spec.Process.Capabilities func (v *Validator) CheckCapabilities() (msgs []string) { process := v.spec.Process - if v.spec.Platform.OS == "linux" { + if v.platform == "linux" { var effective, permitted, inheritable, ambient bool caps := make(map[string][]string) @@ -336,7 +309,7 @@ func (v *Validator) CheckCapabilities() (msgs []string) { } } } else { - logrus.Warnf("process.capabilities validation not yet implemented for OS %q", v.spec.Platform.OS) + logrus.Warnf("process.capabilities validation not yet implemented for OS %q", v.platform) } return @@ -402,7 +375,7 @@ 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.platform, v.HostSpecific) if err != nil { msgs = append(msgs, err.Error()) return @@ -423,25 +396,25 @@ func (v *Validator) CheckMounts() (msgs []string) { return } -// CheckOS checks v.spec.Platform.OS +// CheckOS checks v.platform func (v *Validator) CheckOS() (msgs []string) { logrus.Debugf("check os") - if v.spec.Platform.OS != "linux" { + if v.platform != "linux" { if v.spec.Linux != nil { - msgs = append(msgs, fmt.Sprintf("'linux' MUST NOT be set when platform.os is %q", v.spec.Platform.OS)) + msgs = append(msgs, fmt.Sprintf("'linux' MUST NOT be set when target platform is %q", v.platform)) } } - if v.spec.Platform.OS != "solaris" { + if v.platform != "solaris" { if v.spec.Solaris != nil { - msgs = append(msgs, fmt.Sprintf("'solaris' MUST NOT be set when platform.os is %q", v.spec.Platform.OS)) + msgs = append(msgs, fmt.Sprintf("'solaris' MUST NOT be set when target platform is %q", v.platform)) } } - if v.spec.Platform.OS != "windows" { + if v.platform != "windows" { if v.spec.Windows != nil { - msgs = append(msgs, fmt.Sprintf("'windows' MUST NOT be set when platform.os is %q", v.spec.Platform.OS)) + msgs = append(msgs, fmt.Sprintf("'windows' MUST NOT be set when target platform is %q", v.platform)) } } @@ -502,7 +475,7 @@ func (v *Validator) CheckLinux() (msgs []string) { } } - if v.spec.Platform.OS == "linux" && !typeList[rspec.UTSNamespace].newExist && v.spec.Hostname != "" { + if v.platform == "linux" && !typeList[rspec.UTSNamespace].newExist && v.spec.Hostname != "" { msgs = append(msgs, fmt.Sprintf("On Linux, hostname requires a new UTS namespace to be specified as well")) } @@ -684,7 +657,7 @@ func (v *Validator) rlimitValid(rlimit rspec.LinuxRlimit) (msgs []string) { msgs = append(msgs, fmt.Sprintf("hard limit of rlimit %s should not be less than soft limit", rlimit.Type)) } - if v.spec.Platform.OS == "linux" { + if v.platform == "linux" { for _, val := range defaultRlimits { if val == rlimit.Type { return @@ -692,7 +665,7 @@ func (v *Validator) rlimitValid(rlimit rspec.LinuxRlimit) (msgs []string) { } msgs = append(msgs, fmt.Sprintf("rlimit type %q is invalid", rlimit.Type)) } else { - logrus.Warnf("process.rlimits validation not yet implemented for OS %q", v.spec.Platform.OS) + logrus.Warnf("process.rlimits validation not yet implemented for platform %q", v.platform) } return diff --git a/validate/validate_test.go b/validate/validate_test.go index 0dd379ac7..4d182f128 100644 --- a/validate/validate_test.go +++ b/validate/validate_test.go @@ -47,7 +47,7 @@ func TestCheckRootfsPath(t *testing.T) { {filepath.Join(tmpBundle, rootfsNonExists), false}, } for _, c := range cases { - v := NewValidator(&rspec.Spec{Root: rspec.Root{Path: c.val}}, tmpBundle, false) + v := NewValidator(&rspec.Spec{Root: rspec.Root{Path: c.val}}, tmpBundle, false, "linux") checkErrors(t, "CheckRootfsPath "+c.val, v.CheckRootfsPath(), c.expected) } } @@ -64,7 +64,7 @@ func TestCheckSemVer(t *testing.T) { } for _, c := range cases { - v := NewValidator(&rspec.Spec{Version: c.val}, "", false) + v := NewValidator(&rspec.Spec{Version: c.val}, "", false, "linux") checkErrors(t, "CheckSemVer "+c.val, v.CheckSemVer(), c.expected) } } diff --git a/validation/validation_test.go b/validation/validation_test.go index 141db9a1f..0e6232919 100644 --- a/validation/validation_test.go +++ b/validation/validation_test.go @@ -14,11 +14,13 @@ import ( ) var ( - runtime = "runc" + runtime = "runc" + platform = "linux" ) func init() { runtime = os.Getenv("RUNTIME") + platform = os.Getenv("PLATFORM") } func runtimeValidate(runtime string, g *generate.Generator) error { @@ -79,7 +81,7 @@ func runtimeValidate(runtime string, g *generate.Generator) error { func getDefaultGenerator() *generate.Generator { g := generate.New() g.SetRootPath(".") - g.SetProcessArgs([]string{"/runtimetest"}) + g.SetProcessArgs([]string{"/runtimetest", "--platform", platform}) return &g } From 1af3b094a0bc5c6d261d913038c8eee63048949f Mon Sep 17 00:00:00 2001 From: Qiang Huang Date: Wed, 5 Jul 2017 13:12:38 +0800 Subject: [PATCH 2/2] Add check for the input platform Signed-off-by: Qiang Huang --- validate/validate.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/validate/validate.go b/validate/validate.go index 8710086f1..94d4f9519 100644 --- a/validate/validate.go +++ b/validate/validate.go @@ -88,12 +88,12 @@ 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.CheckRootfsPath()...) msgs = append(msgs, v.CheckMandatoryFields()...) msgs = append(msgs, v.CheckSemVer()...) msgs = append(msgs, v.CheckMounts()...) msgs = append(msgs, v.CheckProcess()...) - msgs = append(msgs, v.CheckOS()...) msgs = append(msgs, v.CheckHooks()...) if v.spec.Linux != nil { msgs = append(msgs, v.CheckLinux()...) @@ -396,9 +396,14 @@ func (v *Validator) CheckMounts() (msgs []string) { return } -// CheckOS checks v.platform -func (v *Validator) CheckOS() (msgs []string) { - logrus.Debugf("check os") +// CheckPlatform checks v.platform +func (v *Validator) CheckPlatform() (msgs []string) { + 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)) + return + } if v.platform != "linux" { if v.spec.Linux != nil {