diff --git a/cmd/runtimetest/main.go b/cmd/runtimetest/main.go index 858f216bd..5b4819644 100644 --- a/cmd/runtimetest/main.go +++ b/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,9 +780,9 @@ 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 @@ -789,7 +790,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) @@ -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) diff --git a/error/rfc2199.go b/error/error.go similarity index 99% rename from error/rfc2199.go rename to error/error.go index 2b4ab6267..c2345c1d8 100644 --- a/error/rfc2199.go +++ b/error/error.go @@ -48,7 +48,6 @@ type Error struct { Level Level Reference string Err error - ErrCode int } // ParseLevel takes a string level and returns the OCI compliance level constant. diff --git a/error/runtime_spec.go b/specerror/error.go similarity index 63% rename from error/runtime_spec.go rename to specerror/error.go index 031a3ccf0..199aa2d4a 100644 --- a/error/runtime_spec.go +++ b/specerror/error.go @@ -1,15 +1,18 @@ -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. +// SpecErrorCode represents the spec violation, enumerating both +// configuration violations and runtime violations. type SpecErrorCode int const ( @@ -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 + + // ErrCode is a matchable holds a SpecErrorCode + ErrCode SpecErrorCode +} + var ( containerFormatRef = func(version string) (reference string, err error) { return fmt.Sprintf(referenceTemplate, version, "bundle.md#container-format"), nil @@ -78,56 +90,63 @@ var ( var ociErrors = map[SpecErrorCode]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 SpecErrorCode, 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, + }, + ErrCode: 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 { @@ -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.ErrCode == code { return code } } diff --git a/validate/validate.go b/validate/validate.go index bbad95a8b..0d62a53ef 100644 --- a/validate/validate.go +++ b/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)) diff --git a/validate/validate_test.go b/validate/validate_test.go index be89a008e..b2d59ec1e 100644 --- a/validate/validate_test.go +++ b/validate/validate_test.go @@ -11,7 +11,7 @@ import ( rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" - rerr "github.com/opencontainers/runtime-tools/error" + "github.com/opencontainers/runtime-tools/specerror" ) func TestNewValidator(t *testing.T) { @@ -53,40 +53,40 @@ func TestCheckRoot(t *testing.T) { cases := []struct { val rspec.Spec platform string - expected rerr.SpecErrorCode + expected specerror.SpecErrorCode }{ - {rspec.Spec{Windows: &rspec.Windows{HyperV: &rspec.WindowsHyperV{}}, Root: &rspec.Root{}}, "windows", rerr.RootOnHyperV}, - {rspec.Spec{Windows: &rspec.Windows{HyperV: &rspec.WindowsHyperV{}}, Root: nil}, "windows", rerr.NonError}, - {rspec.Spec{Root: nil}, "linux", rerr.RootOnNonHyperV}, - {rspec.Spec{Root: &rspec.Root{Path: "maverick-rootfs"}}, "linux", rerr.PathName}, - {rspec.Spec{Root: &rspec.Root{Path: "rootfs"}}, "linux", rerr.NonError}, - {rspec.Spec{Root: &rspec.Root{Path: filepath.Join(tmpBundle, rootfsNonExists)}}, "linux", rerr.PathExistence}, - {rspec.Spec{Root: &rspec.Root{Path: filepath.Join(tmpBundle, rootfsNonDir)}}, "linux", rerr.PathExistence}, - {rspec.Spec{Root: &rspec.Root{Path: filepath.Join(tmpBundle, "rootfs")}}, "linux", rerr.NonError}, - {rspec.Spec{Root: &rspec.Root{Path: "rootfs/rootfs"}}, "linux", rerr.ArtifactsInSingleDir}, - {rspec.Spec{Root: &rspec.Root{Readonly: true}}, "windows", rerr.ReadonlyOnWindows}, + {rspec.Spec{Windows: &rspec.Windows{HyperV: &rspec.WindowsHyperV{}}, Root: &rspec.Root{}}, "windows", specerror.RootOnHyperV}, + {rspec.Spec{Windows: &rspec.Windows{HyperV: &rspec.WindowsHyperV{}}, Root: nil}, "windows", specerror.NonError}, + {rspec.Spec{Root: nil}, "linux", specerror.RootOnNonHyperV}, + {rspec.Spec{Root: &rspec.Root{Path: "maverick-rootfs"}}, "linux", specerror.PathName}, + {rspec.Spec{Root: &rspec.Root{Path: "rootfs"}}, "linux", specerror.NonError}, + {rspec.Spec{Root: &rspec.Root{Path: filepath.Join(tmpBundle, rootfsNonExists)}}, "linux", specerror.PathExistence}, + {rspec.Spec{Root: &rspec.Root{Path: filepath.Join(tmpBundle, rootfsNonDir)}}, "linux", specerror.PathExistence}, + {rspec.Spec{Root: &rspec.Root{Path: filepath.Join(tmpBundle, "rootfs")}}, "linux", specerror.NonError}, + {rspec.Spec{Root: &rspec.Root{Path: "rootfs/rootfs"}}, "linux", specerror.ArtifactsInSingleDir}, + {rspec.Spec{Root: &rspec.Root{Readonly: true}}, "windows", specerror.ReadonlyOnWindows}, } for _, c := range cases { v := NewValidator(&c.val, tmpBundle, false, c.platform) err := v.CheckRoot() - assert.Equal(t, c.expected, rerr.FindError(err, c.expected), fmt.Sprintf("Fail to check Root: %v %d", err, c.expected)) + assert.Equal(t, c.expected, specerror.FindError(err, c.expected), fmt.Sprintf("Fail to check Root: %v %d", err, c.expected)) } } func TestCheckSemVer(t *testing.T) { cases := []struct { val string - expected rerr.SpecErrorCode + expected specerror.SpecErrorCode }{ - {rspec.Version, rerr.NonError}, + {rspec.Version, specerror.NonError}, //FIXME: validate currently only handles rpsec.Version - {"0.0.1", rerr.NonRFCError}, - {"invalid", rerr.SpecVersion}, + {"0.0.1", specerror.NonRFCError}, + {"invalid", specerror.SpecVersion}, } for _, c := range cases { v := NewValidator(&rspec.Spec{Version: c.val}, "", false, "linux") err := v.CheckSemVer() - assert.Equal(t, c.expected, rerr.FindError(err, c.expected), "Fail to check SemVer "+c.val) + assert.Equal(t, c.expected, specerror.FindError(err, c.expected), "Fail to check SemVer "+c.val) } } diff --git a/validation/validation_test.go b/validation/validation_test.go index 163613e8a..ad09c7ec7 100644 --- a/validation/validation_test.go +++ b/validation/validation_test.go @@ -12,8 +12,8 @@ import ( "github.com/satori/go.uuid" "github.com/stretchr/testify/assert" - rerr "github.com/opencontainers/runtime-tools/error" "github.com/opencontainers/runtime-tools/generate" + "github.com/opencontainers/runtime-tools/specerror" ) var ( @@ -114,9 +114,9 @@ func TestValidateCreate(t *testing.T) { errExpected bool err error }{ - {"", false, rerr.NewError(rerr.CreateWithID, "'Create' MUST generate an error if the ID is not provided", rspecs.Version)}, - {containerID, true, rerr.NewError(rerr.CreateNewContainer, "'Create' MUST create a new container", rspecs.Version)}, - {containerID, false, rerr.NewError(rerr.CreateWithUniqueID, "'Create' MUST generate an error if the ID provided is not unique", rspecs.Version)}, + {"", false, specerror.NewError(specerror.CreateWithID, "'Create' MUST generate an error if the ID is not provided", rspecs.Version)}, + {containerID, true, specerror.NewError(specerror.CreateNewContainer, "'Create' MUST create a new container", rspecs.Version)}, + {containerID, false, specerror.NewError(specerror.CreateWithUniqueID, "'Create' MUST generate an error if the ID provided is not unique", rspecs.Version)}, } for _, c := range cases {