Add details for field.Required

Some fields are only required if other fields have certain values.
This patch adds some additional error messaging to inform the user
why these fields were necessary.

Fixes issue #7118

Signed-off-by: Stephen Gallagher <[email protected]>

Author: sgallagher

pkg/authorization/api/validation/validation.go

@@ -297,7 +297,7 @@ func validateRoleBindingSubject(subject kapi.ObjectReference, isNamespaced bool,
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), subject.Name, strings.Join(reasons, ", ")))
 		}
 		if !isNamespaced && len(subject.Namespace) == 0 {
-			allErrs = append(allErrs, field.Required(fldPath.Child("namespace"), ""))
+			allErrs = append(allErrs, field.Required(fldPath.Child("namespace"), "Service account subjects for ClusterRoleBindings must have a namespace"))
 		}
 
 	case authorizationapi.UserKind:

pkg/cmd/server/api/validation/etcd.go

@@ -42,7 +42,7 @@ func ValidateEtcdConnectionInfo(config api.EtcdConnectionInfo, server *api.EtcdC
 		// Require a client cert to connect to an etcd that requires client certs
 		if len(server.ServingInfo.ClientCA) > 0 {
 			if len(config.ClientCert.CertFile) == 0 {
-				allErrs = append(allErrs, field.Required(fldPath.Child("certFile"), ""))
+				allErrs = append(allErrs, field.Required(fldPath.Child("certFile"), "A client certificate must be provided for this etcd server"))
 			}
 		}
 

pkg/cmd/server/api/validation/oauth.go

@@ -243,13 +243,11 @@ func ValidateRequestHeaderIdentityProvider(provider *api.RequestHeaderIdentityPr
 		validationResults.AddErrors(field.Required(fieldPath.Child("provider", "headers"), ""))
 	}
 	if identityProvider.UseAsChallenger && len(provider.ChallengeURL) == 0 {
-		err := field.Required(fieldPath.Child("provider", "challengeURL"), "")
-		err.Detail = "challengeURL is required if challenge=true"
+		err := field.Required(fieldPath.Child("provider", "challengeURL"), "challengeURL is required if challenge is true")
 		validationResults.AddErrors(err)
 	}
 	if identityProvider.UseAsLogin && len(provider.LoginURL) == 0 {
-		err := field.Required(fieldPath.Child("provider", "loginURL"), "")
-		err.Detail = "loginURL is required if login=true"
+		err := field.Required(fieldPath.Child("provider", "loginURL"), "loginURL is required if login=true")
 		validationResults.AddErrors(err)
 	}
 

pkg/cmd/server/api/validation/validation.go

@@ -76,15 +76,20 @@ func ValidateHostPort(value string, fldPath *field.Path) field.ErrorList {
 func ValidateCertInfo(certInfo api.CertInfo, required bool, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
-	if required || len(certInfo.CertFile) > 0 || len(certInfo.KeyFile) > 0 {
+	if required {
 		if len(certInfo.CertFile) == 0 {
-			allErrs = append(allErrs, field.Required(fldPath.Child("certFile"), ""))
+			allErrs = append(allErrs, field.Required(fldPath.Child("certFile"), "The certificate file must be provided"))
 		}
 		if len(certInfo.KeyFile) == 0 {
-			allErrs = append(allErrs, field.Required(fldPath.Child("keyFile"), ""))
+			allErrs = append(allErrs, field.Required(fldPath.Child("keyFile"), "The certificate key must be provided"))
 		}
 	}
 
+	if (len(certInfo.CertFile) == 0) != (len(certInfo.KeyFile) == 0) {
+		allErrs = append(allErrs, field.Required(fldPath.Child("certFile"), "Both the certificate file and the certificate key must be provided together or not at all"))
+		allErrs = append(allErrs, field.Required(fldPath.Child("keyFile"), "Both the certificate file and the certificate key must be provided together or not at all"))
+	}
+
 	if len(certInfo.CertFile) > 0 {
 		allErrs = append(allErrs, ValidateFile(certInfo.CertFile, fldPath.Child("certFile"))...)
 	}

pkg/image/api/validation/validation.go

@@ -192,7 +192,7 @@ func ValidateImageStreamTagReference(tagRef api.TagReference, fldPath *field.Pat
 	var errs field.ErrorList
 	if tagRef.From != nil {
 		if len(tagRef.From.Name) == 0 {
-			errs = append(errs, field.Required(fldPath.Child("from", "name"), "name is required"))
+			errs = append(errs, field.Required(fldPath.Child("from", "name"), ""))
 		}
 		switch tagRef.From.Kind {
 		case "DockerImage":
@@ -324,7 +324,7 @@ func ValidateImageStreamImport(isi *api.ImageStreamImport) field.ErrorList {
 		switch from.Kind {
 		case "DockerImage":
 			if len(spec.From.Name) == 0 {
-				errs = append(errs, field.Required(repoPath.Child("from", "name"), ""))
+				errs = append(errs, field.Required(repoPath.Child("from", "name"), "Docker image references require a name"))
 			} else {
 				if ref, err := api.ParseDockerImageReference(from.Name); err != nil {
 					errs = append(errs, field.Invalid(repoPath.Child("from", "name"), from.Name, err.Error()))

pkg/user/api/validation/validation.go

@@ -155,10 +155,10 @@ func ValidateIdentity(identity *api.Identity) field.ErrorList {
 		allErrs = append(allErrs, field.Invalid(userPath.Child("name"), identity.User.Name, strings.Join(reasons, ", ")))
 	}
 	if len(identity.User.Name) == 0 && len(identity.User.UID) != 0 {
-		allErrs = append(allErrs, field.Invalid(userPath.Child("uid"), identity.User.UID, "may not be set if user.name is empty"))
+		allErrs = append(allErrs, field.Invalid(userPath.Child("username"), "username is required when uid is provided"))
 	}
 	if len(identity.User.Name) != 0 && len(identity.User.UID) == 0 {
-		allErrs = append(allErrs, field.Required(userPath.Child("uid"), ""))
+		allErrs = append(allErrs, field.Required(userPath.Child("uid"), "uid is required when username is provided"))
 	}
 	return allErrs
 }