Merge pull request #25709 from asalkeld/null-deref

hack/test-cmd.sh

@@ -902,6 +902,18 @@ __EOF__
   kubectl delete hpa frontend "${kube_flags[@]}"
   kubectl delete rc  frontend "${kube_flags[@]}"
 
+  ## kubectl create should not panic on empty string lists in a template
+  ERROR_FILE="${KUBE_TEMP}/validation-error"
+  kubectl create -f hack/testdata/invalid-rc-with-empty-args.yaml "${kube_flags[@]}" 2> "${ERROR_FILE}" || true
+  # Post-condition: should get an error reporting the empty string
+  if grep -q "unexpected nil value for field" "${ERROR_FILE}"; then
+    kube::log::status "\"kubectl create with empty string list returns error as expected: $(cat ${ERROR_FILE})"
+  else
+    kube::log::status "\"kubectl create with empty string list returns unexpected error or non-error: $(cat ${ERROR_FILE})"
+    exit 1
+  fi
+  rm "${ERROR_FILE}"
+
   ## kubectl apply should create the resource that doesn't exist yet
   # Pre-Condition: no POD exists
   kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" ''

hack/testdata/invalid-rc-with-empty-args.yaml

+apiVersion: v1
+kind: ReplicationController
+metadata:
+  name: kube-dns-v10
+  namespace: kube-system
+spec:
+  replicas: 1
+  selector:
+    k8s-app: kube-dns
+  template:
+    metadata:
+      labels:
+        k8s-app: kube-dns
+    spec:
+      containers:
+      - name: carbon-relay
+        image: banno/carbon-relay
+        args:
+        -
+# above is the empty arg string.
\ No newline at end of file

pkg/api/validation/schema.go

@@ -297,6 +297,10 @@ func (s *SwaggerSchema) isGenericArray(p swagger.ModelProperty) bool {
 var versionRegexp = regexp.MustCompile(`^v.+\..*`)
 
 func (s *SwaggerSchema) validateField(value interface{}, fieldName, fieldType string, fieldDetails *swagger.ModelProperty) []error {
+	allErrs := []error{}
+	if reflect.TypeOf(value) == nil {
+		return append(allErrs, fmt.Errorf("unexpected nil value for field %v", fieldName))
+	}
 	// TODO: caesarxuchao: because we have multiple group/versions and objects
 	// may reference objects in other group, the commented out way of checking
 	// if a filedType is a type defined by us is outdated. We use a hacky way
@@ -310,7 +314,6 @@ func (s *SwaggerSchema) validateField(value interface{}, fieldName, fieldType st
 		// if strings.HasPrefix(fieldType, apiVersion) {
 		return s.ValidateObject(value, fieldName, fieldType)
 	}
-	allErrs := []error{}
 	switch fieldType {
 	case "string":
 		// Be loose about what we accept for 'string' since we use IntOrString in a couple of places

pkg/api/validation/schema_test.go

@@ -209,6 +209,7 @@ func TestInvalid(t *testing.T) {
 		"invalidPod1.json", // command is a string, instead of []string.
 		"invalidPod2.json", // hostPort if of type string, instead of int.
 		"invalidPod3.json", // volumes is not an array of objects.
+		"invalidPod4.yaml", // string list with empty string.
 		"invalidPod.yaml",  // command is a string, instead of []string.
 	}
 	for _, test := range tests {

pkg/api/validation/testdata/v1/invalidPod4.yaml

+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    name: redis-master
+  name: name
+spec:
+  containers:
+  - image: gcr.io/fake_project/fake_image:fake_tag
+    name: master
+    args:
+    -
+    command:
+    -
\ No newline at end of file