Skip to content

K
k3s
Commit 6d31c2bf authored by Clayton Coleman's avatar Clayton Coleman
Browse files
Options

util.EncodeJSON proven harmful, remove it everywhere

People were misusing EncodeJSON in tests when they should be using runtime.EncodeOrDie(testapi.Codec(), obj). Removing the potential for cutting self on sharp objects.
parent d4108ec4
Hide whitespace changes
Inline Side-by-side
Showing with 49 additions and 81 deletions
pkg/cloudprovider/controller/minioncontroller_test.go
View file @ 6d31c2bf
......@@ -30,7 +30,7 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
)
func newMinionList(count int) api.MinionList {
func newMinionList(count int) *api.MinionList {
minions := []api.Minion{}
for i := 0; i < count; i++ {
minions = append(minions, api.Minion{
......@@ -39,7 +39,7 @@ func newMinionList(count int) api.MinionList {
},
})
}
return api.MinionList{
return &api.MinionList{
Items: minions,
}
}
......@@ -52,7 +52,7 @@ type serverResponse struct {
func makeTestServer(t *testing.T, minionResponse serverResponse) (*httptest.Server, *util.FakeHandler) {
fakeMinionHandler := util.FakeHandler{
StatusCode: minionResponse.statusCode,
ResponseBody: util.EncodeJSON(minionResponse.obj),
ResponseBody: runtime.EncodeOrDie(testapi.Codec(), minionResponse.obj.(runtime.Object)),
}
mux := http.NewServeMux()
mux.Handle("/api/"+testapi.Version()+"/minions", &fakeMinionHandler)
......
pkg/controller/replication_controller_test.go
View file @ 6d31c2bf
......@@ -284,10 +284,10 @@ func TestSynchonize(t *testing.T) {
Node: &etcd.Node{
Nodes: []*etcd.Node{
{
Value: util.EncodeJSON(controllerSpec1),
Value: runtime.EncodeOrDie(testapi.Codec(), &controllerSpec1),
},
{
Value: util.EncodeJSON(controllerSpec2),
Value: runtime.EncodeOrDie(testapi.Codec(), &controllerSpec2),
},
},
},
......
pkg/kubelet/server_test.go
View file @ 6d31c2bf
......@@ -30,7 +30,6 @@ import (
"testing"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/google/cadvisor/info"
)
......@@ -97,6 +96,15 @@ func newServerTest() *serverTestFramework {
return fw
}
// encodeJSON returns obj marshalled as a JSON string, panicing on any errors
func encodeJSON(obj interface{}) string {
data, err := json.Marshal(obj)
if err != nil {
panic(err)
}
return string(data)
}
func readResp(resp *http.Response) (string, error) {
defer resp.Body.Close()
body, err := ioutil.ReadAll(resp.Body)
......@@ -124,7 +132,7 @@ func TestContainer(t *testing.T) {
},
},
}
body := bytes.NewBuffer([]byte(util.EncodeJSON(expected[0]))) // Only send a single ContainerManifest
body := bytes.NewBuffer([]byte(encodeJSON(expected[0]))) // Only send a single ContainerManifest
resp, err := http.Post(fw.testHTTPServer.URL+"/container", "application/json", body)
if err != nil {
t.Errorf("Post returned: %v", err)
......@@ -199,7 +207,7 @@ func TestContainers(t *testing.T) {
},
},
}
body := bytes.NewBuffer([]byte(util.EncodeJSON(expected)))
body := bytes.NewBuffer([]byte(encodeJSON(expected)))
resp, err := http.Post(fw.testHTTPServer.URL+"/containers", "application/json", body)
if err != nil {
t.Errorf("Post returned: %v", err)
......
pkg/service/endpoints_controller_test.go
View file @ 6d31c2bf
......@@ -30,7 +30,7 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
)
func newPodList(count int) api.PodList {
func newPodList(count int) *api.PodList {
pods := []api.Pod{}
for i := 0; i < count; i++ {
pods = append(pods, api.Pod{
......@@ -54,7 +54,7 @@ func newPodList(count int) api.PodList {
},
})
}
return api.PodList{
return &api.PodList{
TypeMeta: api.TypeMeta{APIVersion: testapi.Version(), Kind: "PodList"},
Items: pods,
}
......@@ -170,15 +170,15 @@ type serverResponse struct {
func makeTestServer(t *testing.T, podResponse serverResponse, serviceResponse serverResponse, endpointsResponse serverResponse) (*httptest.Server, *util.FakeHandler) {
fakePodHandler := util.FakeHandler{
StatusCode: podResponse.statusCode,
ResponseBody: util.EncodeJSON(podResponse.obj),
ResponseBody: runtime.EncodeOrDie(testapi.Codec(), podResponse.obj.(runtime.Object)),
}
fakeServiceHandler := util.FakeHandler{
StatusCode: serviceResponse.statusCode,
ResponseBody: util.EncodeJSON(serviceResponse.obj),
ResponseBody: runtime.EncodeOrDie(testapi.Codec(), serviceResponse.obj.(runtime.Object)),
}
fakeEndpointsHandler := util.FakeHandler{
StatusCode: endpointsResponse.statusCode,
ResponseBody: util.EncodeJSON(endpointsResponse.obj),
ResponseBody: runtime.EncodeOrDie(testapi.Codec(), endpointsResponse.obj.(runtime.Object)),
}
mux := http.NewServeMux()
mux.Handle("/api/"+testapi.Version()+"/pods", &fakePodHandler)
......@@ -195,8 +195,8 @@ func makeTestServer(t *testing.T, podResponse serverResponse, serviceResponse se
func TestSyncEndpointsEmpty(t *testing.T) {
testServer, _ := makeTestServer(t,
serverResponse{http.StatusOK, newPodList(0)},
serverResponse{http.StatusOK, api.ServiceList{}},
serverResponse{http.StatusOK, api.Endpoints{}})
serverResponse{http.StatusOK, &api.ServiceList{}},
serverResponse{http.StatusOK, &api.Endpoints{}})
defer testServer.Close()
client := client.NewOrDie(&client.Config{Host: testServer.URL, Version: testapi.Version()})
endpoints := NewEndpointController(client)
......@@ -208,8 +208,8 @@ func TestSyncEndpointsEmpty(t *testing.T) {
func TestSyncEndpointsError(t *testing.T) {
testServer, _ := makeTestServer(t,
serverResponse{http.StatusOK, newPodList(0)},
serverResponse{http.StatusInternalServerError, api.ServiceList{}},
serverResponse{http.StatusOK, api.Endpoints{}})
serverResponse{http.StatusInternalServerError, &api.ServiceList{}},
serverResponse{http.StatusOK, &api.Endpoints{}})
defer testServer.Close()
client := client.NewOrDie(&client.Config{Host: testServer.URL, Version: testapi.Version()})
endpoints := NewEndpointController(client)
......@@ -233,8 +233,8 @@ func TestSyncEndpointsItemsPreexisting(t *testing.T) {
}
testServer, endpointsHandler := makeTestServer(t,
serverResponse{http.StatusOK, newPodList(1)},
serverResponse{http.StatusOK, serviceList},
serverResponse{http.StatusOK, api.Endpoints{
serverResponse{http.StatusOK, &serviceList},
serverResponse{http.StatusOK, &api.Endpoints{
ObjectMeta: api.ObjectMeta{
Name: "foo",
ResourceVersion: "1",
......@@ -272,8 +272,8 @@ func TestSyncEndpointsItemsPreexistingIdentical(t *testing.T) {
}
testServer, endpointsHandler := makeTestServer(t,
serverResponse{http.StatusOK, newPodList(1)},
serverResponse{http.StatusOK, serviceList},
serverResponse{http.StatusOK, api.Endpoints{
serverResponse{http.StatusOK, &serviceList},
serverResponse{http.StatusOK, &api.Endpoints{
ObjectMeta: api.ObjectMeta{
ResourceVersion: "1",
},
......@@ -303,8 +303,8 @@ func TestSyncEndpointsItems(t *testing.T) {
}
testServer, endpointsHandler := makeTestServer(t,
serverResponse{http.StatusOK, newPodList(1)},
serverResponse{http.StatusOK, serviceList},
serverResponse{http.StatusOK, api.Endpoints{}})
serverResponse{http.StatusOK, &serviceList},
serverResponse{http.StatusOK, &api.Endpoints{}})
defer testServer.Close()
client := client.NewOrDie(&client.Config{Host: testServer.URL, Version: testapi.Version()})
endpoints := NewEndpointController(client)
......@@ -333,9 +333,9 @@ func TestSyncEndpointsPodError(t *testing.T) {
},
}
testServer, _ := makeTestServer(t,
serverResponse{http.StatusInternalServerError, api.PodList{}},
serverResponse{http.StatusOK, serviceList},
serverResponse{http.StatusOK, api.Endpoints{}})
serverResponse{http.StatusInternalServerError, &api.PodList{}},
serverResponse{http.StatusOK, &serviceList},
serverResponse{http.StatusOK, &api.Endpoints{}})
defer testServer.Close()
client := client.NewOrDie(&client.Config{Host: testServer.URL, Version: testapi.Version()})
endpoints := NewEndpointController(client)
......
pkg/tools/etcd_tools_test.go
View file @ 6d31c2bf
......@@ -24,10 +24,9 @@ import (
"testing"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/meta"
"github.com/GoogleCloudPlatform/kubernetes/pkg/api/testapi"
"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
"github.com/coreos/go-etcd/etcd"
)
......@@ -100,7 +99,7 @@ func TestExtractToList(t *testing.T) {
}
var got api.PodList
helper := EtcdHelper{fakeClient, latest.Codec, versioner}
helper := EtcdHelper{fakeClient, testapi.Codec(), versioner}
err := helper.ExtractToList("/some/key", &got)
if err != nil {
t.Errorf("Unexpected error %v", err)
......@@ -151,7 +150,7 @@ func TestExtractToListAcrossDirectories(t *testing.T) {
}
var got api.PodList
helper := EtcdHelper{fakeClient, latest.Codec, versioner}
helper := EtcdHelper{fakeClient, testapi.Codec(), versioner}
err := helper.ExtractToList("/some/key", &got)
if err != nil {
t.Errorf("Unexpected error %v", err)
......@@ -198,7 +197,7 @@ func TestExtractToListExcludesDirectories(t *testing.T) {
}
var got api.PodList
helper := EtcdHelper{fakeClient, latest.Codec, versioner}
helper := EtcdHelper{fakeClient, testapi.Codec(), versioner}
err := helper.ExtractToList("/some/key", &got)
if err != nil {
t.Errorf("Unexpected error %v", err)
......@@ -211,8 +210,8 @@ func TestExtractToListExcludesDirectories(t *testing.T) {
func TestExtractObj(t *testing.T) {
fakeClient := NewFakeEtcdClient(t)
expect := api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}
fakeClient.Set("/some/key", util.EncodeJSON(expect), 0)
helper := EtcdHelper{fakeClient, latest.Codec, versioner}
fakeClient.Set("/some/key", runtime.EncodeOrDie(testapi.Codec(), &expect), 0)
helper := EtcdHelper{fakeClient, testapi.Codec(), versioner}
var got api.Pod
err := helper.ExtractObj("/some/key", &got, false)
if err != nil {
......@@ -266,12 +265,12 @@ func TestExtractObjNotFoundErr(t *testing.T) {
func TestCreateObj(t *testing.T) {
obj := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}
fakeClient := NewFakeEtcdClient(t)
helper := EtcdHelper{fakeClient, latest.Codec, versioner}
helper := EtcdHelper{fakeClient, testapi.Codec(), versioner}
err := helper.CreateObj("/some/key", obj, 5)
if err != nil {
t.Errorf("Unexpected error %#v", err)
}
data, err := latest.Codec.Encode(obj)
data, err := testapi.Codec().Encode(obj)
if err != nil {
t.Errorf("Unexpected error %#v", err)
}
......@@ -287,12 +286,12 @@ func TestCreateObj(t *testing.T) {
func TestSetObj(t *testing.T) {
obj := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}
fakeClient := NewFakeEtcdClient(t)
helper := EtcdHelper{fakeClient, latest.Codec, versioner}
helper := EtcdHelper{fakeClient, testapi.Codec(), versioner}
err := helper.SetObj("/some/key", obj)
if err != nil {
t.Errorf("Unexpected error %#v", err)
}
data, err := latest.Codec.Encode(obj)
data, err := testapi.Codec().Encode(obj)
if err != nil {
t.Errorf("Unexpected error %#v", err)
}
......@@ -310,18 +309,18 @@ func TestSetObjWithVersion(t *testing.T) {
fakeClient.Data["/some/key"] = EtcdResponseWithError{
R: &etcd.Response{
Node: &etcd.Node{
Value: runtime.EncodeOrDie(latest.Codec, obj),
Value: runtime.EncodeOrDie(testapi.Codec(), obj),
ModifiedIndex: 1,
},
},
}
helper := EtcdHelper{fakeClient, latest.Codec, versioner}
helper := EtcdHelper{fakeClient, testapi.Codec(), versioner}
err := helper.SetObj("/some/key", obj)
if err != nil {
t.Fatalf("Unexpected error %#v", err)
}
data, err := latest.Codec.Encode(obj)
data, err := testapi.Codec().Encode(obj)
if err != nil {
t.Fatalf("Unexpected error %#v", err)
}
......@@ -335,12 +334,12 @@ func TestSetObjWithVersion(t *testing.T) {
func TestSetObjWithoutResourceVersioner(t *testing.T) {
obj := &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}
fakeClient := NewFakeEtcdClient(t)
helper := EtcdHelper{fakeClient, latest.Codec, nil}
helper := EtcdHelper{fakeClient, testapi.Codec(), nil}
err := helper.SetObj("/some/key", obj)
if err != nil {
t.Errorf("Unexpected error %#v", err)
}
data, err := latest.Codec.Encode(obj)
data, err := testapi.Codec().Encode(obj)
if err != nil {
t.Errorf("Unexpected error %#v", err)
}
......
pkg/util/util.go
View file @ 6d31c2bf
......@@ -60,12 +60,6 @@ func Forever(f func(), period time.Duration) {
}
}
// EncodeJSON returns obj marshalled as a JSON string, ignoring any errors.
func EncodeJSON(obj interface{}) string {
data, _ := json.Marshal(obj)
return string(data)
}
// IntOrString is a type that can hold an int or a string. When used in
// JSON or YAML marshalling and unmarshalling, it produces or consumes the
// inner type. This allows you to have, for example, a JSON field that can
......
pkg/util/util_test.go
View file @ 6d31c2bf
......@@ -24,39 +24,6 @@ import (
"gopkg.in/v1/yaml"
)
type FakeTypeMeta struct {
ID string
}
type FakePod struct {
FakeTypeMeta `json:",inline" yaml:",inline"`
Labels map[string]string
Int int
Str string
}
func TestEncodeJSON(t *testing.T) {
pod := FakePod{
FakeTypeMeta: FakeTypeMeta{ID: "foo"},
Labels: map[string]string{
"foo": "bar",
"baz": "blah",
},
Int: -6,
Str: "a string",
}
body := EncodeJSON(pod)
expectedBody, err := json.Marshal(pod)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if string(expectedBody) != body {
t.Errorf("JSON doesn't match. Expected %s, saw %s", expectedBody, body)
}
}
func TestHandleCrash(t *testing.T) {
count := 0
expect := 10
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment