Browse code

Fix 'kube apply' panic on nil items

Bug 1146024 - https://bugzilla.redhat.com/show_bug.cgi?id=1146024

Vojtech Vitek (V-Teq) authored on 2014/09/26 06:11:55
Showing 3 changed files
... ...
@@ -479,6 +479,9 @@ func (c *KubeConfig) executeControllerRequest(method string, client *kubeclient.
479 479
 
480 480
 // executeTemplateRequest transform the JSON file with Config template into a
481 481
 // valid Config JSON.
482
+//
483
+// TODO: Print the output for each resource on success, as "create" method
484
+//       does in the executeAPIRequest().
482 485
 func (c *KubeConfig) executeTemplateRequest(method string, client *osclient.Client) bool {
483 486
 	if method != "process" {
484 487
 		return false
... ...
@@ -5,80 +5,82 @@ import (
5 5
 	"fmt"
6 6
 
7 7
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
8
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
8 9
 
9 10
 	clientapi "github.com/openshift/origin/pkg/cmd/client/api"
10 11
 )
11 12
 
12
-// configJSON stores the raw Config JSON representation
13
-// TODO: Replace this with configapi.Config when it handles the unregistred types.
14
-type configJSON struct {
15
-	Items []interface{} `json:"items" yaml:"items"`
16
-}
17
-
18 13
 // Apply creates and manages resources defined in the Config. It wont stop on
19 14
 // error, but it will finish the job and then return list of errors.
15
+//
16
+// TODO: Return the output for each resource on success, so the client can
17
+//       print it out.
20 18
 func Apply(data []byte, storage clientapi.ClientMappings) (errs errors.ErrorList) {
21 19
 
22
-	// Unmarshal the Config JSON using default json package instead of
23
-	// api.Decode()
24
-	conf := configJSON{}
20
+	// Unmarshal the Config JSON manually instead of using runtime.Decode()
21
+	conf := struct {
22
+		Items []json.RawMessage `json:"items" yaml:"items"`
23
+	}{}
25 24
 	if err := json.Unmarshal(data, &conf); err != nil {
26 25
 		return append(errs, fmt.Errorf("Unable to parse Config: %v", err))
27 26
 	}
28 27
 
29
-	for _, item := range conf.Items {
30
-		kind, itemID, parseErrs := parseKindAndID(item)
31
-		if len(parseErrs) != 0 {
32
-			errs = append(errs, parseErrs...)
28
+	if len(conf.Items) == 0 {
29
+		return append(errs, fmt.Errorf("Config.items is empty"))
30
+	}
31
+
32
+	for i, item := range conf.Items {
33
+		if item == nil || (len(item) == 4 && string(item) == "null") {
34
+			errs = append(errs, fmt.Errorf("Config.items[%v] is null", i))
33 35
 			continue
34 36
 		}
35 37
 
36
-		client, path := getClientAndPath(kind, storage)
38
+		_, kind, err := runtime.VersionAndKind(item)
39
+		if err != nil {
40
+			errs = append(errs, fmt.Errorf("Config.items[%v]: %v", i, err))
41
+			continue
42
+		}
43
+
44
+		if kind == "" {
45
+			errs = append(errs, fmt.Errorf("Config.items[%v] has an empty 'kind'", i))
46
+			continue
47
+		}
48
+
49
+		client, path, err := getClientAndPath(kind, storage)
50
+		if err != nil {
51
+			errs = append(errs, fmt.Errorf("Config.items[%v]: %v", i, err))
52
+			continue
53
+		}
37 54
 		if client == nil {
38
-			errs = append(errs, fmt.Errorf("The resource %s is not a known type - unable to create %s", kind, itemID))
55
+			errs = append(errs, fmt.Errorf("Config.items[%v]: Invalid client for 'kind=%v'", i, kind))
39 56
 			continue
40 57
 		}
41 58
 
42
-		// Serialize the single Config item back into JSON
43
-		itemJSON, _ := json.Marshal(item)
59
+		jsonResource, err := item.MarshalJSON()
60
+		if err != nil {
61
+			errs = append(errs, err)
62
+			continue
63
+		}
44 64
 
45
-		request := client.Verb("POST").Path(path).Body(itemJSON)
46
-		_, err := request.Do().Get()
65
+		request := client.Verb("POST").Path(path).Body(jsonResource)
66
+		_, err = request.Do().Get()
47 67
 		if err != nil {
48
-			errs = append(errs, fmt.Errorf("[%s#%s] Failed to create: %v", kind, itemID, err))
68
+			errs = append(errs, fmt.Errorf("Failed to create Config.items[%v] of 'kind=%v': %v", i, kind, err))
49 69
 		}
50 70
 	}
51 71
 
52 72
 	return
53 73
 }
54 74
 
55
-// getClientAndPath returns the RESTClient and path defined for given resource
56
-// kind.
57
-func getClientAndPath(kind string, mappings clientapi.ClientMappings) (client clientapi.RESTClient, path string) {
75
+// getClientAndPath returns the RESTClient and path defined for a given
76
+// resource kind. Returns an error when no RESTClient is found.
77
+func getClientAndPath(kind string, mappings clientapi.ClientMappings) (clientapi.RESTClient, string, error) {
58 78
 	for k, m := range mappings {
59 79
 		if m.Kind == kind {
60
-			return m.Client, k
80
+			return m.Client, k, nil
61 81
 		}
62 82
 	}
63
-	return
64
-}
65
-
66
-// parseKindAndID extracts the 'kind' and 'id' fields from the Config item JSON
67
-// and report errors if these fields are missing.
68
-func parseKindAndID(item interface{}) (kind, id string, errs errors.ErrorList) {
69
-	itemMap := item.(map[string]interface{})
70
-
71
-	kind, ok := itemMap["kind"].(string)
72
-	if !ok {
73
-		errs = append(errs, reportError(item, "Missing 'kind' field for Config item"))
74
-	}
75
-
76
-	id, ok = itemMap["id"].(string)
77
-	if !ok {
78
-		errs = append(errs, reportError(item, "Missing 'id' field for Config item"))
79
-	}
80
-
81
-	return
83
+	return nil, "", fmt.Errorf("No client found for 'kind=%v'", kind)
82 84
 }
83 85
 
84 86
 // reportError provides a human-readable error message that include the Config
... ...
@@ -1,43 +1,74 @@
1 1
 package config
2 2
 
3 3
 import (
4
-	"encoding/json"
5 4
 	"io/ioutil"
5
+	"net/http"
6
+	"net/http/httptest"
6 7
 	"testing"
7 8
 
9
+	kubeapi "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
8 10
 	klatest "github.com/GoogleCloudPlatform/kubernetes/pkg/api/latest"
9 11
 	kubeclient "github.com/GoogleCloudPlatform/kubernetes/pkg/client"
12
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
10 13
 	clientapi "github.com/openshift/origin/pkg/cmd/client/api"
11 14
 )
12 15
 
13
-func TestParseKindAndItem(t *testing.T) {
14
-	data, _ := ioutil.ReadFile("config_test.json")
15
-	conf := configJSON{}
16
-	if err := json.Unmarshal(data, &conf); err != nil {
17
-		t.Errorf("Failed to parse Config: %v", err)
16
+func TestApplyInvalidConfig(t *testing.T) {
17
+	clients := clientapi.ClientMappings{
18
+		"InvalidClientMapping": {"InvalidClientResource", nil, nil},
18 19
 	}
19
-
20
-	kind, itemID, err := parseKindAndID(conf.Items[0])
21
-	if len(err) != 0 {
22
-		t.Errorf("Failed to parse kind and id from the Config item: %v", err)
20
+	invalidConfigs := []string{
21
+		`{}`,
22
+		`{ "foo": "bar" }`,
23
+		`{ "items": null }`,
24
+		`{ "items": "bar" }`,
25
+		`{ "items": [ null ] }`,
26
+		`{ "items": [ { "foo": "bar" } ] }`,
27
+		`{ "items": [ { "kind": "", "apiVersion": "v1beta1" } ] }`,
28
+		`{ "items": [ { "kind": "UnknownResource", "apiVersion": "v1beta1" } ] }`,
29
+		`{ "items": [ { "kind": "InvalidClientResource", "apiVersion": "v1beta1" } ] }`,
23 30
 	}
24
-
25
-	if kind != "Service" && itemID != "frontend" {
26
-		t.Errorf("Invalid kind and id, should be Service and frontend: %s, %s", kind, itemID)
31
+	for _, invalidConfig := range invalidConfigs {
32
+		errs := Apply([]byte(invalidConfig), clients)
33
+		if len(errs) == 0 {
34
+			t.Errorf("Expected error while applying invalid Config '%v'", invalidConfig)
35
+		}
27 36
 	}
28 37
 }
29 38
 
30
-func TestApply(t *testing.T) {
31
-	clients := clientapi.ClientMappings{}
32
-	invalidData := []byte(`{"items": [ { "foo": "bar" } ]}`)
33
-	errs := Apply(invalidData, clients)
34
-	if len(errs) == 0 {
35
-		t.Errorf("Expected missing kind field for Config item, got %v", errs)
39
+type FakeResource struct {
40
+	kubeapi.JSONBase `json:",inline" yaml:",inline"`
41
+}
42
+
43
+func (*FakeResource) IsAnAPIObject() {}
44
+
45
+func TestApplySendsData(t *testing.T) {
46
+	fakeScheme := runtime.NewScheme()
47
+	// TODO: The below should work with "FakeResource" name instead.
48
+	fakeScheme.AddKnownTypeWithName("", "", &FakeResource{})
49
+	fakeScheme.AddKnownTypeWithName("v1beta1", "", &FakeResource{})
50
+	fakeCodec := runtime.CodecFor(fakeScheme, "v1beta1")
51
+
52
+	received := make(chan bool, 1)
53
+	fakeServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
54
+		received <- true
55
+		if r.RequestURI != "/api/v1beta1/FakeMapping" {
56
+			t.Errorf("Unexpected RESTClient RequestURI. Expected: %v, got: %v.", "/api/v1beta1/FakeMapping", r.RequestURI)
57
+		}
58
+	}))
59
+
60
+	fakeClient, _ := kubeclient.NewRESTClient(fakeServer.URL, nil, "/api/v1beta1/", fakeCodec)
61
+	clients := clientapi.ClientMappings{
62
+		"FakeMapping": {"FakeResource", fakeClient, fakeCodec},
36 63
 	}
37
-	uErrs := Apply([]byte(`{ "foo": }`), clients)
38
-	if len(uErrs) == 0 {
39
-		t.Errorf("Expected unmarshal error, got nothing")
64
+	config := `{ "apiVersion": "v1beta1", "items": [ { "kind": "FakeResource", "apiVersion": "v1beta1" } ] }`
65
+
66
+	errs := Apply([]byte(config), clients)
67
+	if len(errs) != 0 {
68
+		t.Errorf("Unexpected error while applying valid Config '%v': %v", config, errs)
40 69
 	}
70
+
71
+	<-received
41 72
 }
42 73
 
43 74
 func TestGetClientAndPath(t *testing.T) {
... ...
@@ -46,7 +77,7 @@ func TestGetClientAndPath(t *testing.T) {
46 46
 		"pods":     {"Pod", kubeClient.RESTClient, klatest.Codec},
47 47
 		"services": {"Service", kubeClient.RESTClient, klatest.Codec},
48 48
 	}
49
-	client, path := getClientAndPath("Service", testClientMappings)
49
+	client, path, _ := getClientAndPath("Service", testClientMappings)
50 50
 	if client != kubeClient.RESTClient {
51 51
 		t.Errorf("Failed to get client for Service")
52 52
 	}