Merge pull request #14495 from derekwaynecarr/improve_quota_validation

pkg/api/validation/validation.go

@@ -220,6 +220,15 @@ func ValidatePositiveField(value int64, fieldName string) errs.ValidationErrorLi
 	return allErrs
 }
 
+// Validates that a Quantity is not negative
+func ValidatePositiveQuantity(value resource.Quantity, fieldName string) errs.ValidationErrorList {
+	allErrs := errs.ValidationErrorList{}
+	if value.Cmp(resource.Quantity{}) < 0 {
+		allErrs = append(allErrs, errs.NewFieldInvalid(fieldName, value.String(), isNegativeErrorMsg))
+	}
+	return allErrs
+}
+
 // ValidateObjectMeta validates an object's metadata on creation. It expects that name generation has already
 // been performed.
 func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn ValidateNameFunc) errs.ValidationErrorList {
@@ -1701,14 +1710,17 @@ func ValidateResourceQuota(resourceQuota *api.ResourceQuota) errs.ValidationErro
 	allErrs := errs.ValidationErrorList{}
 	allErrs = append(allErrs, ValidateObjectMeta(&resourceQuota.ObjectMeta, true, ValidateResourceQuotaName).Prefix("metadata")...)
 
-	for k := range resourceQuota.Spec.Hard {
+	for k, v := range resourceQuota.Spec.Hard {
 		allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
+		allErrs = append(allErrs, ValidatePositiveQuantity(v, string(k))...)
 	}
-	for k := range resourceQuota.Status.Hard {
+	for k, v := range resourceQuota.Status.Hard {
 		allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
+		allErrs = append(allErrs, ValidatePositiveQuantity(v, string(k))...)
 	}
-	for k := range resourceQuota.Status.Used {
+	for k, v := range resourceQuota.Status.Used {
 		allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
+		allErrs = append(allErrs, ValidatePositiveQuantity(v, string(k))...)
 	}
 	return allErrs
 }

pkg/api/validation/validation_test.go

@@ -3131,12 +3131,23 @@ func TestValidateResourceQuota(t *testing.T) {
 			api.ResourceCPU:                    resource.MustParse("100"),
 			api.ResourceMemory:                 resource.MustParse("10000"),
 			api.ResourcePods:                   resource.MustParse("10"),
-			api.ResourceServices:               resource.MustParse("10"),
+			api.ResourceServices:               resource.MustParse("0"),
 			api.ResourceReplicationControllers: resource.MustParse("10"),
 			api.ResourceQuotas:                 resource.MustParse("10"),
 		},
 	}
 
+	negativeSpec := api.ResourceQuotaSpec{
+		Hard: api.ResourceList{
+			api.ResourceCPU:                    resource.MustParse("-100"),
+			api.ResourceMemory:                 resource.MustParse("-10000"),
+			api.ResourcePods:                   resource.MustParse("-10"),
+			api.ResourceServices:               resource.MustParse("-10"),
+			api.ResourceReplicationControllers: resource.MustParse("-10"),
+			api.ResourceQuotas:                 resource.MustParse("-10"),
+		},
+	}
+
 	successCases := []api.ResourceQuota{
 		{
 			ObjectMeta: api.ObjectMeta{
@@ -3173,6 +3184,10 @@ func TestValidateResourceQuota(t *testing.T) {
 			api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: spec},
 			DNS1123LabelErrorMsg,
 		},
+		"negative-limits": {
+			api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: negativeSpec},
+			isNegativeErrorMsg,
+		},
 	}
 	for k, v := range errorCases {
 		errs := ValidateResourceQuota(&v.R)
@@ -3182,7 +3197,7 @@ func TestValidateResourceQuota(t *testing.T) {
 		for i := range errs {
 			field := errs[i].(*errors.ValidationError).Field
 			detail := errs[i].(*errors.ValidationError).Detail
-			if field != "metadata.name" && field != "metadata.namespace" {
+			if field != "metadata.name" && field != "metadata.namespace" && !api.IsStandardResourceName(field) {
 				t.Errorf("%s: missing prefix for: %v", k, field)
 			}
 			if detail != v.D {