diff --git a/pkg/api/rest/update_test.go b/pkg/api/rest/update_test.go index 5817a93a4e3..5c6eaa656ba 100644 --- a/pkg/api/rest/update_test.go +++ b/pkg/api/rest/update_test.go @@ -32,8 +32,9 @@ func TestBeforeUpdate(t *testing.T) { { obj: &api.Service{ ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: "#$%%invalid", + Name: "foo", + ResourceVersion: "1", + Namespace: "#$%%invalid", }, }, old: &api.Service{}, @@ -42,14 +43,16 @@ func TestBeforeUpdate(t *testing.T) { { obj: &api.Service{ ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: "valid", + Name: "foo", + ResourceVersion: "1", + Namespace: "valid", }, }, old: &api.Service{ ObjectMeta: api.ObjectMeta{ - Name: "bar", - Namespace: "valid", + Name: "bar", + ResourceVersion: "1", + Namespace: "valid", }, }, expectErr: true, @@ -57,8 +60,9 @@ func TestBeforeUpdate(t *testing.T) { { obj: &api.Service{ ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: "valid", + Name: "foo", + ResourceVersion: "1", + Namespace: "valid", }, Spec: api.ServiceSpec{ PortalIP: "1.2.3.4", @@ -66,8 +70,9 @@ func TestBeforeUpdate(t *testing.T) { }, old: &api.Service{ ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: "valid", + Name: "foo", + ResourceVersion: "1", + Namespace: "valid", }, Spec: api.ServiceSpec{ PortalIP: "4.3.2.1", @@ -78,8 +83,9 @@ func TestBeforeUpdate(t *testing.T) { { obj: &api.Service{ ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: api.NamespaceDefault, + Name: "foo", + ResourceVersion: "1", + Namespace: api.NamespaceDefault, }, Spec: api.ServiceSpec{ PortalIP: "1.2.3.4", @@ -88,8 +94,9 @@ func TestBeforeUpdate(t *testing.T) { }, old: &api.Service{ ObjectMeta: api.ObjectMeta{ - Name: "foo", - Namespace: api.NamespaceDefault, + Name: "foo", + ResourceVersion: "1", + Namespace: api.NamespaceDefault, }, Spec: api.ServiceSpec{ PortalIP: "1.2.3.4", diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 9700b460d8c..9376cfa9840 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -232,6 +232,11 @@ func ValidateObjectMetaUpdate(old, meta *api.ObjectMeta) errs.ValidationErrorLis meta.CreationTimestamp = old.CreationTimestamp } + // Reject updates that don't specify a resource version + if meta.ResourceVersion == "" { + allErrs = append(allErrs, errs.NewFieldInvalid("resourceVersion", meta.ResourceVersion, "resourceVersion must be specified for an update")) + } + if old.Name != meta.Name { allErrs = append(allErrs, errs.NewFieldInvalid("name", meta.Name, "field is immutable")) } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 798a3179d36..6a300da30bc 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -56,20 +56,20 @@ func TestValidateObjectMetaCustomName(t *testing.T) { func TestValidateObjectMetaUpdateIgnoresCreationTimestamp(t *testing.T) { if errs := ValidateObjectMetaUpdate( - &api.ObjectMeta{Name: "test", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, - &api.ObjectMeta{Name: "test"}, + &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, + &api.ObjectMeta{Name: "test", ResourceVersion: "1"}, ); len(errs) != 0 { t.Fatalf("unexpected errors: %v", errs) } if errs := ValidateObjectMetaUpdate( - &api.ObjectMeta{Name: "test"}, - &api.ObjectMeta{Name: "test", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, + &api.ObjectMeta{Name: "test", ResourceVersion: "1"}, + &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, ); len(errs) != 0 { t.Fatalf("unexpected errors: %v", errs) } if errs := ValidateObjectMetaUpdate( - &api.ObjectMeta{Name: "test", CreationTimestamp: util.NewTime(time.Unix(11, 0))}, - &api.ObjectMeta{Name: "test", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, + &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(11, 0))}, + &api.ObjectMeta{Name: "test", ResourceVersion: "1", CreationTimestamp: util.NewTime(time.Unix(10, 0))}, ); len(errs) != 0 { t.Fatalf("unexpected errors: %v", errs) } @@ -1208,6 +1208,8 @@ func TestValidatePodUpdate(t *testing.T) { } for _, test := range tests { + test.a.ObjectMeta.ResourceVersion = "1" + test.b.ObjectMeta.ResourceVersion = "1" errs := ValidatePodUpdate(&test.a, &test.b) if test.isValid { if len(errs) != 0 { @@ -1522,6 +1524,8 @@ func TestValidateReplicationControllerUpdate(t *testing.T) { }, } for _, successCase := range successCases { + successCase.old.ObjectMeta.ResourceVersion = "1" + successCase.update.ObjectMeta.ResourceVersion = "1" if errs := ValidateReplicationControllerUpdate(&successCase.old, &successCase.update); len(errs) != 0 { t.Errorf("expected success: %v", errs) } @@ -2126,6 +2130,8 @@ func TestValidateMinionUpdate(t *testing.T) { }, true}, } for i, test := range tests { + test.oldMinion.ObjectMeta.ResourceVersion = "1" + test.minion.ObjectMeta.ResourceVersion = "1" errs := ValidateMinionUpdate(&test.oldMinion, &test.minion) if test.valid && len(errs) > 0 { t.Errorf("%d: Unexpected error: %v", i, errs) @@ -2295,6 +2301,8 @@ func TestValidateServiceUpdate(t *testing.T) { }, true}, } for i, test := range tests { + test.oldService.ObjectMeta.ResourceVersion = "1" + test.service.ObjectMeta.ResourceVersion = "1" errs := ValidateServiceUpdate(&test.oldService, &test.service) if test.valid && len(errs) > 0 { t.Errorf("%d: Unexpected error: %v", i, errs) @@ -2560,6 +2568,8 @@ func TestValidateNamespaceFinalizeUpdate(t *testing.T) { }, true}, } for i, test := range tests { + test.namespace.ObjectMeta.ResourceVersion = "1" + test.oldNamespace.ObjectMeta.ResourceVersion = "1" errs := ValidateNamespaceFinalizeUpdate(&test.namespace, &test.oldNamespace) if test.valid && len(errs) > 0 { t.Errorf("%d: Unexpected error: %v", i, errs) @@ -2600,6 +2610,8 @@ func TestValidateNamespaceStatusUpdate(t *testing.T) { }, false}, } for i, test := range tests { + test.namespace.ObjectMeta.ResourceVersion = "1" + test.oldNamespace.ObjectMeta.ResourceVersion = "1" errs := ValidateNamespaceStatusUpdate(&test.oldNamespace, &test.namespace) if test.valid && len(errs) > 0 { t.Errorf("%d: Unexpected error: %v", i, errs) @@ -2670,6 +2682,8 @@ func TestValidateNamespaceUpdate(t *testing.T) { }, true}, } for i, test := range tests { + test.namespace.ObjectMeta.ResourceVersion = "1" + test.oldNamespace.ObjectMeta.ResourceVersion = "1" errs := ValidateNamespaceUpdate(&test.oldNamespace, &test.namespace) if test.valid && len(errs) > 0 { t.Errorf("%d: Unexpected error: %v", i, errs) diff --git a/pkg/registry/service/rest_test.go b/pkg/registry/service/rest_test.go index 506d6a6018a..1e3491e3f0c 100644 --- a/pkg/registry/service/rest_test.go +++ b/pkg/registry/service/rest_test.go @@ -129,15 +129,21 @@ func TestServiceStorageValidatesCreate(t *testing.T) { func TestServiceRegistryUpdate(t *testing.T) { ctx := api.NewDefaultContext() storage, registry, _ := NewTestREST(t, nil) - registry.CreateService(ctx, &api.Service{ - ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault}, + svc, err := registry.CreateService(ctx, &api.Service{ + ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1", Namespace: api.NamespaceDefault}, Spec: api.ServiceSpec{ Port: 6502, Selector: map[string]string{"bar": "baz1"}, }, }) + + if err != nil { + t.Fatalf("Expected no error: %v", err) + } updated_svc, created, err := storage.Update(ctx, &api.Service{ - ObjectMeta: api.ObjectMeta{Name: "foo"}, + ObjectMeta: api.ObjectMeta{ + Name: "foo", + ResourceVersion: svc.ResourceVersion}, Spec: api.ServiceSpec{ Port: 6502, Selector: map[string]string{"bar": "baz2"}, @@ -305,7 +311,7 @@ func TestServiceRegistryUpdateExternalService(t *testing.T) { // Create non-external load balancer. svc1 := &api.Service{ - ObjectMeta: api.ObjectMeta{Name: "foo"}, + ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"}, Spec: api.ServiceSpec{ Port: 6502, Selector: map[string]string{"bar": "baz"}, @@ -546,7 +552,7 @@ func TestServiceRegistryIPUpdate(t *testing.T) { rest.portalMgr.randomAttempts = 0 svc := &api.Service{ - ObjectMeta: api.ObjectMeta{Name: "foo"}, + ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, Port: 6502, @@ -589,7 +595,7 @@ func TestServiceRegistryIPExternalLoadBalancer(t *testing.T) { rest.portalMgr.randomAttempts = 0 svc := &api.Service{ - ObjectMeta: api.ObjectMeta{Name: "foo"}, + ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"}, Spec: api.ServiceSpec{ Selector: map[string]string{"bar": "baz"}, Port: 6502,