From 0af59a5f9efab65d6db259a55d10f0e0f207f463 Mon Sep 17 00:00:00 2001 From: Daman Date: Wed, 19 Apr 2023 13:35:13 +0530 Subject: [PATCH] pkg/cluster: raise error if host-ports are configured again with same addr and protocol Signed-off-by: Daman --- pkg/internal/apis/config/validate.go | 63 ++++++++++++ pkg/internal/apis/config/validate_test.go | 118 ++++++++++++++++++++++ 2 files changed, 181 insertions(+) diff --git a/pkg/internal/apis/config/validate.go b/pkg/internal/apis/config/validate.go index c05ff8b5da..78286cc877 100644 --- a/pkg/internal/apis/config/validate.go +++ b/pkg/internal/apis/config/validate.go @@ -23,6 +23,7 @@ import ( "strings" "sigs.k8s.io/kind/pkg/errors" + "sigs.k8s.io/kind/pkg/internal/sets" ) // similar to valid docker container names, but since we will prefix @@ -118,11 +119,16 @@ func (n *Node) Validate() error { if err := validatePort(mapping.HostPort); err != nil { errs = append(errs, errors.Wrapf(err, "invalid hostPort")) } + if err := validatePort(mapping.ContainerPort); err != nil { errs = append(errs, errors.Wrapf(err, "invalid containerPort")) } } + if err := validatePortMappings(n.ExtraPortMappings); err != nil { + errs = append(errs, errors.Wrapf(err, "invalid portMapping")) + } + if len(errs) > 0 { return errors.NewAggregate(errs) } @@ -130,6 +136,63 @@ func (n *Node) Validate() error { return nil } +func validatePortMappings(portMappings []PortMapping) error { + errMsg := "port mapping with same listen address, port and protocol already configured" + + wildcardAddrIPv4 := net.ParseIP("0.0.0.0") + wildcardAddrIPv6 := net.ParseIP("::") + + // bindMap has the following key-value structure + // PORT/PROTOCOL: [ IP ] + // { 80/TCP: [ 127.0.0.1, 192.168.2.3 ], 80/UDP: [ 0.0.0.0 ] } + bindMap := make(map[string]sets.String) + + formatPortProtocol := func(port int32, protocol PortMappingProtocol) string { + return fmt.Sprintf("%d/%s", port, protocol) + } + + for _, portMapping := range portMappings { + addr := net.ParseIP(portMapping.ListenAddress) + addrString := addr.String() + + portProtocol := formatPortProtocol(portMapping.HostPort, portMapping.Protocol) + possibleErr := fmt.Errorf("%s: %s:%s", errMsg, addrString, portProtocol) + + if _, ok := bindMap[portProtocol]; ok { + // in golang 0.0.0.0 and [::] are equivalent, for any wild card address if there + // exists any other address for same port & protocol immediately return error. + // https://github.com/golang/go/issues/48723 + if addr.Equal(wildcardAddrIPv4) || addr.Equal(wildcardAddrIPv6) { + if bindMap[portProtocol].Len() > 0 { + return possibleErr + } + } + + // direct duplicate check: + // return error if same combination of ip, port and protocol already exists in bindMap. + if bindMap[portProtocol].Has(addrString) { + return possibleErr + } + + // wildcard address already present check: + // return error if wildcard address is already present for same port & protocol + for _, otherAddrString := range bindMap[portProtocol].UnsortedList() { + if otherAddrString == wildcardAddrIPv4.String() || otherAddrString == wildcardAddrIPv6.String() { + return possibleErr + } + } + + } else { + // initialize the set + bindMap[portProtocol] = sets.NewString() + } + + // add the entry to bindMap + bindMap[portProtocol].Insert(addrString) + } + return nil +} + func validatePort(port int32) error { // NOTE: -1 is a special value for auto-selecting the port in the container // backend where possible as opposed to in kind itself. diff --git a/pkg/internal/apis/config/validate_test.go b/pkg/internal/apis/config/validate_test.go index a6912fccf1..6d4ef54a0e 100644 --- a/pkg/internal/apis/config/validate_test.go +++ b/pkg/internal/apis/config/validate_test.go @@ -17,6 +17,8 @@ limitations under the License. package config import ( + "fmt" + "sigs.k8s.io/kind/pkg/internal/assert" "testing" "sigs.k8s.io/kind/pkg/errors" @@ -428,3 +430,119 @@ func TestPortValidate(t *testing.T) { }) } } + +func TestValidatePortMappings(t *testing.T) { + newPortMapping := func(addr string, port int, protocol string) PortMapping { + return PortMapping{ + HostPort: int32(port), + ListenAddress: addr, + Protocol: PortMappingProtocol(protocol), + } + } + errMsg := "port mapping with same listen address, port and protocol already configured" + cases := []struct { + testName string + portMappings []PortMapping + expectErr string + }{ + { + 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"), + }, + 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"), + }, + 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"), + }, + // error expected: exact duplicate + expectErr: fmt.Sprintf("%s: 127.0.0.1:80/TCP", errMsg), + }, + + { + testName: "exact duplicate port mappings ipv6", + portMappings: []PortMapping{ + newPortMapping("::1", 80, "TCP"), + newPortMapping("::1", 80, "UDP"), + newPortMapping("::1", 80, "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"), + }, + // error expected: 0.0.0.0 & [::] are same in golang + // https://github.com/golang/go/issues/48723 + expectErr: fmt.Sprintf("%s: [::]:80/UDP", errMsg), + }, + { + testName: "subset already configured ipv4", + portMappings: []PortMapping{ + newPortMapping("127.0.0.1", 80, "TCP"), + newPortMapping("0.0.0.0", 80, "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"), + }, + // 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"), + }, + // 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"), + }, + // error expected: port mapping is already defined for wildcard interface - :: + expectErr: fmt.Sprintf("%s: [::1]:80/SCTP", errMsg), + }, + } + + for _, tc := range cases { + tc := tc //capture loop variable + t.Run(tc.testName, func(t *testing.T) { + t.Parallel() + + err := validatePortMappings(tc.portMappings) + assert.ExpectError(t, len(tc.expectErr) > 0, err) + }) + } +}