Update the use of sets in EndpointSlice validation

Don't use sets for validating port name and zone hint uniqueness,
since constructing a new set each time is likely to be less efficient
than just doing a linear search.

Keep the sets for supportedAddressTypes and supportedPortProtocols
(since they're only constructed once) but switch to the generic set
API.
This commit is contained in:
Dan Winship 2025-03-18 07:01:02 -04:00
parent 73f54b67b2
commit 6ca82f9c16

View File

@ -18,6 +18,7 @@ package validation
import ( import (
"fmt" "fmt"
"slices"
corev1 "k8s.io/api/core/v1" corev1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality" apiequality "k8s.io/apimachinery/pkg/api/equality"
@ -33,15 +34,15 @@ import (
) )
var ( var (
supportedAddressTypes = sets.NewString( supportedAddressTypes = sets.New(
string(discovery.AddressTypeIPv4), discovery.AddressTypeIPv4,
string(discovery.AddressTypeIPv6), discovery.AddressTypeIPv6,
string(discovery.AddressTypeFQDN), discovery.AddressTypeFQDN,
) )
supportedPortProtocols = sets.NewString( supportedPortProtocols = sets.New(
string(api.ProtocolTCP), api.ProtocolTCP,
string(api.ProtocolUDP), api.ProtocolUDP,
string(api.ProtocolSCTP), api.ProtocolSCTP,
) )
maxTopologyLabels = 16 maxTopologyLabels = 16
maxAddresses = 100 maxAddresses = 100
@ -172,7 +173,9 @@ func validatePorts(endpointPorts []discovery.EndpointPort, fldPath *field.Path)
return allErrs return allErrs
} }
portNames := sets.String{} // Even though a sets.Set would be more idiomatic, we use a []string here to avoid
// extra allocations (especially since there are presumably only a few ports anyway).
portNames := make([]string, 0, len(endpointPorts))
for i, endpointPort := range endpointPorts { for i, endpointPort := range endpointPorts {
idxPath := fldPath.Index(i) idxPath := fldPath.Index(i)
@ -180,16 +183,16 @@ func validatePorts(endpointPorts []discovery.EndpointPort, fldPath *field.Path)
allErrs = append(allErrs, apivalidation.ValidateDNS1123Label(*endpointPort.Name, idxPath.Child("name"))...) allErrs = append(allErrs, apivalidation.ValidateDNS1123Label(*endpointPort.Name, idxPath.Child("name"))...)
} }
if portNames.Has(*endpointPort.Name) { if slices.Contains(portNames, *endpointPort.Name) {
allErrs = append(allErrs, field.Duplicate(idxPath.Child("name"), endpointPort.Name)) allErrs = append(allErrs, field.Duplicate(idxPath.Child("name"), endpointPort.Name))
} else { } else {
portNames.Insert(*endpointPort.Name) portNames = append(portNames, *endpointPort.Name)
} }
if endpointPort.Protocol == nil { if endpointPort.Protocol == nil {
allErrs = append(allErrs, field.Required(idxPath.Child("protocol"), "")) allErrs = append(allErrs, field.Required(idxPath.Child("protocol"), ""))
} else if !supportedPortProtocols.Has(string(*endpointPort.Protocol)) { } else if !supportedPortProtocols.Has(*endpointPort.Protocol) {
allErrs = append(allErrs, field.NotSupported(idxPath.Child("protocol"), *endpointPort.Protocol, supportedPortProtocols.List())) allErrs = append(allErrs, field.NotSupported(idxPath.Child("protocol"), *endpointPort.Protocol, sets.List(supportedPortProtocols)))
} }
if endpointPort.AppProtocol != nil { if endpointPort.AppProtocol != nil {
@ -205,8 +208,8 @@ func validateAddressType(addressType discovery.AddressType) field.ErrorList {
if addressType == "" { if addressType == "" {
allErrs = append(allErrs, field.Required(field.NewPath("addressType"), "")) allErrs = append(allErrs, field.Required(field.NewPath("addressType"), ""))
} else if !supportedAddressTypes.Has(string(addressType)) { } else if !supportedAddressTypes.Has(addressType) {
allErrs = append(allErrs, field.NotSupported(field.NewPath("addressType"), addressType, supportedAddressTypes.List())) allErrs = append(allErrs, field.NotSupported(field.NewPath("addressType"), addressType, sets.List(supportedAddressTypes)))
} }
return allErrs return allErrs
@ -221,13 +224,15 @@ func validateHints(endpointHints *discovery.EndpointHints, fldPath *field.Path)
return allErrs return allErrs
} }
zoneNames := sets.String{} // Even though a sets.Set would be more idiomatic, we use a []string here to avoid
// extra allocations (especially since there is normally only one zone anyway).
zoneNames := make([]string, 0, len(endpointHints.ForZones))
for i, forZone := range endpointHints.ForZones { for i, forZone := range endpointHints.ForZones {
zonePath := fzPath.Index(i).Child("name") zonePath := fzPath.Index(i).Child("name")
if zoneNames.Has(forZone.Name) { if slices.Contains(zoneNames, forZone.Name) {
allErrs = append(allErrs, field.Duplicate(zonePath, forZone.Name)) allErrs = append(allErrs, field.Duplicate(zonePath, forZone.Name))
} else { } else {
zoneNames.Insert(forZone.Name) zoneNames = append(zoneNames, forZone.Name)
} }
for _, msg := range validation.IsValidLabelValue(forZone.Name) { for _, msg := range validation.IsValidLabelValue(forZone.Name) {