From 0c66fe94e74448c98f26fb94474f243ae48bf1ce Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 3 Dec 2017 15:16:58 -0800 Subject: [PATCH 1/5] vendor/github.com/mndrix/tap-go: Bump to 629fa407 To pick up t.YAML(...) for more visible error messages. See ad47e7d5 (Makefile: Change from prove to node-tap, 2017-11-30, #439) about how node-tap handles YAML blocks vs. diagnostics. Generated with: $ go get -u github.com/mndrix/tap-go $ godep update github.com/mndrix/tap-go $ emacs Godeps/Godeps.json # remove gopkg.in/yaml.v2 entry $ rm -rf vendor/gopkg.in/yaml.v2 $ git add vendor/github.com/mndrix We don't need gopkg.in/yaml.v2 because we're using tap-go's JSON output. Signed-off-by: W. Trevor King --- Godeps/Godeps.json | 2 +- vendor/github.com/mndrix/tap-go/Makefile | 8 +++---- vendor/github.com/mndrix/tap-go/tap.go | 12 ++++++++++ vendor/github.com/mndrix/tap-go/yaml_json.go | 22 +++++++++++++++++++ vendor/github.com/mndrix/tap-go/yaml_yaml.go | 23 ++++++++++++++++++++ 5 files changed, 62 insertions(+), 5 deletions(-) create mode 100644 vendor/github.com/mndrix/tap-go/yaml_json.go create mode 100644 vendor/github.com/mndrix/tap-go/yaml_yaml.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 9a174c7b0..609e8c436 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -26,7 +26,7 @@ }, { "ImportPath": "github.com/mndrix/tap-go", - "Rev": "56cca451570bb0366b29f30e1aeb5cf9736a1e18" + "Rev": "629fa407e90bbf3781e448e8e9b8931523ec823c" }, { "ImportPath": "github.com/mrunalp/fileutils", diff --git a/vendor/github.com/mndrix/tap-go/Makefile b/vendor/github.com/mndrix/tap-go/Makefile index 986541d7c..5911e250e 100644 --- a/vendor/github.com/mndrix/tap-go/Makefile +++ b/vendor/github.com/mndrix/tap-go/Makefile @@ -1,5 +1,5 @@ -TESTS = auto check diagnostic failing known skip todo writer -GOPATH = $(CURDIR)/gopath +TESTS = auto check diagnostic failing known skip todo writer yaml +GOPATH ?= $(CURDIR)/gopath .PHONY: $(TESTS) @@ -9,8 +9,8 @@ all: $(foreach t,$(TESTS),test/$(t)/test) clean: rm -f test/*/test -test/%/test: test/%/main.go tap.go - go build -o $@ $< +test/%/test: test/%/*.go tap.go yaml_json.go yaml_yaml.go + go build -o $@ -tags yaml ./test/$* $(TESTS): %: test/%/test prove -v -e '' test/$@/test diff --git a/vendor/github.com/mndrix/tap-go/tap.go b/vendor/github.com/mndrix/tap-go/tap.go index c30fd77d8..88df22d16 100644 --- a/vendor/github.com/mndrix/tap-go/tap.go +++ b/vendor/github.com/mndrix/tap-go/tap.go @@ -151,3 +151,15 @@ func (t *T) Diagnostic(message string) { func (t *T) Diagnosticf(format string, a ...interface{}) { t.printf("# "+escapeNewlines(format)+"\n", a...) } + +// YAML generates a YAML block from the message. +func (t *T) YAML(message interface{}) error { + bytes, err := yaml(message, " ") + if err != nil { + return err + } + t.printf(" ---\n ") + t.printf(string(bytes)) + t.printf(" ...\n") + return nil +} diff --git a/vendor/github.com/mndrix/tap-go/yaml_json.go b/vendor/github.com/mndrix/tap-go/yaml_json.go new file mode 100644 index 000000000..f848e6426 --- /dev/null +++ b/vendor/github.com/mndrix/tap-go/yaml_json.go @@ -0,0 +1,22 @@ +// +build !yaml + +package tap + +import ( + "encoding/json" +) + +// yaml serializes a message to YAML. This implementation uses JSON, +// which is a subset of YAML [1] and is implemented by Go's standard +// library. +// +// [1]: http://www.yaml.org/spec/1.2/spec.html#id2759572 +func yaml(message interface{}, prefix string) (marshaled []byte, err error) { + marshaled, err = json.MarshalIndent(message, prefix, " ") + if err != nil { + return marshaled, err + } + + marshaled = append(marshaled, []byte("\n")...) + return marshaled, err +} diff --git a/vendor/github.com/mndrix/tap-go/yaml_yaml.go b/vendor/github.com/mndrix/tap-go/yaml_yaml.go new file mode 100644 index 000000000..93c6f2e80 --- /dev/null +++ b/vendor/github.com/mndrix/tap-go/yaml_yaml.go @@ -0,0 +1,23 @@ +// +build yaml + +package tap + +import ( + "bytes" + + goyaml "gopkg.in/yaml.v2" +) + +// yaml serializes a message to YAML. This implementation uses +// non-JSON YAML, which has better prove support [1]. +// +// [1]: https://rt.cpan.org/Public/Bug/Display.html?id=121606 +func yaml(message interface{}, prefix string) (marshaled []byte, err error) { + marshaled, err = goyaml.Marshal(message) + if err != nil { + return marshaled, err + } + + marshaled = bytes.Replace(marshaled, []byte("\n"), []byte("\n"+prefix), -1) + return marshaled[:len(marshaled)-len(prefix)], err +} From b880d57071bf399a93f9fe083cc47a3dd79bbdce Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 3 Dec 2017 15:28:25 -0800 Subject: [PATCH 2/5] *: Transition from tap Diagnostic(...) to YAML(...) See ad47e7d5 (Makefile: Change from prove to node-tap, 2017-11-30, #439) about how node-tap handles YAML blocks vs. diagnostics. The README's localvalidation line is back after I accidentally removed it in ad47e7d5. Signed-off-by: W. Trevor King --- README.md | 3 +++ cmd/runtimetest/main.go | 4 ++-- validation/create.go | 14 ++++++++++---- validation/process_capabilities.go | 2 +- validation/root_readonly_true.go | 2 +- validation/util/test.go | 6 +++--- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 5f16436bc..a67eda763 100644 --- a/README.md +++ b/README.md @@ -39,15 +39,18 @@ $ npm install tap ```console $ make runtimetest validation-executables +$ sudo make RUNTIME=runc localvalidation RUNTIME=runc tap validation/linux_rootfs_propagation_shared.t validation/create.t validation/default.t validation/linux_readonly_paths.t validation/linux_masked_paths.t validation/mounts.t validation/process.t validation/root_readonly_false.t validation/linux_sysctl.t validation/linux_devices.t validation/linux_gid_mappings.t validation/process_oom_score_adj.t validation/process_capabilities.t validation/process_rlimits.t validation/root_readonly_true.t validation/linux_rootfs_propagation_unbindable.t validation/hostname.t validation/linux_uid_mappings.t validation/linux_rootfs_propagation_shared.t ........ 18/19 not ok rootfs propagation + error: 'rootfs should be shared, but not' validation/create.t ................................... 4/4 validation/default.t ................................ 19/19 validation/linux_readonly_paths.t ................... 19/19 validation/linux_masked_paths.t ..................... 18/19 not ok masked paths + error: /masktest should not be readable validation/mounts.t ................................... 0/1 Skipped: 1 diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go index c3da9a6fa..53352f055 100644 --- a/cmd/runtimetest/main.go +++ b/cmd/runtimetest/main.go @@ -945,7 +945,7 @@ func run(context *cli.Context) error { } else { t.Fail(v.description) } - t.Diagnostic(err.Error()) + t.YAML(map[string]string{"error": err.Error()}) } } else { if e, ok := err.(*rfc2119.Error); ok { @@ -953,7 +953,7 @@ func run(context *cli.Context) error { } else { t.Fail(v.description) } - t.Diagnostic(err.Error()) + t.YAML(map[string]string{"error": err.Error()}) } } } diff --git a/validation/create.go b/validation/create.go index 892311277..bb87bf877 100644 --- a/validation/create.go +++ b/validation/create.go @@ -50,18 +50,24 @@ func main() { r.SetID(c.id) stderr, err := r.Create() t.Ok((err == nil) == c.errExpected, c.err.(*specerror.Error).Err.Err.Error()) - t.Diagnostic(c.err.(*specerror.Error).Err.Reference) + diagnostic := map[string]string{ + "reference": c.err.(*specerror.Error).Err.Reference, + } if err != nil { - t.Diagnostic(err.Error()) + diagnostic["error"] = err.Error() } if len(stderr) > 0 { - t.Diagnostic(string(stderr)) + diagnostic["stderr"] = string(stderr) } + t.YAML(diagnostic) if err == nil { state, _ := r.State() t.Ok(state.ID == c.id, "") - t.Diagnosticf("container PID: %d, state ID: %d", c.id, state.ID) + t.YAML(map[string]string{ + "container ID": c.id, + "state ID": state.ID, + }) } } diff --git a/validation/process_capabilities.go b/validation/process_capabilities.go index 3c676c554..dc9be82bd 100644 --- a/validation/process_capabilities.go +++ b/validation/process_capabilities.go @@ -9,7 +9,7 @@ import ( func main() { if "linux" != runtime.GOOS { - util.Skip("linux-specific process.capabilities test", runtime.GOOS) + util.Skip("linux-specific process.capabilities test", map[string]string{"OS": runtime.GOOS}) os.Exit(0) } diff --git a/validation/root_readonly_true.go b/validation/root_readonly_true.go index 05907a02c..a75012036 100644 --- a/validation/root_readonly_true.go +++ b/validation/root_readonly_true.go @@ -9,7 +9,7 @@ import ( func main() { if "windows" == runtime.GOOS { - util.Skip("non-Windows root.readonly test", runtime.GOOS) + util.Skip("non-Windows root.readonly test", map[string]string{"OS": runtime.GOOS}) os.Exit(0) } diff --git a/validation/util/test.go b/validation/util/test.go index 3ecc86d2a..e41051836 100644 --- a/validation/util/test.go +++ b/validation/util/test.go @@ -42,12 +42,12 @@ func Fatal(err error) { } // Skip skips a full TAP suite. -func Skip(message string, diagnostic string) { +func Skip(message string, diagnostic interface{}) { t := tap.New() t.Header(1) t.Skip(1, message) - if diagnostic != "" { - t.Diagnostic(diagnostic) + if diagnostic != nil { + t.YAML(diagnostic) } } From a12de423621a5a9a24b1f0645bb7a223373d3571 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 4 Dec 2017 09:03:25 -0800 Subject: [PATCH 3/5] validation/create: Label the state ID comparison test So we get: ok 3 - 'state' MUST return the state of a container instead of: ok 3 - Signed-off-by: W. Trevor King --- validation/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/validation/create.go b/validation/create.go index bb87bf877..e2d4b1eb1 100644 --- a/validation/create.go +++ b/validation/create.go @@ -63,7 +63,7 @@ func main() { if err == nil { state, _ := r.State() - t.Ok(state.ID == c.id, "") + t.Ok(state.ID == c.id, "'state' MUST return the state of a container") t.YAML(map[string]string{ "container ID": c.id, "state ID": state.ID, From 0c2e37e5292f872eae9f0af2f851e867e3af22a4 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 4 Dec 2017 09:04:15 -0800 Subject: [PATCH 4/5] validation/util/container: Use a local UUID for stdout/stderr So validation/create.t gives: ok 4 - create MUST generate an error if the ID provided is not unique --- { "error": "exit status 1", "reference": "https://github.com/opencontainers/runtime-spec/blob/v1.0.0/runtime.md#create", "stderr": "container with id exists: 4a142dd0-e5bc-48b3-9abd-49c1a8f7c498\n" } ... instead of: ok 4 - create MUST generate an error if the ID provided is not unique --- { "error": "open /tmp/ocitest842577116/stdout-178f8311-9cc7-42c5-b91e-abbe29eaf582: file exists", "reference": "https://github.com/opencontainers/runtime-spec/blob/v1.0.0/runtime.md#create" } ... The old code hit the internal error, while the new code is checking for a runtime error. Signed-off-by: W. Trevor King --- validation/util/container.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/validation/util/container.go b/validation/util/container.go index 652c316f8..629ac02f2 100644 --- a/validation/util/container.go +++ b/validation/util/container.go @@ -12,6 +12,7 @@ import ( rspecs "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" + "github.com/satori/go.uuid" ) // Runtime represents the basic requirement of a container runtime @@ -63,12 +64,13 @@ func (r *Runtime) Create() (stderr []byte, err error) { // } cmd := exec.Command(r.RuntimeCommand, args...) cmd.Dir = r.BundleDir - r.stdout, err = os.OpenFile(filepath.Join(r.BundleDir, fmt.Sprintf("stdout-%s", r.ID)), os.O_CREATE|os.O_EXCL|os.O_RDWR, 0600) + id := uuid.NewV4().String() + r.stdout, err = os.OpenFile(filepath.Join(r.BundleDir, fmt.Sprintf("stdout-%s", id)), os.O_CREATE|os.O_EXCL|os.O_RDWR, 0600) if err != nil { return []byte(""), err } cmd.Stdout = r.stdout - r.stderr, err = os.OpenFile(filepath.Join(r.BundleDir, fmt.Sprintf("stderr-%s", r.ID)), os.O_CREATE|os.O_EXCL|os.O_RDWR, 0600) + r.stderr, err = os.OpenFile(filepath.Join(r.BundleDir, fmt.Sprintf("stderr-%s", id)), os.O_CREATE|os.O_EXCL|os.O_RDWR, 0600) if err != nil { return []byte(""), err } From e85081acd382c78de71edcdf8301ae5aea511d16 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 4 Dec 2017 10:10:22 -0800 Subject: [PATCH 5/5] Makefile: Clearer warning on missing validation executable(s) Instead of reporting total ................................................. 0/0 and passing (which node-tap seems to do when given missing files as arguments), report: missing test executable; run 'make runtimetest validation-executables' and error out. Having a separate step to build the executables is useful for: * Not building them as root, which reduces the change of security issues by using as little privilege as possible. * Not rebuilding them on each test, since we haven't told Make about the full dependency tree for the test executables. The separate build step was introduced in e11b77f3 (validation: Use prove(1) as a TAP harness, 2017-11-25, #439), but I didn't do a good job of motivating it in that commit message. Signed-off-by: W. Trevor King --- Makefile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Makefile b/Makefile index a7a858123..4cd61063a 100644 --- a/Makefile +++ b/Makefile @@ -40,6 +40,14 @@ clean: rm -f oci-runtime-tool runtimetest *.1 $(VALIDATION_TESTS) localvalidation: + @for EXECUTABLE in runtimetest $(VALIDATION_TESTS); \ + do \ + if test ! -x "$${EXECUTABLE}"; \ + then \ + echo "missing test executable $${EXECUTABLE}; run 'make runtimetest validation-executables'" >&2; \ + exit 1; \ + fi; \ + done RUNTIME=$(RUNTIME) $(TAP) $(VALIDATION_TESTS) .PHONY: validation-executables