diff --git a/federation/apis/federation/validation/validation.go b/federation/apis/federation/validation/validation.go index f6f3dac483818..f7cac2e264de2 100644 --- a/federation/apis/federation/validation/validation.go +++ b/federation/apis/federation/validation/validation.go @@ -22,9 +22,7 @@ import ( "k8s.io/kubernetes/pkg/util/validation/field" ) -func ValidateClusterName(name string, prefix bool) (bool, string) { - return validation.NameIsDNSSubdomain(name, prefix) -} +var ValidateClusterName = validation.NameIsDNSSubdomain func ValidateClusterSpec(spec *federation.ClusterSpec, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/api/rest/resttest/resttest.go b/pkg/api/rest/resttest/resttest.go index 6184f236791cb..df554ee83167d 100644 --- a/pkg/api/rest/resttest/resttest.go +++ b/pkg/api/rest/resttest/resttest.go @@ -366,7 +366,7 @@ func (t *Tester) testCreateValidatesNames(valid runtime.Object) { ctx := t.TestContext() _, err := t.storage.(rest.Creater).Create(ctx, objCopy) if !errors.IsInvalid(err) { - t.Errorf("%s: Expected to get an invalid resource error, got %v", invalidName, err) + t.Errorf("%s: Expected to get an invalid resource error, got '%v'", invalidName, err) } } @@ -378,7 +378,7 @@ func (t *Tester) testCreateValidatesNames(valid runtime.Object) { ctx := t.TestContext() _, err := t.storage.(rest.Creater).Create(ctx, objCopy) if !errors.IsInvalid(err) { - t.Errorf("%s: Expected to get an invalid resource error, got %v", invalidSuffix, err) + t.Errorf("%s: Expected to get an invalid resource error, got '%v'", invalidSuffix, err) } } } diff --git a/pkg/api/unversioned/validation/validation.go b/pkg/api/unversioned/validation/validation.go index 3fdd4c2453f68..47852e3e29f2a 100644 --- a/pkg/api/unversioned/validation/validation.go +++ b/pkg/api/unversioned/validation/validation.go @@ -55,8 +55,8 @@ func ValidateLabelSelectorRequirement(sr unversioned.LabelSelectorRequirement, f // ValidateLabelName validates that the label name is correctly defined. func ValidateLabelName(labelName string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - for _, err := range validation.IsQualifiedName(labelName) { - allErrs = append(allErrs, field.Invalid(fldPath, labelName, err)) + for _, msg := range validation.IsQualifiedName(labelName) { + allErrs = append(allErrs, field.Invalid(fldPath, labelName, msg)) } return allErrs } @@ -66,8 +66,8 @@ func ValidateLabels(labels map[string]string, fldPath *field.Path) field.ErrorLi allErrs := field.ErrorList{} for k, v := range labels { allErrs = append(allErrs, ValidateLabelName(k, fldPath)...) - for _, err := range validation.IsValidLabelValue(v) { - allErrs = append(allErrs, field.Invalid(fldPath, v, err)) + for _, msg := range validation.IsValidLabelValue(v) { + allErrs = append(allErrs, field.Invalid(fldPath, v, msg)) } } return allErrs diff --git a/pkg/api/validation/name.go b/pkg/api/validation/name.go index e36775b601d55..cf2eb8bb2990a 100644 --- a/pkg/api/validation/name.go +++ b/pkg/api/validation/name.go @@ -28,36 +28,36 @@ var NameMayNotBe = []string{".", ".."} var NameMayNotContain = []string{"/", "%"} // IsValidPathSegmentName validates the name can be safely encoded as a path segment -func IsValidPathSegmentName(name string) (bool, string) { +func IsValidPathSegmentName(name string) []string { for _, illegalName := range NameMayNotBe { if name == illegalName { - return false, fmt.Sprintf(`name may not be %q`, illegalName) + return []string{fmt.Sprintf(`may not be '%s'`, illegalName)} } } for _, illegalContent := range NameMayNotContain { if strings.Contains(name, illegalContent) { - return false, fmt.Sprintf(`name may not contain %q`, illegalContent) + return []string{fmt.Sprintf(`may not contain '%s'`, illegalContent)} } } - return true, "" + return nil } // IsValidPathSegmentPrefix validates the name can be used as a prefix for a name which will be encoded as a path segment // It does not check for exact matches with disallowed names, since an arbitrary suffix might make the name valid -func IsValidPathSegmentPrefix(name string) (bool, string) { +func IsValidPathSegmentPrefix(name string) []string { for _, illegalContent := range NameMayNotContain { if strings.Contains(name, illegalContent) { - return false, fmt.Sprintf(`name may not contain %q`, illegalContent) + return []string{fmt.Sprintf(`may not contain '%s'`, illegalContent)} } } - return true, "" + return nil } // ValidatePathSegmentName validates the name can be safely encoded as a path segment -func ValidatePathSegmentName(name string, prefix bool) (bool, string) { +func ValidatePathSegmentName(name string, prefix bool) []string { if prefix { return IsValidPathSegmentPrefix(name) } else { diff --git a/pkg/api/validation/name_test.go b/pkg/api/validation/name_test.go index b8687cdccd521..f952a960e771b 100644 --- a/pkg/api/validation/name_test.go +++ b/pkg/api/validation/name_test.go @@ -25,32 +25,27 @@ func TestValidatePathSegmentName(t *testing.T) { testcases := map[string]struct { Name string Prefix bool - ExpectedOK bool ExpectedMsg string }{ "empty": { Name: "", Prefix: false, - ExpectedOK: true, ExpectedMsg: "", }, "empty,prefix": { Name: "", Prefix: true, - ExpectedOK: true, ExpectedMsg: "", }, "valid": { Name: "foo.bar.baz", Prefix: false, - ExpectedOK: true, ExpectedMsg: "", }, "valid,prefix": { Name: "foo.bar.baz", Prefix: true, - ExpectedOK: true, ExpectedMsg: "", }, @@ -58,92 +53,80 @@ func TestValidatePathSegmentName(t *testing.T) { "valid complex": { Name: "sha256:ABCDEF012345@ABCDEF012345", Prefix: false, - ExpectedOK: true, ExpectedMsg: "", }, // Make sure non-ascii characters are tolerated "valid extended charset": { Name: "Iñtërnâtiônàlizætiøn", Prefix: false, - ExpectedOK: true, ExpectedMsg: "", }, "dot": { Name: ".", Prefix: false, - ExpectedOK: false, ExpectedMsg: ".", }, "dot leading": { Name: ".test", Prefix: false, - ExpectedOK: true, ExpectedMsg: "", }, "dot,prefix": { Name: ".", Prefix: true, - ExpectedOK: true, // allowed because a suffix could make it valid ExpectedMsg: "", }, "dot dot": { Name: "..", Prefix: false, - ExpectedOK: false, ExpectedMsg: "..", }, "dot dot leading": { Name: "..test", Prefix: false, - ExpectedOK: true, ExpectedMsg: "", }, "dot dot,prefix": { Name: "..", Prefix: true, - ExpectedOK: true, // allowed because a suffix could make it valid ExpectedMsg: "", }, "slash": { Name: "foo/bar", Prefix: false, - ExpectedOK: false, ExpectedMsg: "/", }, "slash,prefix": { Name: "foo/bar", Prefix: true, - ExpectedOK: false, ExpectedMsg: "/", }, "percent": { Name: "foo%bar", Prefix: false, - ExpectedOK: false, ExpectedMsg: "%", }, "percent,prefix": { Name: "foo%bar", Prefix: true, - ExpectedOK: false, ExpectedMsg: "%", }, } for k, tc := range testcases { - ok, msg := ValidatePathSegmentName(tc.Name, tc.Prefix) - if ok != tc.ExpectedOK { - t.Errorf("%s: expected ok=%v, got %v", k, tc.ExpectedOK, ok) + msgs := ValidatePathSegmentName(tc.Name, tc.Prefix) + if len(tc.ExpectedMsg) == 0 && len(msgs) > 0 { + t.Errorf("%s: expected no message, got %v", k, msgs) } - if len(tc.ExpectedMsg) == 0 && len(msg) > 0 { - t.Errorf("%s: expected no message, got %v", k, msg) + if len(tc.ExpectedMsg) > 0 && len(msgs) == 0 { + t.Errorf("%s: expected error message, got none", k) } - if len(tc.ExpectedMsg) > 0 && !strings.Contains(msg, tc.ExpectedMsg) { - t.Errorf("%s: expected message containing %q, got %v", k, tc.ExpectedMsg, msg) + if len(tc.ExpectedMsg) > 0 && !strings.Contains(msgs[0], tc.ExpectedMsg) { + t.Errorf("%s: expected message containing %q, got %v", k, tc.ExpectedMsg, msgs[0]) } } } diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 47d2309d6ca31..cfdc03b757918 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -92,8 +92,8 @@ func ValidateAnnotations(annotations map[string]string, fldPath *field.Path) fie allErrs := field.ErrorList{} var totalSize int64 for k, v := range annotations { - for _, err := range validation.IsQualifiedName(strings.ToLower(k)) { - allErrs = append(allErrs, field.Invalid(fldPath, k, err)) + for _, msg := range validation.IsQualifiedName(strings.ToLower(k)) { + allErrs = append(allErrs, field.Invalid(fldPath, k, msg)) } totalSize += (int64)(len(k)) + (int64)(len(v)) } @@ -162,9 +162,11 @@ func ValidateOwnerReferences(ownerReferences []api.OwnerReference, fldPath *fiel } // ValidateNameFunc validates that the provided name is valid for a given resource type. -// Not all resources have the same validation rules for names. Prefix is true if the -// name will have a value appended to it. -type ValidateNameFunc func(name string, prefix bool) (bool, string) +// Not all resources have the same validation rules for names. Prefix is true +// if the name will have a value appended to it. If the names is not valid, +// this returns a list of descriptions of individual characteristics of the +// value that were not valid. Otherwise this returns an empty list or nil. +type ValidateNameFunc func(name string, prefix bool) []string // maskTrailingDash replaces the final character of a string with a subdomain safe // value if is a dash. @@ -178,106 +180,86 @@ func maskTrailingDash(name string) string { // ValidatePodName can be used to check whether the given pod name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidatePodName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +var ValidatePodName = NameIsDNSSubdomain // ValidateReplicationControllerName can be used to check whether the given replication // controller name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateReplicationControllerName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +var ValidateReplicationControllerName = NameIsDNSSubdomain // ValidateServiceName can be used to check whether the given service name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateServiceName(name string, prefix bool) (bool, string) { - return NameIsDNS952Label(name, prefix) -} +var ValidateServiceName = NameIsDNS952Label // ValidateNodeName can be used to check whether the given node name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateNodeName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +var ValidateNodeName = NameIsDNSSubdomain // ValidateNamespaceName can be used to check whether the given namespace name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateNamespaceName(name string, prefix bool) (bool, string) { - return NameIsDNSLabel(name, prefix) -} +var ValidateNamespaceName = NameIsDNSLabel // ValidateLimitRangeName can be used to check whether the given limit range name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateLimitRangeName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +var ValidateLimitRangeName = NameIsDNSSubdomain // ValidateResourceQuotaName can be used to check whether the given // resource quota name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateResourceQuotaName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +var ValidateResourceQuotaName = NameIsDNSSubdomain // ValidateSecretName can be used to check whether the given secret name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateSecretName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +var ValidateSecretName = NameIsDNSSubdomain // ValidateServiceAccountName can be used to check whether the given service account name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateServiceAccountName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +var ValidateServiceAccountName = NameIsDNSSubdomain // ValidateEndpointsName can be used to check whether the given endpoints name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateEndpointsName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +var ValidateEndpointsName = NameIsDNSSubdomain // NameIsDNSSubdomain is a ValidateNameFunc for names that must be a DNS subdomain. -func NameIsDNSSubdomain(name string, prefix bool) (bool, string) { +func NameIsDNSSubdomain(name string, prefix bool) []string { if prefix { name = maskTrailingDash(name) } if validation.IsDNS1123Subdomain(name) { - return true, "" + return nil } - return false, DNSSubdomainErrorMsg + return []string{DNSSubdomainErrorMsg} } // NameIsDNSLabel is a ValidateNameFunc for names that must be a DNS 1123 label. -func NameIsDNSLabel(name string, prefix bool) (bool, string) { +func NameIsDNSLabel(name string, prefix bool) []string { if prefix { name = maskTrailingDash(name) } if validation.IsDNS1123Label(name) { - return true, "" + return nil } - return false, DNS1123LabelErrorMsg + return []string{DNS1123LabelErrorMsg} } // NameIsDNS952Label is a ValidateNameFunc for names that must be a DNS 952 label. -func NameIsDNS952Label(name string, prefix bool) (bool, string) { +func NameIsDNS952Label(name string, prefix bool) []string { if prefix { name = maskTrailingDash(name) } if validation.IsDNS952Label(name) { - return true, "" + return nil } - return false, DNS952LabelErrorMsg + return []string{DNS952LabelErrorMsg} } // Validates that given value is not negative. @@ -314,8 +296,8 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val allErrs := field.ErrorList{} if len(meta.GenerateName) != 0 { - if ok, qualifier := nameFn(meta.GenerateName, true); !ok { - allErrs = append(allErrs, field.Invalid(fldPath.Child("generateName"), meta.GenerateName, qualifier)) + for _, msg := range nameFn(meta.GenerateName, true) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("generateName"), meta.GenerateName, msg)) } } // If the generated name validates, but the calculated value does not, it's a problem with generation, and we @@ -324,15 +306,17 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val if len(meta.Name) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("name"), "name or generateName is required")) } else { - if ok, qualifier := nameFn(meta.Name, false); !ok { - allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), meta.Name, qualifier)) + for _, msg := range nameFn(meta.Name, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), meta.Name, msg)) } } if requiresNamespace { if len(meta.Namespace) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("namespace"), "")) - } else if ok, _ := ValidateNamespaceName(meta.Namespace, false); !ok { - allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), meta.Namespace, DNS1123LabelErrorMsg)) + } else { + for _, msg := range ValidateNamespaceName(meta.Namespace, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), meta.Namespace, msg)) + } } } else { if len(meta.Namespace) != 0 { @@ -816,9 +800,9 @@ func validateAzureFile(azure *api.AzureFileVolumeSource, fldPath *field.Path) fi return allErrs } -func ValidatePersistentVolumeName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +// ValidatePersistentVolumeName checks that a name is appropriate for a +// PersistentVolumeName object. +var ValidatePersistentVolumeName = NameIsDNSSubdomain var supportedAccessModes = sets.NewString(string(api.ReadWriteOnce), string(api.ReadOnlyMany), string(api.ReadWriteMany)) @@ -1456,13 +1440,13 @@ func ValidatePodSpec(spec *api.PodSpec, fldPath *field.Path) field.ErrorList { allErrs = append(allErrs, ValidatePodSecurityContext(spec.SecurityContext, spec, fldPath, fldPath.Child("securityContext"))...) allErrs = append(allErrs, validateImagePullSecrets(spec.ImagePullSecrets, fldPath.Child("imagePullSecrets"))...) if len(spec.ServiceAccountName) > 0 { - if ok, msg := ValidateServiceAccountName(spec.ServiceAccountName, false); !ok { + for _, msg := range ValidateServiceAccountName(spec.ServiceAccountName, false) { allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceAccountName"), spec.ServiceAccountName, msg)) } } if len(spec.NodeName) > 0 { - if ok, msg := ValidateNodeName(spec.NodeName, false); !ok { + for _, msg := range ValidateNodeName(spec.NodeName, false) { allErrs = append(allErrs, field.Invalid(fldPath.Child("nodeName"), spec.NodeName, msg)) } } @@ -1556,8 +1540,8 @@ func validatePodAffinityTerm(podAffinityTerm api.PodAffinityTerm, allowEmptyTopo allErrs := field.ErrorList{} allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(podAffinityTerm.LabelSelector, fldPath.Child("matchExpressions"))...) for _, name := range podAffinityTerm.Namespaces { - if ok, _ := ValidateNamespaceName(name, false); !ok { - allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), name, DNS1123LabelErrorMsg)) + for _, msg := range ValidateNamespaceName(name, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), name, msg)) } } if !allowEmptyTopologyKey && len(podAffinityTerm.TopologyKey) == 0 { @@ -2146,8 +2130,8 @@ func ValidateNodeUpdate(node, oldNode *api.Node) field.ErrorList { // Refer to docs/design/resources.md for more details. func validateResourceName(value string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - for _, err := range validation.IsQualifiedName(value) { - allErrs = append(allErrs, field.Invalid(fldPath, value, err)) + for _, msg := range validation.IsQualifiedName(value) { + allErrs = append(allErrs, field.Invalid(fldPath, value, msg)) } if len(allErrs) != 0 { return allErrs @@ -2189,8 +2173,8 @@ func validateResourceQuotaResourceName(value string, fldPath *field.Path) field. // Validate limit range types func validateLimitRangeTypeName(value string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - for _, err := range validation.IsQualifiedName(value) { - allErrs = append(allErrs, field.Invalid(fldPath, value, err)) + for _, msg := range validation.IsQualifiedName(value) { + allErrs = append(allErrs, field.Invalid(fldPath, value, msg)) } if len(allErrs) != 0 { return allErrs @@ -2449,9 +2433,7 @@ func ValidateSecretUpdate(newSecret, oldSecret *api.Secret) field.ErrorList { // ValidateConfigMapName can be used to check whether the given ConfigMap name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateConfigMapName(name string, prefix bool) (bool, string) { - return NameIsDNSSubdomain(name, prefix) -} +var ValidateConfigMapName = NameIsDNSSubdomain // ValidateConfigMap tests whether required fields in the ConfigMap are set. func ValidateConfigMap(cfg *api.ConfigMap) field.ErrorList { @@ -2663,8 +2645,8 @@ func ValidateNamespace(namespace *api.Namespace) field.ErrorList { // Validate finalizer names func validateFinalizerName(stringValue string, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - for _, err := range validation.IsQualifiedName(stringValue) { - allErrs = append(allErrs, field.Invalid(fldPath, stringValue, err)) + for _, msg := range validation.IsQualifiedName(stringValue) { + allErrs = append(allErrs, field.Invalid(fldPath, stringValue, msg)) } if len(allErrs) != 0 { return allErrs @@ -2870,8 +2852,8 @@ func ValidateLoadBalancerStatus(status *api.LoadBalancerStatus, fldPath *field.P } } if len(ingress.Hostname) > 0 { - if valid, errMsg := NameIsDNSSubdomain(ingress.Hostname, false); !valid { - allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), ingress.Hostname, errMsg)) + for _, msg := range NameIsDNSSubdomain(ingress.Hostname, false) { + allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), ingress.Hostname, msg)) } if isIP := (net.ParseIP(ingress.Hostname) != nil); isIP { allErrs = append(allErrs, field.Invalid(idxPath.Child("hostname"), ingress.Hostname, "must be a DNS name, not an IP address")) diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index cd45d79c00a25..3154a4b44bdaf 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -47,11 +47,11 @@ func TestValidateObjectMetaCustomName(t *testing.T) { errs := ValidateObjectMeta( &api.ObjectMeta{Name: "test", GenerateName: "foo"}, false, - func(s string, prefix bool) (bool, string) { + func(s string, prefix bool) []string { if s == "test" { - return true, "" + return nil } - return false, "name-gen" + return []string{"name-gen"} }, field.NewPath("field")) if len(errs) != 1 { @@ -67,8 +67,8 @@ func TestValidateObjectMetaNamespaces(t *testing.T) { errs := ValidateObjectMeta( &api.ObjectMeta{Name: "test", Namespace: "foo.bar"}, true, - func(s string, prefix bool) (bool, string) { - return true, "" + func(s string, prefix bool) []string { + return nil }, field.NewPath("field")) if len(errs) != 1 { @@ -86,8 +86,8 @@ func TestValidateObjectMetaNamespaces(t *testing.T) { errs = ValidateObjectMeta( &api.ObjectMeta{Name: "test", Namespace: string(b)}, true, - func(s string, prefix bool) (bool, string) { - return true, "" + func(s string, prefix bool) []string { + return nil }, field.NewPath("field")) if len(errs) != 1 { @@ -132,8 +132,8 @@ func TestValidateObjectMetaOwnerReferences(t *testing.T) { errs := ValidateObjectMeta( &api.ObjectMeta{Name: "test", Namespace: "test", OwnerReferences: tc.ownerReferences}, true, - func(s string, prefix bool) (bool, string) { - return true, "" + func(s string, prefix bool) []string { + return nil }, field.NewPath("field")) if len(errs) != 0 && !tc.expectError { diff --git a/pkg/apis/apps/validation/validation.go b/pkg/apis/apps/validation/validation.go index ce83ef8c52069..acbdfedfe1ebd 100644 --- a/pkg/apis/apps/validation/validation.go +++ b/pkg/apis/apps/validation/validation.go @@ -28,11 +28,10 @@ import ( "k8s.io/kubernetes/pkg/util/validation/field" ) -// ValidatePetSetName can be used to check whether the given PetSet -// name is valid. +// ValidatePetSetName can be used to check whether the given PetSet name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidatePetSetName(name string, prefix bool) (bool, string) { +func ValidatePetSetName(name string, prefix bool) []string { // TODO: Validate that there's name for the suffix inserted by the pets. // Currently this is just "-index". In the future we may allow a user // specified list of suffixes and we need to validate the longest one. diff --git a/pkg/apis/autoscaling/validation/validation.go b/pkg/apis/autoscaling/validation/validation.go index fee1119616148..3432e3cb1c40c 100644 --- a/pkg/apis/autoscaling/validation/validation.go +++ b/pkg/apis/autoscaling/validation/validation.go @@ -39,10 +39,7 @@ func ValidateScale(scale *autoscaling.Scale) field.ErrorList { // ValidateHorizontalPodAutoscaler can be used to check whether the given autoscaler name is valid. // Prefix indicates this name will be used as part of generation, in which case trailing dashes are allowed. -func ValidateHorizontalPodAutoscalerName(name string, prefix bool) (bool, string) { - // TODO: finally move it to pkg/api/validation and use nameIsDNSSubdomain function - return apivalidation.ValidateReplicationControllerName(name, prefix) -} +var ValidateHorizontalPodAutoscalerName = apivalidation.ValidateReplicationControllerName func validateHorizontalPodAutoscalerSpec(autoscaler autoscaling.HorizontalPodAutoscalerSpec, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -68,14 +65,18 @@ func ValidateCrossVersionObjectReference(ref autoscaling.CrossVersionObjectRefer allErrs := field.ErrorList{} if len(ref.Kind) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("kind"), "")) - } else if ok, msg := apivalidation.IsValidPathSegmentName(ref.Kind); !ok { - allErrs = append(allErrs, field.Invalid(fldPath.Child("kind"), ref.Kind, msg)) + } else { + for _, msg := range apivalidation.IsValidPathSegmentName(ref.Kind) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("kind"), ref.Kind, msg)) + } } if len(ref.Name) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) - } else if ok, msg := apivalidation.IsValidPathSegmentName(ref.Name); !ok { - allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), ref.Name, msg)) + } else { + for _, msg := range apivalidation.IsValidPathSegmentName(ref.Name) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), ref.Name, msg)) + } } return allErrs diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index e621466feedc5..5328ef10c3a1d 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -43,21 +43,21 @@ func ValidateThirdPartyResourceUpdate(update, old *extensions.ThirdPartyResource return allErrs } -func ValidateThirdPartyResourceName(name string, prefix bool) (bool, string) { +func ValidateThirdPartyResourceName(name string, prefix bool) []string { // Make sure it's a valid DNS subdomain - if ok, msg := apivalidation.NameIsDNSSubdomain(name, prefix); !ok { - return ok, msg + if msgs := apivalidation.NameIsDNSSubdomain(name, prefix); len(msgs) != 0 { + return msgs } // Make sure it's at least three segments (kind + two-segment group name) if !prefix { parts := strings.Split(name, ".") if len(parts) < 3 { - return false, "must be at least three segments long: .." + return []string{"must be at least three segments long: .."} } } - return true, "" + return nil } func ValidateThirdPartyResource(obj *extensions.ThirdPartyResource) field.ErrorList { @@ -137,14 +137,10 @@ func ValidateDaemonSetSpec(spec *extensions.DaemonSetSpec, fldPath *field.Path) // ValidateDaemonSetName can be used to check whether the given daemon set name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateDaemonSetName(name string, prefix bool) (bool, string) { - return apivalidation.NameIsDNSSubdomain(name, prefix) -} +var ValidateDaemonSetName = apivalidation.NameIsDNSSubdomain // Validates that the given name can be used as a deployment name. -func ValidateDeploymentName(name string, prefix bool) (bool, string) { - return apivalidation.NameIsDNSSubdomain(name, prefix) -} +var ValidateDeploymentName = apivalidation.NameIsDNSSubdomain func ValidatePositiveIntOrPercent(intOrPercent intstr.IntOrString, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -305,9 +301,7 @@ func ValidateIngress(ingress *extensions.Ingress) field.ErrorList { } // ValidateIngressName validates that the given name can be used as an Ingress name. -func ValidateIngressName(name string, prefix bool) (bool, string) { - return apivalidation.NameIsDNSSubdomain(name, prefix) -} +var ValidateIngressName = apivalidation.NameIsDNSSubdomain func validateIngressTLS(spec *extensions.IngressSpec, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -357,8 +351,8 @@ func validateIngressRules(IngressRules []extensions.IngressRule, fldPath *field. if len(ih.Host) > 0 { // TODO: Ports and ips are allowed in the host part of a url // according to RFC 3986, consider allowing them. - if valid, errMsg := apivalidation.NameIsDNSSubdomain(ih.Host, false); !valid { - allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, errMsg)) + for _, msg := range apivalidation.NameIsDNSSubdomain(ih.Host, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, msg)) } if isIP := (net.ParseIP(ih.Host) != nil); isIP { allErrs = append(allErrs, field.Invalid(fldPath.Index(i).Child("host"), ih.Host, "must be a DNS name, not an IP address")) @@ -413,8 +407,10 @@ func validateIngressBackend(backend *extensions.IngressBackend, fldPath *field.P // All backends must reference a single local service by name, and a single service port by name or number. if len(backend.ServiceName) == 0 { return append(allErrs, field.Required(fldPath.Child("serviceName"), "")) - } else if ok, errMsg := apivalidation.ValidateServiceName(backend.ServiceName, false); !ok { - allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceName"), backend.ServiceName, errMsg)) + } else { + for _, msg := range apivalidation.ValidateServiceName(backend.ServiceName, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceName"), backend.ServiceName, msg)) + } } if backend.ServicePort.Type == intstr.String { if !validation.IsDNS1123Label(backend.ServicePort.StrVal) { @@ -444,9 +440,7 @@ func ValidateScale(scale *extensions.Scale) field.ErrorList { // name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidateReplicaSetName(name string, prefix bool) (bool, string) { - return apivalidation.NameIsDNSSubdomain(name, prefix) -} +var ValidateReplicaSetName = apivalidation.NameIsDNSSubdomain // ValidateReplicaSet tests if required fields in the ReplicaSet are set. func ValidateReplicaSet(rs *extensions.ReplicaSet) field.ErrorList { @@ -526,9 +520,7 @@ func ValidatePodTemplateSpecForReplicaSet(template *api.PodTemplateSpec, selecto // pod security policy name is valid. // Prefix indicates this name will be used as part of generation, in which case // trailing dashes are allowed. -func ValidatePodSecurityPolicyName(name string, prefix bool) (bool, string) { - return apivalidation.NameIsDNSSubdomain(name, prefix) -} +var ValidatePodSecurityPolicyName = apivalidation.NameIsDNSSubdomain func ValidatePodSecurityPolicy(psp *extensions.PodSecurityPolicy) field.ErrorList { allErrs := field.ErrorList{} diff --git a/pkg/apis/rbac/validation/validation.go b/pkg/apis/rbac/validation/validation.go index 0487bd539e6c5..69481a1fb62c0 100644 --- a/pkg/apis/rbac/validation/validation.go +++ b/pkg/apis/rbac/validation/validation.go @@ -25,7 +25,7 @@ import ( // Minimal validation of names for roles and bindings. Identical to the validation for Openshift. See: // * https://github.com/kubernetes/kubernetes/blob/60db50/pkg/api/validation/name.go // * https://github.com/openshift/origin/blob/388478/pkg/api/helpers.go -func minimalNameRequirements(name string, prefix bool) (bool, string) { +func minimalNameRequirements(name string, prefix bool) []string { return validation.IsValidPathSegmentName(name) } @@ -86,16 +86,16 @@ func validateRoleBinding(roleBinding *rbac.RoleBinding, isNamespaced bool, fldPa // roleRef namespace is empty when referring to global policy. if len(roleBinding.RoleRef.Namespace) > 0 { - if ok, reason := validation.ValidateNamespaceName(roleBinding.RoleRef.Namespace, false); !ok { - allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "namespace"), roleBinding.RoleRef.Namespace, reason)) + for _, msg := range validation.ValidateNamespaceName(roleBinding.RoleRef.Namespace, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "namespace"), roleBinding.RoleRef.Namespace, msg)) } } if len(roleBinding.RoleRef.Name) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("roleRef", "name"), "")) } else { - if valid, err := minimalNameRequirements(roleBinding.RoleRef.Name, false); !valid { - allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "name"), roleBinding.RoleRef.Name, err)) + for _, msg := range minimalNameRequirements(roleBinding.RoleRef.Name, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("roleRef", "name"), roleBinding.RoleRef.Name, msg)) } } @@ -119,8 +119,10 @@ func validateRoleBindingSubject(subject rbac.Subject, isNamespaced bool, fldPath switch subject.Kind { case rbac.ServiceAccountKind: - if valid, reason := validation.ValidateServiceAccountName(subject.Name, false); len(subject.Name) > 0 && !valid { - allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), subject.Name, reason)) + if len(subject.Name) > 0 { + for _, msg := range validation.ValidateServiceAccountName(subject.Name, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), subject.Name, msg)) + } } if !isNamespaced && len(subject.Namespace) == 0 { allErrs = append(allErrs, field.Required(fldPath.Child("namespace"), "")) diff --git a/pkg/client/restclient/request.go b/pkg/client/restclient/request.go index 7050eab9a9849..a063f0898d88f 100644 --- a/pkg/client/restclient/request.go +++ b/pkg/client/restclient/request.go @@ -179,8 +179,8 @@ func (r *Request) Resource(resource string) *Request { r.err = fmt.Errorf("resource already set to %q, cannot change to %q", r.resource, resource) return r } - if ok, msg := validation.IsValidPathSegmentName(resource); !ok { - r.err = fmt.Errorf("invalid resource %q: %s", resource, msg) + if msgs := validation.IsValidPathSegmentName(resource); len(msgs) != 0 { + r.err = fmt.Errorf("invalid resource %q: %v", resource, msgs) return r } r.resource = resource @@ -199,8 +199,8 @@ func (r *Request) SubResource(subresources ...string) *Request { return r } for _, s := range subresources { - if ok, msg := validation.IsValidPathSegmentName(s); !ok { - r.err = fmt.Errorf("invalid subresource %q: %s", s, msg) + if msgs := validation.IsValidPathSegmentName(s); len(msgs) != 0 { + r.err = fmt.Errorf("invalid subresource %q: %v", s, msgs) return r } } @@ -221,8 +221,8 @@ func (r *Request) Name(resourceName string) *Request { r.err = fmt.Errorf("resource name already set to %q, cannot change to %q", r.resourceName, resourceName) return r } - if ok, msg := validation.IsValidPathSegmentName(resourceName); !ok { - r.err = fmt.Errorf("invalid resource name %q: %s", resourceName, msg) + if msgs := validation.IsValidPathSegmentName(resourceName); len(msgs) != 0 { + r.err = fmt.Errorf("invalid resource name %q: %v", resourceName, msgs) return r } r.resourceName = resourceName @@ -238,8 +238,8 @@ func (r *Request) Namespace(namespace string) *Request { r.err = fmt.Errorf("namespace already set to %q, cannot change to %q", r.namespace, namespace) return r } - if ok, msg := validation.IsValidPathSegmentName(namespace); !ok { - r.err = fmt.Errorf("invalid namespace %q: %s", namespace, msg) + if msgs := validation.IsValidPathSegmentName(namespace); len(msgs) != 0 { + r.err = fmt.Errorf("invalid namespace %q: %v", namespace, msgs) return r } r.namespaceSet = true diff --git a/pkg/controller/controller_utils.go b/pkg/controller/controller_utils.go index 7945741c66896..0748017346b76 100644 --- a/pkg/controller/controller_utils.go +++ b/pkg/controller/controller_utils.go @@ -399,7 +399,7 @@ func getPodsAnnotationSet(template *api.PodTemplateSpec, object runtime.Object) func getPodsPrefix(controllerName string) string { // use the dash (if the name isn't too long) to make the pod name a bit prettier prefix := fmt.Sprintf("%s-", controllerName) - if ok, _ := validation.ValidatePodName(prefix, true); !ok { + if len(validation.ValidatePodName(prefix, true)) != 0 { prefix = controllerName } return prefix diff --git a/pkg/kubelet/client/kubelet_client.go b/pkg/kubelet/client/kubelet_client.go index cd48f05c3df76..bbaade4eeeadf 100644 --- a/pkg/kubelet/client/kubelet_client.go +++ b/pkg/kubelet/client/kubelet_client.go @@ -21,6 +21,7 @@ import ( "fmt" "net" "net/http" + "strings" "time" "k8s.io/kubernetes/pkg/api" @@ -98,8 +99,8 @@ func NewStaticKubeletClient(config *KubeletClientConfig) (KubeletClient, error) // In default HTTPKubeletClient ctx is unused. func (c *HTTPKubeletClient) GetConnectionInfo(ctx api.Context, nodeName string) (string, uint, http.RoundTripper, error) { - if ok, msg := validation.ValidateNodeName(nodeName, false); !ok { - return "", 0, nil, fmt.Errorf("invalid node name: %s", msg) + if errs := validation.ValidateNodeName(nodeName, false); len(errs) != 0 { + return "", 0, nil, fmt.Errorf("invalid node name: %s", strings.Join(errs, ";")) } scheme := "http" if c.Config.EnableHttps { diff --git a/pkg/registry/generic/registry/store.go b/pkg/registry/generic/registry/store.go index 337143ce37d46..3e56d10b45d66 100644 --- a/pkg/registry/generic/registry/store.go +++ b/pkg/registry/generic/registry/store.go @@ -19,6 +19,7 @@ package registry import ( "fmt" "reflect" + "strings" "sync" "k8s.io/kubernetes/pkg/api" @@ -137,8 +138,8 @@ func NamespaceKeyFunc(ctx api.Context, prefix string, name string) (string, erro if len(name) == 0 { return "", kubeerr.NewBadRequest("Name parameter required.") } - if ok, msg := validation.IsValidPathSegmentName(name); !ok { - return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %v.", msg)) + if msgs := validation.IsValidPathSegmentName(name); len(msgs) != 0 { + return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %q: %s", name, strings.Join(msgs, ";"))) } key = key + "/" + name return key, nil @@ -149,8 +150,8 @@ func NoNamespaceKeyFunc(ctx api.Context, prefix string, name string) (string, er if len(name) == 0 { return "", kubeerr.NewBadRequest("Name parameter required.") } - if ok, msg := validation.IsValidPathSegmentName(name); !ok { - return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %v.", msg)) + if msgs := validation.IsValidPathSegmentName(name); len(msgs) != 0 { + return "", kubeerr.NewBadRequest(fmt.Sprintf("Name parameter invalid: %q: %s", name, strings.Join(msgs, ";"))) } key := prefix + "/" + name return key, nil diff --git a/pkg/serviceaccount/util.go b/pkg/serviceaccount/util.go index 60791236429fa..487b0f156923d 100644 --- a/pkg/serviceaccount/util.go +++ b/pkg/serviceaccount/util.go @@ -52,10 +52,10 @@ func SplitUsername(username string) (string, string, error) { return "", "", invalidUsernameErr } namespace, name := parts[0], parts[1] - if ok, _ := validation.ValidateNamespaceName(namespace, false); !ok { + if len(validation.ValidateNamespaceName(namespace, false)) != 0 { return "", "", invalidUsernameErr } - if ok, _ := validation.ValidateServiceAccountName(name, false); !ok { + if len(validation.ValidateServiceAccountName(name, false)) != 0 { return "", "", invalidUsernameErr } return namespace, name, nil diff --git a/pkg/storage/util.go b/pkg/storage/util.go index c8f3b02f198e0..6595a6763828f 100644 --- a/pkg/storage/util.go +++ b/pkg/storage/util.go @@ -71,8 +71,8 @@ func NamespaceKeyFunc(prefix string, obj runtime.Object) (string, error) { return "", err } name := meta.GetName() - if ok, msg := validation.IsValidPathSegmentName(name); !ok { - return "", fmt.Errorf("invalid name: %v", msg) + if msgs := validation.IsValidPathSegmentName(name); len(msgs) != 0 { + return "", fmt.Errorf("invalid name: %v", msgs) } return prefix + "/" + meta.GetNamespace() + "/" + name, nil } @@ -83,8 +83,8 @@ func NoNamespaceKeyFunc(prefix string, obj runtime.Object) (string, error) { return "", err } name := meta.GetName() - if ok, msg := validation.IsValidPathSegmentName(name); !ok { - return "", fmt.Errorf("invalid name: %v", msg) + if msgs := validation.IsValidPathSegmentName(name); len(msgs) != 0 { + return "", fmt.Errorf("invalid name: %v", msgs) } return prefix + "/" + name, nil }