add ResourceQuotaSelector validation and tests

Signed-off-by: Serguei Bezverkhi <sbezverk@cisco.com>

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",

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"],
+)

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
+}

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()))
+						}
+					}
+				})
+			}
+		}
+	}
+}

pkg/apis/core/validation/validation.go

@@ -4906,9 +4906,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
 }

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)

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",

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.