Skip to content

Commit

Permalink
extra-port-mapping bug fix
Browse files Browse the repository at this point in the history 
Signed-off-by: Daman Arora <[email protected]>
  • Loading branch information
@aroradaman
aroradaman committed Jul 9, 2023
1 parent 3610f60 commit a7df778
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 30 deletions.
17 changes: 16 additions & 1 deletion pkg/internal/apis/config/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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("::")
Expand All @@ -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()

Expand Down
79 changes: 50 additions & 29 deletions pkg/internal/apis/config/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -479,20 +500,20 @@ 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),
},
{
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
Expand All @@ -501,35 +522,35 @@ 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),
},
{
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),
},
{
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),
},
{
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),
Expand Down

0 comments on commit a7df778

Please sign in to comment.