Merge pull request kubernetes#3464 from smarterclayton/fix_invalid_js…

…on_in_services.sh Fix e2e invalid JSON, ensure CLI communicates invalid JSON errors effectively
duda · Jan 14, 2015 · e14d5f3 · e14d5f3
2 parents 87cf7a5 + ccdc20d
commit e14d5f3
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 9 deletions.
diff --git a/hack/e2e-suite/services.sh b/hack/e2e-suite/services.sh
@@ -87,9 +87,9 @@ function start_service() {
                                           "containerPort": 9376,
                                           "protocol": "TCP"
                                       }
-                                  ],
+                                  ]
                               }
-                          ],
+                          ]
                       }
                   },
                   "labels": {

diff --git a/hack/e2e.go b/hack/e2e.go
@@ -531,9 +531,9 @@ func kubecfgArgs() string {
 // kubectl command (begining with a space).
 func kubectlArgs() string {
 	if *checkVersionSkew {
-		return " --match-server-version"
+		return " --match-server-version --v=4"
 	}
-	return ""
+	return " --v=4"
 }
 
 func bashWrap(cmd string) string {

diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh
@@ -115,6 +115,9 @@ for version in "${kube_api_versions[@]}"; do
     [ "$(kubectl get minions -t $'{{ .apiVersion }}' "${kube_flags[@]}")" == "${version}" ]
   fi
 
+  # passing no arguments to create is an error
+  [ ! $(kubectl create) ]
+
   kube::log::status "Testing kubectl(${version}:pods)"
   kubectl get pods "${kube_flags[@]}"
   kubectl create -f examples/guestbook/redis-master.json "${kube_flags[@]}"
@@ -138,7 +141,17 @@ for version in "${kube_api_versions[@]}"; do
   output_service=$(kubectl get service frontend -o json --output-version=v1beta3 "${kube_flags[@]}")
   kubectl delete service frontend "${kube_flags[@]}"
   echo "${output_service}" | kubectl create -f - "${kube_flags[@]}"
+  kubectl create -f - "${kube_flags[@]}" << __EOF__
+      {
+          "kind": "Service",
+          "apiVersion": "v1beta1",
+          "id": "service-${version}-test",
+          "port": 80,
+          "protocol": "TCP"
+      }
+__EOF__
   kubectl get services "${kube_flags[@]}"
+  kubectl get services "service-${version}-test" "${kube_flags[@]}"
   kubectl delete service frontend "${kube_flags[@]}"
 
   kube::log::status "Testing kubectl(${version}:replicationcontrollers)"

diff --git a/pkg/kubectl/cmd/create.go b/pkg/kubectl/cmd/create.go
@@ -55,6 +55,7 @@ Examples:
 				Flatten().
 				Do()
 
+			count := 0
 			err = r.Visit(func(info *resource.Info) error {
 				data, err := info.Mapping.Codec.Encode(info.Object)
 				if err != nil {
@@ -66,11 +67,15 @@ Examples:
 				if err := resource.NewHelper(info.Client, info.Mapping).Create(info.Namespace, true, data); err != nil {
 					return err
 				}
+				count++
 				// TODO: if generation of names added to server side, change this to use the server's name
 				fmt.Fprintf(out, "%s\n", info.Name)
 				return nil
 			})
 			checkErr(err)
+			if count == 0 {
+				checkErr(fmt.Errorf("no objects passed to create"))
+			}
 		},
 	}
 	cmd.Flags().VarP(&flags.Filenames, "filename", "f", "Filename, directory, or URL to file to use to create the resource")

diff --git a/pkg/kubectl/resource/visitor.go b/pkg/kubectl/resource/visitor.go
@@ -17,6 +17,7 @@ limitations under the License.
 package resource
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -337,7 +338,10 @@ type StreamVisitor struct {
 }
 
 // NewStreamVisitor creates a visitor that will return resources that were encoded into the provided
-// stream. If ignoreErrors is set, unrecognized or invalid objects will be skipped and logged.
+// stream. If ignoreErrors is set, unrecognized or invalid objects will be skipped and logged. An
+// empty stream is treated as an error for now.
+// TODO: convert ignoreErrors into a func(data, error, count) bool that consumers can use to decide
+// what to do with ignored errors.
 func NewStreamVisitor(r io.Reader, mapper *Mapper, source string, ignoreErrors bool) Visitor {
 	return &StreamVisitor{r, mapper, source, ignoreErrors}
 }
@@ -353,10 +357,14 @@ func (v *StreamVisitor) Visit(fn VisitorFunc) error {
 			}
 			return err
 		}
+		ext.RawJSON = bytes.TrimSpace(ext.RawJSON)
+		if len(ext.RawJSON) == 0 || bytes.Equal(ext.RawJSON, []byte("null")) {
+			continue
+		}
 		info, err := v.InfoForData(ext.RawJSON, v.Source)
 		if err != nil {
 			if v.IgnoreErrors {
-				glog.V(2).Infof("Unable to read item from stream %q: %v", err)
+				glog.Warningf("Could not read an encoded object from %s: %v", v.Source, err)
 				glog.V(4).Infof("Unreadable: %s", string(ext.RawJSON))
 				continue
 			}

diff --git a/pkg/util/yaml/decoder.go b/pkg/util/yaml/decoder.go
@@ -149,6 +149,6 @@ func guessJSONStream(r io.Reader, size int) (io.Reader, bool) {
 // Return true if the first non-whitespace bytes in buf is
 // prefix
 func hasPrefix(buf []byte, prefix []byte) bool {
-	buf = bytes.TrimLeftFunc(buf, unicode.IsSpace)
-	return bytes.HasPrefix(buf, prefix)
+	trim := bytes.TrimLeftFunc(buf, unicode.IsSpace)
+	return bytes.HasPrefix(trim, prefix)
 }
diff --git a/pkg/util/yaml/decoder_test.go b/pkg/util/yaml/decoder_test.go
@@ -61,6 +61,21 @@ func TestSplitYAMLDocument(t *testing.T) {
 	}
 }
 
+func TestGuessJSON(t *testing.T) {
+	if r, isJSON := guessJSONStream(bytes.NewReader([]byte(" \n{}")), 100); !isJSON {
+		t.Fatalf("expected stream to be JSON")
+	} else {
+		b := make([]byte, 30)
+		n, err := r.Read(b)
+		if err != nil || n != 4 {
+			t.Fatalf("unexpected body: %d / %v", n, err)
+		}
+		if string(b[:n]) != " \n{}" {
+			t.Fatalf("unexpected body: %q", string(b[:n]))
+		}
+	}
+}
+
 func TestScanYAML(t *testing.T) {
 	s := bufio.NewScanner(bytes.NewReader([]byte(`---
 stuff: 1
@@ -130,7 +145,10 @@ func TestYAMLOrJSONDecoder(t *testing.T) {
 		{" \na: b", 2, false, false, []generic{
 			{"a": "b"},
 		}},
-		{` \n{"a": "b"}`, 2, false, true, []generic{
+		{" \n{\"a\": \"b\"}", 2, false, true, []generic{
+			{"a": "b"},
+		}},
+		{" \n{\"a\": \"b\"}", 3, true, false, []generic{
 			{"a": "b"},
 		}},
 		{`   {"a":"b"}`, 100, true, false, []generic{
-Original file line number
+Diff line change
@@ Expand Up / @@ -87,9 +87,9 @@ function start_service() { @@
                                               "containerPort": 9376,
                                               "protocol": "TCP"
                                           }
-                                      ],
+                                      ]
                                   }
-                              ],
+                              ]
                           }
                       },
                       "labels": {
@@ Expand Down @@