Skip to content

Commit

Permalink
Merge pull request kubernetes#5546 from gmarek/client1
Browse files Browse the repository at this point in the history 
Cleanup of validation.go - recovered version of kubernetes#5289
  • Loading branch information
@thockin
thockin committed Mar 18, 2015
2 parents faab509 + a3b137c commit e26b810
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 78 deletions.
2 changes: 1 addition & 1 deletion pkg/api/errors/errors_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestNewInvalid(t *testing.T) {
},
},
{
NewFieldRequired("field[0].name", "bar"),
NewFieldRequired("field[0].name"),
&api.StatusDetails{
Kind: "kind",
ID: "name",
Expand Down
5 changes: 2 additions & 3 deletions pkg/api/errors/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,8 @@ func (v *ValidationError) Error() string {
}

// NewFieldRequired returns a *ValidationError indicating "value required"
// TODO: remove "value"
func NewFieldRequired(field string, value interface{}) *ValidationError {
return &ValidationError{ValidationErrorTypeRequired, field, value, ""}
func NewFieldRequired(field string) *ValidationError {
return &ValidationError{ValidationErrorTypeRequired, field, "", ""}
}

// NewFieldInvalid returns a *ValidationError indicating "invalid value"
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/errors/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestMakeFuncs(t *testing.T) {
ValidationErrorTypeNotFound,
},
{
func() *ValidationError { return NewFieldRequired("f", "v") },
func() *ValidationError { return NewFieldRequired("f") },
ValidationErrorTypeRequired,
},
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/validation/events.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func ValidateEvent(event *api.Event) errs.ValidationErrorList {
if event.Namespace != event.InvolvedObject.Namespace {
allErrs = append(allErrs, errs.NewFieldInvalid("involvedObject.namespace", event.InvolvedObject.Namespace, "namespace does not match involvedObject"))
}
if !util.IsDNSSubdomain(event.Namespace) {
if !util.IsDNS1123Subdomain(event.Namespace) {
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", event.Namespace, ""))
}
return allErrs
Expand Down
78 changes: 39 additions & 39 deletions pkg/api/validation/validation.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ func intervalErrorMsg(lo, hi int) string {
}

var labelValueErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.LabelValueMaxLength, util.LabelValueFmt)
var qualifiedNameErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123SubdomainMaxLength, util.QualifiedNameFmt)
var qualifiedNameErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.QualifiedNameMaxLength, util.QualifiedNameFmt)
var dnsSubdomainErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123SubdomainMaxLength, util.DNS1123SubdomainFmt)
var dnsLabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123LabelMaxLength, util.DNS1123LabelFmt)
var dns1123LabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS1123LabelMaxLength, util.DNS1123LabelFmt)
var dns952LabelErrorMsg string = fmt.Sprintf("must have at most %d characters and match regex %s", util.DNS952LabelMaxLength, util.DNS952LabelFmt)
var pdPartitionErrorMsg string = intervalErrorMsg(0, 255)
var portRangeErrorMsg string = intervalErrorMsg(0, 65536)
Expand Down Expand Up @@ -157,7 +157,7 @@ func nameIsDNSSubdomain(name string, prefix bool) (bool, string) {
if prefix {
name = maskTrailingDash(name)
}
if util.IsDNSSubdomain(name) {
if util.IsDNS1123Subdomain(name) {
return true, ""
}
return false, dnsSubdomainErrorMsg
Expand Down Expand Up @@ -187,7 +187,7 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val
// if the generated name validates, but the calculated value does not, it's a problem with generation, and we
// report it here. This may confuse users, but indicates a programming bug and still must be validated.
if len(meta.Name) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("name", meta.Name))
allErrs = append(allErrs, errs.NewFieldRequired("name"))
} else {
if ok, qualifier := nameFn(meta.Name, false); !ok {
allErrs = append(allErrs, errs.NewFieldInvalid("name", meta.Name, qualifier))
Expand All @@ -196,8 +196,8 @@ func ValidateObjectMeta(meta *api.ObjectMeta, requiresNamespace bool, nameFn Val

if requiresNamespace {
if len(meta.Namespace) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("namespace", meta.Namespace))
} else if !util.IsDNSSubdomain(meta.Namespace) {
allErrs = append(allErrs, errs.NewFieldRequired("namespace"))
} else if !util.IsDNS1123Subdomain(meta.Namespace) {
allErrs = append(allErrs, errs.NewFieldInvalid("namespace", meta.Namespace, dnsSubdomainErrorMsg))
}
} else {
Expand Down Expand Up @@ -252,9 +252,9 @@ func validateVolumes(volumes []api.Volume) (util.StringSet, errs.ValidationError
for i, vol := range volumes {
el := validateSource(&vol.VolumeSource).Prefix("source")
if len(vol.Name) == 0 {
el = append(el, errs.NewFieldRequired("name", vol.Name))
} else if !util.IsDNSLabel(vol.Name) {
el = append(el, errs.NewFieldInvalid("name", vol.Name, dnsLabelErrorMsg))
el = append(el, errs.NewFieldRequired("name"))
} else if !util.IsDNS1123Label(vol.Name) {
el = append(el, errs.NewFieldInvalid("name", vol.Name, dns1123LabelErrorMsg))
} else if allNames.Has(vol.Name) {
el = append(el, errs.NewFieldDuplicate("name", vol.Name))
}
Expand Down Expand Up @@ -299,26 +299,26 @@ func validateSource(source *api.VolumeSource) errs.ValidationErrorList {
func validateHostPathVolumeSource(hostDir *api.HostPathVolumeSource) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
if hostDir.Path == "" {
allErrs = append(allErrs, errs.NewFieldRequired("path", hostDir.Path))
allErrs = append(allErrs, errs.NewFieldRequired("path"))
}
return allErrs
}

func validateGitRepoVolumeSource(gitRepo *api.GitRepoVolumeSource) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
if gitRepo.Repository == "" {
allErrs = append(allErrs, errs.NewFieldRequired("repository", gitRepo.Repository))
allErrs = append(allErrs, errs.NewFieldRequired("repository"))
}
return allErrs
}

func validateGCEPersistentDiskVolumeSource(PD *api.GCEPersistentDiskVolumeSource) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
if PD.PDName == "" {
allErrs = append(allErrs, errs.NewFieldRequired("pdName", PD.PDName))
allErrs = append(allErrs, errs.NewFieldRequired("pdName"))
}
if PD.FSType == "" {
allErrs = append(allErrs, errs.NewFieldRequired("fsType", PD.FSType))
allErrs = append(allErrs, errs.NewFieldRequired("fsType"))
}
if PD.Partition < 0 || PD.Partition > 255 {
allErrs = append(allErrs, errs.NewFieldInvalid("partition", PD.Partition, pdPartitionErrorMsg))
Expand All @@ -329,10 +329,10 @@ func validateGCEPersistentDiskVolumeSource(PD *api.GCEPersistentDiskVolumeSource
func validateSecretVolumeSource(secretSource *api.SecretVolumeSource) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
if secretSource.Target.Name == "" {
allErrs = append(allErrs, errs.NewFieldRequired("target.name", ""))
allErrs = append(allErrs, errs.NewFieldRequired("target.name"))
}
if secretSource.Target.Namespace == "" {
allErrs = append(allErrs, errs.NewFieldRequired("target.namespace", ""))
allErrs = append(allErrs, errs.NewFieldRequired("target.namespace"))
}
if secretSource.Target.Kind != "Secret" {
allErrs = append(allErrs, errs.NewFieldInvalid("target.kind", secretSource.Target.Kind, "Secret"))
Expand All @@ -349,8 +349,8 @@ func validatePorts(ports []api.ContainerPort) errs.ValidationErrorList {
for i, port := range ports {
pErrs := errs.ValidationErrorList{}
if len(port.Name) > 0 {
if len(port.Name) > util.DNS1123LabelMaxLength || !util.IsDNSLabel(port.Name) {
pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, dnsLabelErrorMsg))
if len(port.Name) > util.DNS1123LabelMaxLength || !util.IsDNS1123Label(port.Name) {
pErrs = append(pErrs, errs.NewFieldInvalid("name", port.Name, dns1123LabelErrorMsg))
} else if allNames.Has(port.Name) {
pErrs = append(pErrs, errs.NewFieldDuplicate("name", port.Name))
} else {
Expand All @@ -366,7 +366,7 @@ func validatePorts(ports []api.ContainerPort) errs.ValidationErrorList {
pErrs = append(pErrs, errs.NewFieldInvalid("hostPort", port.HostPort, portRangeErrorMsg))
}
if len(port.Protocol) == 0 {
pErrs = append(pErrs, errs.NewFieldRequired("protocol", port.Protocol))
pErrs = append(pErrs, errs.NewFieldRequired("protocol"))
} else if !supportedPortProtocols.Has(strings.ToUpper(string(port.Protocol))) {
pErrs = append(pErrs, errs.NewFieldNotSupported("protocol", port.Protocol))
}
Expand All @@ -381,7 +381,7 @@ func validateEnv(vars []api.EnvVar) errs.ValidationErrorList {
for i, ev := range vars {
vErrs := errs.ValidationErrorList{}
if len(ev.Name) == 0 {
vErrs = append(vErrs, errs.NewFieldRequired("name", ev.Name))
vErrs = append(vErrs, errs.NewFieldRequired("name"))
}
if !util.IsCIdentifier(ev.Name) {
vErrs = append(vErrs, errs.NewFieldInvalid("name", ev.Name, cIdentifierErrorMsg))
Expand All @@ -397,12 +397,12 @@ func validateVolumeMounts(mounts []api.VolumeMount, volumes util.StringSet) errs
for i, mnt := range mounts {
mErrs := errs.ValidationErrorList{}
if len(mnt.Name) == 0 {
mErrs = append(mErrs, errs.NewFieldRequired("name", mnt.Name))
mErrs = append(mErrs, errs.NewFieldRequired("name"))
} else if !volumes.Has(mnt.Name) {
mErrs = append(mErrs, errs.NewFieldNotFound("name", mnt.Name))
}
if len(mnt.MountPath) == 0 {
mErrs = append(mErrs, errs.NewFieldRequired("mountPath", mnt.MountPath))
mErrs = append(mErrs, errs.NewFieldRequired("mountPath"))
}
allErrs = append(allErrs, mErrs.PrefixIndex(i)...)
}
Expand Down Expand Up @@ -458,20 +458,20 @@ func checkHostPortConflicts(containers []api.Container) errs.ValidationErrorList
func validateExecAction(exec *api.ExecAction) errs.ValidationErrorList {
allErrors := errs.ValidationErrorList{}
if len(exec.Command) == 0 {
allErrors = append(allErrors, errs.NewFieldRequired("command", exec.Command))
allErrors = append(allErrors, errs.NewFieldRequired("command"))
}
return allErrors
}

func validateHTTPGetAction(http *api.HTTPGetAction) errs.ValidationErrorList {
allErrors := errs.ValidationErrorList{}
if len(http.Path) == 0 {
allErrors = append(allErrors, errs.NewFieldRequired("path", http.Path))
allErrors = append(allErrors, errs.NewFieldRequired("path"))
}
if http.Port.Kind == util.IntstrInt && !util.IsValidPortNum(http.Port.IntVal) {
allErrors = append(allErrors, errs.NewFieldInvalid("port", http.Port, portRangeErrorMsg))
} else if http.Port.Kind == util.IntstrString && len(http.Port.StrVal) == 0 {
allErrors = append(allErrors, errs.NewFieldRequired("port", http.Port.StrVal))
allErrors = append(allErrors, errs.NewFieldRequired("port"))
}
return allErrors
}
Expand All @@ -481,7 +481,7 @@ func validateTCPSocketAction(tcp *api.TCPSocketAction) errs.ValidationErrorList
if tcp.Port.Kind == util.IntstrInt && !util.IsValidPortNum(tcp.Port.IntVal) {
allErrors = append(allErrors, errs.NewFieldInvalid("port", tcp.Port, portRangeErrorMsg))
} else if tcp.Port.Kind == util.IntstrString && len(tcp.Port.StrVal) == 0 {
allErrors = append(allErrors, errs.NewFieldRequired("port", tcp.Port.StrVal))
allErrors = append(allErrors, errs.NewFieldRequired("port"))
}
return allErrors
}
Expand Down Expand Up @@ -525,7 +525,7 @@ func validatePullPolicy(ctr *api.Container) errs.ValidationErrorList {
case api.PullAlways, api.PullIfNotPresent, api.PullNever:
break
case "":
allErrors = append(allErrors, errs.NewFieldRequired("", ctr.ImagePullPolicy))
allErrors = append(allErrors, errs.NewFieldRequired(""))
default:
allErrors = append(allErrors, errs.NewFieldNotSupported("", ctr.ImagePullPolicy))
}
Expand All @@ -541,9 +541,9 @@ func validateContainers(containers []api.Container, volumes util.StringSet) errs
cErrs := errs.ValidationErrorList{}
capabilities := capabilities.Get()
if len(ctr.Name) == 0 {
cErrs = append(cErrs, errs.NewFieldRequired("name", ctr.Name))
} else if !util.IsDNSLabel(ctr.Name) {
cErrs = append(cErrs, errs.NewFieldInvalid("name", ctr.Name, dnsLabelErrorMsg))
cErrs = append(cErrs, errs.NewFieldRequired("name"))
} else if !util.IsDNS1123Label(ctr.Name) {
cErrs = append(cErrs, errs.NewFieldInvalid("name", ctr.Name, dns1123LabelErrorMsg))
} else if allNames.Has(ctr.Name) {
cErrs = append(cErrs, errs.NewFieldDuplicate("name", ctr.Name))
} else if ctr.Privileged && !capabilities.AllowPrivileged {
Expand All @@ -552,7 +552,7 @@ func validateContainers(containers []api.Container, volumes util.StringSet) errs
allNames.Insert(ctr.Name)
}
if len(ctr.Image) == 0 {
cErrs = append(cErrs, errs.NewFieldRequired("image", ctr.Image))
cErrs = append(cErrs, errs.NewFieldRequired("image"))
}
if ctr.Lifecycle != nil {
cErrs = append(cErrs, validateLifecycle(ctr.Lifecycle).Prefix("lifecycle")...)
Expand Down Expand Up @@ -587,7 +587,7 @@ func ValidateManifest(manifest *api.ContainerManifest) errs.ValidationErrorList
allErrs := errs.ValidationErrorList{}

if len(manifest.Version) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("version", manifest.Version))
allErrs = append(allErrs, errs.NewFieldRequired("version"))
} else if !supportedManifestVersions.Has(strings.ToLower(manifest.Version)) {
allErrs = append(allErrs, errs.NewFieldNotSupported("version", manifest.Version))
}
Expand All @@ -605,7 +605,7 @@ func validateRestartPolicy(restartPolicy *api.RestartPolicy) errs.ValidationErro
case api.RestartPolicyAlways, api.RestartPolicyOnFailure, api.RestartPolicyNever:
break
case "":
allErrors = append(allErrors, errs.NewFieldRequired("", *restartPolicy))
allErrors = append(allErrors, errs.NewFieldRequired(""))
default:
allErrors = append(allErrors, errs.NewFieldNotSupported("", restartPolicy))
}
Expand All @@ -619,7 +619,7 @@ func validateDNSPolicy(dnsPolicy *api.DNSPolicy) errs.ValidationErrorList {
case api.DNSClusterFirst, api.DNSDefault:
break
case "":
allErrors = append(allErrors, errs.NewFieldRequired("", *dnsPolicy))
allErrors = append(allErrors, errs.NewFieldRequired(""))
default:
allErrors = append(allErrors, errs.NewFieldNotSupported("", dnsPolicy))
}
Expand Down Expand Up @@ -707,22 +707,22 @@ func ValidateService(service *api.Service) errs.ValidationErrorList {
allErrs = append(allErrs, errs.NewFieldInvalid("spec.port", service.Spec.Port, portRangeErrorMsg))
}
if len(service.Spec.Protocol) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("spec.protocol", service.Spec.Protocol))
allErrs = append(allErrs, errs.NewFieldRequired("spec.protocol"))
} else if !supportedPortProtocols.Has(strings.ToUpper(string(service.Spec.Protocol))) {
allErrs = append(allErrs, errs.NewFieldNotSupported("spec.protocol", service.Spec.Protocol))
}
if service.Spec.ContainerPort.Kind == util.IntstrInt && service.Spec.ContainerPort.IntVal != 0 && !util.IsValidPortNum(service.Spec.ContainerPort.IntVal) {
allErrs = append(allErrs, errs.NewFieldInvalid("spec.containerPort", service.Spec.Port, portRangeErrorMsg))
} else if service.Spec.ContainerPort.Kind == util.IntstrString && len(service.Spec.ContainerPort.StrVal) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("spec.containerPort", service.Spec.ContainerPort.StrVal))
allErrs = append(allErrs, errs.NewFieldRequired("spec.containerPort"))
}

if service.Spec.Selector != nil {
allErrs = append(allErrs, ValidateLabels(service.Spec.Selector, "spec.selector")...)
}

if service.Spec.SessionAffinity == "" {
allErrs = append(allErrs, errs.NewFieldRequired("spec.sessionAffinity", service.Spec.SessionAffinity))
allErrs = append(allErrs, errs.NewFieldRequired("spec.sessionAffinity"))
} else if !supportedSessionAffinityType.Has(string(service.Spec.SessionAffinity)) {
allErrs = append(allErrs, errs.NewFieldNotSupported("spec.sessionAffinity", service.Spec.SessionAffinity))
}
Expand Down Expand Up @@ -767,14 +767,14 @@ func ValidateReplicationControllerSpec(spec *api.ReplicationControllerSpec) errs

selector := labels.Set(spec.Selector).AsSelector()
if selector.Empty() {
allErrs = append(allErrs, errs.NewFieldRequired("selector", spec.Selector))
allErrs = append(allErrs, errs.NewFieldRequired("selector"))
}
if spec.Replicas < 0 {
allErrs = append(allErrs, errs.NewFieldInvalid("replicas", spec.Replicas, isNegativeErrorMsg))
}

if spec.Template == nil {
allErrs = append(allErrs, errs.NewFieldRequired("template", spec.Template))
allErrs = append(allErrs, errs.NewFieldRequired("template"))
} else {
labels := labels.Set(spec.Template.Labels)
if !selector.Matches(labels) {
Expand Down Expand Up @@ -893,7 +893,7 @@ func ValidateSecret(secret *api.Secret) errs.ValidationErrorList {

totalSize := 0
for key, value := range secret.Data {
if !util.IsDNSSubdomain(key) {
if !util.IsDNS1123Subdomain(key) {
allErrs = append(allErrs, errs.NewFieldInvalid(fmt.Sprintf("data[%s]", key), key, cIdentifierErrorMsg))
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/api/validation/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ func TestValidateVolumes(t *testing.T) {
t.Errorf("%s: expected errors to have field %s: %v", k, v.F, errs[i])
}
detail := errs[i].(*errors.ValidationError).Detail
if detail != "" && detail != dnsLabelErrorMsg {
t.Errorf("%s: expected error detail either empty or %s, got %s", k, dnsLabelErrorMsg, detail)
if detail != "" && detail != dns1123LabelErrorMsg {
t.Errorf("%s: expected error detail either empty or %s, got %s", k, dns1123LabelErrorMsg, detail)
}
}
}
Expand Down Expand Up @@ -277,8 +277,8 @@ func TestValidatePorts(t *testing.T) {
F string
D string
}{
"name > 63 characters": {[]api.ContainerPort{{Name: strings.Repeat("a", 64), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dnsLabelErrorMsg},
"name not a DNS label": {[]api.ContainerPort{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dnsLabelErrorMsg},
"name > 63 characters": {[]api.ContainerPort{{Name: strings.Repeat("a", 64), ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dns1123LabelErrorMsg},
"name not a DNS label": {[]api.ContainerPort{{Name: "a.b.c", ContainerPort: 80, Protocol: "TCP"}}, errors.ValidationErrorTypeInvalid, "[0].name", dns1123LabelErrorMsg},
"name not unique": {[]api.ContainerPort{
{Name: "abc", ContainerPort: 80, Protocol: "TCP"},
{Name: "abc", ContainerPort: 81, Protocol: "TCP"},
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,12 @@ func CreateObjects(typer runtime.ObjectTyper, mapper meta.RESTMapper, clientFor
func CreateObject(client RESTClientPoster, mapping *meta.RESTMapping, obj runtime.Object) *errs.ValidationError {
name, err := mapping.MetadataAccessor.Name(obj)
if err != nil || name == "" {
return errs.NewFieldRequired("name", err)
return errs.NewFieldRequired("name")
}

namespace, err := mapping.Namespace(obj)
if err != nil {
return errs.NewFieldRequired("namespace", err)
return errs.NewFieldRequired("namespace")
}

// TODO: This should be using RESTHelper
Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/pod/etcd/etcd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func (r *BindingREST) Create(ctx api.Context, obj runtime.Object) (out runtime.O
return nil, errors.NewInvalid("binding", binding.Name, errors.ValidationErrorList{errors.NewFieldInvalid("to.kind", binding.Target.Kind, "must be empty, 'Node', or 'Minion'")})
}
if len(binding.Target.Name) == 0 {
return nil, errors.NewInvalid("binding", binding.Name, errors.ValidationErrorList{errors.NewFieldRequired("to.name", binding.Target.Name)})
return nil, errors.NewInvalid("binding", binding.Name, errors.ValidationErrorList{errors.NewFieldRequired("to.name")})
}
err = r.assignPod(ctx, binding.Name, binding.Target.Name, binding.Annotations)
out = &api.Status{Status: api.StatusSuccess}
Expand Down
Loading

0 comments on commit e26b810

Please sign in to comment.