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 PosixProcRlimitsTypeValueError 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].

Also make CheckRlimits a no-op on Windows, because the spec does not
define process.rlimits for that OS [5].

[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
[5]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#posix-process

Signed-off-by: W. Trevor King <wking@tremily.us>

Author: wking

specerror/config.go

@@ -46,6 +46,8 @@ const (
 	PosixProcRlimitsTypeGeneError = "The runtime MUST [generate an error](runtime.md#errors) for any values which cannot be mapped to a relevant kernel interface."
 	// PosixProcRlimitsTypeGet represents "For each entry in `rlimits`, a [`getrlimit(3)`][getrlimit.3] on `type` MUST succeed."
 	PosixProcRlimitsTypeGet = "For each entry in `rlimits`, a [`getrlimit(3)`][getrlimit.3] on `type` MUST succeed."
+	// PosixProcRlimitsTypeValueError represents "valid values are defined in the ... man page"
+	PosixProcRlimitsTypeValueError = "valid values are defined in the ... man page"
 	// PosixProcRlimitsSoftMatchCur represents "`rlim.rlim_cur` MUST match the configured value."
 	PosixProcRlimitsSoftMatchCur = "`rlim.rlim_cur` MUST match the configured value."
 	// PosixProcRlimitsHardMatchMax represents "`rlim.rlim_max` MUST match the configured value."
@@ -159,6 +161,7 @@ func init() {
 	register(ProcArgsOneEntryRequired, rfc2119.Required, processRef)
 	register(PosixProcRlimitsTypeGeneError, rfc2119.Must, posixProcessRef)
 	register(PosixProcRlimitsTypeGet, rfc2119.Must, posixProcessRef)
+	register(PosixProcRlimitsTypeValueError, rfc2119.Should, posixProcessRef)
 	register(PosixProcRlimitsSoftMatchCur, rfc2119.Must, posixProcessRef)
 	register(PosixProcRlimitsHardMatchMax, rfc2119.Must, posixProcessRef)
 	register(PosixProcRlimitsErrorOnDup, rfc2119.Must, posixProcessRef)

validate/validate.go

@@ -424,6 +424,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++ {
@@ -870,14 +874,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.PosixProcRlimitsTypeValueError, 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.PosixProcRlimitsTypeValueError, 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

@@ -188,3 +188,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.PosixProcRlimitsTypeValueError,
+		},
+		{
+			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.PosixProcRlimitsTypeValueError,
+		},
+	}
+	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))
+	}
+}