pkg/cluster: raise error if host-ports are configured again with same…

… addr and protocol

pkg/internal/apis/config/validate.go

@@ -20,9 +20,11 @@ import (
 	"fmt"
 	"net"
 	"regexp"
+	"strconv"
 	"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
@@ -113,14 +115,22 @@ func (n *Node) Validate() error {
 		errs = append(errs, errors.New("image is a required field"))
 	}
 
+	portMappingsSet := sets.NewString()
+	addrSet := sets.NewString()
+
 	// validate extra port forwards
 	for _, mapping := range n.ExtraPortMappings {
 		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 := validatePortMapping(portMappingsSet, addrSet, mapping); err != nil {
+			errs = append(errs, errors.Wrapf(err, "invalid portMapping"))
+		}
 	}
 
 	if len(errs) > 0 {
@@ -130,6 +140,68 @@ func (n *Node) Validate() error {
 	return nil
 }
 
+func formatPortMapping(addr net.IP, port int, protocol PortMappingProtocol) string {
+	return fmt.Sprintf("%s/%s",
+		net.JoinHostPort(addr.String(), strconv.Itoa(port)),
+		protocol,
+	)
+}
+
+func validatePortMapping(portMappingsSet sets.String, addrSet sets.String, mapping PortMapping) error {
+	wildcardAddrIPv4 := net.ParseIP("0.0.0.0")
+	wildcardAddrIPv6 := net.ParseIP("::")
+
+	addr := net.ParseIP(mapping.ListenAddress)
+	port := int(mapping.HostPort)
+	protocol := mapping.Protocol
+	ipFamily := getIPFamily(addr)
+
+	portMappingString := formatPortMapping(addr, port, protocol)
+	possibleErr := fmt.Errorf("port mapping with same listen address, port and protocol already configured: %s", portMappingString)
+
+	// base case - direct existence check for listen address, port and protocol
+	if portMappingsSet.Has(portMappingString) {
+		return possibleErr
+	}
+
+	var mappingString string
+
+	// handle for wild card addresses
+	if addr.Equal(wildcardAddrIPv4) || addr.Equal(wildcardAddrIPv6) {
+		// we won't be able to bind sockets to a wild card IP address if there exists
+		// a port mapping for any non-wild card address with same port, protocol and IPFamily.
+		for _, val := range addrSet.List() {
+			ip := net.ParseIP(val)
+			if !ip.Equal(wildcardAddrIPv4) && !ip.Equal(wildcardAddrIPv6) {
+				// filter on IPFamily
+				if getIPFamily(ip) == ipFamily {
+					mappingString = formatPortMapping(ip, port, protocol)
+					if portMappingsSet.Has(mappingString) {
+						return possibleErr
+					}
+				}
+			}
+		}
+	} else {
+		// check if port and protocol combination is already defined for wildcard IPv4 address
+		mappingString = formatPortMapping(wildcardAddrIPv4, port, protocol)
+		if portMappingsSet.Has(mappingString) {
+			return possibleErr
+		}
+
+		// check if port and protocol combination is already defined for wildcard IPv6 address
+		mappingString = formatPortMapping(wildcardAddrIPv6, port, protocol)
+		if portMappingsSet.Has(mappingString) {
+			return possibleErr
+		}
+
+	}
+
+	portMappingsSet.Insert(portMappingString)
+	addrSet.Insert(addr.String())
+	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.
@@ -183,6 +255,14 @@ func validateSubnets(subnetStr string, ipFamily ClusterIPFamily) error {
 	return nil
 }
 
+// getIPFamily returns the IPFamily for the given IP.
+func getIPFamily(ip net.IP) ClusterIPFamily {
+	if ip.To4() != nil {
+		return IPv4Family
+	}
+	return IPv6Family
+}
+
 // isDualStackCIDRs returns if
 // - all are valid cidrs
 // - at least one cidr from each family (v4 or v6)

pkg/internal/apis/config/validate_test.go

@@ -17,9 +17,12 @@ limitations under the License.
 package config
 
 import (
+	"fmt"
+	"net"
 	"testing"
 
 	"sigs.k8s.io/kind/pkg/errors"
+	"sigs.k8s.io/kind/pkg/internal/sets"
 )
 
 func TestClusterValidate(t *testing.T) {
@@ -428,3 +431,69 @@ func TestPortValidate(t *testing.T) {
 		})
 	}
 }
+
+func TestValidatePortMapping(t *testing.T) {
+	portMappingsSet := sets.NewString()
+	addSet := sets.NewString()
+
+	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 {
+		mapping   PortMapping
+		expectErr string
+	}{
+		{newPortMapping("127.0.0.1", 80, "UDP"), ""},
+		{newPortMapping("127.0.0.1", 80, "TCP"), ""},
+		{newPortMapping("::1", 80, "TCP"), ""},
+		{newPortMapping("::1", 80, "UDP"), ""},
+
+		// error expected: same addr, port and protocol
+		{newPortMapping("127.0.0.1", 80, "UDP"),
+			fmt.Sprintf("%s: 127.0.0.1:80/UDP", errMsg)},
+
+		// error expected: subset of 0.0.0.0[127.0.0.1] is already defined for same port and protocol
+		{newPortMapping("0.0.0.0", 80, "UDP"),
+			fmt.Sprintf("%s: 0.0.0.0:80/UDP", errMsg)},
+
+		{newPortMapping("0.0.0.0", 3000, "TCP"), ""},
+		{newPortMapping("::", 3000, "TCP"), ""},
+
+		// error expected: subset of ::[1e3d:6e85:424d:a011:a72e:9780:5f6f:a6fc] is already defined for same port and protocol
+		{newPortMapping("1e3d:6e85:424d:a011:a72e:9780:5f6f:a6fc", 3000, "TCP"),
+			fmt.Sprintf("%s: [1e3d:6e85:424d:a011:a72e:9780:5f6f:a6fc]:3000/TCP", errMsg)},
+
+		{newPortMapping("6516:944d:e070:a1d1:2e91:8437:a6b3:edf9", 3000, "UDP"), ""},
+
+		// error expected: same port and protocol is already defined for wildcard interface - 0.0.0.0
+		{newPortMapping("127.0.0.1", 3000, "TCP"),
+			fmt.Sprintf("%s: 127.0.0.1:3000/TCP", errMsg)},
+
+		{newPortMapping("::", 5000, "TCP"), ""},
+
+		// error expected: subset of ::[::1] is already defined for same port and protocol
+		{newPortMapping("::1", 5000, "TCP"),
+			fmt.Sprintf("%s: [::1]:5000/TCP", errMsg)},
+	}
+
+	for _, tc := range cases {
+		err := validatePortMapping(portMappingsSet, addSet, tc.mapping)
+		if err != nil {
+			portMappingsSet.Insert(formatPortMapping(net.ParseIP(tc.mapping.ListenAddress), int(tc.mapping.HostPort), tc.mapping.Protocol))
+			addSet.Insert(net.ParseIP(tc.mapping.ListenAddress).String())
+		}
+
+		if err == nil && len(tc.expectErr) > 0 {
+			t.Errorf("Test failed, unexpected error: %s", tc.expectErr)
+		}
+
+		if err != nil && err.Error() != tc.expectErr {
+			t.Errorf("Test failed, error: %s expected error: %s", err, tc.expectErr)
+		}
+	}
+}