From e19f4939b1293b8b565a1087dcbb71c1ef46fe47 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Tue, 31 Mar 2015 11:13:44 -0700 Subject: [PATCH] fix tests broken by stronger validation --- pkg/api/rest/update_test.go | 126 +++++----- pkg/api/validation/validation_test.go | 324 ++++++++++---------------- 2 files changed, 179 insertions(+), 271 deletions(-) diff --git a/pkg/api/rest/update_test.go b/pkg/api/rest/update_test.go index 5c6eaa656ba..79266183051 100644 --- a/pkg/api/rest/update_test.go +++ b/pkg/api/rest/update_test.go @@ -23,96 +23,86 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" ) +func makeValidService() api.Service { + return api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "valid", + Namespace: "default", + Labels: map[string]string{}, + Annotations: map[string]string{}, + ResourceVersion: "1", + }, + Spec: api.ServiceSpec{ + Selector: map[string]string{"key": "val"}, + SessionAffinity: "None", + Ports: []api.ServicePort{{Name: "p", Protocol: "TCP", Port: 8675}}, + }, + } +} + +// TODO: This should be done on types that are not part of our API func TestBeforeUpdate(t *testing.T) { - tests := []struct { - old runtime.Object - obj runtime.Object + testCases := []struct { + name string + tweakSvc func(oldSvc, newSvc *api.Service) // given basic valid services, each test case can customize them expectErr bool }{ { - obj: &api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - ResourceVersion: "1", - Namespace: "#$%%invalid", - }, + name: "no change", + tweakSvc: func(oldSvc, newSvc *api.Service) { + // nothing }, - old: &api.Service{}, - expectErr: true, + expectErr: false, }, { - obj: &api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - ResourceVersion: "1", - Namespace: "valid", - }, + name: "change port", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Spec.Ports[0].Port++ }, - old: &api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "bar", - ResourceVersion: "1", - Namespace: "valid", - }, + expectErr: false, + }, + { + name: "bad namespace", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Namespace = "#$%%invalid" }, expectErr: true, }, { - obj: &api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - ResourceVersion: "1", - Namespace: "valid", - }, - Spec: api.ServiceSpec{ - PortalIP: "1.2.3.4", - }, - }, - old: &api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - ResourceVersion: "1", - Namespace: "valid", - }, - Spec: api.ServiceSpec{ - PortalIP: "4.3.2.1", - }, + name: "change name", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Name += "2" }, expectErr: true, }, { - obj: &api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - ResourceVersion: "1", - Namespace: api.NamespaceDefault, - }, - Spec: api.ServiceSpec{ - PortalIP: "1.2.3.4", - Selector: map[string]string{"foo": "bar"}, - }, + name: "change portal IP", + tweakSvc: func(oldSvc, newSvc *api.Service) { + oldSvc.Spec.PortalIP = "1.2.3.4" + newSvc.Spec.PortalIP = "4.3.2.1" }, - old: &api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - ResourceVersion: "1", - Namespace: api.NamespaceDefault, - }, - Spec: api.ServiceSpec{ - PortalIP: "1.2.3.4", - Selector: map[string]string{"bar": "foo"}, - }, + expectErr: true, + }, + { + name: "change selectpor", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Spec.Selector = map[string]string{"newkey": "newvalue"} }, + expectErr: false, }, } - for _, test := range tests { + + for _, tc := range testCases { + oldSvc := makeValidService() + newSvc := makeValidService() + tc.tweakSvc(&oldSvc, &newSvc) ctx := api.NewDefaultContext() - err := BeforeUpdate(Services, ctx, test.obj, test.old) - if test.expectErr && err == nil { - t.Errorf("unexpected non-error for %v", test) + err := BeforeUpdate(Services, ctx, runtime.Object(&oldSvc), runtime.Object(&newSvc)) + if tc.expectErr && err == nil { + t.Errorf("unexpected non-error for %q", tc.name) } - if !test.expectErr && err != nil { - t.Errorf("unexpected error: %v for %v -> %v", err, test.obj, test.old) + if !tc.expectErr && err != nil { + t.Errorf("unexpected error for %q: %v", tc.name, err) } } } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 7e03baaf2b5..a7942ec2650 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -1223,190 +1223,207 @@ func TestValidatePodUpdate(t *testing.T) { } } +func makeValidService() api.Service { + return api.Service{ + ObjectMeta: api.ObjectMeta{ + Name: "valid", + Namespace: "valid", + Labels: map[string]string{}, + Annotations: map[string]string{}, + ResourceVersion: "1", + }, + Spec: api.ServiceSpec{ + Selector: map[string]string{"key": "val"}, + SessionAffinity: "None", + Ports: []api.ServicePort{{Name: "p", Protocol: "TCP", Port: 8675}}, + }, + } +} + func TestValidateService(t *testing.T) { testCases := []struct { - name string - makeSvc func(svc *api.Service) // given a basic valid service, each test case can customize it - numErrs int + name string + tweakSvc func(svc *api.Service) // given a basic valid service, each test case can customize it + numErrs int }{ { name: "missing namespace", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Namespace = "" }, numErrs: 1, }, { name: "invalid namespace", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Namespace = "-123" }, numErrs: 1, }, { name: "missing name", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Name = "" }, numErrs: 1, }, { name: "invalid name", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Name = "-123" }, numErrs: 1, }, { name: "too long name", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Name = strings.Repeat("a", 25) }, numErrs: 1, }, { name: "invalid generateName", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.GenerateName = "-123" }, numErrs: 1, }, { name: "too long generateName", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.GenerateName = strings.Repeat("a", 25) }, numErrs: 1, }, { name: "invalid label", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Labels["NoUppercaseOrSpecialCharsLike=Equals"] = "bar" }, numErrs: 1, }, { name: "invalid annotation", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Annotations["NoSpecialCharsLike=Equals"] = "bar" }, numErrs: 1, }, { name: "nil selector", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Selector = nil }, numErrs: 0, }, { name: "invalid selector", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Selector["NoSpecialCharsLike=Equals"] = "bar" }, numErrs: 1, }, { name: "missing session affinity", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.SessionAffinity = "" }, numErrs: 1, }, { name: "missing ports", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports = nil }, numErrs: 1, }, { name: "empty port[0] name", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Name = "" }, numErrs: 0, }, { name: "empty port[1] name", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "", Protocol: "TCP", Port: 12345}) }, numErrs: 1, }, { name: "invalid port name", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Name = "INVALID" }, numErrs: 1, }, { name: "missing protocol", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Protocol = "" }, numErrs: 1, }, { name: "invalid protocol", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Protocol = "INVALID" }, numErrs: 1, }, { name: "invalid portal ip", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.PortalIP = "invalid" }, numErrs: 1, }, { name: "missing port", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Port = 0 }, numErrs: 1, }, { name: "invalid port", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Port = 65536 }, numErrs: 1, }, { name: "invalid TargetPort int", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].TargetPort = util.NewIntOrStringFromInt(65536) }, numErrs: 1, }, { name: "invalid publicIPs localhost", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.PublicIPs = []string{"127.0.0.1"} }, numErrs: 1, }, { name: "invalid publicIPs", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.PublicIPs = []string{"0.0.0.0"} }, numErrs: 1, }, { name: "valid publicIPs host", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.PublicIPs = []string{"myhost.mydomain"} }, numErrs: 0, }, { name: "dup port name", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Name = "p" s.Spec.Ports = append(s.Spec.Ports, api.ServicePort{Name: "p", Port: 12345}) }, @@ -1414,14 +1431,14 @@ func TestValidateService(t *testing.T) { }, { name: "valid 1", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { // do nothing }, numErrs: 0, }, { name: "valid 2", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].Protocol = "UDP" s.Spec.Ports[0].TargetPort = util.NewIntOrStringFromInt(12345) }, @@ -1429,21 +1446,21 @@ func TestValidateService(t *testing.T) { }, { name: "valid 3", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.Ports[0].TargetPort = util.NewIntOrStringFromString("http") }, numErrs: 0, }, { name: "valid portal ip - none ", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.PortalIP = "None" }, numErrs: 0, }, { name: "valid portal ip - empty", - makeSvc: func(s *api.Service) { + tweakSvc: func(s *api.Service) { s.Spec.PortalIP = "" s.Spec.Ports[0].TargetPort = util.NewIntOrStringFromString("http") }, @@ -1452,20 +1469,8 @@ func TestValidateService(t *testing.T) { } for _, tc := range testCases { - svc := api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "valid", - Namespace: "valid", - Labels: map[string]string{}, - Annotations: map[string]string{}, - }, - Spec: api.ServiceSpec{ - Selector: map[string]string{"key": "val"}, - SessionAffinity: "None", - Ports: []api.ServicePort{{Name: "p", Protocol: "TCP", Port: 8675}}, - }, - } - tc.makeSvc(&svc) + svc := makeValidService() + tc.tweakSvc(&svc) errs := ValidateService(&svc) if len(errs) != tc.numErrs { t.Errorf("Unexpected error list for case %q: %v", tc.name, utilerrors.NewAggregate(errs)) @@ -2173,172 +2178,85 @@ func TestValidateMinionUpdate(t *testing.T) { } func TestValidateServiceUpdate(t *testing.T) { - tests := []struct { - oldService api.Service - service api.Service - valid bool + testCases := []struct { + name string + tweakSvc func(oldSvc, newSvc *api.Service) // given basic valid services, each test case can customize them + numErrs int }{ - { // 0 - api.Service{}, - api.Service{}, - true}, - { // 1 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo"}}, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "bar"}, - }, false}, - { // 2 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"foo": "bar"}, - }, + { + name: "no change", + tweakSvc: func(oldSvc, newSvc *api.Service) { + // do nothing }, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"foo": "baz"}, - }, - }, true}, - { // 3 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - }, + numErrs: 0, + }, + { + name: "change name", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Name += "2" }, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"foo": "baz"}, - }, - }, true}, - { // 4 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"bar": "foo"}, - }, + numErrs: 1, + }, + { + name: "change namespace", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Namespace += "2" }, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"foo": "baz"}, - }, - }, true}, - { // 5 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Annotations: map[string]string{"bar": "foo"}, - }, + numErrs: 1, + }, + { + name: "change label valid", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Labels["key"] = "other-value" }, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Annotations: map[string]string{"foo": "baz"}, - }, - }, true}, - { // 6 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - }, - Spec: api.ServiceSpec{ - Selector: map[string]string{"foo": "baz"}, - }, + numErrs: 0, + }, + { + name: "add label", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Labels["key2"] = "value2" }, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - }, - Spec: api.ServiceSpec{ - Selector: map[string]string{"foo": "baz"}, - }, - }, true}, - { // 7 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"bar": "foo"}, - }, - Spec: api.ServiceSpec{ - PortalIP: "127.0.0.1", - }, + numErrs: 0, + }, + { + name: "change portal IP", + tweakSvc: func(oldSvc, newSvc *api.Service) { + oldSvc.Spec.PortalIP = "1.2.3.4" + newSvc.Spec.PortalIP = "8.6.7.5" }, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"bar": "fooobaz"}, - }, - Spec: api.ServiceSpec{ - PortalIP: "new", - }, - }, false}, - { // 8 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"bar": "foo"}, - }, - Spec: api.ServiceSpec{ - PortalIP: "127.0.0.1", - }, + numErrs: 1, + }, + { + name: "remove portal IP", + tweakSvc: func(oldSvc, newSvc *api.Service) { + oldSvc.Spec.PortalIP = "1.2.3.4" + newSvc.Spec.PortalIP = "" }, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"bar": "fooobaz"}, - }, - Spec: api.ServiceSpec{ - PortalIP: "", - }, - }, false}, - { // 9 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"bar": "foo"}, - }, - Spec: api.ServiceSpec{ - PortalIP: "127.0.0.1", - }, + numErrs: 1, + }, + { + name: "change affinity", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Spec.SessionAffinity = "ClientIP" }, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"bar": "fooobaz"}, - }, - Spec: api.ServiceSpec{ - PortalIP: "127.0.0.2", - }, - }, false}, - { // 10 - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"foo": "baz"}, - }, + numErrs: 0, + }, + { + name: "remove affinity", + tweakSvc: func(oldSvc, newSvc *api.Service) { + newSvc.Spec.SessionAffinity = "" }, - api.Service{ - ObjectMeta: api.ObjectMeta{ - Name: "foo", - Labels: map[string]string{"Foo": "baz"}, - }, - }, true}, + numErrs: 1, + }, } - 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) - t.Logf("%#v vs %#v", test.oldService.ObjectMeta, test.service.ObjectMeta) - } - if !test.valid && len(errs) == 0 { - t.Errorf("%d: Unexpected non-error", i) + + for _, tc := range testCases { + oldSvc := makeValidService() + newSvc := makeValidService() + tc.tweakSvc(&oldSvc, &newSvc) + errs := ValidateServiceUpdate(&oldSvc, &newSvc) + if len(errs) != tc.numErrs { + t.Errorf("Unexpected error list for case %q: %v", tc.name, utilerrors.NewAggregate(errs)) } } }