Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource Quota should not let fractional values for API resources #15574

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 18 additions & 2 deletions pkg/api/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,20 +80,36 @@ var Semantic = conversion.EqualitiesOrDie(
)

var standardResources = sets.NewString(
string(ResourceMemory),
string(ResourceCPU),
string(ResourceMemory),
string(ResourcePods),
string(ResourceQuotas),
string(ResourceServices),
string(ResourceReplicationControllers),
string(ResourceSecrets),
string(ResourcePersistentVolumeClaims),
string(ResourceStorage))
string(ResourceStorage),
)

// IsStandardResourceName returns true if the resource is known to the system
func IsStandardResourceName(str string) bool {
return standardResources.Has(str)
}

var integerResources = sets.NewString(
string(ResourcePods),
string(ResourceQuotas),
string(ResourceServices),
string(ResourceReplicationControllers),
string(ResourceSecrets),
string(ResourcePersistentVolumeClaims),
)

// IsIntegerResourceName returns true if the resource is measured in integer values
func IsIntegerResourceName(str string) bool {
return integerResources.Has(str)
}

// NewDeleteOptions returns a DeleteOptions indicating the resource should
// be deleted within the specified grace period. Use zero to indicate
// immediate deletion. If you would prefer to use the default grace period,
Expand Down
28 changes: 22 additions & 6 deletions pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (

const cIdentifierErrorMsg string = `must be a C identifier (matching regex ` + validation.CIdentifierFmt + `): e.g. "my_name" or "MyName"`
const isNegativeErrorMsg string = `must be non-negative`
const isNotIntegerErrorMsg string = `must be an integer`

func intervalErrorMsg(lo, hi int) string {
return fmt.Sprintf(`must be greater than %d and less than %d`, lo, hi)
Expand Down Expand Up @@ -1751,15 +1752,27 @@ func ValidateResourceQuota(resourceQuota *api.ResourceQuota) errs.ValidationErro

for k, v := range resourceQuota.Spec.Hard {
allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
allErrs = append(allErrs, ValidatePositiveQuantity(v, string(k))...)
allErrs = append(allErrs, validateResourceQuantityValue(string(k), v)...)
}
for k, v := range resourceQuota.Status.Hard {
allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
allErrs = append(allErrs, ValidatePositiveQuantity(v, string(k))...)
allErrs = append(allErrs, validateResourceQuantityValue(string(k), v)...)
}
for k, v := range resourceQuota.Status.Used {
allErrs = append(allErrs, validateResourceName(string(k), string(resourceQuota.TypeMeta.Kind))...)
allErrs = append(allErrs, ValidatePositiveQuantity(v, string(k))...)
allErrs = append(allErrs, validateResourceQuantityValue(string(k), v)...)
}
return allErrs
}

// validateResourceQuantityValue enforces that specified quantity is valid for specified resource
func validateResourceQuantityValue(resource string, value resource.Quantity) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
allErrs = append(allErrs, ValidatePositiveQuantity(value, resource)...)
if api.IsIntegerResourceName(resource) {
if value.MilliValue()%int64(1000) != int64(0) {
allErrs = append(allErrs, errs.NewFieldInvalid(resource, value, isNotIntegerErrorMsg))
}
}
return allErrs
}
Expand All @@ -1769,8 +1782,9 @@ func ValidateResourceQuota(resourceQuota *api.ResourceQuota) errs.ValidationErro
func ValidateResourceQuotaUpdate(newResourceQuota, oldResourceQuota *api.ResourceQuota) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
allErrs = append(allErrs, ValidateObjectMetaUpdate(&newResourceQuota.ObjectMeta, &oldResourceQuota.ObjectMeta).Prefix("metadata")...)
for k := range newResourceQuota.Spec.Hard {
for k, v := range newResourceQuota.Spec.Hard {
allErrs = append(allErrs, validateResourceName(string(k), string(newResourceQuota.TypeMeta.Kind))...)
allErrs = append(allErrs, validateResourceQuantityValue(string(k), v)...)
}
newResourceQuota.Status = oldResourceQuota.Status
return allErrs
Expand All @@ -1784,11 +1798,13 @@ func ValidateResourceQuotaStatusUpdate(newResourceQuota, oldResourceQuota *api.R
if newResourceQuota.ResourceVersion == "" {
allErrs = append(allErrs, errs.NewFieldRequired("resourceVersion"))
}
for k := range newResourceQuota.Status.Hard {
for k, v := range newResourceQuota.Status.Hard {
allErrs = append(allErrs, validateResourceName(string(k), string(newResourceQuota.TypeMeta.Kind))...)
allErrs = append(allErrs, validateResourceQuantityValue(string(k), v)...)
}
for k := range newResourceQuota.Status.Used {
for k, v := range newResourceQuota.Status.Used {
allErrs = append(allErrs, validateResourceName(string(k), string(newResourceQuota.TypeMeta.Kind))...)
allErrs = append(allErrs, validateResourceQuantityValue(string(k), v)...)
}
newResourceQuota.Spec = oldResourceQuota.Spec
return allErrs
Expand Down
26 changes: 26 additions & 0 deletions pkg/api/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3244,6 +3244,21 @@ func TestValidateResourceQuota(t *testing.T) {
},
}

fractionalComputeSpec := api.ResourceQuotaSpec{
Hard: api.ResourceList{
api.ResourceCPU: resource.MustParse("100m"),
},
}

fractionalPodSpec := api.ResourceQuotaSpec{
Hard: api.ResourceList{
api.ResourcePods: resource.MustParse(".1"),
api.ResourceServices: resource.MustParse(".5"),
api.ResourceReplicationControllers: resource.MustParse("1.25"),
api.ResourceQuotas: resource.MustParse("2.5"),
},
}

successCases := []api.ResourceQuota{
{
ObjectMeta: api.ObjectMeta{
Expand All @@ -3252,6 +3267,13 @@ func TestValidateResourceQuota(t *testing.T) {
},
Spec: spec,
},
{
ObjectMeta: api.ObjectMeta{
Name: "abc",
Namespace: "foo",
},
Spec: fractionalComputeSpec,
},
}

for _, successCase := range successCases {
Expand Down Expand Up @@ -3284,6 +3306,10 @@ func TestValidateResourceQuota(t *testing.T) {
api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: negativeSpec},
isNegativeErrorMsg,
},
"fractional-api-resource": {
api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: fractionalPodSpec},
isNotIntegerErrorMsg,
},
}
for k, v := range errorCases {
errs := ValidateResourceQuota(&v.R)
Expand Down