From 2518d8c0fc018a4529238f629c26681ab5e0ed49 Mon Sep 17 00:00:00 2001 From: derekwaynecarr Date: Tue, 16 Jun 2015 16:16:34 -0400 Subject: [PATCH] Add LimitRange range validation --- pkg/api/validation/validation.go | 49 +++++++++++++++++ pkg/api/validation/validation_test.go | 76 +++++++++++++++++++++++++-- 2 files changed, 120 insertions(+), 5 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index f953098c5e8..964d2b2930d 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1315,15 +1315,64 @@ func ValidateLimitRange(limitRange *api.LimitRange) errs.ValidationErrorList { allErrs = append(allErrs, ValidateObjectMeta(&limitRange.ObjectMeta, true, ValidateLimitRangeName).Prefix("metadata")...) // ensure resource names are properly qualified per docs/resources.md + limitTypeSet := map[api.LimitType]bool{} for i := range limitRange.Spec.Limits { limit := limitRange.Spec.Limits[i] + _, found := limitTypeSet[limit.Type] + if found { + allErrs = append(allErrs, errs.NewFieldDuplicate(fmt.Sprintf("spec.limits[%d].type", i), limit.Type)) + } + limitTypeSet[limit.Type] = true + + keys := util.StringSet{} + min := map[string]int64{} + max := map[string]int64{} + defaults := map[string]int64{} + for k := range limit.Max { allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].max[%s]", i, k))...) + keys.Insert(string(k)) + q := limit.Max[k] + max[string(k)] = q.Value() } for k := range limit.Min { allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].min[%s]", i, k))...) + keys.Insert(string(k)) + q := limit.Min[k] + min[string(k)] = q.Value() + } + for k := range limit.Default { + allErrs = append(allErrs, validateResourceName(string(k), fmt.Sprintf("spec.limits[%d].default[%s]", i, k))...) + keys.Insert(string(k)) + q := limit.Default[k] + defaults[string(k)] = q.Value() + } + + for k := range keys { + minValue, minValueFound := min[k] + maxValue, maxValueFound := max[k] + defaultValue, defaultValueFound := defaults[k] + + if minValueFound && maxValueFound && minValue > maxValue { + minQuantity := limit.Min[api.ResourceName(k)] + maxQuantity := limit.Max[api.ResourceName(k)] + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].max[%s]", i, k), minValue, fmt.Sprintf("min value %s is greater than max value %s", minQuantity.String(), maxQuantity.String()))) + } + + if defaultValueFound && minValueFound && minValue > defaultValue { + minQuantity := limit.Min[api.ResourceName(k)] + defaultQuantity := limit.Default[api.ResourceName(k)] + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].max[%s]", i, k), minValue, fmt.Sprintf("min value %s is greater than default value %s", minQuantity.String(), defaultQuantity.String()))) + } + + if defaultValueFound && maxValueFound && defaultValue > maxValue { + maxQuantity := limit.Max[api.ResourceName(k)] + defaultQuantity := limit.Default[api.ResourceName(k)] + allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("spec.limits[%d].max[%s]", i, k), minValue, fmt.Sprintf("default value %s is greater than max value %s", defaultQuantity.String(), maxQuantity.String()))) + } } } + return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 21868c6d713..47d330b2f67 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -2670,6 +2670,64 @@ func TestValidateLimitRange(t *testing.T) { api.ResourceCPU: resource.MustParse("0"), api.ResourceMemory: resource.MustParse("100"), }, + Default: api.ResourceList{ + api.ResourceCPU: resource.MustParse("50"), + api.ResourceMemory: resource.MustParse("500"), + }, + }, + }, + } + + invalidSpecDuplicateType := api.LimitRangeSpec{ + Limits: []api.LimitRangeItem{ + { + Type: api.LimitTypePod, + Max: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + api.ResourceMemory: resource.MustParse("10000"), + }, + Min: api.ResourceList{ + api.ResourceCPU: resource.MustParse("0"), + api.ResourceMemory: resource.MustParse("100"), + }, + }, + { + Type: api.LimitTypePod, + Min: api.ResourceList{ + api.ResourceCPU: resource.MustParse("0"), + api.ResourceMemory: resource.MustParse("100"), + }, + }, + }, + } + + invalidSpecRangeMaxLessThanMin := api.LimitRangeSpec{ + Limits: []api.LimitRangeItem{ + { + Type: api.LimitTypePod, + Max: api.ResourceList{ + api.ResourceCPU: resource.MustParse("10"), + }, + Min: api.ResourceList{ + api.ResourceCPU: resource.MustParse("1000"), + }, + }, + }, + } + + invalidSpecRangeDefaultOutsideRange := api.LimitRangeSpec{ + Limits: []api.LimitRangeItem{ + { + Type: api.LimitTypePod, + Max: api.ResourceList{ + api.ResourceCPU: resource.MustParse("1000"), + }, + Min: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + }, + Default: api.ResourceList{ + api.ResourceCPU: resource.MustParse("2000"), + }, }, }, } @@ -2710,6 +2768,18 @@ func TestValidateLimitRange(t *testing.T) { api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "^Invalid"}, Spec: spec}, dns1123LabelErrorMsg, }, + "duplicate limit type": { + api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidSpecDuplicateType}, + "", + }, + "min value 1k is greater than max value 10": { + api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidSpecRangeMaxLessThanMin}, + "min value 1k is greater than max value 10", + }, + "invalid spec default outside range": { + api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidSpecRangeDefaultOutsideRange}, + "default value 2k is greater than max value 1k", + }, } for k, v := range errorCases { errs := ValidateLimitRange(&v.R) @@ -2717,11 +2787,7 @@ func TestValidateLimitRange(t *testing.T) { t.Errorf("expected failure for %s", k) } for i := range errs { - field := errs[i].(*errors.ValidationError).Field detail := errs[i].(*errors.ValidationError).Detail - if field != "metadata.name" && field != "metadata.namespace" { - t.Errorf("%s: missing prefix for: %v", k, errs[i]) - } if detail != v.D { t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail) } @@ -2787,7 +2853,7 @@ func TestValidateResourceQuota(t *testing.T) { field := errs[i].(*errors.ValidationError).Field detail := errs[i].(*errors.ValidationError).Detail if field != "metadata.name" && field != "metadata.namespace" { - t.Errorf("%s: missing prefix for: %v", k, errs[i]) + t.Errorf("%s: missing prefix for: %v", k, field) } if detail != v.D { t.Errorf("%s: expected error detail either empty or %s, got %s", k, v.D, detail)