Skip to content

Commit

Permalink
fix listener validation algo
Browse files Browse the repository at this point in the history
  • Loading branch information
Jim Ryan committed Sep 12, 2024
1 parent 57a9d76 commit 294be4a
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 48 deletions.
18 changes: 9 additions & 9 deletions charts/nginx-ingress/values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,17 +357,17 @@ controller:

globalConfiguration:
## Creates the GlobalConfiguration custom resource. Requires controller.enableCustomResources.
create: false
create: true

## The spec of the GlobalConfiguration for defining the global configuration parameters of the Ingress Controller.
spec: {} ## Ensure both curly brackets are removed when adding listeners in YAML format.
# listeners:
# - name: dns-udp
# port: 5353
# protocol: UDP
# - name: dns-tcp
# port: 5353
# protocol: TCP
spec: ## Ensure both curly brackets are removed when adding listeners in YAML format.
listeners:
- name: some-udp
port: 5353
protocol: UDP
- name: some-http
port: 5353
protocol: HTTP

## Enable custom NGINX configuration snippets in Ingress, VirtualServer, VirtualServerRoute and TransportServer resources.
enableSnippets: false
Expand Down
48 changes: 20 additions & 28 deletions pkg/apis/configuration/validation/globalconfiguration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,38 +50,33 @@ func (gcv *GlobalConfigurationValidator) validateGlobalConfigurationSpec(spec *c

func (gcv *GlobalConfigurationValidator) getValidListeners(listeners []conf_v1.Listener, fieldPath *field.Path) ([]conf_v1.Listener, field.ErrorList) {
allErrs := field.ErrorList{}

listenerNames := sets.Set[string]{}
ipv4PortProtocolCombinations := make(map[string]map[int]string) // map[IP]map[Port]Protocol
ipv6PortProtocolCombinations := make(map[string]map[int]string)
ipv4PortProtocolCombinations := make(map[string]map[int][]string) // map[IP]map[Port][]Protocol
ipv6PortProtocolCombinations := make(map[string]map[int][]string)
var validListeners []conf_v1.Listener

for i, l := range listeners {
idxPath := fieldPath.Index(i)
listenerErrs := gcv.validateListener(l, idxPath)
if len(listenerErrs) > 0 {
allErrs = append(allErrs, listenerErrs...)
continue
}

if err := gcv.checkForDuplicateName(listenerNames, l, idxPath); err != nil {
allErrs = append(allErrs, err)
continue
}

if err := gcv.checkIPPortProtocolConflicts(ipv4PortProtocolCombinations, ipv4, l, fieldPath); err != nil {
allErrs = append(allErrs, err)
gcv.updatePortProtocolCombinations(ipv4PortProtocolCombinations, ipv4, l)
continue
}

if err := gcv.checkIPPortProtocolConflicts(ipv6PortProtocolCombinations, ipv6, l, fieldPath); err != nil {
allErrs = append(allErrs, err)
gcv.updatePortProtocolCombinations(ipv6PortProtocolCombinations, ipv6, l)
continue
}

gcv.updatePortProtocolCombinations(ipv4PortProtocolCombinations, ipv4, l)
gcv.updatePortProtocolCombinations(ipv6PortProtocolCombinations, ipv6, l)

validListeners = append(validListeners, l)
}
return validListeners, allErrs
Expand All @@ -97,40 +92,37 @@ func (gcv *GlobalConfigurationValidator) checkForDuplicateName(listenerNames set
}

// checkIPPortProtocolConflicts ensures no duplicate or conflicting port/protocol combinations exist.
func (gcv *GlobalConfigurationValidator) checkIPPortProtocolConflicts(combinations map[string]map[int]string, ipType ipType, listener conf_v1.Listener, fieldPath *field.Path) *field.Error {
func (gcv *GlobalConfigurationValidator) checkIPPortProtocolConflicts(combinations map[string]map[int][]string, ipType ipType, listener conf_v1.Listener, fieldPath *field.Path) *field.Error {
ip := getIP(ipType, listener)

if combinations[ip] == nil {
combinations[ip] = make(map[int]string) // map[ip]map[port]protocol
combinations[ip] = make(map[int][]string) // map[ip]map[port][]protocol
}

existingProtocol, exists := combinations[ip][listener.Port]
existingProtocols, exists := combinations[ip][listener.Port]
if !exists {
return nil
}

switch listener.Protocol {
case "HTTP", "TCP":
if existingProtocol == "HTTP" || existingProtocol == "TCP" {
return field.Invalid(fieldPath.Child("protocol"), listener.Protocol, fmt.Sprintf("Listener %s: Duplicated ip:port protocol combination %d/%s", listener.Name, listener.Port, listener.Protocol))
}
case "UDP":
if existingProtocol == "UDP" {
return field.Invalid(fieldPath.Child("protocol"), listener.Protocol, fmt.Sprintf("Listener %s: Duplicated ip:port protocol combination %d/%s", listener.Name, listener.Port, listener.Protocol))
for _, existingProtocol := range existingProtocols {
switch listener.Protocol {
case "HTTP", "TCP":
if existingProtocol == "HTTP" || existingProtocol == "TCP" {
return field.Invalid(fieldPath.Child("protocol"), listener.Protocol, fmt.Sprintf("Listener %s: Duplicated ip:port protocol combination %d/%s", listener.Name, listener.Port, listener.Protocol))
}
case "UDP":
if existingProtocol == "UDP" {
return field.Invalid(fieldPath.Child("protocol"), listener.Protocol, fmt.Sprintf("Listener %s: Duplicated ip:port protocol combination %d/%s", listener.Name, listener.Port, listener.Protocol))
}
}
}

return nil
}

// updatePortProtocolCombinations updates the port/protocol combinations map with the given listener's details for both IPv4 and IPv6.
func (gcv *GlobalConfigurationValidator) updatePortProtocolCombinations(combinations map[string]map[int]string, ipType ipType, listener conf_v1.Listener) {
func (gcv *GlobalConfigurationValidator) updatePortProtocolCombinations(combinations map[string]map[int][]string, ipType ipType, listener conf_v1.Listener) {
ip := getIP(ipType, listener)

if combinations[ip] == nil {
combinations[ip] = make(map[int]string)
combinations[ip] = make(map[int][]string)
}
combinations[ip][listener.Port] = listener.Protocol
combinations[ip][listener.Port] = append(combinations[ip][listener.Port], listener.Protocol)
}

// getIP returns the appropriate IP address for the given ipType and listener.
Expand Down
30 changes: 19 additions & 11 deletions pkg/apis/configuration/validation/globalconfiguration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ func TestValidateListenerProtocol_FailsOnHttpListenerUsingSamePortAsTCPListener(
}
}

func TestValidateListenerProtocol_FailsOnHttpListenerUsingSamePortAsUDPListener(t *testing.T) {
func TestValidateListenerProtocol_PassesOnHttpListenerUsingSamePortAsUDPListener(t *testing.T) {
t.Parallel()
listeners := []conf_v1.Listener{
{
Expand All @@ -621,18 +621,23 @@ func TestValidateListenerProtocol_FailsOnHttpListenerUsingSamePortAsUDPListener(
Port: 53,
Protocol: "UDP",
},
{
Name: "http-listener",
Port: 53,
Protocol: "HTTP",
},
}
gcv := createGlobalConfigurationValidator()
listeners, allErrs := gcv.getValidListeners(listeners, field.NewPath("listeners"))
if diff := cmp.Diff(listeners, wantListeners); diff != "" {
t.Errorf("getValidListeners() returned unexpected result: (-want +got):\n%s", diff)
}
if len(allErrs) == 0 {
t.Errorf("validateListeners() returned no errors %v for invalid input", allErrs)
if len(allErrs) != 0 {
t.Errorf("validateListeners() returned errors %v invalid input", allErrs)
}
}

func TestValidateListenerProtocol_FailsOnHttpListenerUsingSamePortAsTCPAndUDPListener(t *testing.T) {
func TestValidateListenerProtocol_FailsOnHttpListenerUsingSamePortAsTCP(t *testing.T) {
t.Parallel()
listeners := []conf_v1.Listener{
{
Expand Down Expand Up @@ -663,15 +668,13 @@ func TestValidateListenerProtocol_FailsOnHttpListenerUsingSamePortAsTCPAndUDPLis
Protocol: "UDP",
},
}

gcv := createGlobalConfigurationValidator()

listeners, allErrs := gcv.getValidListeners(listeners, field.NewPath("listeners"))
if diff := cmp.Diff(listeners, wantListeners); diff != "" {
t.Errorf("getValidListeners() returned unexpected result: (-want +got):\n%s", diff)
}
if len(allErrs) == 0 {
t.Errorf("validateListeners() returned no errors %v for invalid input", allErrs)
if len(allErrs) != 1 {
t.Errorf("getValidListeners() returned unexpected number of errors. Got %d, want 1", len(allErrs))
}
}

Expand Down Expand Up @@ -708,7 +711,7 @@ func TestValidateListenerProtocol_FailsOnTCPListenerUsingSamePortAsHTTPListener(
}
}

func TestValidateListenerProtocol_FailsOnUDPListenerUsingSamePortAsHTTPListener(t *testing.T) {
func TestValidateListenerProtocol_PassesOnUDPListenerUsingSamePortAsHTTPListener(t *testing.T) {
t.Parallel()
listeners := []conf_v1.Listener{
{
Expand All @@ -728,6 +731,11 @@ func TestValidateListenerProtocol_FailsOnUDPListenerUsingSamePortAsHTTPListener(
Port: 53,
Protocol: "HTTP",
},
{
Name: "udp-listener",
Port: 53,
Protocol: "UDP",
},
}

gcv := createGlobalConfigurationValidator()
Expand All @@ -736,7 +744,7 @@ func TestValidateListenerProtocol_FailsOnUDPListenerUsingSamePortAsHTTPListener(
if diff := cmp.Diff(listeners, wantListeners); diff != "" {
t.Errorf("getValidListeners() returned unexpected result: (-want +got):\n%s", diff)
}
if len(allErrs) == 0 {
t.Errorf("validateListeners() returned no errors %v for invalid input", allErrs)
if len(allErrs) != 0 {
t.Errorf("validateListeners() returned errors %v for valid input", allErrs)
}
}

0 comments on commit 294be4a

Please sign in to comment.