diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 7704db2904..fc9e3907ea 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -884,6 +884,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}}" '' diff --git a/hack/testdata/invalid-rc-with-empty-args.yaml b/hack/testdata/invalid-rc-with-empty-args.yaml new file mode 100644 index 0000000000..14657f1939 --- /dev/null +++ b/hack/testdata/invalid-rc-with-empty-args.yaml @@ -0,0 +1,20 @@ +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 diff --git a/pkg/api/validation/schema.go b/pkg/api/validation/schema.go index c52a0b6d71..726cb518b5 100644 --- a/pkg/api/validation/schema.go +++ b/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 diff --git a/pkg/api/validation/schema_test.go b/pkg/api/validation/schema_test.go index 3499acab25..876e467d37 100644 --- a/pkg/api/validation/schema_test.go +++ b/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 { diff --git a/pkg/api/validation/testdata/v1/invalidPod4.yaml b/pkg/api/validation/testdata/v1/invalidPod4.yaml new file mode 100644 index 0000000000..f02bf7b336 --- /dev/null +++ b/pkg/api/validation/testdata/v1/invalidPod4.yaml @@ -0,0 +1,14 @@ +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