diff --git a/pkg/BUILD b/pkg/BUILD index 6f74b6995c..04431003b8 100644 --- a/pkg/BUILD +++ b/pkg/BUILD @@ -19,6 +19,7 @@ filegroup( "//pkg/api/podsecuritypolicy:all-srcs", "//pkg/api/ref:all-srcs", "//pkg/api/resource:all-srcs", + "//pkg/api/resourcequota:all-srcs", "//pkg/api/service:all-srcs", "//pkg/api/testapi:all-srcs", "//pkg/api/testing:all-srcs", diff --git a/pkg/api/resourcequota/BUILD b/pkg/api/resourcequota/BUILD new file mode 100644 index 0000000000..04e56ae501 --- /dev/null +++ b/pkg/api/resourcequota/BUILD @@ -0,0 +1,40 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["util.go"], + importpath = "k8s.io/kubernetes/pkg/api/resourcequota", + visibility = ["//visibility:public"], + deps = [ + "//pkg/apis/core:go_default_library", + "//pkg/features:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["util_test.go"], + embed = [":go_default_library"], + deps = [ + "//pkg/apis/core:go_default_library", + "//pkg/features:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature/testing:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/pkg/api/resourcequota/util.go b/pkg/api/resourcequota/util.go new file mode 100644 index 0000000000..322472011f --- /dev/null +++ b/pkg/api/resourcequota/util.go @@ -0,0 +1,41 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resourcequota + +import ( + utilfeature "k8s.io/apiserver/pkg/util/feature" + api "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/features" +) + +// DropDisabledFields removes disabled fields from the ResourceQuota spec. +// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a ResourceQuota spec. +func DropDisabledFields(resSpec *api.ResourceQuotaSpec, oldResSpec *api.ResourceQuotaSpec) { + if !utilfeature.DefaultFeatureGate.Enabled(features.ResourceQuotaScopeSelectors) && !resourceQuotaScopeSelectorInUse(oldResSpec) { + resSpec.ScopeSelector = nil + } +} + +func resourceQuotaScopeSelectorInUse(oldResSpec *api.ResourceQuotaSpec) bool { + if oldResSpec == nil { + return false + } + if oldResSpec.ScopeSelector != nil { + return true + } + return false +} diff --git a/pkg/api/resourcequota/util_test.go b/pkg/api/resourcequota/util_test.go new file mode 100644 index 0000000000..666c45bfe8 --- /dev/null +++ b/pkg/api/resourcequota/util_test.go @@ -0,0 +1,117 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resourcequota + +import ( + "fmt" + "reflect" + "testing" + + "k8s.io/apimachinery/pkg/util/diff" + utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" + api "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/features" +) + +func TestDropDisabledFields(t *testing.T) { + rqWithScopeSelector := func() *api.ResourceQuota { + return &api.ResourceQuota{Spec: api.ResourceQuotaSpec{Scopes: []api.ResourceQuotaScope{"scope-1"}, ScopeSelector: &api.ScopeSelector{ + MatchExpressions: []api.ScopedResourceSelectorRequirement{ + { + ScopeName: api.ResourceQuotaScopePriorityClass, + Operator: api.ScopeSelectorOpIn, + Values: []string{"scope-1"}, + }, + }, + }}} + } + rqWithoutScopeSelector := func() *api.ResourceQuota { + return &api.ResourceQuota{Spec: api.ResourceQuotaSpec{Scopes: []api.ResourceQuotaScope{"scope-1"}, ScopeSelector: nil}} + } + + rqInfo := []struct { + description string + hasScopeSelector bool + resourceQuota func() *api.ResourceQuota + }{ + { + description: "ResourceQuota without Scopes Selector", + hasScopeSelector: false, + resourceQuota: rqWithoutScopeSelector, + }, + { + description: "ResourceQuota with Scope Selector", + hasScopeSelector: true, + resourceQuota: rqWithScopeSelector, + }, + { + description: "is nil", + hasScopeSelector: false, + resourceQuota: func() *api.ResourceQuota { return nil }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldRQInfo := range rqInfo { + for _, newRQInfo := range rqInfo { + oldRQHasSelector, oldrq := oldRQInfo.hasScopeSelector, oldRQInfo.resourceQuota() + newRQHasSelector, newrq := newRQInfo.hasScopeSelector, newRQInfo.resourceQuota() + if newrq == nil { + continue + } + + t.Run(fmt.Sprintf("feature enabled=%v, old ResourceQuota %v, new ResourceQuota %v", enabled, oldRQInfo.description, newRQInfo.description), func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ResourceQuotaScopeSelectors, enabled)() + + var oldRQSpec *api.ResourceQuotaSpec + if oldrq != nil { + oldRQSpec = &oldrq.Spec + } + DropDisabledFields(&newrq.Spec, oldRQSpec) + + // old ResourceQuota should never be changed + if !reflect.DeepEqual(oldrq, oldRQInfo.resourceQuota()) { + t.Errorf("old ResourceQuota changed: %v", diff.ObjectReflectDiff(oldrq, oldRQInfo.resourceQuota())) + } + + switch { + case enabled || oldRQHasSelector: + // new ResourceQuota should not be changed if the feature is enabled, or if the old ResourceQuota had ScopeSelector + if !reflect.DeepEqual(newrq, newRQInfo.resourceQuota()) { + t.Errorf("new ResourceQuota changed: %v", diff.ObjectReflectDiff(newrq, newRQInfo.resourceQuota())) + } + case newRQHasSelector: + // new ResourceQuota should be changed + if reflect.DeepEqual(newrq, newRQInfo.resourceQuota()) { + t.Errorf("new ResourceQuota was not changed") + } + // new ResourceQuota should not have ScopeSelector + if !reflect.DeepEqual(newrq, rqWithoutScopeSelector()) { + t.Errorf("new ResourceQuota had ScopeSelector: %v", diff.ObjectReflectDiff(newrq, rqWithoutScopeSelector())) + } + default: + // new ResourceQuota should not need to be changed + if !reflect.DeepEqual(newrq, newRQInfo.resourceQuota()) { + t.Errorf("new ResourceQuota changed: %v", diff.ObjectReflectDiff(newrq, newRQInfo.resourceQuota())) + } + } + }) + } + } + } +} diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index cfd9bd7493..44da6a3bd9 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4901,9 +4901,6 @@ func validateScopeSelector(resourceQuotaSpec *core.ResourceQuotaSpec, fld *field if resourceQuotaSpec.ScopeSelector == nil { return allErrs } - if !utilfeature.DefaultFeatureGate.Enabled(features.ResourceQuotaScopeSelectors) && resourceQuotaSpec.ScopeSelector != nil { - allErrs = append(allErrs, field.Forbidden(fld.Child("scopeSelector"), "ResourceQuotaScopeSelectors feature-gate is disabled")) - } allErrs = append(allErrs, validateScopedResourceSelectorRequirement(resourceQuotaSpec, fld.Child("scopeSelector"))...) return allErrs } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 03769ca0d2..df88524842 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -11528,13 +11528,11 @@ func TestValidateResourceQuota(t *testing.T) { Spec: nonBestEffortSpec, }, } - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ResourceQuotaScopeSelectors, true)() for _, successCase := range successCases { if errs := ValidateResourceQuota(&successCase); len(errs) != 0 { t.Errorf("expected success: %v", errs) } } - defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ResourceQuotaScopeSelectors, false)() errorCases := map[string]struct { R core.ResourceQuota @@ -11580,10 +11578,6 @@ func TestValidateResourceQuota(t *testing.T) { core.ResourceQuota{ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidScopeNameSpec}, "unsupported scope", }, - "forbidden-quota-scope-selector": { - core.ResourceQuota{ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: scopeSelectorSpec}, - "feature-gate is disabled", - }, } for k, v := range errorCases { errs := ValidateResourceQuota(&v.R) diff --git a/pkg/registry/core/resourcequota/BUILD b/pkg/registry/core/resourcequota/BUILD index 54bba4eaa4..a8eac9a976 100644 --- a/pkg/registry/core/resourcequota/BUILD +++ b/pkg/registry/core/resourcequota/BUILD @@ -15,6 +15,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/registry/core/resourcequota", deps = [ "//pkg/api/legacyscheme:go_default_library", + "//pkg/api/resourcequota:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/core/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", diff --git a/pkg/registry/core/resourcequota/strategy.go b/pkg/registry/core/resourcequota/strategy.go index dc313ac9bf..4e62ca4ec4 100644 --- a/pkg/registry/core/resourcequota/strategy.go +++ b/pkg/registry/core/resourcequota/strategy.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" + resourcequotautil "k8s.io/kubernetes/pkg/api/resourcequota" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/validation" ) @@ -46,6 +47,7 @@ func (resourcequotaStrategy) NamespaceScoped() bool { func (resourcequotaStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { resourcequota := obj.(*api.ResourceQuota) resourcequota.Status = api.ResourceQuotaStatus{} + resourcequotautil.DropDisabledFields(&resourcequota.Spec, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -53,6 +55,7 @@ func (resourcequotaStrategy) PrepareForUpdate(ctx context.Context, obj, old runt newResourcequota := obj.(*api.ResourceQuota) oldResourcequota := old.(*api.ResourceQuota) newResourcequota.Status = oldResourcequota.Status + resourcequotautil.DropDisabledFields(&newResourcequota.Spec, &oldResourcequota.Spec) } // Validate validates a new resourcequota.