validate: Soften unrecognized rlimit types to SHOULD violations

The spec isn't particuarly clear on this, saying [1]:

  * Linux: valid values are defined in the
    [`getrlimit(2)`][getrlimit.2] man page, such as `RLIMIT_MSGQUEUE`.
  * Solaris: valid values are defined in the
    [`getrlimit(3)`][getrlimit.3] man page, such as `RLIMIT_CORE`.

and [2]:

  For each entry in `rlimits`, a [`getrlimit(3)`][getrlimit.3] on
  `type` MUST succeed.

It doesn't say:

  Linux: The value MUST be listed in the getrlimit(2) man page...

and it doesn't require the runtime to support the values listed in the
man page [3,4].  So there are three sets:

* Values listed in the man page
* Values supported by the host kernel
* Values supported by the runtime

And as the spec stands, these sets are only weakly coupled, and any of
them could be a sub- or superset of any other.  In practice, I expect
the sets to all coincide, with the kernel occasionally adding or
removing values, and the man page and runtimes trailing along behind.

To address that, this commit weakens the previous hard error to a
SHOULD-level error.  The PosixProcRlimitsValueError constant is new to
this commit, because the spec contains neither a MUST nor a SHOULD for
this condition, although I expect a SHOULD-level suggestion was
implied by [1].  The posixProcRef constant is cherry-picked from
27503c5 (complete spec errors of config.md, 2017-09-05, opencontainers#458).

[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[3]: opencontainers/runtime-spec#813
[4]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L463

Signed-off-by: W. Trevor King <[email protected]>

specerror/error.go

@@ -44,6 +44,9 @@ const (
 	// ReadonlyOnWindows represents the error code of readonly setting test on Windows
 	ReadonlyOnWindows
 
+	// PosixProcRlimitsValueError represents "valid values are defined in the ... man page"
+	PosixProcRlimitsValueError
+
 	// DefaultFilesystems represents the error code of default filesystems test
 	DefaultFilesystems
 
@@ -79,6 +82,9 @@ var (
 	rootRef = func(version string) (reference string, err error) {
 		return fmt.Sprintf(referenceTemplate, version, "config.md#root"), nil
 	}
+	posixProcRef = func(version string) (reference string, err error) {
+		return fmt.Sprintf(referenceTemplate, version, "config.md#posix-process"), nil
+	}
 	defaultFSRef = func(version string) (reference string, err error) {
 		return fmt.Sprintf(referenceTemplate, version, "config-linux.md#default-filesystems"), nil
 	}
@@ -106,6 +112,8 @@ var ociErrors = map[Code]errorTemplate{
 	ReadonlyFilesystem:  {Level: rfc2119.Must, Reference: rootRef},
 	ReadonlyOnWindows:   {Level: rfc2119.Must, Reference: rootRef},
 
+	PosixProcRlimitsValueError: {Level: rfc2119.Should, Reference: posixProcRef},
+
 	// Config-Linux.md
 	// Default Filesystems
 	DefaultFilesystems: {Level: rfc2119.Should, Reference: defaultFSRef},

validate/validate.go

@@ -51,7 +51,7 @@ var (
 		"RLIMIT_RTPRIO",
 		"RLIMIT_RTTIME",
 		"RLIMIT_SIGPENDING",
-	})
+	}...)
 
 	configSchemaTemplate = "https://raw.githubusercontent.com/opencontainers/runtime-spec/v%s/schema/config-schema.json"
 )
@@ -423,6 +423,10 @@ func (v *Validator) CheckCapabilities() (errs error) {
 
 // CheckRlimits checks v.spec.Process.Rlimits
 func (v *Validator) CheckRlimits() (errs error) {
+	if v.platform == "windows" {
+		return
+	}
+
 	process := v.spec.Process
 	for index, rlimit := range process.Rlimits {
 		for i := index + 1; i < len(process.Rlimits); i++ {
@@ -878,14 +882,14 @@ func (v *Validator) rlimitValid(rlimit rspec.POSIXRlimit) (errs error) {
 				return
 			}
 		}
-		errs = multierror.Append(errs, fmt.Errorf("rlimit type %q is invalid", rlimit.Type))
+		errs = multierror.Append(errs, specerror.NewError(specerror.PosixProcRlimitsValueError, fmt.Errorf("rlimit type %q may not be valid", rlimit.Type), v.spec.Version))
 	} else if v.platform == "solaris" {
 		for _, val := range posixRlimits {
 			if val == rlimit.Type {
 				return
 			}
 		}
-		errs = multierror.Append(errs, fmt.Errorf("rlimit type %q is invalid", rlimit.Type))
+		errs = multierror.Append(errs, specerror.NewError(specerror.PosixProcRlimitsValueError, fmt.Errorf("rlimit type %q may not be valid", rlimit.Type), v.spec.Version))
 	} else {
 		logrus.Warnf("process.rlimits validation not yet implemented for platform %q", v.platform)
 	}

validate/validate_test.go

@@ -154,3 +154,105 @@ func TestCheckSemVer(t *testing.T) {
 		assert.Equal(t, c.expected, specerror.FindError(err, c.expected), "Fail to check SemVer "+c.val)
 	}
 }
+
+func TestCheckProcess(t *testing.T) {
+	cases := []struct {
+		val      rspec.Spec
+		platform string
+		expected specerror.Code
+	}{
+		{
+			val: rspec.Spec{
+				Version: "1.0.0",
+				Process: &rspec.Process{
+					Args: []string{"sh"},
+					Cwd:  "/",
+				},
+			},
+			platform: "linux",
+			expected: specerror.NonError,
+		},
+		{
+			val: rspec.Spec{
+				Version: "1.0.0",
+				Process: &rspec.Process{
+					Args: []string{"sh"},
+					Cwd:  "/",
+					Rlimits: []rspec.POSIXRlimit{
+						{
+							Type: "RLIMIT_NOFILE",
+							Hard: 1024,
+							Soft: 1024,
+						},
+						{
+							Type: "RLIMIT_NPROC",
+							Hard: 512,
+							Soft: 512,
+						},
+					},
+				},
+			},
+			platform: "linux",
+			expected: specerror.NonError,
+		},
+		{
+			val: rspec.Spec{
+				Version: "1.0.0",
+				Process: &rspec.Process{
+					Args: []string{"sh"},
+					Cwd:  "/",
+					Rlimits: []rspec.POSIXRlimit{
+						{
+							Type: "RLIMIT_NOFILE",
+							Hard: 1024,
+							Soft: 1024,
+						},
+					},
+				},
+			},
+			platform: "solaris",
+			expected: specerror.NonError,
+		},
+		{
+			val: rspec.Spec{
+				Version: "1.0.0",
+				Process: &rspec.Process{
+					Args: []string{"sh"},
+					Cwd:  "/",
+					Rlimits: []rspec.POSIXRlimit{
+						{
+							Type: "RLIMIT_DOES_NOT_EXIST",
+							Hard: 512,
+							Soft: 512,
+						},
+					},
+				},
+			},
+			platform: "linux",
+			expected: specerror.PosixProcRlimitsValueError,
+		},
+		{
+			val: rspec.Spec{
+				Version: "1.0.0",
+				Process: &rspec.Process{
+					Args: []string{"sh"},
+					Cwd:  "/",
+					Rlimits: []rspec.POSIXRlimit{
+						{
+							Type: "RLIMIT_NPROC",
+							Hard: 512,
+							Soft: 512,
+						},
+					},
+				},
+			},
+			platform: "solaris",
+			expected: specerror.PosixProcRlimitsValueError,
+		},
+	}
+	for _, c := range cases {
+		v := NewValidator(&c.val, ".", false, c.platform)
+		err := v.CheckProcess()
+		assert.Equal(t, c.expected, specerror.FindError(err, c.expected), fmt.Sprintf("failed CheckProcess: %v %d", err, c.expected))
+	}
+}