diff --git a/pkg/registry/extensions/daemonset/BUILD b/pkg/registry/extensions/daemonset/BUILD index 1f1dd0be240..6865c0cdac5 100644 --- a/pkg/registry/extensions/daemonset/BUILD +++ b/pkg/registry/extensions/daemonset/BUILD @@ -16,8 +16,12 @@ go_library( "//pkg/api:go_default_library", "//pkg/apis/extensions:go_default_library", "//pkg/apis/extensions/validation:go_default_library", + "//vendor/k8s.io/api/apps/v1beta2:go_default_library", + "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/validation:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", @@ -31,6 +35,10 @@ go_test( library = ":go_default_library", deps = [ "//pkg/api:go_default_library", + "//pkg/apis/extensions:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", ], ) diff --git a/pkg/registry/extensions/daemonset/strategy.go b/pkg/registry/extensions/daemonset/strategy.go index c75fa365237..7afdf9b284e 100644 --- a/pkg/registry/extensions/daemonset/strategy.go +++ b/pkg/registry/extensions/daemonset/strategy.go @@ -17,8 +17,14 @@ limitations under the License. package daemonset import ( + "fmt" + + appsv1beta2 "k8s.io/api/apps/v1beta2" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" apiequality "k8s.io/apimachinery/pkg/api/equality" + apivalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" @@ -109,9 +115,29 @@ func (daemonSetStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (daemonSetStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList { - validationErrorList := validation.ValidateDaemonSet(obj.(*extensions.DaemonSet)) - updateErrorList := validation.ValidateDaemonSetUpdate(obj.(*extensions.DaemonSet), old.(*extensions.DaemonSet)) - return append(validationErrorList, updateErrorList...) + newDaemonSet := obj.(*extensions.DaemonSet) + oldDaemonSet := old.(*extensions.DaemonSet) + allErrs := validation.ValidateDaemonSet(obj.(*extensions.DaemonSet)) + allErrs = append(allErrs, validation.ValidateDaemonSetUpdate(newDaemonSet, oldDaemonSet)...) + + // Update is not allowed to set Spec.Selector for all groups/versions except extensions/v1beta1. + // If RequestInfo is nil, it is better to revert to old behavior (i.e. allow update to set Spec.Selector) + // to prevent unintentionally breaking users who may rely on the old behavior. + // TODO(#50791): after extensions/v1beta1 is removed, move selector immutability check inside ValidateDaemonSetUpdate(). + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + switch groupVersion { + case extensionsv1beta1.SchemeGroupVersion: + // no-op for compatibility + case appsv1beta2.SchemeGroupVersion: + // disallow mutation of selector + allErrs = append(allErrs, apivalidation.ValidateImmutableField(newDaemonSet.Spec.Selector, oldDaemonSet.Spec.Selector, field.NewPath("spec").Child("selector"))...) + default: + panic(fmt.Sprintf("unexpected group/version: %v", groupVersion)) + } + } + + return allErrs } // AllowUnconditionalUpdate is the default update policy for daemon set objects. diff --git a/pkg/registry/extensions/daemonset/strategy_test.go b/pkg/registry/extensions/daemonset/strategy_test.go index 0f16ce4d96d..3dca4a72cb4 100644 --- a/pkg/registry/extensions/daemonset/strategy_test.go +++ b/pkg/registry/extensions/daemonset/strategy_test.go @@ -17,10 +17,22 @@ limitations under the License. package daemonset import ( + "reflect" "testing" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" - _ "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/apis/extensions" +) + +const ( + fakeImageName = "fake-name" + fakeImage = "fakeimage" + daemonsetName = "test-daemonset" + namespace = "test-namespace" ) func TestDefaultGarbageCollectionPolicy(t *testing.T) { @@ -31,3 +43,84 @@ func TestDefaultGarbageCollectionPolicy(t *testing.T) { t.Errorf("DefaultGarbageCollectionPolicy() = %#v, want %#v", got, want) } } + +func TestSelectorImmutability(t *testing.T) { + tests := []struct { + requestInfo genericapirequest.RequestInfo + oldSelectorLabels map[string]string + newSelectorLabels map[string]string + expectedErrorList field.ErrorList + }{ + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta2", + Resource: "daemonsets", + }, + map[string]string{"a": "b"}, + map[string]string{"c": "d"}, + field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: field.NewPath("spec").Child("selector").String(), + BadValue: &metav1.LabelSelector{ + MatchLabels: map[string]string{"c": "d"}, + MatchExpressions: []metav1.LabelSelectorRequirement{}, + }, + Detail: "field is immutable", + }, + }, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "extensions", + APIVersion: "v1beta1", + Resource: "daemonsets", + }, + map[string]string{"a": "b"}, + map[string]string{"c": "d"}, + field.ErrorList{}, + }, + } + + for _, test := range tests { + oldDaemonSet := newDaemonSetWithSelectorLabels(test.oldSelectorLabels, 1) + newDaemonSet := newDaemonSetWithSelectorLabels(test.newSelectorLabels, 2) + context := genericapirequest.NewContext() + context = genericapirequest.WithRequestInfo(context, &test.requestInfo) + errorList := daemonSetStrategy{}.ValidateUpdate(context, newDaemonSet, oldDaemonSet) + if !reflect.DeepEqual(test.expectedErrorList, errorList) { + t.Errorf("Unexpected error list, expected: %v, actual: %v", test.expectedErrorList, errorList) + } + } +} + +func newDaemonSetWithSelectorLabels(selectorLabels map[string]string, templateGeneration int64) *extensions.DaemonSet { + return &extensions.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: daemonsetName, + Namespace: namespace, + ResourceVersion: "1", + }, + Spec: extensions.DaemonSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: selectorLabels, + MatchExpressions: []metav1.LabelSelectorRequirement{}, + }, + UpdateStrategy: extensions.DaemonSetUpdateStrategy{ + Type: extensions.OnDeleteDaemonSetStrategyType, + }, + TemplateGeneration: templateGeneration, + Template: api.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: selectorLabels, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{Name: fakeImageName, Image: fakeImage, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: api.TerminationMessageReadFile}}, + }, + }, + }, + } +} diff --git a/pkg/registry/extensions/deployment/BUILD b/pkg/registry/extensions/deployment/BUILD index fcc9466f1ce..4f80dbf7f22 100644 --- a/pkg/registry/extensions/deployment/BUILD +++ b/pkg/registry/extensions/deployment/BUILD @@ -17,10 +17,15 @@ go_library( "//pkg/api:go_default_library", "//pkg/apis/extensions:go_default_library", "//pkg/apis/extensions/validation:go_default_library", + "//vendor/k8s.io/api/apps/v1beta1:go_default_library", + "//vendor/k8s.io/api/apps/v1beta2:go_default_library", + "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/validation:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/internalversion:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", @@ -37,6 +42,8 @@ go_test( "//pkg/apis/extensions:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/intstr:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", ], ) diff --git a/pkg/registry/extensions/deployment/strategy.go b/pkg/registry/extensions/deployment/strategy.go index a90aeb89bae..349f606ca5e 100644 --- a/pkg/registry/extensions/deployment/strategy.go +++ b/pkg/registry/extensions/deployment/strategy.go @@ -17,8 +17,15 @@ limitations under the License. package deployment import ( + "fmt" + + appsv1beta1 "k8s.io/api/apps/v1beta1" + appsv1beta2 "k8s.io/api/apps/v1beta2" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" apiequality "k8s.io/apimachinery/pkg/api/equality" + apivalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" @@ -88,7 +95,31 @@ func (deploymentStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, o // ValidateUpdate is the default update validation for an end user. func (deploymentStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateDeploymentUpdate(obj.(*extensions.Deployment), old.(*extensions.Deployment)) + newDeployment := obj.(*extensions.Deployment) + oldDeployment := old.(*extensions.Deployment) + allErrs := validation.ValidateDeploymentUpdate(newDeployment, oldDeployment) + + // Update is not allowed to set Spec.Selector for all groups/versions except extensions/v1beta1. + // If RequestInfo is nil, it is better to revert to old behavior (i.e. allow update to set Spec.Selector) + // to prevent unintentionally breaking users who may rely on the old behavior. + // TODO(#50791): after apps/v1beta1 and extensions/v1beta1 are removed, + // move selector immutability check inside ValidateDeploymentUpdate(). + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + switch groupVersion { + case appsv1beta1.SchemeGroupVersion: + // no-op for compatibility + case extensionsv1beta1.SchemeGroupVersion: + // no-op for compatibility + case appsv1beta2.SchemeGroupVersion: + // disallow mutation of selector + allErrs = append(allErrs, apivalidation.ValidateImmutableField(newDeployment.Spec.Selector, oldDeployment.Spec.Selector, field.NewPath("spec").Child("selector"))...) + default: + panic(fmt.Sprintf("unexpected group/version: %v", groupVersion)) + } + } + + return allErrs } func (deploymentStrategy) AllowUnconditionalUpdate() bool { diff --git a/pkg/registry/extensions/deployment/strategy_test.go b/pkg/registry/extensions/deployment/strategy_test.go index 74668493540..2a53b347f23 100644 --- a/pkg/registry/extensions/deployment/strategy_test.go +++ b/pkg/registry/extensions/deployment/strategy_test.go @@ -22,11 +22,20 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" ) +const ( + fakeImageName = "fake-name" + fakeImage = "fakeimage" + deploymentName = "test-deployment" + namespace = "test-namespace" +) + func TestStatusUpdates(t *testing.T) { tests := []struct { old runtime.Object @@ -78,3 +87,99 @@ func newDeployment(labels, annotations map[string]string) *extensions.Deployment }, } } + +func TestSelectorImmutability(t *testing.T) { + tests := []struct { + requestInfo genericapirequest.RequestInfo + oldSelectorLabels map[string]string + newSelectorLabels map[string]string + expectedErrorList field.ErrorList + }{ + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta2", + Resource: "deployments", + }, + map[string]string{"a": "b"}, + map[string]string{"c": "d"}, + field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: field.NewPath("spec").Child("selector").String(), + BadValue: &metav1.LabelSelector{ + MatchLabels: map[string]string{"c": "d"}, + MatchExpressions: []metav1.LabelSelectorRequirement{}, + }, + Detail: "field is immutable", + }, + }, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta1", + Resource: "deployments", + }, + map[string]string{"a": "b"}, + map[string]string{"c": "d"}, + field.ErrorList{}, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "extensions", + APIVersion: "v1beta1", + }, + map[string]string{"a": "b"}, + map[string]string{"c": "d"}, + field.ErrorList{}, + }, + } + + for _, test := range tests { + oldDeployment := newDeploymentWithSelectorLabels(test.oldSelectorLabels) + newDeployment := newDeploymentWithSelectorLabels(test.newSelectorLabels) + context := genericapirequest.NewContext() + context = genericapirequest.WithRequestInfo(context, &test.requestInfo) + errorList := deploymentStrategy{}.ValidateUpdate(context, newDeployment, oldDeployment) + if len(test.expectedErrorList) == 0 && len(errorList) == 0 { + continue + } + if !reflect.DeepEqual(test.expectedErrorList, errorList) { + t.Errorf("Unexpected error list, expected: %v, actual: %v", test.expectedErrorList, errorList) + } + } +} + +func newDeploymentWithSelectorLabels(selectorLabels map[string]string) *extensions.Deployment { + return &extensions.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: deploymentName, + Namespace: namespace, + ResourceVersion: "1", + }, + Spec: extensions.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: selectorLabels, + MatchExpressions: []metav1.LabelSelectorRequirement{}, + }, + Strategy: extensions.DeploymentStrategy{ + Type: extensions.RollingUpdateDeploymentStrategyType, + RollingUpdate: &extensions.RollingUpdateDeployment{ + MaxSurge: intstr.FromInt(1), + MaxUnavailable: intstr.FromInt(1), + }, + }, + Template: api.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: selectorLabels, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSDefault, + Containers: []api.Container{{Name: fakeImageName, Image: fakeImage, ImagePullPolicy: api.PullNever, TerminationMessagePolicy: api.TerminationMessageReadFile}}, + }, + }, + }, + } +} diff --git a/pkg/registry/extensions/replicaset/BUILD b/pkg/registry/extensions/replicaset/BUILD index 7d0fe9e135a..01bb4a781d4 100644 --- a/pkg/registry/extensions/replicaset/BUILD +++ b/pkg/registry/extensions/replicaset/BUILD @@ -17,12 +17,16 @@ go_library( "//pkg/api:go_default_library", "//pkg/apis/extensions:go_default_library", "//pkg/apis/extensions/validation:go_default_library", + "//vendor/k8s.io/api/apps/v1beta2:go_default_library", + "//vendor/k8s.io/api/extensions/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/validation:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/internalversion:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/fields:go_default_library", "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", @@ -41,6 +45,7 @@ go_test( "//pkg/api:go_default_library", "//pkg/apis/extensions:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", ], ) diff --git a/pkg/registry/extensions/replicaset/strategy.go b/pkg/registry/extensions/replicaset/strategy.go index e7bf7d5f379..f432f3403c7 100644 --- a/pkg/registry/extensions/replicaset/strategy.go +++ b/pkg/registry/extensions/replicaset/strategy.go @@ -22,10 +22,14 @@ import ( "fmt" "strconv" + appsv1beta2 "k8s.io/api/apps/v1beta2" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" apiequality "k8s.io/apimachinery/pkg/api/equality" + apivalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/generic" @@ -103,9 +107,29 @@ func (rsStrategy) AllowCreateOnUpdate() bool { // ValidateUpdate is the default update validation for an end user. func (rsStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList { - validationErrorList := validation.ValidateReplicaSet(obj.(*extensions.ReplicaSet)) - updateErrorList := validation.ValidateReplicaSetUpdate(obj.(*extensions.ReplicaSet), old.(*extensions.ReplicaSet)) - return append(validationErrorList, updateErrorList...) + newReplicaSet := obj.(*extensions.ReplicaSet) + oldReplicaSet := old.(*extensions.ReplicaSet) + allErrs := validation.ValidateReplicaSet(obj.(*extensions.ReplicaSet)) + allErrs = append(allErrs, validation.ValidateReplicaSetUpdate(newReplicaSet, oldReplicaSet)...) + + // Update is not allowed to set Spec.Selector for all groups/versions except extensions/v1beta1. + // If RequestInfo is nil, it is better to revert to old behavior (i.e. allow update to set Spec.Selector) + // to prevent unintentionally breaking users who may rely on the old behavior. + // TODO(#50791): after extensions/v1beta1 is removed, move selector immutability check inside ValidateReplicaSetUpdate(). + if requestInfo, found := genericapirequest.RequestInfoFrom(ctx); found { + groupVersion := schema.GroupVersion{Group: requestInfo.APIGroup, Version: requestInfo.APIVersion} + switch groupVersion { + case extensionsv1beta1.SchemeGroupVersion: + // no-op for compatibility + case appsv1beta2.SchemeGroupVersion: + // disallow mutation of selector + allErrs = append(allErrs, apivalidation.ValidateImmutableField(newReplicaSet.Spec.Selector, oldReplicaSet.Spec.Selector, field.NewPath("spec").Child("selector"))...) + default: + panic(fmt.Sprintf("unexpected group/version: %v", groupVersion)) + } + } + + return allErrs } func (rsStrategy) AllowUnconditionalUpdate() bool { diff --git a/pkg/registry/extensions/replicaset/strategy_test.go b/pkg/registry/extensions/replicaset/strategy_test.go index 8e9ea6ec40e..27f7b9020e2 100644 --- a/pkg/registry/extensions/replicaset/strategy_test.go +++ b/pkg/registry/extensions/replicaset/strategy_test.go @@ -17,14 +17,23 @@ limitations under the License. package replicaset import ( + "reflect" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" ) +const ( + fakeImageName = "fake-name" + fakeImage = "fakeimage" + replicasetName = "test-replicaset" + namespace = "test-namespace" +) + func TestReplicaSetStrategy(t *testing.T) { ctx := genericapirequest.NewDefaultContext() if !Strategy.NamespaceScoped() { @@ -141,3 +150,80 @@ func TestReplicaSetStatusStrategy(t *testing.T) { t.Errorf("Unexpected error %v", errs) } } + +func TestSelectorImmutability(t *testing.T) { + tests := []struct { + requestInfo genericapirequest.RequestInfo + oldSelectorLabels map[string]string + newSelectorLabels map[string]string + expectedErrorList field.ErrorList + }{ + { + genericapirequest.RequestInfo{ + APIGroup: "apps", + APIVersion: "v1beta2", + Resource: "replicasets", + }, + map[string]string{"a": "b"}, + map[string]string{"c": "d"}, + field.ErrorList{ + &field.Error{ + Type: field.ErrorTypeInvalid, + Field: field.NewPath("spec").Child("selector").String(), + BadValue: &metav1.LabelSelector{ + MatchLabels: map[string]string{"c": "d"}, + MatchExpressions: []metav1.LabelSelectorRequirement{}, + }, + Detail: "field is immutable", + }, + }, + }, + { + genericapirequest.RequestInfo{ + APIGroup: "extensions", + APIVersion: "v1beta1", + Resource: "replicasets", + }, + map[string]string{"a": "b"}, + map[string]string{"c": "d"}, + field.ErrorList{}, + }, + } + + for _, test := range tests { + oldReplicaSet := newReplicaSetWithSelectorLabels(test.oldSelectorLabels) + newReplicaSet := newReplicaSetWithSelectorLabels(test.newSelectorLabels) + context := genericapirequest.NewContext() + context = genericapirequest.WithRequestInfo(context, &test.requestInfo) + errorList := rsStrategy{}.ValidateUpdate(context, newReplicaSet, oldReplicaSet) + if !reflect.DeepEqual(test.expectedErrorList, errorList) { + t.Errorf("Unexpected error list, expected: %v, actual: %v", test.expectedErrorList, errorList) + } + } +} + +func newReplicaSetWithSelectorLabels(selectorLabels map[string]string) *extensions.ReplicaSet { + return &extensions.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: replicasetName, + Namespace: namespace, + ResourceVersion: "1", + }, + Spec: extensions.ReplicaSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: selectorLabels, + MatchExpressions: []metav1.LabelSelectorRequirement{}, + }, + Template: api.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: selectorLabels, + }, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{Name: fakeImageName, Image: fakeImage, ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: api.TerminationMessageReadFile}}, + }, + }, + }, + } +} diff --git a/test/integration/deployment/BUILD b/test/integration/deployment/BUILD index ff6da2dc364..8f1f8e8a9c4 100644 --- a/test/integration/deployment/BUILD +++ b/test/integration/deployment/BUILD @@ -18,6 +18,7 @@ go_test( "//pkg/controller/deployment/util:go_default_library", "//test/integration/framework:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", ], ) diff --git a/test/integration/deployment/deployment_test.go b/test/integration/deployment/deployment_test.go index 756c1f9b92f..7e6ef012d7a 100644 --- a/test/integration/deployment/deployment_test.go +++ b/test/integration/deployment/deployment_test.go @@ -17,9 +17,12 @@ limitations under the License. package deployment import ( + "reflect" + "strings" "testing" "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util" "k8s.io/kubernetes/test/integration/framework" ) @@ -69,3 +72,65 @@ func TestNewDeployment(t *testing.T) { t.Errorf("expected new ReplicaSet last-applied annotation not copied from Deployment %s", deploy.Name) } } + +// selectors are IMMUTABLE for all API versions except apps/v1beta1 and extensions/v1beta1 +func TestDeploymentSelectorImmutability(t *testing.T) { + s, closeFn, c := dcSimpleSetup(t) + defer closeFn() + name := "test-deployment-selector-immutability" + ns := framework.CreateTestingNamespace(name, s, t) + defer framework.DeleteTestingNamespace(ns, s, t) + + tester := &deploymentTester{t: t, c: c, deployment: newDeployment(name, ns.Name, int32(20))} + deploymentExtensionsV1beta1, err := c.ExtensionsV1beta1().Deployments(ns.Name).Create(tester.deployment) + if err != nil { + t.Fatalf("failed to create extensions/v1beta1 deployment %s: %v", tester.deployment.Name, err) + } + + // test to ensure extensions/v1beta1 selector is mutable + newSelectorLabels := map[string]string{"name_extensions_v1beta1": "test_extensions_v1beta1"} + deploymentExtensionsV1beta1.Spec.Selector.MatchLabels = newSelectorLabels + deploymentExtensionsV1beta1.Spec.Template.Labels = newSelectorLabels + updatedDeploymentExtensionsV1beta1, err := c.ExtensionsV1beta1().Deployments(ns.Name).Update(deploymentExtensionsV1beta1) + if err != nil { + t.Fatalf("failed to update extensions/v1beta1 deployment %s: %v", deploymentExtensionsV1beta1.Name, err) + } + if !reflect.DeepEqual(updatedDeploymentExtensionsV1beta1.Spec.Selector.MatchLabels, newSelectorLabels) { + t.Errorf("selector should be changed for extensions/v1beta1, expected: %v, got: %v", newSelectorLabels, updatedDeploymentExtensionsV1beta1.Spec.Selector.MatchLabels) + } + + // test to ensure apps/v1beta1 selector is mutable + deploymentAppsV1beta1, err := c.AppsV1beta1().Deployments(ns.Name).Get(updatedDeploymentExtensionsV1beta1.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get apps/v1beta1 deployment %s: %v", updatedDeploymentExtensionsV1beta1.Name, err) + } + + newSelectorLabels = map[string]string{"name_apps_v1beta1": "test_apps_v1beta1"} + deploymentAppsV1beta1.Spec.Selector.MatchLabels = newSelectorLabels + deploymentAppsV1beta1.Spec.Template.Labels = newSelectorLabels + updatedDeploymentAppsV1beta1, err := c.AppsV1beta1().Deployments(ns.Name).Update(deploymentAppsV1beta1) + if err != nil { + t.Fatalf("failed to update apps/v1beta1 deployment %s: %v", deploymentAppsV1beta1.Name, err) + } + if !reflect.DeepEqual(updatedDeploymentAppsV1beta1.Spec.Selector.MatchLabels, newSelectorLabels) { + t.Errorf("selector should be changed for apps/v1beta1, expected: %v, got: %v", newSelectorLabels, updatedDeploymentAppsV1beta1.Spec.Selector.MatchLabels) + } + + // test to ensure apps/v1beta2 selector is immutable + deploymentAppsV1beta2, err := c.AppsV1beta2().Deployments(ns.Name).Get(updatedDeploymentAppsV1beta1.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get apps/v1beta2 deployment %s: %v", updatedDeploymentAppsV1beta1.Name, err) + } + newSelectorLabels = map[string]string{"name_apps_v1beta2": "test_apps_v1beta2"} + deploymentAppsV1beta2.Spec.Selector.MatchLabels = newSelectorLabels + deploymentAppsV1beta2.Spec.Template.Labels = newSelectorLabels + _, err = c.AppsV1beta2().Deployments(ns.Name).Update(deploymentAppsV1beta2) + if err == nil { + t.Fatalf("failed to provide validation error when changing immutable selector when updating apps/v1beta2 deployment %s", deploymentAppsV1beta2.Name) + } + expectedErrType := "Invalid value" + expectedErrDetail := "field is immutable" + if !strings.Contains(err.Error(), expectedErrType) || !strings.Contains(err.Error(), expectedErrDetail) { + t.Errorf("error message does not match, expected type: %s, expected detail: %s, got: %s", expectedErrType, expectedErrDetail, err.Error()) + } +} diff --git a/test/integration/deployment/util.go b/test/integration/deployment/util.go index 979d535f40f..5180578b902 100644 --- a/test/integration/deployment/util.go +++ b/test/integration/deployment/util.go @@ -115,6 +115,20 @@ func dcSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, *replicaset.R return s, closeFn, rm, dc, informers, clientSet } +// dcSimpleSetup sets up necessities for Deployment integration test, including master, apiserver, +// and clientset, but not controllers and informers +func dcSimpleSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, clientset.Interface) { + masterConfig := framework.NewIntegrationTestMasterConfig() + _, s, closeFn := framework.RunAMaster(masterConfig) + + config := restclient.Config{Host: s.URL} + clientSet, err := clientset.NewForConfig(&config) + if err != nil { + t.Fatalf("error in create clientset: %v", err) + } + return s, closeFn, clientSet +} + // addPodConditionReady sets given pod status to ready at given time func addPodConditionReady(pod *v1.Pod, time metav1.Time) { pod.Status = v1.PodStatus{ diff --git a/test/integration/replicaset/replicaset_test.go b/test/integration/replicaset/replicaset_test.go index 67f9ace3ddc..28195b3d460 100644 --- a/test/integration/replicaset/replicaset_test.go +++ b/test/integration/replicaset/replicaset_test.go @@ -20,6 +20,7 @@ import ( "fmt" "net/http/httptest" "reflect" + "strings" "testing" "time" @@ -149,6 +150,18 @@ func rmSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, *replicaset.R return s, closeFn, rm, informers, clientSet } +func rmSimpleSetup(t *testing.T) (*httptest.Server, framework.CloseFunc, clientset.Interface) { + masterConfig := framework.NewIntegrationTestMasterConfig() + _, s, closeFn := framework.RunAMaster(masterConfig) + + config := restclient.Config{Host: s.URL} + clientSet, err := clientset.NewForConfig(&config) + if err != nil { + t.Fatalf("Error in create clientset: %v", err) + } + return s, closeFn, clientSet +} + // wait for the podInformer to observe the pods. Call this function before // running the RS controller to prevent the rc manager from creating new pods // rather than adopting the existing ones. @@ -462,3 +475,43 @@ func TestUpdateLabelToBeAdopted(t *testing.T) { } close(stopCh) } + +// selectors are IMMUTABLE for all API versions except extensions/v1beta1 +func TestRSSelectorImmutability(t *testing.T) { + s, closeFn, clientSet := rmSimpleSetup(t) + defer closeFn() + ns := framework.CreateTestingNamespace("rs-selector-immutability", s, t) + defer framework.DeleteTestingNamespace(ns, s, t) + rs := newRS("rs", ns.Name, 0) + createRSsPods(t, clientSet, []*v1beta1.ReplicaSet{rs}, []*v1.Pod{}, ns.Name) + + // test to ensure extensions/v1beta1 selector is mutable + newSelectorLabels := map[string]string{"changed_name_extensions_v1beta1": "changed_test_extensions_v1beta1"} + rs.Spec.Selector.MatchLabels = newSelectorLabels + rs.Spec.Template.Labels = newSelectorLabels + replicaset, err := clientSet.ExtensionsV1beta1().ReplicaSets(ns.Name).Update(rs) + if err != nil { + t.Fatalf("failed to update extensions/v1beta1 replicaset %s: %v", replicaset.Name, err) + } + if !reflect.DeepEqual(replicaset.Spec.Selector.MatchLabels, newSelectorLabels) { + t.Errorf("selector should be changed for extensions/v1beta1, expected: %v, got: %v", newSelectorLabels, replicaset.Spec.Selector.MatchLabels) + } + + // test to ensure apps/v1beta2 selector is immutable + rsV1beta2, err := clientSet.AppsV1beta2().ReplicaSets(ns.Name).Get(replicaset.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to get apps/v1beta2 replicaset %s: %v", replicaset.Name, err) + } + newSelectorLabels = map[string]string{"changed_name_apps_v1beta2": "changed_test_apps_v1beta2"} + rsV1beta2.Spec.Selector.MatchLabels = newSelectorLabels + rsV1beta2.Spec.Template.Labels = newSelectorLabels + _, err = clientSet.AppsV1beta2().ReplicaSets(ns.Name).Update(rsV1beta2) + if err == nil { + t.Fatalf("failed to provide validation error when changing immutable selector when updating apps/v1beta2 replicaset %s", rsV1beta2.Name) + } + expectedErrType := "Invalid value" + expectedErrDetail := "field is immutable" + if !strings.Contains(err.Error(), expectedErrType) || !strings.Contains(err.Error(), expectedErrDetail) { + t.Errorf("error message does not match, expected type: %s, expected detail: %s, got: %s", expectedErrType, expectedErrDetail, err.Error()) + } +}