Merge pull request #1429 from laozc/windows-info

Fixes invalid JSON in crictl info

Author: k8s-ci-robot

Diff for: cmd/crictl/util.go

@@ -245,37 +245,48 @@ func outputStatusInfo(status, handlers string, info map[string]string, format st
 	}
 	sort.Strings(keys)
 
-	jsonInfo := "{" + "\"status\":" + status + ","
-	if handlers != "" {
-		jsonInfo += "\"runtimeHandlers\":" + handlers + ","
+	infoMap := map[string]interface{}{}
+
+	var statusVal map[string]interface{}
+	err := json.Unmarshal([]byte(status), &statusVal)
+	if err != nil {
+		return err
+	}
+	infoMap["status"] = statusVal
+
+	var handlersVal map[string]interface{}
+	err = json.Unmarshal([]byte(handlers), &handlersVal)
+	if err != nil {
+		return err
 	}
+	if handlersVal != nil {
+		infoMap["runtimeHandlers"] = handlersVal
+	}
+
 	for _, k := range keys {
-		var res interface{}
-		// We attempt to convert key into JSON if possible else use it directly
-		if err := json.Unmarshal([]byte(info[k]), &res); err != nil {
-			jsonInfo += "\"" + k + "\"" + ":" + "\"" + info[k] + "\","
-		} else {
-			jsonInfo += "\"" + k + "\"" + ":" + info[k] + ","
-		}
+		infoMap[k] = strings.Trim(info[k], "\"")
+	}
+
+	jsonInfo, err := json.Marshal(infoMap)
+	if err != nil {
+		return err
 	}
-	jsonInfo = jsonInfo[:len(jsonInfo)-1]
-	jsonInfo += "}"
 
 	switch format {
 	case "yaml":
-		yamlInfo, err := yaml.JSONToYAML([]byte(jsonInfo))
+		yamlInfo, err := yaml.JSONToYAML(jsonInfo)
 		if err != nil {
 			return err
 		}
 		fmt.Println(string(yamlInfo))
 	case "json":
 		var output bytes.Buffer
-		if err := json.Indent(&output, []byte(jsonInfo), "", "  "); err != nil {
+		if err := json.Indent(&output, jsonInfo, "", "  "); err != nil {
 			return err
 		}
 		fmt.Println(output.String())
 	case "go-template":
-		output, err := tmplExecuteRawJSON(tmplStr, jsonInfo)
+		output, err := tmplExecuteRawJSON(tmplStr, string(jsonInfo))
 		if err != nil {
 			return err
 		}

Diff for: cmd/crictl/util_test.go

@@ -17,7 +17,12 @@ limitations under the License.
 package main
 
 import (
+	"io"
+	"os"
+	"strings"
 	"testing"
+
+	. "github.com/onsi/gomega"
 )
 
 func TestNameFilterByRegex(t *testing.T) {
@@ -64,7 +69,99 @@ func TestNameFilterByRegex(t *testing.T) {
 			if r != tc.isMatch {
 				t.Errorf("expected matched to be %v; actual result is %v", tc.isMatch, r)
 			}
+		})
+	}
+}
 
+func TestOutputStatusInfo(t *testing.T) {
+	const (
+		statusResponse = `{"conditions":[{
+				"message": "no network config found in C:\\Program Files",
+        		"reason": "NetworkPluginNotReady",
+        		"status": false,
+        		"type": "NetworkReady"
+      		}]}`
+	)
+	testCases := []struct {
+		name        string
+		status      string
+		handlers    string
+		info        map[string]string
+		format      string
+		tmplStr     string
+		expectedOut string
+	}{
+		{
+			name:        "YAML format",
+			status:      statusResponse,
+			handlers:    `{"handlers":["handler1","handler2"]}`,
+			info:        map[string]string{"key1": "value1", "key2": "/var/lib"},
+			format:      "yaml",
+			tmplStr:     "",
+			expectedOut: "key1: value1\nkey2: /var/lib\nruntimeHandlers:\n  handlers:\n  - handler1\n  - handler2\nstatus:\n  conditions:\n  - message: no network config found in C:\\Program Files\n    reason: NetworkPluginNotReady\n    status: false\n    type: NetworkReady",
+		},
+		{
+			name:        "JSON format",
+			status:      statusResponse,
+			handlers:    `{"handlers":["handler1","handler2"]}`,
+			info:        map[string]string{"key1": "\"value1\"", "key2": "\"C:\\ProgramFiles\""},
+			format:      "json",
+			tmplStr:     "",
+			expectedOut: "{\n  \"key1\": \"value1\",\n  \"key2\": \"C:\\\\ProgramFiles\",\n  \"runtimeHandlers\": {\n    \"handlers\": [\n      \"handler1\",\n      \"handler2\"\n    ]\n  },\n  \"status\": {\n    \"conditions\": [\n      {\n        \"message\": \"no network config found in C:\\\\Program Files\",\n        \"reason\": \"NetworkPluginNotReady\",\n        \"status\": false,\n        \"type\": \"NetworkReady\"\n      }\n    ]\n  }\n}",
+		},
+		{
+			name:        "Go template format",
+			status:      statusResponse,
+			handlers:    `{"handlers":["handler1","handler2"]}`,
+			info:        map[string]string{"key1": "value1", "key2": "value2"},
+			format:      "go-template",
+			tmplStr:     `NetworkReady: {{ (index .status.conditions 0).status }}`,
+			expectedOut: "NetworkReady: false",
+		},
+	}
+
+	// Run tests
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			captureOutput := func(f func() error) (string, error) {
+				var err error
+				old := os.Stdout
+
+				r, w, _ := os.Pipe()
+				os.Stdout = w
+				defer func() {
+					os.Stdout = old
+				}()
+
+				err = f()
+				if err != nil {
+					return "", err
+				}
+
+				err = w.Close()
+				if err != nil {
+					return "", err
+				}
+
+				out, err := io.ReadAll(r)
+				return strings.TrimRight(string(out), "\n"), err
+			}
+
+			outStr, err := captureOutput(func() error {
+				err := outputStatusInfo(tc.status, tc.handlers, tc.info, tc.format, tc.tmplStr)
+				if err != nil {
+					t.Errorf("Unexpected error: %v", err)
+				}
+				return nil
+			})
+
+			if err != nil {
+				Expect(err).To(BeNil())
+			}
+
+			if outStr != tc.expectedOut {
+				t.Errorf("Expected output:\n%s\nGot:\n%s", tc.expectedOut, outStr)
+			}
 		})
 	}
 }