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

… addr and protocol

pkg/cluster/internal/providers/common/util.go

@@ -0,0 +1,75 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+// AddressPortProtocolMap is used to keep track of declared listen address,
+// port and protocol in node extra port mappings.
+type AddressPortProtocolMap map[string]map[int32]map[string]struct{}
+
+// IsAddrPortProtocolReconfigured checks if a combination of listen address, port and protocol exists in the given map.
+// Also takes care of special cases like 0.0.0.0 wildcard address.
+func IsAddrPortProtocolReconfigured(addrPortProtocolMap AddressPortProtocolMap, addr string, port int32, protocol string) bool {
+	existsInMap := func(addr string, port int32, protocol string) bool {
+		var ok bool
+		if _, ok = addrPortProtocolMap[addr]; ok {
+			if _, ok = addrPortProtocolMap[addr][port]; ok {
+				if _, ok = addrPortProtocolMap[addr][port][protocol]; ok {
+					ok = true
+				}
+			}
+		}
+		return ok
+	}
+
+	// base case - direct existence check for listen address, port and protocol
+	if existsInMap(addr, port, protocol) {
+		return true
+	} else {
+		wildcardAddress := "0.0.0.0"
+		if addr == wildcardAddress {
+			// iterate over all addrs, to check if any non-wildcard addr was declared with same port and protocol.
+			for key, _ := range addrPortProtocolMap {
+				if key != wildcardAddress && existsInMap(key, port, protocol) {
+					return true
+				}
+			}
+			return false
+		} else {
+			// check if port and protocol combination is already defined for 0.0.0.0 address
+			return existsInMap(wildcardAddress, port, protocol)
+		}
+	}
+}
+
+// AddAddrPortProtocolToMap adds combination of listen address, port and protocol to the map.
+func AddAddrPortProtocolToMap(addrPortProtocolMap AddressPortProtocolMap, addr string, port int32, protocol string) {
+	var ok bool
+	if _, ok = addrPortProtocolMap[addr]; ok {
+		if _, ok = addrPortProtocolMap[addr][port]; ok {
+			if _, ok = addrPortProtocolMap[addr][port]; ok {
+				addrPortProtocolMap[addr][port][protocol] = struct{}{}
+			}
+		} else {
+			addrPortProtocolMap[addr][port] = make(map[string]struct{})
+			addrPortProtocolMap[addr][port][protocol] = struct{}{}
+		}
+	} else {
+		addrPortProtocolMap[addr] = make(map[int32]map[string]struct{})
+		addrPortProtocolMap[addr][port] = make(map[string]struct{})
+		addrPortProtocolMap[addr][port][protocol] = struct{}{}
+	}
+}

pkg/cluster/internal/providers/common/util_test.go

@@ -0,0 +1,118 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package common
+
+import (
+	"reflect"
+	"sigs.k8s.io/kind/pkg/internal/assert"
+	"testing"
+)
+
+func TestAddAddrPortProtocolToMap(t *testing.T) {
+	addrPortProtocolMap := make(AddressPortProtocolMap)
+
+	AddAddrPortProtocolToMap(addrPortProtocolMap, "127.0.0.1", int32(80), "UDP")
+	assert.BoolEqual(t, true, reflect.DeepEqual(addrPortProtocolMap, AddressPortProtocolMap{
+		"127.0.0.1": {
+			int32(80): {
+				"UDP": struct{}{},
+			},
+		},
+	}))
+
+	AddAddrPortProtocolToMap(addrPortProtocolMap, "127.0.0.1", int32(80), "TCP")
+	assert.BoolEqual(t, true, reflect.DeepEqual(addrPortProtocolMap, AddressPortProtocolMap{
+		"127.0.0.1": {
+			int32(80): {
+				"UDP": struct{}{},
+				"TCP": struct{}{},
+			},
+		},
+	}))
+
+	AddAddrPortProtocolToMap(addrPortProtocolMap, "0.0.0.0", int32(5000), "UDP")
+	assert.BoolEqual(t, true, reflect.DeepEqual(addrPortProtocolMap, AddressPortProtocolMap{
+		"127.0.0.1": {
+			int32(80): {
+				"UDP": struct{}{},
+				"TCP": struct{}{},
+			},
+		},
+		"0.0.0.0": {
+			int32(5000): {
+				"UDP": struct{}{},
+			},
+		},
+	}))
+
+	AddAddrPortProtocolToMap(addrPortProtocolMap, "0.0.0.0", int32(3000), "SCTP")
+	assert.BoolEqual(t, true, reflect.DeepEqual(addrPortProtocolMap, AddressPortProtocolMap{
+		"127.0.0.1": {
+			int32(80): {
+				"UDP": struct{}{},
+				"TCP": struct{}{},
+			},
+		},
+		"0.0.0.0": {
+			int32(5000): {
+				"UDP": struct{}{},
+			},
+			int32(3000): {
+				"SCTP": struct{}{},
+			},
+		},
+	}))
+}
+
+func TestIsAddrPortProtocolRedeclared(t *testing.T) {
+	addrPortProtocolMap := make(AddressPortProtocolMap)
+
+	testCases := []struct {
+		addr     string
+		port     int
+		protocol string
+		expected bool
+	}{
+		{"127.0.0.1", 80, "UDP", false},
+		{"127.0.0.1", 80, "TCP", false},
+
+		// expected true: same addr, port and protocol
+		{"127.0.0.1", 80, "UDP", true},
+
+		// should return true: subset of 0.0.0.0[127.0.0.1] is already defined for same port and protocol
+		{"0.0.0.0", 80, "UDP", true},
+
+		// should return true: subset of 0.0.0.0[127.0.0.1] is already defined for same port and protocol
+		{"0.0.0.0", 80, "TCP", true},
+
+		{"0.0.0.0", 3000, "UDP", false},
+		{"0.0.0.0", 3000, "TCP", false},
+
+		// should return true: same addr, port and protocol
+		{"0.0.0.0", 3000, "UDP", true},
+
+		// should return true: same port and protocol is already defined for wildcard interface - 0.0.0.0
+		{"127.0.0.1", 3000, "UDP", true},
+	}
+
+	for _, testCase := range testCases {
+		assert.BoolEqual(t, testCase.expected,
+			IsAddrPortProtocolReconfigured(addrPortProtocolMap, testCase.addr, int32(testCase.port), testCase.protocol))
+
+		AddAddrPortProtocolToMap(addrPortProtocolMap, testCase.addr, int32(testCase.port), testCase.protocol)
+	}
+}

pkg/cluster/internal/providers/docker/provision.go

@@ -353,6 +353,7 @@ func generateMountBindings(mounts ...config.Mount) []string {
 
 // generatePortMappings converts the portMappings list to a list of args for docker
 func generatePortMappings(clusterIPFamily config.ClusterIPFamily, portMappings ...config.PortMapping) ([]string, error) {
+	addrPortProtocolMap := make(common.AddressPortProtocolMap)
 	args := make([]string, 0, len(portMappings))
 	for _, pm := range portMappings {
 		// do provider internal defaulting
@@ -389,6 +390,13 @@ func generatePortMappings(clusterIPFamily config.ClusterIPFamily, portMappings .
 		// generate the actual mapping arg
 		protocol := string(pm.Protocol)
 		hostPortBinding := net.JoinHostPort(pm.ListenAddress, fmt.Sprintf("%d", hostPort))
+
+		// check if host-port is configured more than once
+		if common.IsAddrPortProtocolReconfigured(addrPortProtocolMap, pm.ListenAddress, hostPort, protocol) {
+			return nil, errors.Errorf("host-port configured again with same listen address and protocol: %s/%s", hostPortBinding, protocol)
+		}
+
+		common.AddAddrPortProtocolToMap(addrPortProtocolMap, pm.ListenAddress, hostPort, protocol)
 		args = append(args, fmt.Sprintf("--publish=%s:%d/%s", hostPortBinding, pm.ContainerPort, protocol))
 	}
 	return args, nil