From de086059f96c508123019f5139e94e4b5b3010f6 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Sat, 30 Sep 2017 16:17:57 +1000 Subject: [PATCH 1/3] validate: allow unset "type" fields in resource devices whitelist According to the spec, an unset .Type field in the .Linux.Resources.Devices list is permitted (and it maps to "all"). This was broken recently, making our own default profile (as well as umoci's and runc's) not pass validation anymore. Signed-off-by: Aleksa Sarai --- validate/validate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validate/validate.go b/validate/validate.go index a4777a806..066da6dda 100644 --- a/validate/validate.go +++ b/validate/validate.go @@ -749,7 +749,7 @@ func (v *Validator) CheckLinuxResources() (errs error) { } for index := 0; index < len(r.Devices); index++ { switch r.Devices[index].Type { - case "a", "b", "c": + case "a", "b", "c", "": default: errs = multierror.Append(errs, fmt.Errorf("type of devices %s is invalid", r.Devices[index].Type)) } From a6f475f50ca4a8406421a96948f820d22fe75619 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 2 Oct 2017 23:37:43 +1100 Subject: [PATCH 2/3] config: correct rootfs default It doesn't make sense for us to not include a rootfs in our default configuration -- as it means that we're providing an invalid configuration. Signed-off-by: Aleksa Sarai --- generate/generate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generate/generate.go b/generate/generate.go index 4f4330cc9..5a1f5543e 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -41,7 +41,7 @@ func New() Generator { spec := rspec.Spec{ Version: rspec.Version, Root: &rspec.Root{ - Path: "", + Path: "rootfs", Readonly: false, }, Process: &rspec.Process{ From 6df06d91a40a707464b5cd8cfba17e25f0716afa Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 2 Oct 2017 23:39:25 +1100 Subject: [PATCH 3/3] validation: add a generate smoke-test To avoid cases where our own validation code would consider our defaults unsafe (which has happened in the past several times), add a smoke-test to ensure that this won't happen. Our defaults should not be intentionally invalid, as that confuses downstreams like umoci which use runtime-tools for the default as well as for validation of the generated configuration. Signed-off-by: Aleksa Sarai --- validation/generate_test.go | 56 +++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 validation/generate_test.go diff --git a/validation/generate_test.go b/validation/generate_test.go new file mode 100644 index 000000000..fe6893821 --- /dev/null +++ b/validation/generate_test.go @@ -0,0 +1,56 @@ +package validation + +import ( + "io/ioutil" + "os" + "path/filepath" + "runtime" + "testing" + + rfc2119 "github.com/opencontainers/runtime-tools/error" + "github.com/opencontainers/runtime-tools/generate" + "github.com/opencontainers/runtime-tools/specerror" + "github.com/opencontainers/runtime-tools/validate" +) + +// Smoke test to ensure that _at the very least_ our default configuration +// passes the validation tests. If this test fails, something is _very_ wrong +// and needs to be fixed immediately (as it will break downstreams that depend +// on us for a "sane default" and do compliance testing -- such as umoci). +func TestGenerateValid(t *testing.T) { + bundle, err := ioutil.TempDir("", "TestGenerateValid_bundle") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(bundle) + + // Create our toy bundle. + rootfsPath := filepath.Join(bundle, "rootfs") + if err := os.Mkdir(rootfsPath, 0755); err != nil { + t.Fatal(err) + } + configPath := filepath.Join(bundle, "config.json") + g := generate.New() + if err := (&g).SaveToFile(configPath, generate.ExportOptions{Seccomp: false}); err != nil { + t.Fatal(err) + } + + // Validate the bundle. + v, err := validate.NewValidatorFromPath(bundle, true, runtime.GOOS) + if err != nil { + t.Errorf("unexpected NewValidatorFromPath error: %+v", err) + } + if err := v.CheckAll(); err != nil { + levelErrors, err := specerror.SplitLevel(err, rfc2119.Must) + if err != nil { + t.Errorf("unexpected non-multierror: %+v", err) + return + } + for _, e := range levelErrors.Warnings { + t.Logf("unexpected warning: %v", e) + } + if err := levelErrors.Error; err != nil { + t.Errorf("unexpected MUST error(s): %+v", err) + } + } +}