Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion completions/bash/ocitools
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ _ocitools_validate() {

case "$cur" in
-*)
COMPREPLY=( $( compgen -W "--hooks --path --help" -- "$cur" ) )
COMPREPLY=( $( compgen -W "--host-specific --path --help" -- "$cur" ) )
;;
esac

Expand Down
11 changes: 9 additions & 2 deletions man/ocitools-validate.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,15 @@ Validate an OCI bundle
**--path=PATH
Path to bundle

**--hooks**
Check specified hooks exist and are executable on the host.
**--host-specific**
Check host specific configs.
By default, validation only tests for compatibility with a hypothetical host.
With this flag, validation will also run more specific tests to see whether
the current host is capable of launching a container from the configuration.
For example, validating a compliant Windows configuration on a Linux machine
will pass without this flag ("there may be a Windows host capable of
launching this container"), but will fail with it ("this host is not capable
of launching this container").
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: there's trailing whitespace here (in case we care in this repository; I don't if we do).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Thanks.


# SEE ALSO
**ocitools**(1)
Expand Down
81 changes: 70 additions & 11 deletions validate.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"bufio"
"encoding/json"
"fmt"
"io/ioutil"
Expand All @@ -20,7 +21,7 @@ import (

var bundleValidateFlags = []cli.Flag{
cli.StringFlag{Name: "path", Value: ".", Usage: "path to a bundle"},
cli.BoolFlag{Name: "hooks", Usage: "Check specified hooks exist and are executable on the host."},
cli.BoolFlag{Name: "host-specific", Usage: "Check host specific configs."},
}

var (
Expand Down Expand Up @@ -79,20 +80,21 @@ var bundleValidateCommand = cli.Command{
return fmt.Errorf("The root path %q is not a directory.", rootfsPath)
}

hooksCheck := context.Bool("hooks")
bundleValidate(spec, rootfsPath, hooksCheck)
hostCheck := context.Bool("host-specific")
bundleValidate(spec, rootfsPath, hostCheck)
logrus.Infof("Bundle validation succeeded.")
return nil
},
}

func bundleValidate(spec rspec.Spec, rootfs string, hooksCheck bool) {
func bundleValidate(spec rspec.Spec, rootfs string, hostCheck bool) {
checkMandatoryField(spec)
checkSemVer(spec.Version)
checkPlatform(spec.Platform)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably also get a hostCheck that compares spec.Platform against the current host (runtime.GOOS?).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Paltform check has nothing to do with hostCheck

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Mon, Jun 27, 2016 at 12:00:12AM -0700, Ma Shimiao wrote:

checkPlatform(spec.Platform)

I think Paltform check has nothing to do with hostCheck

The spec has 1:

The runtime MUST generate an error if it does not support the
configured os.

I guess a VM-based runtime could support launching a Windows bundle
from a Linux host (etc.), but your /proc/filesystems approach here is
assuming runtimes are using a container approach, and not a VM
approach. With a container approach, a runtime based on the host's
Linux kernel is not going to be able to support a Windows container.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what spec means. I think that's runtime's business. checkPlatform seems can't judge you mentioned above.
Or could you give me more detailed information about how to modify checkPlatform?
Thanks a lot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Mon, Jun 27, 2016 at 07:27:17AM -0700, Ma Shimiao wrote:

checkPlatform(spec.Platform)

I'm not sure what spec means. I think that's runtime's
business. checkPlatform seems can't judge you mentioned above. Or
could you give me more detailed information about how to modify
checkPlatform?

Let's just punt this to a follow-up PR.

checkProcess(spec.Process, rootfs)
checkMounts(spec, hostCheck)
checkLinux(spec)
checkHooks(spec.Hooks, hooksCheck)
checkHooks(spec.Hooks, hostCheck)
}

func checkSemVer(version string) {
Expand Down Expand Up @@ -129,19 +131,19 @@ func checkPlatform(platform rspec.Platform) {
logrus.Fatalf("Operation system %q of the bundle is not supported yet.", platform.OS)
}

func checkHooks(hooks rspec.Hooks, hooksCheck bool) {
checkEventHooks("pre-start", hooks.Prestart, hooksCheck)
checkEventHooks("post-start", hooks.Poststart, hooksCheck)
checkEventHooks("post-stop", hooks.Poststop, hooksCheck)
func checkHooks(hooks rspec.Hooks, hostCheck bool) {
checkEventHooks("pre-start", hooks.Prestart, hostCheck)
checkEventHooks("post-start", hooks.Poststart, hostCheck)
checkEventHooks("post-stop", hooks.Poststop, hostCheck)
}

func checkEventHooks(hookType string, hooks []rspec.Hook, hooksCheck bool) {
func checkEventHooks(hookType string, hooks []rspec.Hook, hostCheck bool) {
for _, hook := range hooks {
if !filepath.IsAbs(hook.Path) {
logrus.Fatalf("The %s hook %v: is not absolute path", hookType, hook.Path)
}

if hooksCheck {
if hostCheck {
fi, err := os.Stat(hook.Path)
if err != nil {
logrus.Fatalf("Cannot find %s hook: %v", hookType, hook.Path)
Expand Down Expand Up @@ -192,6 +194,63 @@ func checkProcess(process rspec.Process, rootfs string) {
}
}

func supportedMountTypes(OS string, hostCheck bool) (map[string]bool, error) {
supportedTypes := make(map[string]bool)

if OS != "linux" && OS != "windows" {
logrus.Warnf("%v is not supported to check mount type", OS)
return nil, nil
} else if OS == "windows" {
supportedTypes["ntfs"] = true
return supportedTypes, nil
}

if hostCheck {
f, err := os.Open("/proc/filesystems")
if err != nil {
return nil, err
}
defer f.Close()

s := bufio.NewScanner(f)
for s.Scan() {
if err := s.Err(); err != nil {
return supportedTypes, err
}

text := s.Text()
parts := strings.Split(text, "\t")
if len(parts) > 1 {
supportedTypes[parts[1]] = true
} else {
supportedTypes[parts[0]] = true
}
}

supportedTypes["bind"] = true

return supportedTypes, nil
} else {
logrus.Warn("Checking linux mount types without --host-specific is not supported yet")
return nil, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you don't need an else. If you get this far, you didn't match the if condition.

You probably want a warn here, with something like “Linux mount checks without --host-specific are not implemented yet”.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed. Thanks.

}
}

func checkMounts(spec rspec.Spec, hostCheck bool) {
supportedTypes, err := supportedMountTypes(spec.Platform.OS, hostCheck)
if err != nil {
logrus.Fatal(err)
}

if supportedTypes != nil {
for _, mount := range spec.Mounts {
if !supportedTypes[mount.Type] {
logrus.Fatalf("Unsupported mount type %q", mount.Type)
}
}
}
}

//Linux only
func checkLinux(spec rspec.Spec) {
utsExists := false
Expand Down