diff --git a/pkg/apis/configuration/validation/globalconfiguration.go b/pkg/apis/configuration/validation/globalconfiguration.go index 689fa625ef..6045508fac 100644 --- a/pkg/apis/configuration/validation/globalconfiguration.go +++ b/pkg/apis/configuration/validation/globalconfiguration.go @@ -50,12 +50,10 @@ 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) @@ -63,25 +61,22 @@ func (gcv *GlobalConfigurationValidator) getValidListeners(listeners []conf_v1.L 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 @@ -97,33 +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] - if exists { - if existingProtocol == listener.Protocol { - return field.Duplicate(fieldPath, fmt.Sprintf("Listener %s: Duplicated port/protocol combination %d/%s", listener.Name, listener.Port, listener.Protocol)) - } else if listener.Protocol == "HTTP" || existingProtocol == "HTTP" { - return field.Invalid(fieldPath.Child("port"), listener.Port, fmt.Sprintf("Listener %s: Port %d is used with a different protocol (current: %s, new: %s)", listener.Name, listener.Port, existingProtocol, listener.Protocol)) + existingProtocols, exists := combinations[ip][listener.Port] + if !exists { + return nil + } + 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 %s:%d %s", listener.Name, ip, 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 %s:%d %s", listener.Name, ip, 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. diff --git a/pkg/apis/configuration/validation/globalconfiguration_test.go b/pkg/apis/configuration/validation/globalconfiguration_test.go index 709967db71..2acd448397 100644 --- a/pkg/apis/configuration/validation/globalconfiguration_test.go +++ b/pkg/apis/configuration/validation/globalconfiguration_test.go @@ -234,6 +234,27 @@ func TestValidateListeners_PassesOnValidIPListeners(t *testing.T) { {Name: "listener-2", IPv6IP: "2001:0db8:85a3:0000:0000:8a2e:0370:7334", Port: 8080, Protocol: "HTTP"}, }, }, + { + name: "UDP and HTTP Listeners with Same Port", + listeners: []conf_v1.Listener{ + {Name: "listener-1", IPv4IP: "127.0.0.1", Port: 8080, Protocol: "UDP"}, + {Name: "listener-2", IPv4IP: "127.0.0.1", Port: 8080, Protocol: "HTTP"}, + }, + }, + { + name: "HTTP Listeners with Same Port but different IPv4 and IPv6 ip addresses", + listeners: []conf_v1.Listener{ + {Name: "listener-1", IPv4IP: "127.0.0.2", IPv6IP: "::1", Port: 8080, Protocol: "HTTP"}, + {Name: "listener-2", IPv4IP: "127.0.0.1", Port: 8080, Protocol: "HTTP"}, + }, + }, + { + name: "UDP and TCP Listeners with Same Port", + listeners: []conf_v1.Listener{ + {Name: "listener-1", IPv4IP: "127.0.0.1", Port: 8080, Protocol: "UDP"}, + {Name: "listener-2", IPv4IP: "127.0.0.1", Port: 8080, Protocol: "TCP"}, + }, + }, } gcv := createGlobalConfigurationValidator() @@ -587,7 +608,7 @@ func TestValidateListenerProtocol_FailsOnHttpListenerUsingSamePortAsTCPListener( } } -func TestValidateListenerProtocol_FailsOnHttpListenerUsingSamePortAsUDPListener(t *testing.T) { +func TestValidateListenerProtocol_PassesOnHttpListenerUsingSamePortAsUDPListener(t *testing.T) { t.Parallel() listeners := []conf_v1.Listener{ { @@ -607,18 +628,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{ { @@ -649,15 +675,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)) } } @@ -694,7 +718,7 @@ func TestValidateListenerProtocol_FailsOnTCPListenerUsingSamePortAsHTTPListener( } } -func TestValidateListenerProtocol_FailsOnUDPListenerUsingSamePortAsHTTPListener(t *testing.T) { +func TestValidateListenerProtocol_PassesOnUDPListenerUsingSamePortAsHTTPListener(t *testing.T) { t.Parallel() listeners := []conf_v1.Listener{ { @@ -714,6 +738,11 @@ func TestValidateListenerProtocol_FailsOnUDPListenerUsingSamePortAsHTTPListener( Port: 53, Protocol: "HTTP", }, + { + Name: "udp-listener", + Port: 53, + Protocol: "UDP", + }, } gcv := createGlobalConfigurationValidator() @@ -722,7 +751,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) } } diff --git a/tests/data/udp-http-listeners-together/global-configuration.yaml b/tests/data/udp-http-listeners-together/global-configuration.yaml new file mode 100644 index 0000000000..861b3e1e04 --- /dev/null +++ b/tests/data/udp-http-listeners-together/global-configuration.yaml @@ -0,0 +1,13 @@ +apiVersion: k8s.nginx.org/v1 +kind: GlobalConfiguration +metadata: + name: nginx-configuration + namespace: nginx-ingress +spec: + listeners: + - name: udp-listener + port: 5454 + protocol: UDP + - name: http-listener + port: 5454 + protocol: HTTP diff --git a/tests/data/udp-http-listeners-together/transport-server.yaml b/tests/data/udp-http-listeners-together/transport-server.yaml new file mode 100644 index 0000000000..d5d4a2d07e --- /dev/null +++ b/tests/data/udp-http-listeners-together/transport-server.yaml @@ -0,0 +1,14 @@ +apiVersion: k8s.nginx.org/v1 +kind: TransportServer +metadata: + name: transport-server +spec: + listener: + name: udp-listener + protocol: UDP + upstreams: + - name: dns-app + service: coredns + port: 5353 + action: + pass: dns-app diff --git a/tests/data/udp-http-listeners-together/virtual-server.yaml b/tests/data/udp-http-listeners-together/virtual-server.yaml new file mode 100644 index 0000000000..e813f8a0d1 --- /dev/null +++ b/tests/data/udp-http-listeners-together/virtual-server.yaml @@ -0,0 +1,22 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server-status +spec: + listener: + http: http-listener + host: virtual-server-status.example.com + upstreams: + - name: backend2 + service: backend2-svc + port: 80 + - name: backend1 + service: backend1-svc + port: 80 + routes: + - path: /backend1 + action: + pass: backend1 + - path: /backend2 + action: + pass: backend2 diff --git a/tests/suite/test_udp_http_listeners_together.py b/tests/suite/test_udp_http_listeners_together.py new file mode 100644 index 0000000000..ca99e3fbd2 --- /dev/null +++ b/tests/suite/test_udp_http_listeners_together.py @@ -0,0 +1,104 @@ +import time + +import pytest +from settings import TEST_DATA +from suite.utils.custom_resources_utils import ( + create_ts_from_yaml, + delete_ts, + patch_gc_from_yaml, + read_custom_resource, + read_ts, +) +from suite.utils.resources_utils import get_first_pod_name, get_ts_nginx_template_conf, wait_before_test +from suite.utils.vs_vsr_resources_utils import get_vs_nginx_template_conf, patch_virtual_server_from_yaml + + +@pytest.mark.vs +@pytest.mark.ts +@pytest.mark.parametrize( + "crd_ingress_controller, virtual_server_setup, transport_server_setup", + [ + ( + { + "type": "complete", + "extra_args": [ + "-enable-custom-resources", + "-global-configuration=nginx-ingress/nginx-configuration", + ], + }, + {"example": "virtual-server-status", "app_type": "simple"}, + {"example": "transport-server-status", "app_type": "simple"}, + ) + ], + indirect=True, +) +class TestUDPandHTTPListenersTogether: + def test_udp_and_http_listeners_together( + self, + kube_apis, + ingress_controller_prerequisites, + crd_ingress_controller, + virtual_server_setup, + transport_server_setup, + ): + + wait_before_test() + existing_ts = read_ts(kube_apis.custom_objects, transport_server_setup.namespace, transport_server_setup.name) + delete_ts(kube_apis.custom_objects, existing_ts, transport_server_setup.namespace) + + global_config_file = f"{TEST_DATA}/udp-http-listeners-together/global-configuration.yaml" + transport_server_file = f"{TEST_DATA}/udp-http-listeners-together/transport-server.yaml" + virtual_server_file = f"{TEST_DATA}/udp-http-listeners-together/virtual-server.yaml" + gc_resource_name = "nginx-configuration" + gc_namespace = "nginx-ingress" + + patch_gc_from_yaml(kube_apis.custom_objects, gc_resource_name, global_config_file, gc_namespace) + create_ts_from_yaml(kube_apis.custom_objects, transport_server_file, transport_server_setup.namespace) + patch_virtual_server_from_yaml( + kube_apis.custom_objects, "virtual-server-status", virtual_server_file, virtual_server_setup.namespace + ) + wait_before_test() + + ic_pod_name = get_first_pod_name(kube_apis.v1, ingress_controller_prerequisites.namespace) + ts_config = get_ts_nginx_template_conf( + kube_apis.v1, + transport_server_setup.namespace, + transport_server_setup.name, + ic_pod_name, + ingress_controller_prerequisites.namespace, + ) + vs_config = get_vs_nginx_template_conf( + kube_apis.v1, + virtual_server_setup.namespace, + virtual_server_setup.vs_name, + ic_pod_name, + ingress_controller_prerequisites.namespace, + ) + assert "listen 5454;" in vs_config + assert "listen 5454 udp;" in ts_config + + for _ in range(30): + transport_server_response = read_custom_resource( + kube_apis.custom_objects, + transport_server_setup.namespace, + "transportservers", + "transport-server", + ) + if "status" in transport_server_response and transport_server_response["status"]["state"] == "Valid": + break + time.sleep(1) + else: + pytest.fail("TransportServer status did not become 'Valid' within the timeout period") + + for _ in range(30): + virtual_server_response = read_custom_resource( + kube_apis.custom_objects, + virtual_server_setup.namespace, + "virtualservers", + "virtual-server-status", + ) + if "status" in virtual_server_response and virtual_server_response["status"]["state"] == "Valid": + break + time.sleep(1) + else: + pytest.fail("VirtualServer status did not become 'Valid' within the timeout period") diff --git a/tests/suite/test_virtual_server_custom_listeners.py b/tests/suite/test_virtual_server_custom_listeners.py index a9e8a70a9b..034ed26f5f 100644 --- a/tests/suite/test_virtual_server_custom_listeners.py +++ b/tests/suite/test_virtual_server_custom_listeners.py @@ -158,7 +158,7 @@ class TestVirtualServerCustomListeners: "https_listener_in_config": True, "expected_response_codes": [404, 404, 0, 200], "expected_vs_error_msg": "Listener http-8085 is not defined in GlobalConfiguration", - "expected_gc_error_msg": "Listener http-8085: Duplicated port/protocol combination 8085/HTTP", + "expected_gc_error_msg": "Listener http-8085: Duplicated ip:port protocol combination 0.0.0.0:8085 HTTP", }, { "gc_yaml": "global-configuration-forbidden-port-http", @@ -339,7 +339,7 @@ def test_custom_listeners( "https_listener_in_config": True, "expected_response_codes": [404, 404, 0, 200], "expected_vs_error_msg": "Listener http-8085 is not defined in GlobalConfiguration", - "expected_gc_error_msg": "Listener http-8085: Duplicated port/protocol combination 8085/HTTP", + "expected_gc_error_msg": "Listener http-8085: Duplicated ip:port protocol combination 0.0.0.0:8085 HTTP", }, { "gc_yaml": "global-configuration-forbidden-port-http",