Merge pull request #3175 from daman1807/main

Raise error if host-ports are configured again with same listen address and protocol
kubernetes-sigs · Apr 29, 2023 · b9d362c · b9d362c
2 parents 31a4d78 + 8c1e47a
commit b9d362c
Show file tree

Hide file tree

Showing 2 changed files with 180 additions and 0 deletions.
diff --git 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,18 +119,79 @@ 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)
 	}
 
 	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)
+
+		// in golang 0.0.0.0 and [::] are equivalent, convert [::] -> 0.0.0.0
+		// https://github.com/golang/go/issues/48723
+		if addr.Equal(wildcardAddrIPv6) {
+			addr = wildcardAddrIPv4
+			addrString = addr.String()
+		}
+
+		if _, ok := bindMap[portProtocol]; ok {
+
+			// wildcard address case:
+			// return error if there already exists any listen address for same port and protocol
+			if addr.Equal(wildcardAddrIPv4) {
+				if bindMap[portProtocol].Len() > 0 {
+					return possibleErr
+				}
+			}
+
+			// direct duplicate & wild card present check:
+			// return error if same combination of ip, port and protocol already exists in bindMap.
+			// return error if wildcard address is already present for same port & protocol
+			if bindMap[portProtocol].Has(addrString) || bindMap[portProtocol].Has(wildcardAddrIPv4.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
@@ -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)
+		})
+	}
+}