From df064bd53d258380711d8365dac4d1985e313e14 Mon Sep 17 00:00:00 2001 From: derekwaynecarr Date: Mon, 22 Feb 2016 11:14:11 -0500 Subject: [PATCH] ResourceQuota API validation for scopes and new resource types --- pkg/api/helpers.go | 73 ++++++++++++++++++ pkg/api/validation/validation.go | 105 ++++++++++++++++++++++---- pkg/api/validation/validation_test.go | 105 ++++++++++++++++++++++++++ 3 files changed, 270 insertions(+), 13 deletions(-) diff --git a/pkg/api/helpers.go b/pkg/api/helpers.go index b373b9eee81..b037b53c850 100644 --- a/pkg/api/helpers.go +++ b/pkg/api/helpers.go @@ -79,9 +79,82 @@ var Semantic = conversion.EqualitiesOrDie( }, ) +var standardResourceQuotaScopes = sets.NewString( + string(ResourceQuotaScopeTerminating), + string(ResourceQuotaScopeNotTerminating), + string(ResourceQuotaScopeBestEffort), + string(ResourceQuotaScopeNotBestEffort), +) + +// IsStandardResourceQuotaScope returns true if the scope is a standard value +func IsStandardResourceQuotaScope(str string) bool { + return standardResourceQuotaScopes.Has(str) +} + +var podObjectCountQuotaResources = sets.NewString( + string(ResourcePods), +) + +var podComputeQuotaResources = sets.NewString( + string(ResourceCPU), + string(ResourceMemory), + string(ResourceLimitsCPU), + string(ResourceLimitsMemory), + string(ResourceRequestsCPU), + string(ResourceRequestsMemory), +) + +// IsResourceQuotaScopeValidForResource returns true if the resource applies to the specified scope +func IsResourceQuotaScopeValidForResource(scope ResourceQuotaScope, resource string) bool { + switch scope { + case ResourceQuotaScopeTerminating, ResourceQuotaScopeNotTerminating, ResourceQuotaScopeNotBestEffort: + return podObjectCountQuotaResources.Has(resource) || podComputeQuotaResources.Has(resource) + case ResourceQuotaScopeBestEffort: + return podObjectCountQuotaResources.Has(resource) + default: + return true + } +} + +var standardContainerResources = sets.NewString( + string(ResourceCPU), + string(ResourceMemory), +) + +// IsStandardContainerResourceName returns true if the container can make a resource request +// for the specified resource +func IsStandardContainerResourceName(str string) bool { + return standardContainerResources.Has(str) +} + +var standardQuotaResources = sets.NewString( + string(ResourceCPU), + string(ResourceMemory), + string(ResourceRequestsCPU), + string(ResourceRequestsMemory), + string(ResourceLimitsCPU), + string(ResourceLimitsMemory), + string(ResourcePods), + string(ResourceQuotas), + string(ResourceServices), + string(ResourceReplicationControllers), + string(ResourceSecrets), + string(ResourcePersistentVolumeClaims), +) + +// IsStandardQuotaResourceName returns true if the resource is known to +// the quota tracking system +func IsStandardQuotaResourceName(str string) bool { + return standardQuotaResources.Has(str) +} + var standardResources = sets.NewString( string(ResourceCPU), string(ResourceMemory), + string(ResourceRequestsCPU), + string(ResourceRequestsMemory), + string(ResourceLimitsCPU), + string(ResourceLimitsMemory), string(ResourcePods), string(ResourceQuotas), string(ResourceServices), diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 47d94242f6b..5f0d8767be1 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -44,6 +44,7 @@ import ( var RepairMalformedUpdates bool = true const isNegativeErrorMsg string = `must be greater than or equal to 0` +const isInvalidQuotaResource string = `must be a standard resource for quota` const fieldImmutableErrorMsg string = `field is immutable` const cIdentifierErrorMsg string = `must be a C identifier (matching regex ` + validation.CIdentifierFmt + `): e.g. "my_name" or "MyName"` const isNotIntegerErrorMsg string = `must be an integer` @@ -1971,6 +1972,30 @@ func validateResourceName(value string, fldPath *field.Path) field.ErrorList { return field.ErrorList{} } +// Validate container resource name +// Refer to docs/design/resources.md for more details. +func validateContainerResourceName(value string, fldPath *field.Path) field.ErrorList { + allErrs := validateResourceName(value, fldPath) + if len(strings.Split(value, "/")) == 1 { + if !api.IsStandardContainerResourceName(value) { + return append(allErrs, field.Invalid(fldPath, value, "must be a standard resource for containers")) + } + } + return field.ErrorList{} +} + +// Validate resource names that can go in a resource quota +// Refer to docs/design/resources.md for more details. +func validateResourceQuotaResourceName(value string, fldPath *field.Path) field.ErrorList { + allErrs := validateResourceName(value, fldPath) + if len(strings.Split(value, "/")) == 1 { + if !api.IsStandardQuotaResourceName(value) { + return append(allErrs, field.Invalid(fldPath, value, isInvalidQuotaResource)) + } + } + return field.ErrorList{} +} + // ValidateLimitRange tests if required fields in the LimitRange are set. func ValidateLimitRange(limitRange *api.LimitRange) field.ErrorList { allErrs := ValidateObjectMeta(&limitRange.ObjectMeta, true, ValidateLimitRangeName, field.NewPath("metadata")) @@ -1995,12 +2020,12 @@ func ValidateLimitRange(limitRange *api.LimitRange) field.ErrorList { maxLimitRequestRatios := map[string]resource.Quantity{} for k, q := range limit.Max { - allErrs = append(allErrs, validateResourceName(string(k), idxPath.Child("max").Key(string(k)))...) + allErrs = append(allErrs, validateContainerResourceName(string(k), idxPath.Child("max").Key(string(k)))...) keys.Insert(string(k)) max[string(k)] = q } for k, q := range limit.Min { - allErrs = append(allErrs, validateResourceName(string(k), idxPath.Child("min").Key(string(k)))...) + allErrs = append(allErrs, validateContainerResourceName(string(k), idxPath.Child("min").Key(string(k)))...) keys.Insert(string(k)) min[string(k)] = q } @@ -2014,19 +2039,19 @@ func ValidateLimitRange(limitRange *api.LimitRange) field.ErrorList { } } else { for k, q := range limit.Default { - allErrs = append(allErrs, validateResourceName(string(k), idxPath.Child("default").Key(string(k)))...) + allErrs = append(allErrs, validateContainerResourceName(string(k), idxPath.Child("default").Key(string(k)))...) keys.Insert(string(k)) defaults[string(k)] = q } for k, q := range limit.DefaultRequest { - allErrs = append(allErrs, validateResourceName(string(k), idxPath.Child("defaultRequest").Key(string(k)))...) + allErrs = append(allErrs, validateContainerResourceName(string(k), idxPath.Child("defaultRequest").Key(string(k)))...) keys.Insert(string(k)) defaultRequests[string(k)] = q } } for k, q := range limit.MaxLimitRequestRatio { - allErrs = append(allErrs, validateResourceName(string(k), idxPath.Child("maxLimitRequestRatio").Key(string(k)))...) + allErrs = append(allErrs, validateContainerResourceName(string(k), idxPath.Child("maxLimitRequestRatio").Key(string(k)))...) keys.Insert(string(k)) maxLimitRequestRatios[string(k)] = q } @@ -2249,7 +2274,7 @@ func ValidateResourceRequirements(requirements *api.ResourceRequirements, fldPat for resourceName, quantity := range requirements.Limits { fldPath := limPath.Key(string(resourceName)) // Validate resource name. - allErrs = append(allErrs, validateResourceName(string(resourceName), fldPath)...) + allErrs = append(allErrs, validateContainerResourceName(string(resourceName), fldPath)...) if api.IsStandardResourceName(string(resourceName)) { allErrs = append(allErrs, validateBasicResource(quantity, fldPath.Key(string(resourceName)))...) } @@ -2265,7 +2290,7 @@ func ValidateResourceRequirements(requirements *api.ResourceRequirements, fldPat for resourceName, quantity := range requirements.Requests { fldPath := reqPath.Key(string(resourceName)) // Validate resource name. - allErrs = append(allErrs, validateResourceName(string(resourceName), fldPath)...) + allErrs = append(allErrs, validateContainerResourceName(string(resourceName), fldPath)...) if api.IsStandardResourceName(string(resourceName)) { allErrs = append(allErrs, validateBasicResource(quantity, fldPath.Key(string(resourceName)))...) } @@ -2273,6 +2298,41 @@ func ValidateResourceRequirements(requirements *api.ResourceRequirements, fldPat return allErrs } +// validateResourceQuotaScopes ensures that each enumerated hard resource constraint is valid for set of scopes +func validateResourceQuotaScopes(resourceQuota *api.ResourceQuota) field.ErrorList { + allErrs := field.ErrorList{} + if len(resourceQuota.Spec.Scopes) == 0 { + return allErrs + } + hardLimits := sets.NewString() + for k := range resourceQuota.Spec.Hard { + hardLimits.Insert(string(k)) + } + fldPath := field.NewPath("spec", "scopes") + scopeSet := sets.NewString() + for _, scope := range resourceQuota.Spec.Scopes { + if !api.IsStandardResourceQuotaScope(string(scope)) { + allErrs = append(allErrs, field.Invalid(fldPath, resourceQuota.Spec.Scopes, "unsupported scope")) + } + for _, k := range hardLimits.List() { + if api.IsStandardQuotaResourceName(k) && !api.IsResourceQuotaScopeValidForResource(scope, k) { + allErrs = append(allErrs, field.Invalid(fldPath, resourceQuota.Spec.Scopes, "unsupported scope applied to resource")) + } + } + scopeSet.Insert(string(scope)) + } + invalidScopePairs := []sets.String{ + sets.NewString(string(api.ResourceQuotaScopeBestEffort), string(api.ResourceQuotaScopeNotBestEffort)), + sets.NewString(string(api.ResourceQuotaScopeTerminating), string(api.ResourceQuotaScopeNotTerminating)), + } + for _, invalidScopePair := range invalidScopePairs { + if scopeSet.HasAll(invalidScopePair.List()...) { + allErrs = append(allErrs, field.Invalid(fldPath, resourceQuota.Spec.Scopes, "conflicting scopes")) + } + } + return allErrs +} + // ValidateResourceQuota tests if required fields in the ResourceQuota are set. func ValidateResourceQuota(resourceQuota *api.ResourceQuota) field.ErrorList { allErrs := ValidateObjectMeta(&resourceQuota.ObjectMeta, true, ValidateResourceQuotaName, field.NewPath("metadata")) @@ -2280,21 +2340,24 @@ func ValidateResourceQuota(resourceQuota *api.ResourceQuota) field.ErrorList { fldPath := field.NewPath("spec", "hard") for k, v := range resourceQuota.Spec.Hard { resPath := fldPath.Key(string(k)) - allErrs = append(allErrs, validateResourceName(string(k), resPath)...) + allErrs = append(allErrs, validateResourceQuotaResourceName(string(k), resPath)...) allErrs = append(allErrs, validateResourceQuantityValue(string(k), v, resPath)...) } + allErrs = append(allErrs, validateResourceQuotaScopes(resourceQuota)...) + fldPath = field.NewPath("status", "hard") for k, v := range resourceQuota.Status.Hard { resPath := fldPath.Key(string(k)) - allErrs = append(allErrs, validateResourceName(string(k), resPath)...) + allErrs = append(allErrs, validateResourceQuotaResourceName(string(k), resPath)...) allErrs = append(allErrs, validateResourceQuantityValue(string(k), v, resPath)...) } fldPath = field.NewPath("status", "used") for k, v := range resourceQuota.Status.Used { resPath := fldPath.Key(string(k)) - allErrs = append(allErrs, validateResourceName(string(k), resPath)...) + allErrs = append(allErrs, validateResourceQuotaResourceName(string(k), resPath)...) allErrs = append(allErrs, validateResourceQuantityValue(string(k), v, resPath)...) } + return allErrs } @@ -2317,9 +2380,25 @@ func ValidateResourceQuotaUpdate(newResourceQuota, oldResourceQuota *api.Resourc fldPath := field.NewPath("spec", "hard") for k, v := range newResourceQuota.Spec.Hard { resPath := fldPath.Key(string(k)) - allErrs = append(allErrs, validateResourceName(string(k), resPath)...) + allErrs = append(allErrs, validateResourceQuotaResourceName(string(k), resPath)...) allErrs = append(allErrs, validateResourceQuantityValue(string(k), v, resPath)...) } + + // ensure scopes cannot change, and that resources are still valid for scope + fldPath = field.NewPath("spec", "scopes") + oldScopes := sets.NewString() + newScopes := sets.NewString() + for _, scope := range newResourceQuota.Spec.Scopes { + newScopes.Insert(string(scope)) + } + for _, scope := range oldResourceQuota.Spec.Scopes { + oldScopes.Insert(string(scope)) + } + if !oldScopes.Equal(newScopes) { + allErrs = append(allErrs, field.Invalid(fldPath, newResourceQuota.Spec.Scopes, "field is immutable")) + } + allErrs = append(allErrs, validateResourceQuotaScopes(newResourceQuota)...) + newResourceQuota.Status = oldResourceQuota.Status return allErrs } @@ -2334,13 +2413,13 @@ func ValidateResourceQuotaStatusUpdate(newResourceQuota, oldResourceQuota *api.R fldPath := field.NewPath("status", "hard") for k, v := range newResourceQuota.Status.Hard { resPath := fldPath.Key(string(k)) - allErrs = append(allErrs, validateResourceName(string(k), resPath)...) + allErrs = append(allErrs, validateResourceQuotaResourceName(string(k), resPath)...) allErrs = append(allErrs, validateResourceQuantityValue(string(k), v, resPath)...) } fldPath = field.NewPath("status", "used") for k, v := range newResourceQuota.Status.Used { resPath := fldPath.Key(string(k)) - allErrs = append(allErrs, validateResourceName(string(k), resPath)...) + allErrs = append(allErrs, validateResourceQuotaResourceName(string(k), resPath)...) allErrs = append(allErrs, validateResourceQuantityValue(string(k), v, resPath)...) } newResourceQuota.Spec = oldResourceQuota.Spec diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 4c60d66f895..daa43655165 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -4044,6 +4044,10 @@ func TestValidateResourceQuota(t *testing.T) { Hard: api.ResourceList{ api.ResourceCPU: resource.MustParse("100"), api.ResourceMemory: resource.MustParse("10000"), + api.ResourceRequestsCPU: resource.MustParse("100"), + api.ResourceRequestsMemory: resource.MustParse("10000"), + api.ResourceLimitsCPU: resource.MustParse("100"), + api.ResourceLimitsMemory: resource.MustParse("10000"), api.ResourcePods: resource.MustParse("10"), api.ResourceServices: resource.MustParse("0"), api.ResourceReplicationControllers: resource.MustParse("10"), @@ -4051,6 +4055,42 @@ func TestValidateResourceQuota(t *testing.T) { }, } + terminatingSpec := api.ResourceQuotaSpec{ + Hard: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + api.ResourceLimitsCPU: resource.MustParse("200"), + }, + Scopes: []api.ResourceQuotaScope{api.ResourceQuotaScopeTerminating}, + } + + nonTerminatingSpec := api.ResourceQuotaSpec{ + Hard: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + }, + Scopes: []api.ResourceQuotaScope{api.ResourceQuotaScopeNotTerminating}, + } + + bestEffortSpec := api.ResourceQuotaSpec{ + Hard: api.ResourceList{ + api.ResourcePods: resource.MustParse("100"), + }, + Scopes: []api.ResourceQuotaScope{api.ResourceQuotaScopeBestEffort}, + } + + nonBestEffortSpec := api.ResourceQuotaSpec{ + Hard: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + }, + Scopes: []api.ResourceQuotaScope{api.ResourceQuotaScopeNotBestEffort}, + } + + // storage is not yet supported as a quota tracked resource + invalidQuotaResourceSpec := api.ResourceQuotaSpec{ + Hard: api.ResourceList{ + api.ResourceStorage: resource.MustParse("10"), + }, + } + negativeSpec := api.ResourceQuotaSpec{ Hard: api.ResourceList{ api.ResourceCPU: resource.MustParse("-100"), @@ -4077,6 +4117,27 @@ func TestValidateResourceQuota(t *testing.T) { }, } + invalidTerminatingScopePairsSpec := api.ResourceQuotaSpec{ + Hard: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + }, + Scopes: []api.ResourceQuotaScope{api.ResourceQuotaScopeTerminating, api.ResourceQuotaScopeNotTerminating}, + } + + invalidBestEffortScopePairsSpec := api.ResourceQuotaSpec{ + Hard: api.ResourceList{ + api.ResourcePods: resource.MustParse("100"), + }, + Scopes: []api.ResourceQuotaScope{api.ResourceQuotaScopeBestEffort, api.ResourceQuotaScopeNotBestEffort}, + } + + invalidScopeNameSpec := api.ResourceQuotaSpec{ + Hard: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + }, + Scopes: []api.ResourceQuotaScope{api.ResourceQuotaScope("foo")}, + } + successCases := []api.ResourceQuota{ { ObjectMeta: api.ObjectMeta{ @@ -4092,6 +4153,34 @@ func TestValidateResourceQuota(t *testing.T) { }, Spec: fractionalComputeSpec, }, + { + ObjectMeta: api.ObjectMeta{ + Name: "abc", + Namespace: "foo", + }, + Spec: terminatingSpec, + }, + { + ObjectMeta: api.ObjectMeta{ + Name: "abc", + Namespace: "foo", + }, + Spec: nonTerminatingSpec, + }, + { + ObjectMeta: api.ObjectMeta{ + Name: "abc", + Namespace: "foo", + }, + Spec: bestEffortSpec, + }, + { + ObjectMeta: api.ObjectMeta{ + Name: "abc", + Namespace: "foo", + }, + Spec: nonBestEffortSpec, + }, } for _, successCase := range successCases { @@ -4128,6 +4217,22 @@ func TestValidateResourceQuota(t *testing.T) { api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: fractionalPodSpec}, isNotIntegerErrorMsg, }, + "invalid-quota-resource": { + api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidQuotaResourceSpec}, + isInvalidQuotaResource, + }, + "invalid-quota-terminating-pair": { + api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidTerminatingScopePairsSpec}, + "conflicting scopes", + }, + "invalid-quota-besteffort-pair": { + api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidBestEffortScopePairsSpec}, + "conflicting scopes", + }, + "invalid-quota-scope-name": { + api.ResourceQuota{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidScopeNameSpec}, + "unsupported scope", + }, } for k, v := range errorCases { errs := ValidateResourceQuota(&v.R)