Merge pull request #481 from wking/warn-on-potentially-invalid-rlimits

validate: Soften unrecognized rlimit types to SHOULD violations

Author: Ma Shimiao

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))
+	}
+}