From db95798b3928c2a2499255e7bed7c2d85a0c68d0 Mon Sep 17 00:00:00 2001 From: Girish Kalele Date: Wed, 24 Aug 2016 09:57:38 -0700 Subject: [PATCH] Enforce EndpointAddress.NodeName validation + added unit tests --- pkg/api/validation/validation.go | 52 +++++++++++++++++++++++---- pkg/api/validation/validation_test.go | 39 ++++++++++++++++++++ 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 49aecc06033..5d01850ebba 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -3358,17 +3358,50 @@ func ValidateNamespaceFinalizeUpdate(newNamespace, oldNamespace *api.Namespace) return allErrs } +// Construct lookup map of old subset IPs to NodeNames. +func updateEpAddrToNodeNameMap(ipToNodeName map[string]string, addresses []api.EndpointAddress) { + for n := range addresses { + if addresses[n].NodeName == nil { + continue + } + ipToNodeName[addresses[n].IP] = *addresses[n].NodeName + } +} + +// Build a map across all subsets of IP -> NodeName +func buildEndpointAddressNodeNameMap(subsets []api.EndpointSubset) map[string]string { + ipToNodeName := make(map[string]string) + for i := range subsets { + updateEpAddrToNodeNameMap(ipToNodeName, subsets[i].Addresses) + updateEpAddrToNodeNameMap(ipToNodeName, subsets[i].NotReadyAddresses) + } + return ipToNodeName +} + +func validateEpAddrNodeNameTransition(addr *api.EndpointAddress, ipToNodeName map[string]string, fldPath *field.Path) field.ErrorList { + errList := field.ErrorList{} + existingNodeName, found := ipToNodeName[addr.IP] + if !found { + return errList + } + if addr.NodeName == nil || *addr.NodeName == existingNodeName { + return errList + } + // NodeName entry found for this endpoint IP, but user is attempting to change NodeName + return append(errList, field.Forbidden(fldPath, fmt.Sprintf("Cannot change NodeName for %s to %s", addr.IP, *addr.NodeName))) +} + // ValidateEndpoints tests if required fields are set. func ValidateEndpoints(endpoints *api.Endpoints) field.ErrorList { allErrs := ValidateObjectMeta(&endpoints.ObjectMeta, true, ValidateEndpointsName, field.NewPath("metadata")) allErrs = append(allErrs, ValidateEndpointsSpecificAnnotations(endpoints.Annotations, field.NewPath("annotations"))...) - allErrs = append(allErrs, validateEndpointSubsets(endpoints.Subsets, field.NewPath("subsets"))...) + allErrs = append(allErrs, validateEndpointSubsets(endpoints.Subsets, []api.EndpointSubset{}, field.NewPath("subsets"))...) return allErrs } -func validateEndpointSubsets(subsets []api.EndpointSubset, fldPath *field.Path) field.ErrorList { +func validateEndpointSubsets(subsets []api.EndpointSubset, oldSubsets []api.EndpointSubset, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - + ipToNodeName := buildEndpointAddressNodeNameMap(oldSubsets) for i := range subsets { ss := &subsets[i] idxPath := fldPath.Index(i) @@ -3381,10 +3414,10 @@ func validateEndpointSubsets(subsets []api.EndpointSubset, fldPath *field.Path) allErrs = append(allErrs, field.Required(idxPath.Child("ports"), "")) } for addr := range ss.Addresses { - allErrs = append(allErrs, validateEndpointAddress(&ss.Addresses[addr], idxPath.Child("addresses").Index(addr))...) + allErrs = append(allErrs, validateEndpointAddress(&ss.Addresses[addr], idxPath.Child("addresses").Index(addr), ipToNodeName)...) } for addr := range ss.NotReadyAddresses { - allErrs = append(allErrs, validateEndpointAddress(&ss.NotReadyAddresses[addr], idxPath.Child("notReadyAddresses").Index(addr))...) + allErrs = append(allErrs, validateEndpointAddress(&ss.NotReadyAddresses[addr], idxPath.Child("notReadyAddresses").Index(addr), ipToNodeName)...) } for port := range ss.Ports { allErrs = append(allErrs, validateEndpointPort(&ss.Ports[port], len(ss.Ports) > 1, idxPath.Child("ports").Index(port))...) @@ -3394,7 +3427,7 @@ func validateEndpointSubsets(subsets []api.EndpointSubset, fldPath *field.Path) return allErrs } -func validateEndpointAddress(address *api.EndpointAddress, fldPath *field.Path) field.ErrorList { +func validateEndpointAddress(address *api.EndpointAddress, fldPath *field.Path, ipToNodeName map[string]string) field.ErrorList { allErrs := field.ErrorList{} for _, msg := range validation.IsValidIP(address.IP) { allErrs = append(allErrs, field.Invalid(fldPath.Child("ip"), address.IP, msg)) @@ -3402,6 +3435,11 @@ func validateEndpointAddress(address *api.EndpointAddress, fldPath *field.Path) if len(address.Hostname) > 0 { allErrs = append(allErrs, ValidateDNS1123Label(address.Hostname, fldPath.Child("hostname"))...) } + // During endpoint update, validate NodeName is DNS1123 compliant and transition rules allow the update + if address.NodeName != nil { + allErrs = append(allErrs, ValidateDNS1123Label(*address.NodeName, fldPath.Child("nodeName"))...) + } + allErrs = append(allErrs, validateEpAddrNodeNameTransition(address, ipToNodeName, fldPath.Child("nodeName"))...) if len(allErrs) > 0 { return allErrs } @@ -3456,7 +3494,7 @@ func validateEndpointPort(port *api.EndpointPort, requireName bool, fldPath *fie // ValidateEndpointsUpdate tests to make sure an endpoints update can be applied. func ValidateEndpointsUpdate(newEndpoints, oldEndpoints *api.Endpoints) field.ErrorList { allErrs := ValidateObjectMetaUpdate(&newEndpoints.ObjectMeta, &oldEndpoints.ObjectMeta, field.NewPath("metadata")) - allErrs = append(allErrs, validateEndpointSubsets(newEndpoints.Subsets, field.NewPath("subsets"))...) + allErrs = append(allErrs, validateEndpointSubsets(newEndpoints.Subsets, oldEndpoints.Subsets, field.NewPath("subsets"))...) allErrs = append(allErrs, ValidateEndpointsSpecificAnnotations(newEndpoints.Annotations, field.NewPath("annotations"))...) return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index 5032120b3e1..2edc77137f0 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -7992,3 +7992,42 @@ func TestValidateSysctls(t *testing.T) { } } } + +func newNodeNameEndpoint(nodeName string) *api.Endpoints { + ep := &api.Endpoints{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + ResourceVersion: "1", + }, + Subsets: []api.EndpointSubset{ + { + NotReadyAddresses: []api.EndpointAddress{}, + Ports: []api.EndpointPort{{Name: "https", Port: 443, Protocol: "TCP"}}, + Addresses: []api.EndpointAddress{ + { + IP: "8.8.8.8", + Hostname: "zookeeper1", + NodeName: &nodeName}}}}} + return ep +} + +func TestEndpointAddressNodeNameUpdateRestrictions(t *testing.T) { + oldEndpoint := newNodeNameEndpoint("kubernetes-minion-setup-by-backend") + updatedEndpoint := newNodeNameEndpoint("kubernetes-changed-nodename") + // Check that NodeName cannot be changed during update (if already set) + errList := ValidateEndpoints(updatedEndpoint) + errList = append(errList, ValidateEndpointsUpdate(updatedEndpoint, oldEndpoint)...) + if len(errList) == 0 { + t.Error("Endpoint should not allow changing of Subset.Addresses.NodeName on update") + } +} + +func TestEndpointAddressNodeNameInvalidDNS1123(t *testing.T) { + // Check NodeName DNS validation + endpoint := newNodeNameEndpoint("illegal.nodename") + errList := ValidateEndpoints(endpoint) + if len(errList) == 0 { + t.Error("Endpoint should reject invalid NodeName") + } +}