From a9dbd7e322c2e57502e17711f1c926184e854c86 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 2 Oct 2017 15:26:00 -0700 Subject: [PATCH 1/2] cmd/oci-runtime-tool: Implement --compliance-level The man-page entry and Bash completions for this option were added in 4029999d (oci error: add error level and reference, 2017-03-31, #354), but the option was left unimplemented. The implementation in this commit is very similar to the existing runtimetest implementation from 4029999d, except we warn instead of silently skipping non-fatal spec violations. The warning (vs. error) on invalid level arguments follows the runtimetest implementation from 6316a4e0 (use released version as reference; improve Parse error, 2017-04-12, #354). I'd personally prefer hard errors on invalid level arguments, but didn't want to break runtime-tools consistency over that. Signed-off-by: W. Trevor King --- cmd/oci-runtime-tool/main.go | 5 +++++ cmd/oci-runtime-tool/validate.go | 26 ++++++++++++++++++++++++-- man/oci-runtime-tool.1.md | 3 ++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/cmd/oci-runtime-tool/main.go b/cmd/oci-runtime-tool/main.go index 2268a0ac5..c4f4f589b 100644 --- a/cmd/oci-runtime-tool/main.go +++ b/cmd/oci-runtime-tool/main.go @@ -22,6 +22,11 @@ func main() { } app.Usage = "OCI (Open Container Initiative) runtime tools" app.Flags = []cli.Flag{ + cli.StringFlag{ + Name: "compliance-level", + Value: "must", + Usage: "compliance level (may, should, or must).", + }, cli.BoolFlag{ Name: "host-specific", Usage: "generate host-specific configs or do host-specific validations", diff --git a/cmd/oci-runtime-tool/validate.go b/cmd/oci-runtime-tool/validate.go index 58cdeb921..8030428c7 100644 --- a/cmd/oci-runtime-tool/validate.go +++ b/cmd/oci-runtime-tool/validate.go @@ -3,7 +3,11 @@ package main import ( "fmt" + "github.com/hashicorp/go-multierror" + rfc2119 "github.com/opencontainers/runtime-tools/error" + "github.com/opencontainers/runtime-tools/specerror" "github.com/opencontainers/runtime-tools/validate" + "github.com/sirupsen/logrus" "github.com/urfave/cli" ) @@ -19,6 +23,12 @@ var bundleValidateCommand = cli.Command{ Before: before, Action: func(context *cli.Context) error { hostSpecific := context.GlobalBool("host-specific") + complianceLevelString := context.GlobalString("compliance-level") + complianceLevel, err := rfc2119.ParseLevel(complianceLevelString) + if err != nil { + complianceLevel = rfc2119.Must + logrus.Warningf("%s, using 'MUST' by default.", err.Error()) + } inputPath := context.String("path") platform := context.String("platform") v, err := validate.NewValidatorFromPath(inputPath, hostSpecific, platform) @@ -27,8 +37,20 @@ var bundleValidateCommand = cli.Command{ } if err := v.CheckAll(); err != nil { - return err - + merr, ok := err.(*multierror.Error) + if !ok { + return err + } + var validationErrors error + for _, err = range merr.Errors { + e, ok := err.(*specerror.Error) + if ok && e.Err.Level < complianceLevel { + logrus.Warn(e) + continue + } + validationErrors = multierror.Append(validationErrors, err) + } + return validationErrors } fmt.Println("Bundle validation succeeded.") return nil diff --git a/man/oci-runtime-tool.1.md b/man/oci-runtime-tool.1.md index 880b587ff..5a66fe33b 100644 --- a/man/oci-runtime-tool.1.md +++ b/man/oci-runtime-tool.1.md @@ -33,7 +33,8 @@ oci-runtime-tool is a collection of tools for working with the [OCI runtime spec Log level (panic, fatal, error, warn, info, or debug) (default: "error"). **--compliance-level**=LEVEL - Compliance level (may, should or must) (default: "must"). + Compliance level (`may`, `should`, or `must`) (default: `must`). + For example, a SHOULD-level violation is fatal if `--compliance-level` is `may` or `should` but non-fatal if `--compliance-level` is `must`. **-v**, **--version** Print version information. From 6089f63225c5a04b3ee1c98e75ad8854e7bf822a Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 3 Oct 2017 12:07:47 -0700 Subject: [PATCH 2/2] specerror: Add SplitLevel helper This doesn't save much time for folks who are iterating over the removed errors (e.g. to print warnings about them), but Aleksa asked for it [1] and Ma suggested the helper struct [2]. I haven't used it in runtimetest. I'm waiting for the in-flight 7aa3987 (cmd/runtimetest/main: Use TAP diagnostics for errors, 2017-07-28, #439) to settle, since that commit cleans up some of the runtimetest error-processing code with: validations := defaultValidations if platform == "linux" { validations = append(validations, linuxValidations...) } and a single loop over the constructed validations array. [1]: https://github.com/opencontainers/runtime-tools/pull/492#discussion_r142488597 [2]: https://github.com/opencontainers/runtime-tools/pull/492#discussion_r143620920 Signed-off-by: W. Trevor King --- cmd/oci-runtime-tool/validate.go | 17 +++++------------ specerror/error.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/cmd/oci-runtime-tool/validate.go b/cmd/oci-runtime-tool/validate.go index 8030428c7..f506aa28b 100644 --- a/cmd/oci-runtime-tool/validate.go +++ b/cmd/oci-runtime-tool/validate.go @@ -3,7 +3,6 @@ package main import ( "fmt" - "github.com/hashicorp/go-multierror" rfc2119 "github.com/opencontainers/runtime-tools/error" "github.com/opencontainers/runtime-tools/specerror" "github.com/opencontainers/runtime-tools/validate" @@ -37,20 +36,14 @@ var bundleValidateCommand = cli.Command{ } if err := v.CheckAll(); err != nil { - merr, ok := err.(*multierror.Error) - if !ok { + levelErrors, err := specerror.SplitLevel(err, complianceLevel) + if err != nil { return err } - var validationErrors error - for _, err = range merr.Errors { - e, ok := err.(*specerror.Error) - if ok && e.Err.Level < complianceLevel { - logrus.Warn(e) - continue - } - validationErrors = multierror.Append(validationErrors, err) + for _, e := range levelErrors.Warnings { + logrus.Warn(e) } - return validationErrors + return levelErrors.Error } fmt.Println("Bundle validation succeeded.") return nil diff --git a/specerror/error.go b/specerror/error.go index c75bb6b14..069cbc1e6 100644 --- a/specerror/error.go +++ b/specerror/error.go @@ -69,6 +69,16 @@ type Error struct { Code Code } +// LevelErrors represents Errors filtered into fatal and warnings. +type LevelErrors struct { + // Warnings holds Errors that were below a compliance-level threshold. + Warnings []*Error + + // Error holds errors that were at or above a compliance-level + // threshold, as well as errors that are not Errors. + Error *multierror.Error +} + var ( containerFormatRef = func(version string) (reference string, err error) { return fmt.Sprintf(referenceTemplate, version, "bundle.md#container-format"), nil @@ -168,3 +178,23 @@ func FindError(err error, code Code) Code { } return NonRFCError } + +// SplitLevel removes RFC 2119 errors with a level less than 'level' +// from the source error. If the source error is not a multierror, it +// is returned unchanged. +func SplitLevel(errIn error, level rfc2119.Level) (levelErrors LevelErrors, errOut error) { + merr, ok := errIn.(*multierror.Error) + if !ok { + return levelErrors, errIn + } + for _, err := range merr.Errors { + e, ok := err.(*Error) + if ok && e.Err.Level < level { + fmt.Println(e) + levelErrors.Warnings = append(levelErrors.Warnings, e) + continue + } + levelErrors.Error = multierror.Append(levelErrors.Error, err) + } + return levelErrors, nil +}