Skip to content

Commit d6e5f82

Browse files
liangchenyeliangchenye
authored
Merge pull request #455 from wking/specerror-package
specerror: Pull runtime-spec-specific error handling into its own package
2 parents 12b47b9 + f9152f1 commit d6e5f82

File tree

6 files changed

+107
-82
lines changed

6 files changed

+107
-82
lines changed

‎cmd/runtimetest/main.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ import (
2222
"github.com/urfave/cli"
2323

2424
"github.com/opencontainers/runtime-tools/cmd/runtimetest/mount"
25-
rerr "github.com/opencontainers/runtime-tools/error"
25+
rfc2119 "github.com/opencontainers/runtime-tools/error"
26+
"github.com/opencontainers/runtime-tools/specerror"
2627
)
2728

2829
// 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 {
313314
if spec.Root.Readonly {
314315
err := testWriteAccess("/")
315316
if err == nil {
316-
return rerr.NewError(rerr.ReadonlyFilesystem, "Rootfs should be readonly", rspec.Version)
317+
return specerror.NewError(specerror.ReadonlyFilesystem, fmt.Errorf("rootfs must be readonly"), rspec.Version)
317318
}
318319
}
319320

@@ -325,7 +326,7 @@ func validateDefaultFS(spec *rspec.Spec) error {
325326

326327
mountInfos, err := mount.GetMounts()
327328
if err != nil {
328-
rerr.NewError(rerr.DefaultFilesystems, err.Error(), spec.Version)
329+
specerror.NewError(specerror.DefaultFilesystems, err, spec.Version)
329330
}
330331

331332
mountsMap := make(map[string]string)
@@ -335,7 +336,7 @@ func validateDefaultFS(spec *rspec.Spec) error {
335336

336337
for fs, fstype := range defaultFS {
337338
if !(mountsMap[fs] == fstype) {
338-
return rerr.NewError(rerr.DefaultFilesystems, fmt.Sprintf("%v SHOULD exist and expected type is %v", fs, fstype), rspec.Version)
339+
return specerror.NewError(specerror.DefaultFilesystems, fmt.Errorf("%v SHOULD exist and expected type is %v", fs, fstype), rspec.Version)
339340
}
340341
}
341342

@@ -779,17 +780,17 @@ func run(context *cli.Context) error {
779780
t.Header(0)
780781

781782
complianceLevelString := context.String("compliance-level")
782-
complianceLevel, err := rerr.ParseLevel(complianceLevelString)
783+
complianceLevel, err := rfc2119.ParseLevel(complianceLevelString)
783784
if err != nil {
784-
complianceLevel = rerr.Must
785+
complianceLevel = rfc2119.Must
785786
logrus.Warningf("%s, using 'MUST' by default.", err.Error())
786787
}
787788
var validationErrors error
788789
for _, v := range defaultValidations {
789790
err := v.test(spec)
790791
t.Ok(err == nil, v.description)
791792
if err != nil {
792-
if e, ok := err.(*rerr.Error); ok && e.Level < complianceLevel {
793+
if e, ok := err.(*specerror.Error); ok && e.Err.Level < complianceLevel {
793794
continue
794795
}
795796
validationErrors = multierror.Append(validationErrors, err)
@@ -801,7 +802,7 @@ func run(context *cli.Context) error {
801802
err := v.test(spec)
802803
t.Ok(err == nil, v.description)
803804
if err != nil {
804-
if e, ok := err.(*rerr.Error); ok && e.Level < complianceLevel {
805+
if e, ok := err.(*specerror.Error); ok && e.Err.Level < complianceLevel {
805806
continue
806807
}
807808
validationErrors = multierror.Append(validationErrors, err)

‎error/rfc2199.go‎ renamed to ‎error/error.go‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"strings"
88
)
99

10-
// Level represents the OCI compliance levels
10+
// Level represents the RFC 2119 compliance levels
1111
type Level int
1212

1313
const (
@@ -43,15 +43,19 @@ const (
4343
Required
4444
)
4545

46-
// Error represents an error with compliance level and OCI reference.
46+
// Error represents an error with compliance level and specification reference.
4747
type Error struct {
48-
Level Level
48+
// Level represents the RFC 2119 compliance level.
49+
Level Level
50+
51+
// Reference is a URL for the violated specification requirement.
4952
Reference string
50-
Err error
51-
ErrCode int
53+
54+
// Err holds additional details about the violation.
55+
Err error
5256
}
5357

54-
// ParseLevel takes a string level and returns the OCI compliance level constant.
58+
// ParseLevel takes a string level and returns the RFC 2119 compliance level constant.
5559
func ParseLevel(level string) (Level, error) {
5660
switch strings.ToUpper(level) {
5761
case "MAY":
@@ -82,7 +86,7 @@ func ParseLevel(level string) (Level, error) {
8286
return l, fmt.Errorf("%q is not a valid compliance level", level)
8387
}
8488

85-
// Error returns the error message with OCI reference
89+
// Error returns the error message with specification reference.
8690
func (err *Error) Error() string {
8791
return fmt.Sprintf("%s\nRefer to: %s", err.Err.Error(), err.Reference)
8892
}

‎error/runtime_spec.go‎ renamed to ‎specerror/error.go‎

Lines changed: 54 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,23 @@
1-
package error
1+
// Package specerror implements runtime-spec-specific tooling for
2+
// tracking RFC 2119 violations.
3+
package specerror
24

35
import (
4-
"errors"
56
"fmt"
67

78
"github.com/hashicorp/go-multierror"
9+
rfc2119 "github.com/opencontainers/runtime-tools/error"
810
)
911

1012
const referenceTemplate = "https://github.com/opencontainers/runtime-spec/blob/v%s/%s"
1113

12-
// SpecErrorCode represents the compliance content.
13-
type SpecErrorCode int
14+
// Code represents the spec violation, enumerating both
15+
// configuration violations and runtime violations.
16+
type Code int
1417

1518
const (
1619
// NonError represents that an input is not an error
17-
NonError SpecErrorCode = iota
20+
NonError Code = iota
1821
// NonRFCError represents that an error is not a rfc2119 error
1922
NonRFCError
2023

@@ -53,10 +56,19 @@ const (
5356
)
5457

5558
type errorTemplate struct {
56-
Level Level
59+
Level rfc2119.Level
5760
Reference func(version string) (reference string, err error)
5861
}
5962

63+
// Error represents a runtime-spec violation.
64+
type Error struct {
65+
// Err holds the RFC 2119 violation.
66+
Err rfc2119.Error
67+
68+
// Code is a matchable holds a Code
69+
Code Code
70+
}
71+
6072
var (
6173
containerFormatRef = func(version string) (reference string, err error) {
6274
return fmt.Sprintf(referenceTemplate, version, "bundle.md#container-format"), nil
@@ -75,62 +87,69 @@ var (
7587
}
7688
)
7789

78-
var ociErrors = map[SpecErrorCode]errorTemplate{
90+
var ociErrors = map[Code]errorTemplate{
7991
// Bundle.md
8092
// Container Format
81-
ConfigFileExistence: {Level: Must, Reference: containerFormatRef},
82-
ArtifactsInSingleDir: {Level: Must, Reference: containerFormatRef},
93+
ConfigFileExistence: {Level: rfc2119.Must, Reference: containerFormatRef},
94+
ArtifactsInSingleDir: {Level: rfc2119.Must, Reference: containerFormatRef},
8395

8496
// Config.md
8597
// Specification Version
86-
SpecVersion: {Level: Must, Reference: specVersionRef},
98+
SpecVersion: {Level: rfc2119.Must, Reference: specVersionRef},
8799
// Root
88-
RootOnNonHyperV: {Level: Required, Reference: rootRef},
89-
RootOnHyperV: {Level: Must, Reference: rootRef},
100+
RootOnNonHyperV: {Level: rfc2119.Required, Reference: rootRef},
101+
RootOnHyperV: {Level: rfc2119.Must, Reference: rootRef},
90102
// TODO: add tests for 'PathFormatOnWindows'
91-
PathFormatOnWindows: {Level: Must, Reference: rootRef},
92-
PathName: {Level: Should, Reference: rootRef},
93-
PathExistence: {Level: Must, Reference: rootRef},
94-
ReadonlyFilesystem: {Level: Must, Reference: rootRef},
95-
ReadonlyOnWindows: {Level: Must, Reference: rootRef},
103+
PathFormatOnWindows: {Level: rfc2119.Must, Reference: rootRef},
104+
PathName: {Level: rfc2119.Should, Reference: rootRef},
105+
PathExistence: {Level: rfc2119.Must, Reference: rootRef},
106+
ReadonlyFilesystem: {Level: rfc2119.Must, Reference: rootRef},
107+
ReadonlyOnWindows: {Level: rfc2119.Must, Reference: rootRef},
96108

97109
// Config-Linux.md
98110
// Default Filesystems
99-
DefaultFilesystems: {Level: Should, Reference: defaultFSRef},
111+
DefaultFilesystems: {Level: rfc2119.Should, Reference: defaultFSRef},
100112

101113
// Runtime.md
102114
// Create
103-
CreateWithID: {Level: Must, Reference: runtimeCreateRef},
104-
CreateWithUniqueID: {Level: Must, Reference: runtimeCreateRef},
105-
CreateNewContainer: {Level: Must, Reference: runtimeCreateRef},
115+
CreateWithID: {Level: rfc2119.Must, Reference: runtimeCreateRef},
116+
CreateWithUniqueID: {Level: rfc2119.Must, Reference: runtimeCreateRef},
117+
CreateNewContainer: {Level: rfc2119.Must, Reference: runtimeCreateRef},
118+
}
119+
120+
// Error returns the error message with specification reference.
121+
func (err *Error) Error() string {
122+
return err.Err.Error()
106123
}
107124

108125
// NewError creates an Error referencing a spec violation. The error
109-
// can be cast to a *runtime-tools.error.Error for extracting
110-
// structured information about the level of the violation and a
111-
// reference to the violated spec condition.
126+
// can be cast to an *Error for extracting structured information
127+
// about the level of the violation and a reference to the violated
128+
// spec condition.
112129
//
113130
// A version string (for the version of the spec that was violated)
114131
// must be set to get a working URL.
115-
func NewError(code SpecErrorCode, msg string, version string) (err error) {
132+
func NewError(code Code, err error, version string) error {
116133
template := ociErrors[code]
117-
reference, err := template.Reference(version)
118-
if err != nil {
119-
return err
134+
reference, err2 := template.Reference(version)
135+
if err2 != nil {
136+
return err2
120137
}
121138
return &Error{
122-
Level: template.Level,
123-
Reference: reference,
124-
Err: errors.New(msg),
125-
ErrCode: int(code),
139+
Err: rfc2119.Error{
140+
Level: template.Level,
141+
Reference: reference,
142+
Err: err,
143+
},
144+
Code: code,
126145
}
127146
}
128147

129148
// FindError finds an error from a source error (multiple error) and
130-
// returns the error code if founded.
149+
// returns the error code if found.
131150
// If the source error is nil or empty, return NonError.
132151
// If the source error is not a multiple error, return NonRFCError.
133-
func FindError(err error, code SpecErrorCode) SpecErrorCode {
152+
func FindError(err error, code Code) Code {
134153
if err == nil {
135154
return NonError
136155
}
@@ -141,7 +160,7 @@ func FindError(err error, code SpecErrorCode) SpecErrorCode {
141160
}
142161
for _, e := range merr.Errors {
143162
if rfcErr, ok := e.(*Error); ok {
144-
if rfcErr.ErrCode == int(code) {
163+
if rfcErr.Code == code {
145164
return code
146165
}
147166
}

‎validate/validate.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"github.com/sirupsen/logrus"
2424
"github.com/syndtr/gocapability/capability"
2525

26-
rerr "github.com/opencontainers/runtime-tools/error"
26+
"github.com/opencontainers/runtime-tools/specerror"
2727
)
2828

2929
const specConfig = "config.json"
@@ -86,7 +86,7 @@ func NewValidatorFromPath(bundlePath string, hostSpecific bool, platform string)
8686
configPath := filepath.Join(bundlePath, specConfig)
8787
content, err := ioutil.ReadFile(configPath)
8888
if err != nil {
89-
return Validator{}, rerr.NewError(rerr.ConfigFileExistence, err.Error(), rspec.Version)
89+
return Validator{}, specerror.NewError(specerror.ConfigFileExistence, err, rspec.Version)
9090
}
9191
if !utf8.Valid(content) {
9292
return Validator{}, fmt.Errorf("%q is not encoded in UTF-8", configPath)
@@ -120,13 +120,13 @@ func (v *Validator) CheckRoot() (errs error) {
120120
if v.platform == "windows" && v.spec.Windows != nil && v.spec.Windows.HyperV != nil {
121121
if v.spec.Root != nil {
122122
errs = multierror.Append(errs,
123-
rerr.NewError(rerr.RootOnHyperV, "for Hyper-V containers, Root must not be set", rspec.Version))
123+
specerror.NewError(specerror.RootOnHyperV, fmt.Errorf("for Hyper-V containers, Root must not be set"), rspec.Version))
124124
return
125125
}
126126
return
127127
} else if v.spec.Root == nil {
128128
errs = multierror.Append(errs,
129-
rerr.NewError(rerr.RootOnNonHyperV, "for non-Hyper-V containers, Root must be set", rspec.Version))
129+
specerror.NewError(specerror.RootOnNonHyperV, fmt.Errorf("for non-Hyper-V containers, Root must be set"), rspec.Version))
130130
return
131131
}
132132

@@ -138,7 +138,7 @@ func (v *Validator) CheckRoot() (errs error) {
138138

139139
if filepath.Base(v.spec.Root.Path) != "rootfs" {
140140
errs = multierror.Append(errs,
141-
rerr.NewError(rerr.PathName, "Path name should be the conventional 'rootfs'", rspec.Version))
141+
specerror.NewError(specerror.PathName, fmt.Errorf("path name should be the conventional 'rootfs'"), rspec.Version))
142142
}
143143

144144
var rootfsPath string
@@ -158,22 +158,22 @@ func (v *Validator) CheckRoot() (errs error) {
158158

159159
if fi, err := os.Stat(rootfsPath); err != nil {
160160
errs = multierror.Append(errs,
161-
rerr.NewError(rerr.PathExistence, fmt.Sprintf("Cannot find the root path %q", rootfsPath), rspec.Version))
161+
specerror.NewError(specerror.PathExistence, fmt.Errorf("cannot find the root path %q", rootfsPath), rspec.Version))
162162
} else if !fi.IsDir() {
163163
errs = multierror.Append(errs,
164-
rerr.NewError(rerr.PathExistence, fmt.Sprintf("The root path %q is not a directory", rootfsPath), rspec.Version))
164+
specerror.NewError(specerror.PathExistence, fmt.Errorf("root.path %q is not a directory", rootfsPath), rspec.Version))
165165
}
166166

167167
rootParent := filepath.Dir(absRootPath)
168168
if absRootPath == string(filepath.Separator) || rootParent != absBundlePath {
169169
errs = multierror.Append(errs,
170-
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))
170+
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))
171171
}
172172

173173
if v.platform == "windows" {
174174
if v.spec.Root.Readonly {
175175
errs = multierror.Append(errs,
176-
rerr.NewError(rerr.ReadonlyOnWindows, "root.readonly field MUST be omitted or false when target platform is windows", rspec.Version))
176+
specerror.NewError(specerror.ReadonlyOnWindows, fmt.Errorf("root.readonly field MUST be omitted or false when target platform is windows"), rspec.Version))
177177
}
178178
}
179179

@@ -188,7 +188,7 @@ func (v *Validator) CheckSemVer() (errs error) {
188188
_, err := semver.Parse(version)
189189
if err != nil {
190190
errs = multierror.Append(errs,
191-
rerr.NewError(rerr.SpecVersion, fmt.Sprintf("%q is not valid SemVer: %s", version, err.Error()), rspec.Version))
191+
specerror.NewError(specerror.SpecVersion, fmt.Errorf("%q is not valid SemVer: %s", version, err.Error()), rspec.Version))
192192
}
193193
if version != rspec.Version {
194194
errs = multierror.Append(errs, fmt.Errorf("validate currently only handles version %s, but the supplied configuration targets %s", rspec.Version, version))

0 commit comments

Comments
 (0)