From e3df0d57d3ead77a9abec382672f74960cdc7c73 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 16 Aug 2016 21:09:58 -0700 Subject: [PATCH 1/3] validate: With --host-specific, compare config platform vs. runtime This is a bit aggressive. For example, maybe a Linux kernel was compiled with CONFIG_64BIT, CONFIG_X86, and CONFIG_X86_X32. Your ocitools build may be 386 and validating an amd64 image (or vice versa) and both would run fine. I'm not sure x32 is supported by Go, but you could have and x32 container if somebody submits it to the OCI specs as an extention arch. So I'd rather have an arch mismatch be a warning than a fatal error. But in most cases, you'll want the arch to match (e.g. no arm configs if ocitools is amd64), so I don't want to ignore the missmatch completely. In the absence of warning-support, an overly strict check seems safer than an overly lax check, because the caller can always read the error messages and decide if they care ;). Signed-off-by: W. Trevor King --- validate/validate.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/validate/validate.go b/validate/validate.go index bce44d974..274feaa56 100644 --- a/validate/validate.go +++ b/validate/validate.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "reflect" + "runtime" "strings" "unicode" "unicode/utf8" @@ -164,6 +165,16 @@ func (v *Validator) CheckPlatform() (msgs []string) { } msgs = append(msgs, fmt.Sprintf("Operation system %q of the bundle is not supported yet.", platform.OS)) + if hostCheck { + if platform.OS != runtime.GOOS { + msgs = append(msgs, fmt.Sprintf("platform.os is %s, not %s", platform.OS, runtime.GOOS)) + } + if platform.Arch != runtime.GOARCH { + // warning, not an error, since kernels can support multiple architectures + msgs = append(msgs, fmt.Sprintf("platform.arch is %s, not %s", platform.Arch, runtime.GOARCH)) + } + } + return } From ea326f04c391035e653c93e10739971a6aeb5b1c Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 16 Aug 2016 21:41:10 -0700 Subject: [PATCH 2/3] generate: Respect runtime.GOOS when generating default template Don't fill in a bunch of Linux stuff if runtime.GOOS isn't Linux ;). We don't have sensible defaults for other OSes yet, so error out in those cases. Signed-off-by: W. Trevor King --- cmd/oci-runtime-tool/generate.go | 8 +-- generate/generate.go | 86 +++++++++++++++++++------------- 2 files changed, 55 insertions(+), 39 deletions(-) diff --git a/cmd/oci-runtime-tool/generate.go b/cmd/oci-runtime-tool/generate.go index ea934fcb8..c9a682660 100644 --- a/cmd/oci-runtime-tool/generate.go +++ b/cmd/oci-runtime-tool/generate.go @@ -92,21 +92,23 @@ var generateCommand = cli.Command{ Before: before, Action: func(context *cli.Context) error { // Start from the default template. - specgen := generate.New() + specgen, err := generate.New(nil) + if err != nil { + return err + } var template string if context.IsSet("template") { template = context.String("template") } if template != "" { - var err error specgen, err = generate.NewFromFile(template) if err != nil { return err } } - err := setupSpec(&specgen, context) + err = setupSpec(&specgen, context) if err != nil { return err } diff --git a/generate/generate.go b/generate/generate.go index 20bc601bb..f191fdb27 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -30,8 +30,14 @@ type ExportOptions struct { Seccomp bool // seccomp toggles if only seccomp should be exported } -// New creates a spec Generator with the default spec. -func New() Generator { +// New creates a spec Generator with the default spec for the target +// OS (which defaults to runtime.GOOS). +func New(os *string) (generator Generator, err error) { + var goos string + goos = runtime.GOOS + if os == nil { + os = &goos + } spec := rspec.Spec{ Version: rspec.Version, Platform: rspec.Platform{ @@ -44,41 +50,45 @@ func New() Generator { }, Process: rspec.Process{ Terminal: false, - User: rspec.User{}, Args: []string{ "sh", }, - Env: []string{ - "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", - "TERM=xterm", - }, - Cwd: "/", - Capabilities: []string{ - "CAP_CHOWN", - "CAP_DAC_OVERRIDE", - "CAP_FSETID", - "CAP_FOWNER", - "CAP_MKNOD", - "CAP_NET_RAW", - "CAP_SETGID", - "CAP_SETUID", - "CAP_SETFCAP", - "CAP_SETPCAP", - "CAP_NET_BIND_SERVICE", - "CAP_SYS_CHROOT", - "CAP_KILL", - "CAP_AUDIT_WRITE", - }, - Rlimits: []rspec.Rlimit{ - { - Type: "RLIMIT_NOFILE", - Hard: uint64(1024), - Soft: uint64(1024), - }, - }, }, Hostname: "mrsdalloway", - Mounts: []rspec.Mount{ + } + + if *os == "linux" { + spec.Process.User = rspec.User{} + spec.Process.Env = []string{ + "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "TERM=xterm", + } + spec.Process.Cwd = "/" + spec.Process.Capabilities = []string{ + "CAP_CHOWN", + "CAP_DAC_OVERRIDE", + "CAP_FSETID", + "CAP_FOWNER", + "CAP_MKNOD", + "CAP_NET_RAW", + "CAP_SETGID", + "CAP_SETUID", + "CAP_SETFCAP", + "CAP_SETPCAP", + "CAP_NET_BIND_SERVICE", + "CAP_SYS_CHROOT", + "CAP_KILL", + "CAP_AUDIT_WRITE", + } + spec.Process.Rlimits = []rspec.Rlimit{ + { + Type: "RLIMIT_NOFILE", + Hard: uint64(1024), + Soft: uint64(1024), + }, + } + + spec.Mounts = []rspec.Mount{ { Destination: "/proc", Type: "proc", @@ -115,8 +125,9 @@ func New() Generator { Source: "sysfs", Options: []string{"nosuid", "noexec", "nodev", "ro"}, }, - }, - Linux: &rspec.Linux{ + } + + spec.Linux = &rspec.Linux{ Resources: &rspec.Resources{ Devices: []rspec.DeviceCgroup{ { @@ -143,12 +154,15 @@ func New() Generator { }, }, Devices: []rspec.Device{}, - }, + } + } else { + return generator, fmt.Errorf("no defaults configured for %s", *os) } spec.Linux.Seccomp = seccomp.DefaultProfile(&spec) - return Generator{ + generator = Generator{ spec: &spec, } + return generator, nil } // NewFromSpec creates a spec Generator from a given spec. From fedbf906113b3482b78b29fab1e1881ba7b08e76 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 16 Aug 2016 22:07:09 -0700 Subject: [PATCH 3/3] generate: Respect --os when generating default templates Also add --host-specific checks to SetPlatformOS and SetPlatformArch. The SetPlatformArch check is currently just a warning (details in eda1ac7, validate: With --host-specific, compare config platform vs. runtime, 2016-08-16) but I've added an error to its API so callers will be ready when we do actually raise errors (e.g. for '--arch arm' on an amd64 box). Also add IsSet checks to --os and --arch so we don't clobber their template values. Before this commit on my amd64 Linux box: $ echo '{"platform": {"os": "windows", "arch": "386"}}' >template.json $ ocitools generate --template template.json | jq .platform { "os": "linux", "arch": "amd64" } After this commit: $ ocitools generate --template template.json | jq .platform { "os": "windows", "arch": "386" } Signed-off-by: W. Trevor King --- cmd/oci-runtime-tool/generate.go | 27 +++++++++++++++++++++------ generate/generate.go | 13 +++++++++++-- man/oci-runtime-tool-generate.1.md | 8 +++++++- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/cmd/oci-runtime-tool/generate.go b/cmd/oci-runtime-tool/generate.go index c9a682660..57261c52d 100644 --- a/cmd/oci-runtime-tool/generate.go +++ b/cmd/oci-runtime-tool/generate.go @@ -3,7 +3,6 @@ package main import ( "fmt" "os" - "runtime" "strconv" "strings" @@ -15,7 +14,7 @@ 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.StringFlag{Name: "arch", 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"}, @@ -52,7 +51,7 @@ var generateFlags = []cli.Flag{ cli.StringFlag{Name: "network", Usage: "network namespace"}, 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: "os", Usage: "operating system the container is created for"}, cli.StringFlag{Name: "output", Usage: "output file (defaults to stdout)"}, cli.StringFlag{Name: "pid", Usage: "pid namespace"}, cli.StringSliceFlag{Name: "poststart", Usage: "set command to run in poststart hooks"}, @@ -92,7 +91,12 @@ var generateCommand = cli.Command{ Before: before, Action: func(context *cli.Context) error { // Start from the default template. - specgen, err := generate.New(nil) + var osPointer *string + if context.IsSet("os") { + goos := context.String("os") + osPointer = &goos + } + specgen, err := generate.New(osPointer) if err != nil { return err } @@ -143,8 +147,19 @@ 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("os") { + err := g.SetPlatformOS(context.String("os")) + if err != nil { + return err + } + } + + if context.IsSet("arch") { + err := g.SetPlatformArch(context.String("arch")) + if err != nil { + return err + } + } if context.IsSet("label") { annotations := context.StringSlice("label") diff --git a/generate/generate.go b/generate/generate.go index f191fdb27..ba09c2755 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -11,6 +11,7 @@ import ( rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate/seccomp" + "github.com/Sirupsen/logrus" "github.com/syndtr/gocapability/capability" ) @@ -284,15 +285,23 @@ func (g *Generator) RemoveAnnotation(key string) { } // SetPlatformOS sets g.spec.Process.OS. -func (g *Generator) SetPlatformOS(os string) { +func (g *Generator) SetPlatformOS(os string) error { + if g.HostSpecific && os != runtime.GOOS { + return fmt.Errorf("cannot set platform.os to %s on a %s host", os, runtime.GOOS) + } g.initSpec() g.spec.Platform.OS = os + return nil } // SetPlatformArch sets g.spec.Platform.Arch. -func (g *Generator) SetPlatformArch(arch string) { +func (g *Generator) SetPlatformArch(arch string) error { + if g.HostSpecific && arch != runtime.GOARCH { + logrus.Warnf("setting platform.arch to %s on a %s host", arch, runtime.GOARCH) + } g.initSpec() g.spec.Platform.Arch = arch + return nil } // SetProcessUID sets g.spec.Process.User.UID. diff --git a/man/oci-runtime-tool-generate.1.md b/man/oci-runtime-tool-generate.1.md index aba61aa4c..2102dc008 100644 --- a/man/oci-runtime-tool-generate.1.md +++ b/man/oci-runtime-tool-generate.1.md @@ -173,7 +173,13 @@ read the configuration from `config.json`. Specifies oom_score_adj for the container. **--os**=OS - Operating system used within the container + Operating system used within the container. When no template is + set, the default is to use the `GOOS` ocitools was built with and to + default other settings based on this setting. For example, if + `GOOS` is `linux`, `ocitools generate` will generate a default Linux + config, `ocitools generate --os=windows` will generate a default + Windows config, and `ocitools --host-specific generate --os=windows` + will exit with an error. **--output**=PATH Instead of writing the configuration JSON to stdout, write it to a