Factor out API defaulting from validation logic

Currently, the validation logic validates fields in an object and supply default values wherever applies. This change factors out defaulting to a set of defaulting callback functions for decoding (see #1502 for more discussion). * This change is based on pull request 2587. * Most defaulting has been migrated to defaults.go where the defaulting functions are added. * validation_test.go and converter_test.go have been adapted to not testing the default values. * Fixed all tests with that create invalid objects with the absence of defaulting logic.
2025-09-16 14:45:28 +00:00 · 2015-01-26 09:52:50 -08:00
parent 1ddb68d8d7
commit 4a72addaeb
40 changed files with 1059 additions and 384 deletions
--- a/pkg/registry/controller/rest_test.go
+++ b/pkg/registry/controller/rest_test.go
@@ -126,6 +126,10 @@ func TestControllerDecode(t *testing.T) {
 						"name": "nginx",
 					},
 				},
+				Spec: api.PodSpec{
+					RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+					DNSPolicy:     api.DNSClusterFirst,
+				},
 			},
 		},
 	}
@@ -225,10 +229,13 @@ var validPodTemplate = api.PodTemplate{
 		Spec: api.PodSpec{
 			Containers: []api.Container{
 				{
-					Name:  "test",
-					Image: "test_image",
+					Name:            "test",
+					Image:           "test_image",
+					ImagePullPolicy: api.PullIfNotPresent,
 				},
 			},
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
 		},
 	},
 }
--- a/pkg/registry/etcd/etcd_test.go
+++ b/pkg/registry/etcd/etcd_test.go
@@ -17,7 +17,6 @@ limitations under the License.
 package etcd

 import (
-	"reflect"
 	"strconv"
 	"strings"
 	"testing"
@@ -441,6 +440,10 @@ func TestEtcdUpdatePodNotScheduled(t *testing.T) {
 				"foo": "bar",
 			},
 		},
+		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
+		},
 	}
 	err := registry.UpdatePod(ctx, &podIn)
 	if err != nil {
@@ -515,9 +518,13 @@ func TestEtcdUpdatePodScheduled(t *testing.T) {
 		Spec: api.PodSpec{
 			Containers: []api.Container{
 				{
-					Image: "foo:v2",
+					Image:                  "foo:v2",
+					ImagePullPolicy:        api.PullIfNotPresent,
+					TerminationMessagePath: api.TerminationMessagePathDefault,
 				},
 			},
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
 		},
 		Status: api.PodStatus{
 			Host: "machine",
@@ -1338,6 +1345,8 @@ func TestEtcdUpdateService(t *testing.T) {
 			Selector: map[string]string{
 				"baz": "bar",
 			},
+			Protocol:        "TCP",
+			SessionAffinity: "None",
 		},
 	}
 	err := registry.UpdateService(ctx, &testService)
@@ -1352,7 +1361,7 @@ func TestEtcdUpdateService(t *testing.T) {
 	// Clear modified indices before the equality test.
 	svc.ResourceVersion = ""
 	testService.ResourceVersion = ""
-	if !reflect.DeepEqual(*svc, testService) {
+	if !api.Semantic.DeepEqual(*svc, testService) {
 		t.Errorf("Unexpected service: got\n %#v\n, wanted\n %#v", svc, testService)
 	}
 }
@@ -1404,7 +1413,7 @@ func TestEtcdGetEndpoints(t *testing.T) {
 		t.Errorf("unexpected error: %v", err)
 	}

-	if e, a := endpoints, got; !reflect.DeepEqual(e, a) {
+	if e, a := endpoints, got; !api.Semantic.DeepEqual(e, a) {
 		t.Errorf("Unexpected endpoints: %#v, expected %#v", e, a)
 	}
 }
@@ -1433,7 +1442,7 @@ func TestEtcdUpdateEndpoints(t *testing.T) {
 	}
 	var endpointsOut api.Endpoints
 	err = latest.Codec.DecodeInto([]byte(response.Node.Value), &endpointsOut)
-	if !reflect.DeepEqual(endpoints, endpointsOut) {
+	if !api.Semantic.DeepEqual(endpoints, endpointsOut) {
 		t.Errorf("Unexpected endpoints: %#v, expected %#v", endpointsOut, endpoints)
 	}
 }
--- a/pkg/registry/generic/etcd/etcd_test.go
+++ b/pkg/registry/generic/etcd/etcd_test.go
@@ -19,7 +19,6 @@ package etcd
 import (
 	"fmt"
 	"path"
-	"reflect"
 	"testing"

 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -146,7 +145,7 @@ func TestEtcdList(t *testing.T) {
 			continue
 		}

-		if e, a := item.out, list; !reflect.DeepEqual(e, a) {
+		if e, a := item.out, list; !api.Semantic.DeepDerivative(e, a) {
 			t.Errorf("%v: Expected %#v, got %#v", name, e, a)
 		}
 	}
@@ -209,7 +208,7 @@ func TestEtcdCreate(t *testing.T) {
 			t.Errorf("%v: unexpected error: %v", name, err)
 		}

-		if e, a := item.expect, fakeClient.Data[path]; !reflect.DeepEqual(e, a) {
+		if e, a := item.expect, fakeClient.Data[path]; !api.Semantic.DeepDerivative(e, a) {
 			t.Errorf("%v:\n%s", name, util.ObjectDiff(e, a))
 		}
 	}
@@ -284,7 +283,7 @@ func TestEtcdUpdate(t *testing.T) {
 			t.Errorf("%v: unexpected error: %v", name, err)
 		}

-		if e, a := item.expect, fakeClient.Data[path]; !reflect.DeepEqual(e, a) {
+		if e, a := item.expect, fakeClient.Data[path]; !api.Semantic.DeepDerivative(e, a) {
 			t.Errorf("%v:\n%s", name, util.ObjectDiff(e, a))
 		}
 	}
@@ -340,7 +339,7 @@ func TestEtcdGet(t *testing.T) {
 			t.Errorf("%v: unexpected error: %v", name, err)
 		}

-		if e, a := item.expect, got; !reflect.DeepEqual(e, a) {
+		if e, a := item.expect, got; !api.Semantic.DeepDerivative(e, a) {
 			t.Errorf("%v:\n%s", name, util.ObjectDiff(e, a))
 		}
 	}
@@ -396,7 +395,7 @@ func TestEtcdDelete(t *testing.T) {
 			t.Errorf("%v: unexpected error: %v", name, err)
 		}

-		if e, a := item.expect, fakeClient.Data[path]; !reflect.DeepEqual(e, a) {
+		if e, a := item.expect, fakeClient.Data[path]; !api.Semantic.DeepDerivative(e, a) {
 			t.Errorf("%v:\n%s", name, util.ObjectDiff(e, a))
 		}
 	}
@@ -432,7 +431,7 @@ func TestEtcdWatch(t *testing.T) {
 		t.Fatalf("unexpected channel close")
 	}

-	if e, a := podA, got.Object; !reflect.DeepEqual(e, a) {
+	if e, a := podA, got.Object; !api.Semantic.DeepDerivative(e, a) {
 		t.Errorf("difference: %s", util.ObjectDiff(e, a))
 	}
 }
--- a/pkg/registry/pod/rest_test.go
+++ b/pkg/registry/pod/rest_test.go
@@ -88,6 +88,10 @@ func TestCreatePodRegistryError(t *testing.T) {
 		ObjectMeta: api.ObjectMeta{
 			Name: "foo",
 		},
+		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
+		},
 	}
 	ctx := api.NewDefaultContext()
 	ch, err := storage.Create(ctx, pod)
@@ -108,6 +112,10 @@ func TestCreatePodSetsIds(t *testing.T) {
 		ObjectMeta: api.ObjectMeta{
 			Name: "foo",
 		},
+		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
+		},
 	}
 	ctx := api.NewDefaultContext()
 	ch, err := storage.Create(ctx, pod)
@@ -135,6 +143,10 @@ func TestCreatePodSetsUID(t *testing.T) {
 		ObjectMeta: api.ObjectMeta{
 			Name: "foo",
 		},
+		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
+		},
 	}
 	ctx := api.NewDefaultContext()
 	ch, err := storage.Create(ctx, pod)
@@ -346,6 +358,10 @@ func TestPodDecode(t *testing.T) {
 		ObjectMeta: api.ObjectMeta{
 			Name: "foo",
 		},
+		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
+		},
 	}
 	body, err := latest.Codec.Encode(expected)
 	if err != nil {
@@ -447,7 +463,12 @@ func TestCreatePod(t *testing.T) {
 		registry: podRegistry,
 		podCache: &fakeCache{statusToReturn: &api.PodStatus{}},
 	}
-	pod := &api.Pod{}
+	pod := &api.Pod{
+		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
+		},
+	}
 	pod.Name = "foo"
 	ctx := api.NewDefaultContext()
 	channel, err := storage.Create(ctx, pod)
@@ -470,6 +491,10 @@ func TestCreatePodWithConflictingNamespace(t *testing.T) {
 	storage := REST{}
 	pod := &api.Pod{
 		ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "not-default"},
+		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
+		},
 	}

 	ctx := api.NewDefaultContext()
@@ -488,6 +513,10 @@ func TestUpdatePodWithConflictingNamespace(t *testing.T) {
 	storage := REST{}
 	pod := &api.Pod{
 		ObjectMeta: api.ObjectMeta{Name: "test", Namespace: "not-default"},
+		Spec: api.PodSpec{
+			RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+			DNSPolicy:     api.DNSClusterFirst,
+		},
 	}

 	ctx := api.NewDefaultContext()
@@ -647,10 +676,13 @@ func TestCreate(t *testing.T) {
 			Spec: api.PodSpec{
 				Containers: []api.Container{
 					{
-						Name:  "test1",
-						Image: "foo",
+						Name:            "test1",
+						Image:           "foo",
+						ImagePullPolicy: api.PullIfNotPresent,
 					},
 				},
+				RestartPolicy: api.RestartPolicy{Always: &api.RestartPolicyAlways{}},
+				DNSPolicy:     api.DNSClusterFirst,
 			},
 		},
 		// invalid
--- a/pkg/registry/service/rest_test.go
+++ b/pkg/registry/service/rest_test.go
@@ -49,8 +49,10 @@ func TestServiceRegistryCreate(t *testing.T) {
 	svc := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "foo"},
 		Spec: api.ServiceSpec{
-			Port:     6502,
-			Selector: map[string]string{"bar": "baz"},
+			Port:            6502,
+			Selector:        map[string]string{"bar": "baz"},
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	ctx := api.NewDefaultContext()
@@ -91,14 +93,18 @@ func TestServiceStorageValidatesCreate(t *testing.T) {
 		"empty ID": {
 			ObjectMeta: api.ObjectMeta{Name: ""},
 			Spec: api.ServiceSpec{
-				Port:     6502,
-				Selector: map[string]string{"bar": "baz"},
+				Port:            6502,
+				Selector:        map[string]string{"bar": "baz"},
+				Protocol:        api.ProtocolTCP,
+				SessionAffinity: api.AffinityTypeNone,
 			},
 		},
 		"empty selector": {
 			ObjectMeta: api.ObjectMeta{Name: "foo"},
 			Spec: api.ServiceSpec{
-				Selector: map[string]string{"bar": "baz"},
+				Selector:        map[string]string{"bar": "baz"},
+				Protocol:        api.ProtocolTCP,
+				SessionAffinity: api.AffinityTypeNone,
 			},
 		},
 	}
@@ -129,8 +135,10 @@ func TestServiceRegistryUpdate(t *testing.T) {
 	c, err := storage.Update(ctx, &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "foo"},
 		Spec: api.ServiceSpec{
-			Port:     6502,
-			Selector: map[string]string{"bar": "baz2"},
+			Port:            6502,
+			Selector:        map[string]string{"bar": "baz2"},
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	})
 	if err != nil {
@@ -164,15 +172,19 @@ func TestServiceStorageValidatesUpdate(t *testing.T) {
 		"empty ID": {
 			ObjectMeta: api.ObjectMeta{Name: ""},
 			Spec: api.ServiceSpec{
-				Port:     6502,
-				Selector: map[string]string{"bar": "baz"},
+				Port:            6502,
+				Selector:        map[string]string{"bar": "baz"},
+				Protocol:        api.ProtocolTCP,
+				SessionAffinity: api.AffinityTypeNone,
 			},
 		},
 		"invalid selector": {
 			ObjectMeta: api.ObjectMeta{Name: "foo"},
 			Spec: api.ServiceSpec{
-				Port:     6502,
-				Selector: map[string]string{"ThisSelectorFailsValidation": "ok"},
+				Port:            6502,
+				Selector:        map[string]string{"ThisSelectorFailsValidation": "ok"},
+				Protocol:        api.ProtocolTCP,
+				SessionAffinity: api.AffinityTypeNone,
 			},
 		},
 	}
@@ -199,6 +211,8 @@ func TestServiceRegistryExternalService(t *testing.T) {
 			Port:                       6502,
 			Selector:                   map[string]string{"bar": "baz"},
 			CreateExternalLoadBalancer: true,
+			Protocol:                   api.ProtocolTCP,
+			SessionAffinity:            api.AffinityTypeNone,
 		},
 	}
 	c, _ := storage.Create(ctx, svc)
@@ -228,6 +242,8 @@ func TestServiceRegistryExternalServiceError(t *testing.T) {
 			Port:                       6502,
 			Selector:                   map[string]string{"bar": "baz"},
 			CreateExternalLoadBalancer: true,
+			Protocol:                   api.ProtocolTCP,
+			SessionAffinity:            api.AffinityTypeNone,
 		},
 	}
 	ctx := api.NewDefaultContext()
@@ -250,7 +266,9 @@ func TestServiceRegistryDelete(t *testing.T) {
 	svc := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "foo"},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
+			Selector:        map[string]string{"bar": "baz"},
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	registry.CreateService(ctx, svc)
@@ -275,6 +293,8 @@ func TestServiceRegistryDeleteExternal(t *testing.T) {
 		Spec: api.ServiceSpec{
 			Selector:                   map[string]string{"bar": "baz"},
 			CreateExternalLoadBalancer: true,
+			Protocol:                   api.ProtocolTCP,
+			SessionAffinity:            api.AffinityTypeNone,
 		},
 	}
 	registry.CreateService(ctx, svc)
@@ -389,8 +409,10 @@ func TestServiceRegistryIPAllocation(t *testing.T) {
 	svc1 := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "foo"},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
-			Port:     6502,
+			Selector:        map[string]string{"bar": "baz"},
+			Port:            6502,
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	ctx := api.NewDefaultContext()
@@ -407,8 +429,10 @@ func TestServiceRegistryIPAllocation(t *testing.T) {
 	svc2 := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "bar"},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
-			Port:     6502,
+			Selector:        map[string]string{"bar": "baz"},
+			Port:            6502,
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		}}
 	ctx = api.NewDefaultContext()
 	c2, _ := rest.Create(ctx, svc2)
@@ -424,9 +448,11 @@ func TestServiceRegistryIPAllocation(t *testing.T) {
 	svc3 := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "quux"},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
-			PortalIP: "1.2.3.93",
-			Port:     6502,
+			Selector:        map[string]string{"bar": "baz"},
+			PortalIP:        "1.2.3.93",
+			Port:            6502,
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	ctx = api.NewDefaultContext()
@@ -448,8 +474,10 @@ func TestServiceRegistryIPReallocation(t *testing.T) {
 	svc1 := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "foo"},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
-			Port:     6502,
+			Selector:        map[string]string{"bar": "baz"},
+			Port:            6502,
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	ctx := api.NewDefaultContext()
@@ -469,8 +497,10 @@ func TestServiceRegistryIPReallocation(t *testing.T) {
 	svc2 := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "bar"},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
-			Port:     6502,
+			Selector:        map[string]string{"bar": "baz"},
+			Port:            6502,
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	ctx = api.NewDefaultContext()
@@ -495,8 +525,10 @@ func TestServiceRegistryIPUpdate(t *testing.T) {
 	svc := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "foo"},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
-			Port:     6502,
+			Selector:        map[string]string{"bar": "baz"},
+			Port:            6502,
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	ctx := api.NewDefaultContext()
@@ -551,6 +583,8 @@ func TestServiceRegistryIPExternalLoadBalancer(t *testing.T) {
 			Selector: map[string]string{"bar": "baz"},
 			Port:     6502,
 			CreateExternalLoadBalancer: true,
+			Protocol:                   api.ProtocolTCP,
+			SessionAffinity:            api.AffinityTypeNone,
 		},
 	}
 	ctx := api.NewDefaultContext()
@@ -586,8 +620,10 @@ func TestServiceRegistryIPReloadFromStorage(t *testing.T) {
 	svc := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
-			Port:     6502,
+			Selector:        map[string]string{"bar": "baz"},
+			Port:            6502,
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	ctx := api.NewDefaultContext()
@@ -596,8 +632,10 @@ func TestServiceRegistryIPReloadFromStorage(t *testing.T) {
 	svc = &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
-			Port:     6502,
+			Selector:        map[string]string{"bar": "baz"},
+			Port:            6502,
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	c, _ = rest1.Create(ctx, svc)
@@ -610,8 +648,10 @@ func TestServiceRegistryIPReloadFromStorage(t *testing.T) {
 	svc = &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault},
 		Spec: api.ServiceSpec{
-			Selector: map[string]string{"bar": "baz"},
-			Port:     6502,
+			Selector:        map[string]string{"bar": "baz"},
+			Port:            6502,
+			Protocol:        api.ProtocolTCP,
+			SessionAffinity: api.AffinityTypeNone,
 		},
 	}
 	c, _ = rest2.Create(ctx, svc)
@@ -671,8 +711,10 @@ func TestCreate(t *testing.T) {
 		// valid
 		&api.Service{
 			Spec: api.ServiceSpec{
-				Selector: map[string]string{"bar": "baz"},
-				Port:     6502,
+				Selector:        map[string]string{"bar": "baz"},
+				Port:            6502,
+				Protocol:        "TCP",
+				SessionAffinity: "None",
 			},
 		},
 		// invalid