From a7df778fd195845158e0218c1cefe3d6b9cb504c Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Sun, 9 Jul 2023 14:31:11 +0530 Subject: [PATCH] extra-port-mapping bug fix Signed-off-by: Daman Arora --- pkg/internal/apis/config/validate.go | 17 ++++- pkg/internal/apis/config/validate_test.go | 79 ++++++++++++++--------- 2 files changed, 66 insertions(+), 30 deletions(-) diff --git a/pkg/internal/apis/config/validate.go b/pkg/internal/apis/config/validate.go index 68185d1579..1aebb29418 100644 --- a/pkg/internal/apis/config/validate.go +++ b/pkg/internal/apis/config/validate.go @@ -137,7 +137,7 @@ func (n *Node) Validate() error { } func validatePortMappings(portMappings []PortMapping) error { - errMsg := "port mapping with same listen address, port and protocol already configured" + errMsg := "port mapping with same listen address, host port and protocol already configured" wildcardAddrIPv4 := net.ParseIP("0.0.0.0") wildcardAddrIPv6 := net.ParseIP("::") @@ -152,6 +152,21 @@ func validatePortMappings(portMappings []PortMapping) error { } for _, portMapping := range portMappings { + // skipping validation if host port is not defined + if portMapping.HostPort == 0 { + continue + } + + // using default listen address + if portMapping.ListenAddress == "" { + portMapping.ListenAddress = wildcardAddrIPv4.String() + } + + // using default protool + if portMapping.Protocol == "" { + portMapping.Protocol = PortMappingProtocolTCP + } + addr := net.ParseIP(portMapping.ListenAddress) addrString := addr.String() diff --git a/pkg/internal/apis/config/validate_test.go b/pkg/internal/apis/config/validate_test.go index 6d4ef54a0e..5b8b089c7a 100644 --- a/pkg/internal/apis/config/validate_test.go +++ b/pkg/internal/apis/config/validate_test.go @@ -432,45 +432,66 @@ func TestPortValidate(t *testing.T) { } func TestValidatePortMappings(t *testing.T) { - newPortMapping := func(addr string, port int, protocol string) PortMapping { + newPortMapping := func(addr string, hostPort, containerPort int, protocol string) PortMapping { return PortMapping{ - HostPort: int32(port), + HostPort: int32(hostPort), + ContainerPort: int32(containerPort), ListenAddress: addr, Protocol: PortMappingProtocol(protocol), } } - errMsg := "port mapping with same listen address, port and protocol already configured" + errMsg := "port mapping with same listen address, host port and protocol already configured" cases := []struct { testName string portMappings []PortMapping expectErr string }{ + { + testName: "unique container ports", + portMappings: []PortMapping{ + newPortMapping("", 0, 1000, ""), + newPortMapping("", 0, 2000, ""), + newPortMapping("", 0, 3000, ""), + newPortMapping("", 0, 4000, ""), + }, + expectErr: "", + }, + { + testName: "duplicate container ports", + portMappings: []PortMapping{ + newPortMapping("", 0, 1000, ""), + newPortMapping("", 0, 2000, ""), + newPortMapping("", 0, 3000, ""), + newPortMapping("", 0, 3000, ""), + }, + expectErr: "", + }, { testName: "unique port mappings ipv4", portMappings: []PortMapping{ - newPortMapping("127.0.0.1", 80, "UDP"), - newPortMapping("127.0.0.1", 80, "TCP"), - newPortMapping("0.0.0.0", 3000, "UDP"), - newPortMapping("0.0.0.0", 5000, "TCP"), + newPortMapping("127.0.0.1", 80, 5000, "UDP"), + newPortMapping("127.0.0.1", 80, 5000, "TCP"), + newPortMapping("0.0.0.0", 3000, 8000, "UDP"), + newPortMapping("0.0.0.0", 5000, 8000, "TCP"), }, expectErr: "", }, { testName: "unique port mappings ipv6", portMappings: []PortMapping{ - newPortMapping("::1", 80, "UDP"), - newPortMapping("::1", 80, "TCP"), - newPortMapping("1e3d:6e85:424d:a011:a72e:9780:5f6f:a6fc", 3000, "UDP"), - newPortMapping("6516:944d:e070:a1d1:2e91:8437:a6b3:edf9", 5000, "TCP"), + newPortMapping("::1", 80, 5000, "UDP"), + newPortMapping("::1", 80, 5000, "TCP"), + newPortMapping("1e3d:6e85:424d:a011:a72e:9780:5f6f:a6fc", 3000, 8000, "UDP"), + newPortMapping("6516:944d:e070:a1d1:2e91:8437:a6b3:edf9", 5000, 8000, "TCP"), }, expectErr: "", }, { testName: "exact duplicate port mappings ipv4", portMappings: []PortMapping{ - newPortMapping("127.0.0.1", 80, "TCP"), - newPortMapping("127.0.0.1", 80, "UDP"), - newPortMapping("127.0.0.1", 80, "TCP"), + newPortMapping("127.0.0.1", 80, 3000, "TCP"), + newPortMapping("127.0.0.1", 80, 5000, "UDP"), + newPortMapping("127.0.0.1", 80, 8000, "TCP"), }, // error expected: exact duplicate expectErr: fmt.Sprintf("%s: 127.0.0.1:80/TCP", errMsg), @@ -479,9 +500,9 @@ func TestValidatePortMappings(t *testing.T) { { testName: "exact duplicate port mappings ipv6", portMappings: []PortMapping{ - newPortMapping("::1", 80, "TCP"), - newPortMapping("::1", 80, "UDP"), - newPortMapping("::1", 80, "TCP"), + newPortMapping("::1", 80, 3000, "TCP"), + newPortMapping("::1", 80, 5000, "UDP"), + newPortMapping("::1", 80, 8000, "TCP"), }, // error expected: exact duplicate expectErr: fmt.Sprintf("%s: [::1]:80/TCP", errMsg), @@ -489,10 +510,10 @@ func TestValidatePortMappings(t *testing.T) { { testName: "wildcard ipv4 & ipv6", portMappings: []PortMapping{ - newPortMapping("127.0.0.1", 80, "TCP"), - newPortMapping("0.0.0.0", 80, "UDP"), - newPortMapping("::1", 80, "TCP"), - newPortMapping("::", 80, "UDP"), + newPortMapping("127.0.0.1", 80, 3000, "TCP"), + newPortMapping("0.0.0.0", 80, 5000, "UDP"), + newPortMapping("::1", 80, 3000, "TCP"), + newPortMapping("::", 80, 5000, "UDP"), }, // error expected: 0.0.0.0 & [::] are same in golang // https://github.com/golang/go/issues/48723 @@ -501,8 +522,8 @@ func TestValidatePortMappings(t *testing.T) { { testName: "subset already configured ipv4", portMappings: []PortMapping{ - newPortMapping("127.0.0.1", 80, "TCP"), - newPortMapping("0.0.0.0", 80, "TCP"), + newPortMapping("127.0.0.1", 80, 3000, "TCP"), + newPortMapping("0.0.0.0", 80, 5000, "TCP"), }, // error expected: subset of 0.0.0.0 -> 127.0.0.1 is already defined for same port and protocol expectErr: fmt.Sprintf("%s: 0.0.0.0:80/TCP", errMsg), @@ -510,8 +531,8 @@ func TestValidatePortMappings(t *testing.T) { { testName: "subset already configured ipv6", portMappings: []PortMapping{ - newPortMapping("::1", 80, "TCP"), - newPortMapping("::", 80, "TCP"), + newPortMapping("::1", 80, 3000, "TCP"), + newPortMapping("::", 80, 5000, "TCP"), }, // error expected: subset of :: -> ::1 is already defined for same port and protocol expectErr: fmt.Sprintf("%s: [::]:80/TCP", errMsg), @@ -519,8 +540,8 @@ func TestValidatePortMappings(t *testing.T) { { testName: "port mapping already configured via wildcard ipv4", portMappings: []PortMapping{ - newPortMapping("0.0.0.0", 80, "TCP"), - newPortMapping("127.0.0.1", 80, "TCP"), + newPortMapping("0.0.0.0", 80, 5000, "TCP"), + newPortMapping("127.0.0.1", 80, 5000, "TCP"), }, // error expected: port mapping is already defined for wildcard interface - 0.0.0.0 expectErr: fmt.Sprintf("%s: 127.0.0.1:80/TCP", errMsg), @@ -528,8 +549,8 @@ func TestValidatePortMappings(t *testing.T) { { testName: "port mapping already configured via wildcard ipv6", portMappings: []PortMapping{ - newPortMapping("::", 80, "SCTP"), - newPortMapping("::1", 80, "SCTP"), + newPortMapping("::", 80, 5000, "SCTP"), + newPortMapping("::1", 80, 5000, "SCTP"), }, // error expected: port mapping is already defined for wildcard interface - :: expectErr: fmt.Sprintf("%s: [::1]:80/SCTP", errMsg),