From c20efb6bae5e7e5ca685d7d1475e73e14e80ac6e Mon Sep 17 00:00:00 2001 From: gmarek Date: Fri, 20 Mar 2015 15:40:20 +0100 Subject: [PATCH] Add validation for Endpoints --- pkg/api/validation/validation.go | 46 ++++++- pkg/api/validation/validation_test.go | 175 +++++++++++++++++++++++++- pkg/util/validation.go | 4 +- pkg/util/validation_test.go | 4 +- 4 files changed, 218 insertions(+), 11 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 9563a7ce566..b99f279ddc2 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -900,7 +900,7 @@ func ValidateService(service *api.Service) errs.ValidationErrorList { for _, ip := range service.Spec.PublicIPs { if ip == "0.0.0.0" { allErrs = append(allErrs, errs.NewFieldInvalid("spec.publicIPs", ip, "is not an IP address")) - } else if util.IsValidIP(ip) && net.ParseIP(ip).IsLoopback() { + } else if util.IsValidIPv4(ip) && net.ParseIP(ip).IsLoopback() { allErrs = append(allErrs, errs.NewFieldInvalid("spec.publicIPs", ip, "publicIP cannot be a loopback")) } } @@ -1276,17 +1276,51 @@ func validateEndpointSubsets(subsets []api.EndpointSubset) errs.ValidationErrorL if len(ss.Ports) == 0 { ssErrs = append(ssErrs, errs.NewFieldRequired("ports")) } - // TODO: validate each address and port + for addr := range ss.Addresses { + ssErrs = append(ssErrs, validateEndpointAddress(&ss.Addresses[addr]).PrefixIndex(addr).Prefix("addresses")...) + } + for port := range ss.Ports { + ssErrs = append(ssErrs, validateEndpointPort(&ss.Ports[port], len(ss.Ports) > 1).PrefixIndex(port).Prefix("ports")...) + } + allErrs = append(allErrs, ssErrs.PrefixIndex(i)...) } return allErrs } -// ValidateEndpointsUpdate tests to make sure an endpoints update can be applied. -func ValidateEndpointsUpdate(oldEndpoints *api.Endpoints, endpoints *api.Endpoints) errs.ValidationErrorList { +func validateEndpointAddress(address *api.EndpointAddress) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldEndpoints.ObjectMeta, &endpoints.ObjectMeta).Prefix("metadata")...) - allErrs = append(allErrs, validateEndpointSubsets(endpoints.Subsets).Prefix("subsets")...) + if !util.IsValidIPv4(address.IP) { + allErrs = append(allErrs, errs.NewFieldInvalid("ip", address.IP, "invalid IPv4 address")) + } + return allErrs +} + +func validateEndpointPort(port *api.EndpointPort, requireName bool) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + if requireName && port.Name == "" { + allErrs = append(allErrs, errs.NewFieldRequired("name")) + } else if port.Name != "" { + if !util.IsDNS1123Label(port.Name) { + allErrs = append(allErrs, errs.NewFieldInvalid("name", port.Name, dns1123LabelErrorMsg)) + } + } + if !util.IsValidPortNum(port.Port) { + allErrs = append(allErrs, errs.NewFieldInvalid("port", port.Port, portRangeErrorMsg)) + } + if len(port.Protocol) == 0 { + allErrs = append(allErrs, errs.NewFieldRequired("protocol")) + } else if !supportedPortProtocols.Has(strings.ToUpper(string(port.Protocol))) { + allErrs = append(allErrs, errs.NewFieldNotSupported("protocol", port.Protocol)) + } + return allErrs +} + +// ValidateEndpointsUpdate tests to make sure an endpoints update can be applied. +func ValidateEndpointsUpdate(oldEndpoints, newEndpoints *api.Endpoints) errs.ValidationErrorList { + allErrs := errs.ValidationErrorList{} + allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldEndpoints.ObjectMeta, &newEndpoints.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, validateEndpointSubsets(newEndpoints.Subsets).Prefix("subsets")...) return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index b866bbec22d..87759f60c3e 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -2812,5 +2812,178 @@ func TestValidateSecret(t *testing.T) { } func TestValidateEndpoints(t *testing.T) { - // TODO: implement this + successCases := map[string]api.Endpoints{ + "simple endpoint": { + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{IP: "10.10.1.1"}, {IP: "10.10.2.2"}}, + Ports: []api.EndpointPort{{Name: "a", Port: 8675, Protocol: "TCP"}, {Name: "b", Port: 309, Protocol: "TCP"}}, + }, + { + Addresses: []api.EndpointAddress{{IP: "10.10.3.3"}}, + Ports: []api.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}, {Name: "b", Port: 76, Protocol: "TCP"}}, + }, + }, + }, + "empty subsets": { + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + }, + "no name required for singleton port": { + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{IP: "10.10.1.1"}}, + Ports: []api.EndpointPort{{Port: 8675, Protocol: "TCP"}}, + }, + }, + }, + } + + for k, v := range successCases { + if errs := ValidateEndpoints(&v); len(errs) != 0 { + t.Errorf("Expected success for %s, got %v", k, errs) + } + } + + errorCases := map[string]struct { + endpoints api.Endpoints + errorType fielderrors.ValidationErrorType + errorDetail string + }{ + "missing namespace": { + endpoints: api.Endpoints{ObjectMeta: api.ObjectMeta{Name: "mysvc"}}, + errorType: "FieldValueRequired", + }, + "missing name": { + endpoints: api.Endpoints{ObjectMeta: api.ObjectMeta{Namespace: "namespace"}}, + errorType: "FieldValueRequired", + }, + "invalid namespace": { + endpoints: api.Endpoints{ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "no@#invalid.;chars\"allowed"}}, + errorType: "FieldValueInvalid", + errorDetail: dnsSubdomainErrorMsg, + }, + "invalid name": { + endpoints: api.Endpoints{ObjectMeta: api.ObjectMeta{Name: "-_Invliad^&Characters", Namespace: "namespace"}}, + errorType: "FieldValueInvalid", + errorDetail: dnsSubdomainErrorMsg, + }, + "empty addresses": { + endpoints: api.Endpoints{ + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Ports: []api.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}}, + }, + }, + }, + errorType: "FieldValueRequired", + }, + "empty ports": { + endpoints: api.Endpoints{ + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{IP: "10.10.3.3"}}, + }, + }, + }, + errorType: "FieldValueRequired", + }, + "invalid IP": { + endpoints: api.Endpoints{ + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{IP: "2001:0db8:85a3:0042:1000:8a2e:0370:7334"}}, + Ports: []api.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}}, + }, + }, + }, + errorType: "FieldValueInvalid", + errorDetail: "invalid IPv4 address", + }, + "Multiple ports, one without name": { + endpoints: api.Endpoints{ + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{IP: "10.10.1.1"}}, + Ports: []api.EndpointPort{{Port: 8675, Protocol: "TCP"}, {Name: "b", Port: 309, Protocol: "TCP"}}, + }, + }, + }, + errorType: "FieldValueRequired", + }, + "Invalid port number": { + endpoints: api.Endpoints{ + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{IP: "10.10.1.1"}}, + Ports: []api.EndpointPort{{Name: "a", Port: 66000, Protocol: "TCP"}}, + }, + }, + }, + errorType: "FieldValueInvalid", + errorDetail: portRangeErrorMsg, + }, + "Invalid protocol": { + endpoints: api.Endpoints{ + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{IP: "10.10.1.1"}}, + Ports: []api.EndpointPort{{Name: "a", Port: 93, Protocol: "Protocol"}}, + }, + }, + }, + errorType: "FieldValueNotSupported", + }, + "Address missing IP": { + endpoints: api.Endpoints{ + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{}}, + Ports: []api.EndpointPort{{Name: "a", Port: 93, Protocol: "TCP"}}, + }, + }, + }, + errorType: "FieldValueInvalid", + errorDetail: "invalid IPv4 address", + }, + "Port missing number": { + endpoints: api.Endpoints{ + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{IP: "10.10.1.1"}}, + Ports: []api.EndpointPort{{Name: "a", Protocol: "TCP"}}, + }, + }, + }, + errorType: "FieldValueInvalid", + errorDetail: portRangeErrorMsg, + }, + "Port missing protocol": { + endpoints: api.Endpoints{ + ObjectMeta: api.ObjectMeta{Name: "mysvc", Namespace: "namespace"}, + Subsets: []api.EndpointSubset{ + { + Addresses: []api.EndpointAddress{{IP: "10.10.1.1"}}, + Ports: []api.EndpointPort{{Name: "a", Port: 93}}, + }, + }, + }, + errorType: "FieldValueRequired", + }, + } + + for k, v := range errorCases { + if errs := ValidateEndpoints(&v.endpoints); len(errs) == 0 || errs[0].(*errors.ValidationError).Type != v.errorType || errs[0].(*errors.ValidationError).Detail != v.errorDetail { + t.Errorf("Expected error type %s with detail %s for %s, got %v", v.errorType, v.errorDetail, k, errs) + } + } } diff --git a/pkg/util/validation.go b/pkg/util/validation.go index b4bc2e8a80b..29fd815c383 100644 --- a/pkg/util/validation.go +++ b/pkg/util/validation.go @@ -91,7 +91,7 @@ func IsValidPortNum(port int) bool { return 0 < port && port < 65536 } -// IsValidIP tests that the argument is a valid IPv4 address. -func IsValidIP(value string) bool { +// IsValidIPv4 tests that the argument is a valid IPv4 address. +func IsValidIPv4(value string) bool { return net.ParseIP(value) != nil && net.ParseIP(value).To4() != nil } diff --git a/pkg/util/validation_test.go b/pkg/util/validation_test.go index 853b0932d9c..0aadff49672 100644 --- a/pkg/util/validation_test.go +++ b/pkg/util/validation_test.go @@ -232,7 +232,7 @@ func TestIsValidIP(t *testing.T) { "0.0.0.0", } for _, val := range goodValues { - if !IsValidIP(val) { + if !IsValidIPv4(val) { t.Errorf("expected true for %q", val) } } @@ -247,7 +247,7 @@ func TestIsValidIP(t *testing.T) { "1.0.0.1.", } for _, val := range badValues { - if IsValidIP(val) { + if IsValidIPv4(val) { t.Errorf("expected false for %q", val) } }