Merge pull request opencontainers#455 from wking/specerror-package

specerror: Pull runtime-spec-specific error handling into its own package

cmd/runtimetest/main.go

@@ -22,7 +22,8 @@ import (
 	"github.com/urfave/cli"
 
 	"github.com/opencontainers/runtime-tools/cmd/runtimetest/mount"
-	rerr "github.com/opencontainers/runtime-tools/error"
+	rfc2119 "github.com/opencontainers/runtime-tools/error"
+	"github.com/opencontainers/runtime-tools/specerror"
 )
 
 // PrGetNoNewPrivs isn't exposed in Golang so we define it ourselves copying the value from
@@ -313,7 +314,7 @@ func validateRootFS(spec *rspec.Spec) error {
 	if spec.Root.Readonly {
 		err := testWriteAccess("/")
 		if err == nil {
-			return rerr.NewError(rerr.ReadonlyFilesystem, "Rootfs should be readonly", rspec.Version)
+			return specerror.NewError(specerror.ReadonlyFilesystem, fmt.Errorf("rootfs must be readonly"), rspec.Version)
 		}
 	}
 
@@ -325,7 +326,7 @@ func validateDefaultFS(spec *rspec.Spec) error {
 
 	mountInfos, err := mount.GetMounts()
 	if err != nil {
-		rerr.NewError(rerr.DefaultFilesystems, err.Error(), spec.Version)
+		specerror.NewError(specerror.DefaultFilesystems, err, spec.Version)
 	}
 
 	mountsMap := make(map[string]string)
@@ -335,7 +336,7 @@ func validateDefaultFS(spec *rspec.Spec) error {
 
 	for fs, fstype := range defaultFS {
 		if !(mountsMap[fs] == fstype) {
-			return rerr.NewError(rerr.DefaultFilesystems, fmt.Sprintf("%v SHOULD exist and expected type is %v", fs, fstype), rspec.Version)
+			return specerror.NewError(specerror.DefaultFilesystems, fmt.Errorf("%v SHOULD exist and expected type is %v", fs, fstype), rspec.Version)
 		}
 	}
 
@@ -779,17 +780,17 @@ func run(context *cli.Context) error {
 	t.Header(0)
 
 	complianceLevelString := context.String("compliance-level")
-	complianceLevel, err := rerr.ParseLevel(complianceLevelString)
+	complianceLevel, err := rfc2119.ParseLevel(complianceLevelString)
 	if err != nil {
-		complianceLevel = rerr.Must
+		complianceLevel = rfc2119.Must
 		logrus.Warningf("%s, using 'MUST' by default.", err.Error())
 	}
 	var validationErrors error
 	for _, v := range defaultValidations {
 		err := v.test(spec)
 		t.Ok(err == nil, v.description)
 		if err != nil {
-			if e, ok := err.(*rerr.Error); ok && e.Level < complianceLevel {
+			if e, ok := err.(*specerror.Error); ok && e.Err.Level < complianceLevel {
 				continue
 			}
 			validationErrors = multierror.Append(validationErrors, err)
@@ -801,7 +802,7 @@ func run(context *cli.Context) error {
 			err := v.test(spec)
 			t.Ok(err == nil, v.description)
 			if err != nil {
-				if e, ok := err.(*rerr.Error); ok && e.Level < complianceLevel {
+				if e, ok := err.(*specerror.Error); ok && e.Err.Level < complianceLevel {
 					continue
 				}
 				validationErrors = multierror.Append(validationErrors, err)

error/rfc2199.go → error/error.go

@@ -7,7 +7,7 @@ import (
 	"strings"
 )
 
-// Level represents the OCI compliance levels
+// Level represents the RFC 2119 compliance levels
 type Level int
 
 const (
@@ -43,15 +43,19 @@ const (
 	Required
 )
 
-// Error represents an error with compliance level and OCI reference.
+// Error represents an error with compliance level and specification reference.
 type Error struct {
-	Level     Level
+	// Level represents the RFC 2119 compliance level.
+	Level Level
+
+	// Reference is a URL for the violated specification requirement.
 	Reference string
-	Err       error
-	ErrCode   int
+
+	// Err holds additional details about the violation.
+	Err error
 }
 
-// ParseLevel takes a string level and returns the OCI compliance level constant.
+// ParseLevel takes a string level and returns the RFC 2119 compliance level constant.
 func ParseLevel(level string) (Level, error) {
 	switch strings.ToUpper(level) {
 	case "MAY":
@@ -82,7 +86,7 @@ func ParseLevel(level string) (Level, error) {
 	return l, fmt.Errorf("%q is not a valid compliance level", level)
 }
 
-// Error returns the error message with OCI reference
+// Error returns the error message with specification reference.
 func (err *Error) Error() string {
 	return fmt.Sprintf("%s\nRefer to: %s", err.Err.Error(), err.Reference)
 }

error/runtime_spec.go → specerror/error.go

@@ -1,20 +1,23 @@
-package error
+// Package specerror implements runtime-spec-specific tooling for
+// tracking RFC 2119 violations.
+package specerror
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/hashicorp/go-multierror"
+	rfc2119 "github.com/opencontainers/runtime-tools/error"
 )
 
 const referenceTemplate = "https://github.com/opencontainers/runtime-spec/blob/v%s/%s"
 
-// SpecErrorCode represents the compliance content.
-type SpecErrorCode int
+// Code represents the spec violation, enumerating both
+// configuration violations and runtime violations.
+type Code int
 
 const (
 	// NonError represents that an input is not an error
-	NonError SpecErrorCode = iota
+	NonError Code = iota
 	// NonRFCError represents that an error is not a rfc2119 error
 	NonRFCError
 
@@ -53,10 +56,19 @@ const (
 )
 
 type errorTemplate struct {
-	Level     Level
+	Level     rfc2119.Level
 	Reference func(version string) (reference string, err error)
 }
 
+// Error represents a runtime-spec violation.
+type Error struct {
+	// Err holds the RFC 2119 violation.
+	Err rfc2119.Error
+
+	// Code is a matchable holds a Code
+	Code Code
+}
+
 var (
 	containerFormatRef = func(version string) (reference string, err error) {
 		return fmt.Sprintf(referenceTemplate, version, "bundle.md#container-format"), nil
@@ -75,62 +87,69 @@ var (
 	}
 )
 
-var ociErrors = map[SpecErrorCode]errorTemplate{
+var ociErrors = map[Code]errorTemplate{
 	// Bundle.md
 	// Container Format
-	ConfigFileExistence:  {Level: Must, Reference: containerFormatRef},
-	ArtifactsInSingleDir: {Level: Must, Reference: containerFormatRef},
+	ConfigFileExistence:  {Level: rfc2119.Must, Reference: containerFormatRef},
+	ArtifactsInSingleDir: {Level: rfc2119.Must, Reference: containerFormatRef},
 
 	// Config.md
 	// Specification Version
-	SpecVersion: {Level: Must, Reference: specVersionRef},
+	SpecVersion: {Level: rfc2119.Must, Reference: specVersionRef},
 	// Root
-	RootOnNonHyperV: {Level: Required, Reference: rootRef},
-	RootOnHyperV:    {Level: Must, Reference: rootRef},
+	RootOnNonHyperV: {Level: rfc2119.Required, Reference: rootRef},
+	RootOnHyperV:    {Level: rfc2119.Must, Reference: rootRef},
 	// TODO: add tests for 'PathFormatOnWindows'
-	PathFormatOnWindows: {Level: Must, Reference: rootRef},
-	PathName:            {Level: Should, Reference: rootRef},
-	PathExistence:       {Level: Must, Reference: rootRef},
-	ReadonlyFilesystem:  {Level: Must, Reference: rootRef},
-	ReadonlyOnWindows:   {Level: Must, Reference: rootRef},
+	PathFormatOnWindows: {Level: rfc2119.Must, Reference: rootRef},
+	PathName:            {Level: rfc2119.Should, Reference: rootRef},
+	PathExistence:       {Level: rfc2119.Must, Reference: rootRef},
+	ReadonlyFilesystem:  {Level: rfc2119.Must, Reference: rootRef},
+	ReadonlyOnWindows:   {Level: rfc2119.Must, Reference: rootRef},
 
 	// Config-Linux.md
 	// Default Filesystems
-	DefaultFilesystems: {Level: Should, Reference: defaultFSRef},
+	DefaultFilesystems: {Level: rfc2119.Should, Reference: defaultFSRef},
 
 	// Runtime.md
 	// Create
-	CreateWithID:       {Level: Must, Reference: runtimeCreateRef},
-	CreateWithUniqueID: {Level: Must, Reference: runtimeCreateRef},
-	CreateNewContainer: {Level: Must, Reference: runtimeCreateRef},
+	CreateWithID:       {Level: rfc2119.Must, Reference: runtimeCreateRef},
+	CreateWithUniqueID: {Level: rfc2119.Must, Reference: runtimeCreateRef},
+	CreateNewContainer: {Level: rfc2119.Must, Reference: runtimeCreateRef},
+}
+
+// Error returns the error message with specification reference.
+func (err *Error) Error() string {
+	return err.Err.Error()
 }
 
 // NewError creates an Error referencing a spec violation.  The error
-// can be cast to a *runtime-tools.error.Error for extracting
-// structured information about the level of the violation and a
-// reference to the violated spec condition.
+// can be cast to an *Error for extracting structured information
+// about the level of the violation and a reference to the violated
+// spec condition.
 //
 // A version string (for the version of the spec that was violated)
 // must be set to get a working URL.
-func NewError(code SpecErrorCode, msg string, version string) (err error) {
+func NewError(code Code, err error, version string) error {
 	template := ociErrors[code]
-	reference, err := template.Reference(version)
-	if err != nil {
-		return err
+	reference, err2 := template.Reference(version)
+	if err2 != nil {
+		return err2
 	}
 	return &Error{
-		Level:     template.Level,
-		Reference: reference,
-		Err:       errors.New(msg),
-		ErrCode:   int(code),
+		Err: rfc2119.Error{
+			Level:     template.Level,
+			Reference: reference,
+			Err:       err,
+		},
+		Code: code,
 	}
 }
 
 // FindError finds an error from a source error (multiple error) and
-// returns the error code if founded.
+// returns the error code if found.
 // If the source error is nil or empty, return NonError.
 // If the source error is not a multiple error, return NonRFCError.
-func FindError(err error, code SpecErrorCode) SpecErrorCode {
+func FindError(err error, code Code) Code {
 	if err == nil {
 		return NonError
 	}
@@ -141,7 +160,7 @@ func FindError(err error, code SpecErrorCode) SpecErrorCode {
 		}
 		for _, e := range merr.Errors {
 			if rfcErr, ok := e.(*Error); ok {
-				if rfcErr.ErrCode == int(code) {
+				if rfcErr.Code == code {
 					return code
 				}
 			}

validate/validate.go

@@ -23,7 +23,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/syndtr/gocapability/capability"
 
-	rerr "github.com/opencontainers/runtime-tools/error"
+	"github.com/opencontainers/runtime-tools/specerror"
 )
 
 const specConfig = "config.json"
@@ -86,7 +86,7 @@ func NewValidatorFromPath(bundlePath string, hostSpecific bool, platform string)
 	configPath := filepath.Join(bundlePath, specConfig)
 	content, err := ioutil.ReadFile(configPath)
 	if err != nil {
-		return Validator{}, rerr.NewError(rerr.ConfigFileExistence, err.Error(), rspec.Version)
+		return Validator{}, specerror.NewError(specerror.ConfigFileExistence, err, rspec.Version)
 	}
 	if !utf8.Valid(content) {
 		return Validator{}, fmt.Errorf("%q is not encoded in UTF-8", configPath)
@@ -120,13 +120,13 @@ func (v *Validator) CheckRoot() (errs error) {
 	if v.platform == "windows" && v.spec.Windows != nil && v.spec.Windows.HyperV != nil {
 		if v.spec.Root != nil {
 			errs = multierror.Append(errs,
-				rerr.NewError(rerr.RootOnHyperV, "for Hyper-V containers, Root must not be set", rspec.Version))
+				specerror.NewError(specerror.RootOnHyperV, fmt.Errorf("for Hyper-V containers, Root must not be set"), rspec.Version))
 			return
 		}
 		return
 	} else if v.spec.Root == nil {
 		errs = multierror.Append(errs,
-			rerr.NewError(rerr.RootOnNonHyperV, "for non-Hyper-V containers, Root must be set", rspec.Version))
+			specerror.NewError(specerror.RootOnNonHyperV, fmt.Errorf("for non-Hyper-V containers, Root must be set"), rspec.Version))
 		return
 	}
 
@@ -138,7 +138,7 @@ func (v *Validator) CheckRoot() (errs error) {
 
 	if filepath.Base(v.spec.Root.Path) != "rootfs" {
 		errs = multierror.Append(errs,
-			rerr.NewError(rerr.PathName, "Path name should be the conventional 'rootfs'", rspec.Version))
+			specerror.NewError(specerror.PathName, fmt.Errorf("path name should be the conventional 'rootfs'"), rspec.Version))
 	}
 
 	var rootfsPath string
@@ -158,22 +158,22 @@ func (v *Validator) CheckRoot() (errs error) {
 
 	if fi, err := os.Stat(rootfsPath); err != nil {
 		errs = multierror.Append(errs,
-			rerr.NewError(rerr.PathExistence, fmt.Sprintf("Cannot find the root path %q", rootfsPath), rspec.Version))
+			specerror.NewError(specerror.PathExistence, fmt.Errorf("cannot find the root path %q", rootfsPath), rspec.Version))
 	} else if !fi.IsDir() {
 		errs = multierror.Append(errs,
-			rerr.NewError(rerr.PathExistence, fmt.Sprintf("The root path %q is not a directory", rootfsPath), rspec.Version))
+			specerror.NewError(specerror.PathExistence, fmt.Errorf("root.path %q is not a directory", rootfsPath), rspec.Version))
 	}
 
 	rootParent := filepath.Dir(absRootPath)
 	if absRootPath == string(filepath.Separator) || rootParent != absBundlePath {
 		errs = multierror.Append(errs,
-			rerr.NewError(rerr.ArtifactsInSingleDir, fmt.Sprintf("root.path is %q, but it MUST be a child of %q", v.spec.Root.Path, absBundlePath), rspec.Version))
+			specerror.NewError(specerror.ArtifactsInSingleDir, fmt.Errorf("root.path is %q, but it MUST be a child of %q", v.spec.Root.Path, absBundlePath), rspec.Version))
 	}
 
 	if v.platform == "windows" {
 		if v.spec.Root.Readonly {
 			errs = multierror.Append(errs,
-				rerr.NewError(rerr.ReadonlyOnWindows, "root.readonly field MUST be omitted or false when target platform is windows", rspec.Version))
+				specerror.NewError(specerror.ReadonlyOnWindows, fmt.Errorf("root.readonly field MUST be omitted or false when target platform is windows"), rspec.Version))
 		}
 	}
 
@@ -188,7 +188,7 @@ func (v *Validator) CheckSemVer() (errs error) {
 	_, err := semver.Parse(version)
 	if err != nil {
 		errs = multierror.Append(errs,
-			rerr.NewError(rerr.SpecVersion, fmt.Sprintf("%q is not valid SemVer: %s", version, err.Error()), rspec.Version))
+			specerror.NewError(specerror.SpecVersion, fmt.Errorf("%q is not valid SemVer: %s", version, err.Error()), rspec.Version))
 	}
 	if version != rspec.Version {
 		errs = multierror.Append(errs, fmt.Errorf("validate currently only handles version %s, but the supplied configuration targets %s", rspec.Version, version))